diff --git a/cilium/accesslog.cc b/cilium/accesslog.cc index 541bb417e..5f20b7b70 100644 --- a/cilium/accesslog.cc +++ b/cilium/accesslog.cc @@ -63,6 +63,8 @@ void AccessLog::log(AccessLog::Entry& log_entry, ::cilium::EntryType entry_type) log_entry.request_logged_ = true; } + ENVOY_LOG_MISC(trace, "Cilium Access log entry: {}", entry.DebugString()); + // encode protobuf std::string msg; entry.SerializeToString(&msg); diff --git a/cilium/bpf_metadata.cc b/cilium/bpf_metadata.cc index 6c7746e9a..2f184f21c 100644 --- a/cilium/bpf_metadata.cc +++ b/cilium/bpf_metadata.cc @@ -403,16 +403,18 @@ Config::extractSocketMetadata(Network::ConnectionSocket& socket) { // Use a pointer as we may need to change the policy in the case of "North/South L7 LB" below. const auto* policy = &getPolicy(pod_ip); - // Resolve the source security ID from conntrack map, or from ip cache - uint32_t source_identity = resolveSourceIdentity(*policy, sip, dip, is_ingress_, is_l7lb_); + // original_source_identity is set to the original source identity and is used in policy logs and + // access logging. This is typically the same as 'source_identity', but for north/south L7 LB, + // where the upstream source address is set to an Ingress IP, the 'source_identity' will be that + // if the Ingress IP. This original source identity is still used in policy logs and access + // logging even in that case. + uint32_t original_source_identity = + resolveSourceIdentity(*policy, sip, dip, is_ingress_, is_l7lb_); + uint32_t source_identity = original_source_identity; // Resolve the destination security ID for egress traffic uint32_t destination_identity = is_ingress_ ? 0 : resolvePolicyId(dip); - // ingress_source_identity is non-zero when the egress path l7 LB should also enforce - // the ingress path policy using the original source identity. - uint32_t ingress_source_identity = 0; - // Use the configured IPv4/IPv6 Ingress IPs as starting point for the sources addresses IpAddressPair source_addresses(ipv4_source_address_, ipv6_source_address_); @@ -463,7 +465,6 @@ Config::extractSocketMetadata(Network::ConnectionSocket& socket) { // Enforce Ingress policy? if (enforce_policy_on_l7lb_) { - ingress_source_identity = source_identity; ingress_policy_name = l7lb_policy_name_.empty() ? ingress_ip->addressAsString() : l7lb_policy_name_; } @@ -519,15 +520,15 @@ Config::extractSocketMetadata(Network::ConnectionSocket& socket) { } ENVOY_LOG(trace, - "cilium.bpf_metadata: mark {}, ingress_source_identity {}, source_identity {}, " + "cilium.bpf_metadata: mark {}, original_source_identity {}, source_identity {}, " "is_ingress {}, is_l7lb_ {}, ingress_policy_name {}, port {}, pod_ip {}", - mark, ingress_source_identity, source_identity, is_ingress_, is_l7lb_, + mark, original_source_identity, source_identity, is_ingress_, is_l7lb_, ingress_policy_name, dip->port(), pod_ip); return {Cilium::BpfMetadata::SocketMetadata( - mark, ingress_source_identity, source_identity, is_ingress_, is_l7lb_, dip->port(), - std::move(pod_ip), std::move(ingress_policy_name), std::move(src_address), - std::move(source_addresses.ipv4_), std::move(source_addresses.ipv6_), std::move(dst_address), - shared_from_this(), proxy_id_, std::move(proxylib_l7proto), sni)}; + mark, original_source_identity, is_ingress_, is_l7lb_, dip->port(), std::move(pod_ip), + std::move(ingress_policy_name), std::move(src_address), std::move(source_addresses.ipv4_), + std::move(source_addresses.ipv6_), std::move(dst_address), shared_from_this(), proxy_id_, + std::move(proxylib_l7proto), sni)}; } Network::FilterStatus Instance::onAccept(Network::ListenerFilterCallbacks& cb) { diff --git a/cilium/bpf_metadata.h b/cilium/bpf_metadata.h index 74d68c067..c5d055d6b 100644 --- a/cilium/bpf_metadata.h +++ b/cilium/bpf_metadata.h @@ -35,19 +35,17 @@ namespace BpfMetadata { #define DEFAULT_CACHE_ENTRY_TTL_MS 3 struct SocketMetadata : public Logger::Loggable { - SocketMetadata(uint32_t mark, uint32_t ingress_source_identity, uint32_t source_identity, - bool ingress, bool l7lb, uint16_t port, std::string&& pod_ip, - std::string&& ingress_policy_name, + SocketMetadata(uint32_t mark, uint32_t source_identity, bool ingress, bool l7lb, uint16_t port, + std::string&& pod_ip, std::string&& ingress_policy_name, Network::Address::InstanceConstSharedPtr original_source_address, Network::Address::InstanceConstSharedPtr source_address_ipv4, Network::Address::InstanceConstSharedPtr source_address_ipv6, Network::Address::InstanceConstSharedPtr original_dest_address, const PolicyResolverSharedPtr& policy_resolver, uint32_t proxy_id, std::string&& proxylib_l7_proto, absl::string_view sni) - : ingress_source_identity_(ingress_source_identity), source_identity_(source_identity), - ingress_(ingress), is_l7lb_(l7lb), port_(port), pod_ip_(std::move(pod_ip)), - ingress_policy_name_(std::move(ingress_policy_name)), proxy_id_(proxy_id), - proxylib_l7_proto_(std::move(proxylib_l7_proto)), sni_(sni), + : source_identity_(source_identity), ingress_(ingress), is_l7lb_(l7lb), port_(port), + pod_ip_(std::move(pod_ip)), ingress_policy_name_(std::move(ingress_policy_name)), + proxy_id_(proxy_id), proxylib_l7_proto_(std::move(proxylib_l7_proto)), sni_(sni), policy_resolver_(policy_resolver), mark_(mark), original_source_address_(std::move(original_source_address)), source_address_ipv4_(std::move(source_address_ipv4)), @@ -56,7 +54,7 @@ struct SocketMetadata : public Logger::Loggable { std::shared_ptr buildCiliumPolicyFilterState() { return std::make_shared( - ingress_source_identity_, source_identity_, ingress_, is_l7lb_, port_, std::move(pod_ip_), + source_identity_, ingress_, is_l7lb_, port_, std::move(pod_ip_), std::move(ingress_policy_name_), policy_resolver_, proxy_id_, sni_); }; @@ -111,7 +109,6 @@ struct SocketMetadata : public Logger::Loggable { socket.connectionInfoProvider().restoreLocalAddress(original_dest_address_); } - uint32_t ingress_source_identity_; uint32_t source_identity_; bool ingress_; bool is_l7lb_; @@ -165,8 +162,8 @@ class Config : public Cilium::PolicyResolver, bool enforce_policy_on_l7lb_; std::string l7lb_policy_name_; std::chrono::milliseconds ipcache_entry_ttl_; - Random::RandomGenerator& random_; + Random::RandomGenerator& random_; std::shared_ptr npmap_{}; Cilium::CtMapSharedPtr ct_maps_{}; Cilium::IpCacheSharedPtr ipcache_{}; diff --git a/cilium/filter_state_cilium_policy.cc b/cilium/filter_state_cilium_policy.cc index ca292597c..43cc8f6e8 100644 --- a/cilium/filter_state_cilium_policy.cc +++ b/cilium/filter_state_cilium_policy.cc @@ -53,15 +53,13 @@ bool CiliumPolicyFilterState::enforceNetworkPolicy(const Network::Connection& co const auto& policy = policy_resolver_->getPolicy(ingress_policy_name_); // Enforce ingress policy for Ingress, on the original destination port - if (ingress_source_identity_ != 0) { - auto ingress_port_policy = policy.findPortPolicy(true, port_); - if (!ingress_port_policy.allowed(proxy_id_, ingress_source_identity_, sni)) { - ENVOY_CONN_LOG(debug, - "Ingress network policy {} DROP for source identity and destination " - "reserved ingress identity: {} proxy_id: {} port: {} sni: \"{}\"", - conn, ingress_policy_name_, ingress_source_identity_, proxy_id_, port_, sni); - return false; - } + auto ingress_port_policy = policy.findPortPolicy(true, port_); + if (!ingress_port_policy.allowed(proxy_id_, source_identity_, sni)) { + ENVOY_CONN_LOG(debug, + "Ingress network policy {} DROP for source identity and destination " + "reserved ingress identity: {} proxy_id: {} port: {} sni: \"{}\"", + conn, ingress_policy_name_, source_identity_, proxy_id_, port_, sni); + return false; } // Enforce egress policy for Ingress @@ -122,28 +120,26 @@ bool CiliumPolicyFilterState::enforceIngressHTTPPolicy( const auto& policy = policy_resolver_->getPolicy(ingress_policy_name_); // Enforce ingress policy for Ingress, on the original destination port - if (ingress_source_identity_ != 0) { - const auto port_policy = policy.findPortPolicy(true, port_); - if (!port_policy.hasHttpRules()) { - ENVOY_CONN_LOG(debug, - "cilium.l7policy: Ingress {} HTTP ingress policy enforcement skipped (no HTTP " - "rules) on proxy_id: {} id: {} port: {}", - conn, ingress_policy_name_, proxy_id_, ingress_source_identity_, port_); - return true; - } + const auto ingress_policy = policy.findPortPolicy(true, port_); + if (!ingress_policy.hasHttpRules()) { + ENVOY_CONN_LOG(debug, + "cilium.l7policy: Ingress {} HTTP ingress policy enforcement skipped (no HTTP " + "rules) on proxy_id: {} id: {} port: {}", + conn, ingress_policy_name_, proxy_id_, source_identity_, port_); + return true; + } - if (!port_policy.allowed(proxy_id_, ingress_source_identity_, headers, log_entry)) { - ENVOY_CONN_LOG( - debug, - "cilium.l7policy: Ingress {} HTTP ingress policy DROP on proxy_id: {} id: {} port: {}", - conn, ingress_policy_name_, proxy_id_, ingress_source_identity_, port_); - return false; - } + if (!ingress_policy.allowed(proxy_id_, source_identity_, headers, log_entry)) { + ENVOY_CONN_LOG( + debug, + "cilium.l7policy: Ingress {} HTTP ingress policy DROP on proxy_id: {} id: {} port: {}", + conn, ingress_policy_name_, proxy_id_, source_identity_, port_); + return false; } // Enforce egress policy for Ingress on the upstream destination identity and port - const auto port_policy = policy.findPortPolicy(false, destination_port); - if (!port_policy.hasHttpRules()) { + const auto egress_policy = policy.findPortPolicy(false, destination_port); + if (!egress_policy.hasHttpRules()) { ENVOY_CONN_LOG(debug, "cilium.l7policy: Ingress {} HTTP egress policy enforcement skipped (no HTTP " "rules) on proxy_id: {} id: {} port: {}", @@ -151,7 +147,7 @@ bool CiliumPolicyFilterState::enforceIngressHTTPPolicy( return true; } - if (!port_policy.allowed(proxy_id_, destination_identity, headers, log_entry)) { + if (!egress_policy.allowed(proxy_id_, destination_identity, headers, log_entry)) { ENVOY_CONN_LOG( debug, "cilium.l7policy: Ingress {} HTTP egress policy DROP on proxy_id: {} id: {} port: {}", diff --git a/cilium/filter_state_cilium_policy.h b/cilium/filter_state_cilium_policy.h index f90632400..960a84db7 100644 --- a/cilium/filter_state_cilium_policy.h +++ b/cilium/filter_state_cilium_policy.h @@ -34,15 +34,13 @@ using PolicyResolverSharedPtr = std::shared_ptr; class CiliumPolicyFilterState : public StreamInfo::FilterState::Object, public Logger::Loggable { public: - CiliumPolicyFilterState(uint32_t ingress_source_identity, uint32_t source_identity, bool ingress, - bool l7lb, uint16_t port, std::string&& pod_ip, - std::string&& ingress_policy_name, + CiliumPolicyFilterState(uint32_t source_identity, bool ingress, bool l7lb, uint16_t port, + std::string&& pod_ip, std::string&& ingress_policy_name, const PolicyResolverSharedPtr& policy_resolver, uint32_t proxy_id, absl::string_view sni) - : ingress_source_identity_(ingress_source_identity), source_identity_(source_identity), - ingress_(ingress), is_l7lb_(l7lb), port_(port), pod_ip_(std::move(pod_ip)), - ingress_policy_name_(std::move(ingress_policy_name)), proxy_id_(proxy_id), sni_(sni), - policy_resolver_(policy_resolver) { + : source_identity_(source_identity), ingress_(ingress), is_l7lb_(l7lb), port_(port), + pod_ip_(std::move(pod_ip)), ingress_policy_name_(std::move(ingress_policy_name)), + proxy_id_(proxy_id), sni_(sni), policy_resolver_(policy_resolver) { ENVOY_LOG( debug, "Cilium CiliumPolicyFilterState(): source_identity: {}, " @@ -79,7 +77,6 @@ class CiliumPolicyFilterState : public StreamInfo::FilterState::Object, static const std::string& key(); // Additional ingress policy enforcement is performed if ingress_source_identity is non-zero - uint32_t ingress_source_identity_; uint32_t source_identity_; bool ingress_; bool is_l7lb_; diff --git a/cilium/l7policy.cc b/cilium/l7policy.cc index d1161ea00..f61aab27b 100644 --- a/cilium/l7policy.cc +++ b/cilium/l7policy.cc @@ -269,7 +269,7 @@ Http::FilterHeadersStatus AccessFilter::decodeHeaders(Http::RequestHeaderMap& he } // Is there an Ingress policy? - if (policy_fs->ingress_policy_name_.length() > 0) { + if (!policy_fs->ingress_policy_name_.empty()) { allowed = policy_fs->enforceIngressHTTPPolicy(conn.ref(), destination_identity, destination_port, headers, *log_entry_); diff --git a/cilium/network_policy.cc b/cilium/network_policy.cc index 5321296f0..75500b64c 100644 --- a/cilium/network_policy.cc +++ b/cilium/network_policy.cc @@ -4,7 +4,6 @@ #include #include -#include #include #include #include diff --git a/tests/bpf_metadata.cc b/tests/bpf_metadata.cc index b10829bf5..d1f9a5f08 100644 --- a/tests/bpf_metadata.cc +++ b/tests/bpf_metadata.cc @@ -191,7 +191,7 @@ TestConfig::extractSocketMetadata(Network::ConnectionSocket& socket) { port, l7proto); return {Cilium::BpfMetadata::SocketMetadata( - 0, 0, source_identity, is_ingress_, is_l7lb_, port, std::move(pod_ip), "", nullptr, nullptr, + 0, source_identity, is_ingress_, is_l7lb_, port, std::move(pod_ip), "", nullptr, nullptr, nullptr, original_dst_address, shared_from_this(), 0, std::move(l7proto), "")}; } diff --git a/tests/metadata_config_test.cc b/tests/metadata_config_test.cc index e3a237198..f1969e702 100644 --- a/tests/metadata_config_test.cc +++ b/tests/metadata_config_test.cc @@ -300,13 +300,12 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbMetadata) { auto policy_fs = socket_metadata->buildCiliumPolicyFilterState(); EXPECT_NE(nullptr, policy_fs); - EXPECT_EQ(8, policy_fs->source_identity_); + EXPECT_EQ(12345678, policy_fs->source_identity_); EXPECT_EQ(false, policy_fs->ingress_); EXPECT_EQ(true, policy_fs->is_l7lb_); EXPECT_EQ(80, policy_fs->port_); EXPECT_EQ("", policy_fs->pod_ip_); EXPECT_EQ("", policy_fs->ingress_policy_name_); - EXPECT_EQ(0, policy_fs->ingress_source_identity_); auto source_addresses_socket_option = socket_metadata->buildSourceAddressSocketOption(-1); EXPECT_NE(nullptr, source_addresses_socket_option); @@ -341,13 +340,12 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbIngressEnforcedMetadata) { auto policy_fs = socket_metadata->buildCiliumPolicyFilterState(); EXPECT_NE(nullptr, policy_fs); - EXPECT_EQ(8, policy_fs->source_identity_); + EXPECT_EQ(12345678, policy_fs->source_identity_); EXPECT_EQ(false, policy_fs->ingress_); EXPECT_EQ(true, policy_fs->is_l7lb_); EXPECT_EQ(80, policy_fs->port_); EXPECT_EQ("", policy_fs->pod_ip_); EXPECT_EQ("10.1.1.42", policy_fs->ingress_policy_name_); - EXPECT_EQ(12345678, policy_fs->ingress_source_identity_); AccessLog::Entry log_entry; log_entry.entry_.set_policy_name("pod"); @@ -411,13 +409,12 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbPodAndIngressEnforcedMetadata) { auto policy_fs = socket_metadata->buildCiliumPolicyFilterState(); EXPECT_NE(nullptr, policy_fs); - EXPECT_EQ(8, policy_fs->source_identity_); + EXPECT_EQ(9999, policy_fs->source_identity_); EXPECT_EQ(false, policy_fs->ingress_); EXPECT_EQ(true, policy_fs->is_l7lb_); EXPECT_EQ(80, policy_fs->port_); EXPECT_EQ("192.168.1.1", policy_fs->pod_ip_); EXPECT_EQ("10.1.1.42", policy_fs->ingress_policy_name_); - EXPECT_EQ(9999, policy_fs->ingress_source_identity_); AccessLog::Entry log_entry; log_entry.entry_.set_policy_name("pod"); @@ -482,13 +479,12 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbIngressEnforcedCIDRMetadata) { auto policy_fs = socket_metadata->buildCiliumPolicyFilterState(); EXPECT_NE(nullptr, policy_fs); - EXPECT_EQ(8, policy_fs->source_identity_); + EXPECT_EQ(2, policy_fs->source_identity_); EXPECT_EQ(false, policy_fs->ingress_); EXPECT_EQ(true, policy_fs->is_l7lb_); EXPECT_EQ(80, policy_fs->port_); EXPECT_EQ("", policy_fs->pod_ip_); EXPECT_EQ("10.1.1.42", policy_fs->ingress_policy_name_); - EXPECT_EQ(2, policy_fs->ingress_source_identity_); AccessLog::Entry log_entry; log_entry.entry_.set_policy_name("pod"); @@ -590,13 +586,12 @@ TEST_F(MetadataConfigTest, EastWestL7LbMetadataNoOriginalSource) { auto policy_fs = socket_metadata->buildCiliumPolicyFilterState(); EXPECT_NE(nullptr, policy_fs); - EXPECT_EQ(8, policy_fs->source_identity_); + EXPECT_EQ(111, policy_fs->source_identity_); EXPECT_EQ(false, policy_fs->ingress_); EXPECT_EQ(true, policy_fs->is_l7lb_); EXPECT_EQ(80, policy_fs->port_); EXPECT_EQ("10.1.1.1", policy_fs->pod_ip_); EXPECT_EQ("", policy_fs->ingress_policy_name_); - EXPECT_EQ(0, policy_fs->ingress_source_identity_); auto source_addresses_socket_option = socket_metadata->buildSourceAddressSocketOption(-1); EXPECT_NE(nullptr, source_addresses_socket_option);