Skip to content

Commit 030d6dc

Browse files
jrajahalmejrajahalme
committed
policy: Use source pod's egress policy also with L7LB if available
Enforce source pod's egress policy, if available, i.e., when the source is a local pod, even in the north/south scenario, where the Ingress IP is used as the upstream source address. Signed-off-by: Jarno Rajahalme <[email protected]>
1 parent a7633fb commit 030d6dc

9 files changed

+348
-118
lines changed

‎cilium/bpf_metadata.cc

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,7 @@ const PolicyInstance& Config::getPolicy(const std::string& pod_ip) const {
349349
// This is the case for L7 LB listeners only. This is needed to allow traffic forwarded by Cilium
350350
// Ingress (which is implemented as an egress listener!).
351351
bool allow_egress = !enforce_policy_on_l7lb_ && !is_ingress_ && is_l7lb_;
352+
// TODO: deny if no npmap_!
352353
if (npmap_ == nullptr)
353354
return allow_egress ? NetworkPolicyMap::GetAllowAllEgressPolicy()
354355
: NetworkPolicyMap::GetDenyAllPolicy();
@@ -371,7 +372,7 @@ Config::extractSocketMetadata(Network::ConnectionSocket& socket) {
371372
return absl::nullopt;
372373
}
373374

374-
std::string pod_ip, other_ip;
375+
std::string pod_ip, other_ip, ingress_policy_name;
375376
if (is_ingress_) {
376377
pod_ip = dip->addressAsString();
377378
other_ip = sip->addressAsString();
@@ -446,31 +447,28 @@ Config::extractSocketMetadata(Network::ConnectionSocket& socket) {
446447
return absl::nullopt;
447448
}
448449

449-
auto& ingress_ip_str = ingress_ip->addressAsString();
450+
// Enforce pod policy only for local pods.
451+
if (policy->getEndpointID() == 0) {
452+
pod_ip = ""; // source is not a local pod
453+
}
454+
455+
// Enforce Ingress policy?
456+
if (enforce_policy_on_l7lb_) {
457+
ingress_source_identity = source_identity;
458+
ingress_policy_name = ingress_ip->addressAsString();
459+
}
450460

451-
auto new_source_identity = resolvePolicyId(ingress_ip);
452-
if (new_source_identity == Cilium::ID::WORLD) {
461+
// Resolve source identity for the Ingress address
462+
source_identity = resolvePolicyId(ingress_ip);
463+
if (source_identity == Cilium::ID::WORLD) {
453464
// No security ID available for the configured source IP
454465
ENVOY_LOG(warn,
455466
"cilium.bpf_metadata (north/south L7 LB): Unknown local Ingress IP source address "
456467
"configured: {}",
457-
ingress_ip_str);
468+
ingress_ip->addressAsString());
458469
return absl::nullopt;
459470
}
460471

461-
// Enforce ingress policy on the incoming Ingress traffic?
462-
if (enforce_policy_on_l7lb_)
463-
ingress_source_identity = source_identity;
464-
465-
source_identity = new_source_identity;
466-
467-
// AllowAllEgressPolicy will be returned if no explicit Ingress policy exists
468-
policy = &getPolicy(ingress_ip_str);
469-
470-
// Set Ingress source IP as pod_ip (In case of egress (how N/S L7 LB is implemented), the pod_ip
471-
// is the source IP)
472-
pod_ip = ingress_ip_str;
473-
474472
// Original source address is never used for north/south LB
475473
src_address = nullptr;
476474
} else if (!use_original_source_address_ || (npmap_ != nullptr && npmap_->exists(other_ip))) {
@@ -511,9 +509,9 @@ Config::extractSocketMetadata(Network::ConnectionSocket& socket) {
511509
}
512510
return absl::optional(Cilium::BpfMetadata::SocketMetadata(
513511
mark, ingress_source_identity, source_identity, is_ingress_, is_l7lb_, dip->port(),
514-
std::move(pod_ip), std::move(src_address), std::move(source_addresses.ipv4_),
515-
std::move(source_addresses.ipv6_), std::move(dst_address), weak_from_this(), proxy_id_,
516-
std::move(proxylib_l7proto), sni));
512+
std::move(pod_ip), std::move(ingress_policy_name), std::move(src_address),
513+
std::move(source_addresses.ipv4_), std::move(source_addresses.ipv6_), std::move(dst_address),
514+
weak_from_this(), proxy_id_, std::move(proxylib_l7proto), sni));
517515
}
518516

519517
Network::FilterStatus Instance::onAccept(Network::ListenerFilterCallbacks& cb) {

‎cilium/bpf_metadata.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ namespace BpfMetadata {
3333
struct SocketMetadata : public Logger::Loggable<Logger::Id::filter> {
3434
SocketMetadata(uint32_t mark, uint32_t ingress_source_identity, uint32_t source_identity,
3535
bool ingress, bool l7lb, uint16_t port, std::string&& pod_ip,
36+
std::string&& ingress_policy_name,
3637
Network::Address::InstanceConstSharedPtr original_source_address,
3738
Network::Address::InstanceConstSharedPtr source_address_ipv4,
3839
Network::Address::InstanceConstSharedPtr source_address_ipv6,
@@ -41,7 +42,8 @@ struct SocketMetadata : public Logger::Loggable<Logger::Id::filter> {
4142
std::string&& proxylib_l7_proto, absl::string_view sni)
4243
: ingress_source_identity_(ingress_source_identity), source_identity_(source_identity),
4344
ingress_(ingress), is_l7lb_(l7lb), port_(port), pod_ip_(std::move(pod_ip)),
44-
proxy_id_(proxy_id), proxylib_l7_proto_(std::move(proxylib_l7_proto)), sni_(sni),
45+
ingress_policy_name_(std::move(ingress_policy_name)), proxy_id_(proxy_id),
46+
proxylib_l7_proto_(std::move(proxylib_l7_proto)), sni_(sni),
4547
policy_resolver_(policy_resolver), mark_(mark),
4648
original_source_address_(std::move(original_source_address)),
4749
source_address_ipv4_(std::move(source_address_ipv4)),
@@ -51,7 +53,7 @@ struct SocketMetadata : public Logger::Loggable<Logger::Id::filter> {
5153
std::shared_ptr<Envoy::Cilium::CiliumPolicyFilterState> buildCiliumPolicyFilterState() {
5254
return std::make_shared<Envoy::Cilium::CiliumPolicyFilterState>(
5355
ingress_source_identity_, source_identity_, ingress_, is_l7lb_, port_, std::move(pod_ip_),
54-
policy_resolver_, proxy_id_, sni_);
56+
std::move(ingress_policy_name_), policy_resolver_, proxy_id_, sni_);
5557
};
5658

5759
std::shared_ptr<Envoy::Cilium::CiliumMarkSocketOption> buildCiliumMarkSocketOption() {
@@ -103,7 +105,8 @@ struct SocketMetadata : public Logger::Loggable<Logger::Id::filter> {
103105
bool ingress_;
104106
bool is_l7lb_;
105107
uint16_t port_;
106-
std::string pod_ip_;
108+
std::string pod_ip_; // pod policy to enforce, if any
109+
std::string ingress_policy_name_; // Ingress policy to enforce, if any
107110
uint32_t proxy_id_;
108111
std::string proxylib_l7_proto_;
109112
std::string sni_;

‎cilium/filter_state_cilium_policy.cc

Lines changed: 122 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,130 @@
1111
namespace Envoy {
1212
namespace Cilium {
1313

14-
const std::string& Envoy::Cilium::CiliumPolicyFilterState::key() {
14+
const std::string& CiliumPolicyFilterState::key() {
1515
CONSTRUCT_ON_FIRST_USE(std::string, "cilium.policy");
1616
}
1717

18+
bool CiliumPolicyFilterState::enforceNetworkPolicy(const Network::Connection& conn,
19+
uint32_t destination_identity,
20+
uint16_t destination_port,
21+
const absl::string_view sni,
22+
/* OUT */ bool& use_proxy_lib,
23+
/* OUT */ std::string& l7_proto,
24+
/* INOUT */ AccessLog::Entry& log_entry) const {
25+
const auto resolver = policy_resolver_.lock();
26+
use_proxy_lib = false;
27+
l7_proto = "";
28+
29+
if (!resolver) {
30+
// No policy resolver
31+
ENVOY_CONN_LOG(debug, "No policy resolver", conn);
32+
return false;
33+
}
34+
35+
// enforce pod policy first, if any
36+
if (pod_ip_.length() > 0) {
37+
const auto& policy = resolver->getPolicy(pod_ip_);
38+
auto remote_id = ingress_ ? source_identity_ : destination_identity;
39+
auto port = ingress_ ? port_ : destination_port;
40+
41+
auto portPolicy = policy.findPortPolicy(ingress_, port);
42+
43+
if (!portPolicy.allowed(remote_id, sni)) {
44+
ENVOY_CONN_LOG(debug, "Pod policy DENY on id: {} port: {} sni: \"{}\"", conn, remote_id,
45+
destination_port, sni);
46+
return false;
47+
}
48+
49+
// populate l7proto_ if available
50+
use_proxy_lib = portPolicy.useProxylib(remote_id, l7_proto);
51+
}
52+
53+
// enforce Ingress policy 2nd, if any
54+
if (ingress_policy_name_.length() > 0) {
55+
log_entry.entry_.set_policy_name(ingress_policy_name_);
56+
const auto& policy = resolver->getPolicy(ingress_policy_name_);
57+
58+
// Enforce ingress policy for Ingress, on the original destination port
59+
if (ingress_source_identity_ != 0) {
60+
auto ingressPortPolicy = policy.findPortPolicy(true, port_);
61+
if (!ingressPortPolicy.allowed(ingress_source_identity_, sni)) {
62+
ENVOY_CONN_LOG(debug,
63+
"Ingress network policy DROP for source identity: {} port: {} sni: \"{}\"",
64+
conn, ingress_source_identity_, destination_port, sni);
65+
return false;
66+
}
67+
}
68+
69+
// Enforce egress policy for Ingress
70+
auto egressPortPolicy = policy.findPortPolicy(false, destination_port);
71+
if (!egressPortPolicy.allowed(destination_identity, sni)) {
72+
ENVOY_CONN_LOG(debug,
73+
"Egress network policy DROP for destination identity: {} port: {} sni: \"{}\"",
74+
conn, destination_identity, destination_port, sni);
75+
return false;
76+
}
77+
}
78+
79+
// Connection allowed by policy
80+
return true;
81+
}
82+
83+
bool CiliumPolicyFilterState::enforceHTTPPolicy(const Network::Connection& conn, bool is_downstream,
84+
uint32_t destination_identity,
85+
uint16_t destination_port,
86+
/* INOUT */ Http::RequestHeaderMap& headers,
87+
/* INOUT */ AccessLog::Entry& log_entry) const {
88+
const auto resolver = policy_resolver_.lock();
89+
90+
if (!resolver) {
91+
// No policy resolver
92+
ENVOY_CONN_LOG(debug, "No policy resolver", conn);
93+
return false;
94+
}
95+
96+
// enforce pod policy first, if any.
97+
// - ingress enforcement in downstream
98+
// - egress enforcement in upstream
99+
// - unless !L7LB, where both are done on downstream filter (only)
100+
// =>
101+
// - is_l7lb_: ingress_ == is_downstream
102+
// - !is_l7lb_: is_downstream
103+
if (pod_ip_.length() > 0 && (is_l7lb_ ? is_downstream == ingress_ : is_downstream)) {
104+
const auto& policy = resolver->getPolicy(pod_ip_);
105+
auto remote_id = ingress_ ? source_identity_ : destination_identity;
106+
auto port = ingress_ ? port_ : destination_port;
107+
if (!policy.allowed(ingress_, remote_id, port, headers, log_entry)) {
108+
ENVOY_CONN_LOG(debug, "Pod HTTP policy DENY on id: {} port: {}", conn, remote_id, port);
109+
return false;
110+
}
111+
}
112+
113+
// enforce Ingress policy 2nd, if any, always on the upstream
114+
if (!is_downstream && ingress_policy_name_.length() > 0) {
115+
log_entry.entry_.set_policy_name(ingress_policy_name_);
116+
const auto& policy = resolver->getPolicy(ingress_policy_name_);
117+
118+
// Enforce ingress policy for Ingress, on the original destination port
119+
if (ingress_source_identity_ != 0) {
120+
if (!policy.allowed(true, ingress_source_identity_, port_, headers, log_entry)) {
121+
ENVOY_CONN_LOG(debug, "Ingress HTTP policy DROP for source identity: {} port: {}", conn,
122+
ingress_source_identity_, port_);
123+
return false;
124+
}
125+
}
126+
127+
// Enforce egress policy for Ingress
128+
if (!policy.allowed(false, destination_identity, destination_port, headers, log_entry)) {
129+
ENVOY_CONN_LOG(debug, "Egress HTTP policy DROP for destination identity: {} port: {}", conn,
130+
destination_identity, destination_port);
131+
return false;
132+
}
133+
}
134+
135+
// Connection allowed by policy
136+
return true;
137+
}
138+
18139
} // namespace Cilium
19140
} // namespace Envoy

‎cilium/filter_state_cilium_policy.h

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,20 @@
33
#include <asm-generic/socket.h>
44
#include <netinet/in.h>
55

6-
#include <cerrno>
76
#include <cstdint>
87
#include <memory>
98
#include <string>
109
#include <utility>
1110

1211
#include "envoy/common/pure.h"
12+
#include "envoy/http/header_map.h"
1313
#include "envoy/network/address.h"
1414
#include "envoy/stream_info/filter_state.h"
1515

1616
#include "source/common/common/logger.h"
1717

1818
#include "absl/strings/string_view.h"
19+
#include "cilium/accesslog.h"
1920
#include "cilium/network_policy.h"
2021
#include "cilium/policy_id.h"
2122

@@ -37,15 +38,18 @@ class CiliumPolicyFilterState : public StreamInfo::FilterState::Object,
3738
public:
3839
CiliumPolicyFilterState(uint32_t ingress_source_identity, uint32_t source_identity, bool ingress,
3940
bool l7lb, uint16_t port, std::string&& pod_ip,
41+
std::string&& ingress_policy_name,
4042
const std::weak_ptr<PolicyResolver>& policy_resolver, uint32_t proxy_id,
4143
absl::string_view sni)
4244
: ingress_source_identity_(ingress_source_identity), source_identity_(source_identity),
4345
ingress_(ingress), is_l7lb_(l7lb), port_(port), pod_ip_(std::move(pod_ip)),
44-
proxy_id_(proxy_id), sni_(sni), policy_resolver_(policy_resolver) {
45-
ENVOY_LOG(debug,
46-
"Cilium CiliumPolicyFilterState(): source_identity: {}, "
47-
"ingress: {}, port: {}, pod_ip: {}, proxy_id: {}, sni: \"{}\"",
48-
source_identity_, ingress_, port_, pod_ip_, proxy_id_, sni_);
46+
ingress_policy_name_(std::move(ingress_policy_name)), proxy_id_(proxy_id), sni_(sni),
47+
policy_resolver_(policy_resolver) {
48+
ENVOY_LOG(
49+
debug,
50+
"Cilium CiliumPolicyFilterState(): source_identity: {}, "
51+
"ingress: {}, port: {}, pod_ip: {}, ingress_policy_name: {}, proxy_id: {}, sni: \"{}\"",
52+
source_identity_, ingress_, port_, pod_ip_, ingress_policy_name_, proxy_id_, sni_);
4953
}
5054

5155
uint32_t resolvePolicyId(const Network::Address::Ip* ip) const {
@@ -62,6 +66,17 @@ class CiliumPolicyFilterState : public StreamInfo::FilterState::Object,
6266
return NetworkPolicyMap::GetDenyAllPolicy();
6367
}
6468

69+
bool enforceNetworkPolicy(const Network::Connection& conn, uint32_t destination_identity,
70+
uint16_t destination_port, const absl::string_view sni,
71+
/* OUT */ bool& use_proxy_lib,
72+
/* OUT */ std::string& l7_proto,
73+
/* INOUT */ AccessLog::Entry& log_entry) const;
74+
75+
bool enforceHTTPPolicy(const Network::Connection& conn, bool is_downstream,
76+
uint32_t destination_identity, uint16_t destination_port,
77+
/* INOUT */ Http::RequestHeaderMap& headers,
78+
/* INOUT */ AccessLog::Entry& log_entry) const;
79+
6580
// policyUseUpstreamDestinationAddress returns 'true' if policy enforcement should be done on the
6681
// basis of the upstream destination address.
6782
bool policyUseUpstreamDestinationAddress() const { return is_l7lb_; }
@@ -75,6 +90,7 @@ class CiliumPolicyFilterState : public StreamInfo::FilterState::Object,
7590
bool is_l7lb_;
7691
uint16_t port_;
7792
std::string pod_ip_;
93+
std::string ingress_policy_name_;
7894
uint32_t proxy_id_;
7995
std::string sni_;
8096

‎cilium/l7policy.cc

Lines changed: 14 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -192,63 +192,46 @@ Http::FilterHeadersStatus AccessFilter::decodeHeaders(Http::RequestHeaderMap& he
192192
return Http::FilterHeadersStatus::StopIteration;
193193
}
194194

195-
uint32_t destination_port = dip->port();
196-
uint32_t destination_identity = policy_fs->resolvePolicyId(dip);
197-
198-
// Policy may have changed since the connection was established, get fresh policy
199-
const auto& policy = policy_fs->getPolicy();
200-
201195
if (log_entry_ == nullptr) {
202196
sendLocalError("cilium.l7policy: No log entry");
203197
return Http::FilterHeadersStatus::StopIteration;
204198
}
205199

206-
bool denied = false;
207-
// Enforce Ingress policy only in the downstream filter
200+
uint32_t destination_identity = policy_fs->resolvePolicyId(dip);
201+
uint16_t destination_port = dip->port();
202+
203+
// Initialize log entry in the beginning of downstream processing
208204
if (!config_->is_upstream_) {
209205
log_entry_->InitFromRequest(
210206
policy_fs->pod_ip_, policy_fs->proxy_id_, policy_fs->ingress_, policy_fs->source_identity_,
211207
callbacks_->streamInfo().downstreamAddressProvider().remoteAddress(), 0,
212208
callbacks_->streamInfo().downstreamAddressProvider().localAddress(),
213209
callbacks_->streamInfo(), headers);
214-
215-
if (policy_fs->ingress_source_identity_ != 0) {
216-
allowed_ = policy.allowed(true, policy_fs->ingress_source_identity_, policy_fs->port_,
217-
headers, *log_entry_);
218-
ENVOY_CONN_LOG(
219-
debug, "cilium.l7policy: Ingress from {} policy lookup for endpoint {} for port {}: {}",
220-
conn.ref(), policy_fs->ingress_source_identity_, policy_fs->pod_ip_, policy_fs->port_,
221-
allowed_ ? "ALLOW" : "DENY");
222-
denied = !allowed_;
223-
}
224-
225-
// Downstream filter leaves L7 LB enforcement and access logging to the upstream
226-
// filter
227-
if (!denied && policy_fs->is_l7lb_) {
228-
return Http::FilterHeadersStatus::Continue;
229-
}
230210
}
231211

232-
if (!denied) {
233-
allowed_ =
234-
policy.allowed(policy_fs->ingress_,
235-
policy_fs->ingress_ ? policy_fs->source_identity_ : destination_identity,
236-
destination_port, headers, *log_entry_);
237-
}
212+
allowed_ = policy_fs->enforceHTTPPolicy(conn.ref(), !config_->is_upstream_, destination_identity,
213+
destination_port, headers, *log_entry_);
214+
238215
ENVOY_CONN_LOG(
239216
debug, "cilium.l7policy: {} ({}->{}) {} policy lookup for endpoint {} for port {}: {}",
240217
conn.ref(), policy_fs->ingress_ ? "ingress" : "egress", policy_fs->source_identity_,
241218
destination_identity, config_->is_upstream_ ? "upstream" : "downstream", policy_fs->pod_ip_,
242219
destination_port, allowed_ ? "ALLOW" : "DENY");
243220

221+
// Downstream filter leaves L7 LB enforcement and access logging to the upstream
222+
// filter
223+
if (!config_->is_upstream_ && allowed_ && policy_fs->is_l7lb_) {
224+
return Http::FilterHeadersStatus::Continue;
225+
}
226+
244227
// Update the log entry with the chosen destination address and current headers, as remaining
245228
// filters, upstream, and/or policy may have altered headers.
246229
log_entry_->UpdateFromRequest(destination_identity, dst_address, headers);
247230

248231
if (!allowed_) {
232+
config_->Log(*log_entry_, ::cilium::EntryType::Denied);
249233
callbacks_->sendLocalReply(Http::Code::Forbidden, config_->denied_403_body_, nullptr,
250234
absl::nullopt, absl::string_view());
251-
config_->Log(*log_entry_, ::cilium::EntryType::Denied);
252235
return Http::FilterHeadersStatus::StopIteration;
253236
}
254237

0 commit comments

Comments
 (0)