diff --git a/cilium/network_filter.cc b/cilium/network_filter.cc index db67badbc..b86474a2d 100644 --- a/cilium/network_filter.cc +++ b/cilium/network_filter.cc @@ -135,6 +135,13 @@ Network::FilterStatus Instance::onNewConnection() { ENVOY_LOG(warn, "cilium.network (egress): No destination address "); return false; } + + const auto& policy = option->getPolicy(); + if (!policy) { + ENVOY_LOG_MISC(warn, "cilium.network: No policy found for pod {}", option->pod_ip_); + return false; + } + if (!option->ingress_) { const auto dip = dst_address->ip(); if (!dip) { @@ -146,7 +153,7 @@ Network::FilterStatus Instance::onNewConnection() { destination_identity = option->resolvePolicyId(dip); if (option->ingress_source_identity_ != 0) { - auto ingress_port_policy = option->initial_policy_->findPortPolicy(true, destination_port_); + auto ingress_port_policy = policy->findPortPolicy(true, destination_port_); if (!ingress_port_policy.allowed(option->ingress_source_identity_, sni)) { ENVOY_CONN_LOG( debug, @@ -157,7 +164,7 @@ Network::FilterStatus Instance::onNewConnection() { } } - auto port_policy = option->initial_policy_->findPortPolicy(option->ingress_, destination_port_); + auto port_policy = policy->findPortPolicy(option->ingress_, destination_port_); remote_id_ = option->ingress_ ? option->identity_ : destination_identity; if (!port_policy.allowed(remote_id_, sni)) { diff --git a/cilium/socket_option.h b/cilium/socket_option.h index f05862b00..2014ca7c9 100644 --- a/cilium/socket_option.h +++ b/cilium/socket_option.h @@ -15,12 +15,16 @@ namespace Cilium { class PolicyInstance; using PolicyInstanceConstSharedPtr = std::shared_ptr; +using PolicyInstanceConstWeakPtr = std::weak_ptr; class PolicyResolver { public: virtual ~PolicyResolver() = default; virtual uint32_t resolvePolicyId(const Network::Address::Ip*) const PURE; + + // getPolicy gets a shared pointer to the policy instance, looking up a fresh one if the + // initial policy is no longer valid. virtual const PolicyInstanceConstSharedPtr getPolicy(const std::string&) const PURE; }; @@ -221,8 +225,10 @@ class SocketMarkOption : public Network::Socket::Option, }; class SocketOption : public SocketMarkOption { + friend class MetadataConfigTest; + public: - SocketOption(PolicyInstanceConstSharedPtr policy, uint32_t mark, uint32_t ingress_source_identity, + SocketOption(PolicyInstanceConstWeakPtr policy, uint32_t mark, uint32_t ingress_source_identity, uint32_t source_identity, bool ingress, bool l7lb, uint16_t port, std::string&& pod_ip, Network::Address::InstanceConstSharedPtr original_source_address, @@ -232,9 +238,9 @@ class SocketOption : public SocketMarkOption { absl::string_view sni) : SocketMarkOption(mark, source_identity, original_source_address, ipv4_source_address, ipv6_source_address), - ingress_source_identity_(ingress_source_identity), initial_policy_(policy), - ingress_(ingress), is_l7lb_(l7lb), port_(port), pod_ip_(std::move(pod_ip)), - proxy_id_(proxy_id), sni_(sni), policy_id_resolver_(policy_id_resolver) { + ingress_source_identity_(ingress_source_identity), ingress_(ingress), is_l7lb_(l7lb), + port_(port), pod_ip_(std::move(pod_ip)), proxy_id_(proxy_id), sni_(sni), + initial_policy_(policy), policy_id_resolver_(policy_id_resolver) { ENVOY_LOG(debug, "Cilium SocketOption(): source_identity: {}, " "ingress: {}, port: {}, pod_ip: {}, source_addresses: {}/{}/{}, mark: {:x} (magic " @@ -244,7 +250,6 @@ class SocketOption : public SocketMarkOption { ipv4_source_address_ ? ipv4_source_address_->asString() : "", ipv6_source_address_ ? ipv6_source_address_->asString() : "", mark_, mark & 0xff00, mark & 0xff, mark >> 16, proxy_id_, sni_); - ASSERT(initial_policy_ != nullptr); } uint32_t resolvePolicyId(const Network::Address::Ip* ip) const { @@ -252,6 +257,11 @@ class SocketOption : public SocketMarkOption { } const PolicyInstanceConstSharedPtr getPolicy() const { + const auto& policy = initial_policy_.lock(); + if (policy) { + return policy; + } + // Get fresh policy if the initial policy no longer exists return policy_id_resolver_->getPolicy(pod_ip_); } @@ -261,13 +271,13 @@ class SocketOption : public SocketMarkOption { // Additional ingress policy enforcement is performed if ingress_source_identity is non-zero uint32_t ingress_source_identity_; - const PolicyInstanceConstSharedPtr initial_policy_; // Never NULL bool ingress_; bool is_l7lb_; uint16_t port_; std::string pod_ip_; uint32_t proxy_id_; std::string sni_; + const PolicyInstanceConstWeakPtr initial_policy_; private: const std::shared_ptr policy_id_resolver_; diff --git a/cilium/tls_wrapper.cc b/cilium/tls_wrapper.cc index acbf9ef96..0dfb68b1e 100644 --- a/cilium/tls_wrapper.cc +++ b/cilium/tls_wrapper.cc @@ -35,6 +35,13 @@ class SslSocketWrapper : public Network::TransportSocket { // TLS a raw socket is used instead, const auto option = Cilium::GetSocketOption(callbacks.connection().socketOptions()); if (option) { + const auto& policy = option->getPolicy(); + if (!policy) { + policy->tlsWrapperMissingPolicyInc(); + ENVOY_LOG_MISC(warn, "cilium.tls_wrapper: No policy found for pod {}", option->pod_ip_); + return; + } + // Resolve the destination security ID and port uint32_t destination_identity = 0; uint32_t destination_port = option->port_; @@ -63,8 +70,7 @@ class SslSocketWrapper : public Network::TransportSocket { const auto& sni = option->sni_; auto remote_id = option->ingress_ ? option->identity_ : destination_identity; - auto port_policy = - option->initial_policy_->findPortPolicy(option->ingress_, destination_port); + auto port_policy = policy->findPortPolicy(option->ingress_, destination_port); const Envoy::Ssl::ContextConfig* config = nullptr; bool raw_socket_allowed = false; Envoy::Ssl::ContextSharedPtr ctx = @@ -89,7 +95,7 @@ class SslSocketWrapper : public Network::TransportSocket { // Set the callbacks socket_->setTransportSocketCallbacks(callbacks); } else { - option->initial_policy_->tlsWrapperMissingPolicyInc(); + policy->tlsWrapperMissingPolicyInc(); std::string ipStr(""); if (option->ingress_) { diff --git a/tests/metadata_config_test.cc b/tests/metadata_config_test.cc index 68c5bd422..941b7af7c 100644 --- a/tests/metadata_config_test.cc +++ b/tests/metadata_config_test.cc @@ -263,7 +263,7 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbMetadata) { EXPECT_EQ("[face::42]:0", option->ipv6_source_address_->asString()); EXPECT_EQ(80, option->port_); EXPECT_EQ("10.1.1.42", option->pod_ip_); - EXPECT_NE(nullptr, option->initial_policy_); + EXPECT_NE(nullptr, option->initial_policy_.lock()); EXPECT_EQ(0, option->ingress_source_identity_); // Check that Ingress security ID is used in the socket mark @@ -296,14 +296,14 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbIngressEnforcedMetadata) { EXPECT_EQ("[face::42]:0", option->ipv6_source_address_->asString()); EXPECT_EQ(80, option->port_); EXPECT_EQ("10.1.1.42", option->pod_ip_); - EXPECT_NE(nullptr, option->initial_policy_); + EXPECT_NE(nullptr, option->initial_policy_.lock()); EXPECT_EQ(12345678, option->ingress_source_identity_); // Check that Ingress security ID is used in the socket mark EXPECT_TRUE((option->mark_ & 0xffff) == 0x0B00 && (option->mark_ >> 16) == 8); // Expect policy accepts security ID 12345678 on ingress on port 80 - auto port_policy = option->initial_policy_->findPortPolicy(true, 80); + auto port_policy = option->getPolicy()->findPortPolicy(true, 80); EXPECT_TRUE(port_policy.allowed(12345678, "")); } @@ -333,14 +333,14 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbIngressEnforcedCIDRMetadata) { EXPECT_EQ("[face::42]:0", option->ipv6_source_address_->asString()); EXPECT_EQ(80, option->port_); EXPECT_EQ("10.1.1.42", option->pod_ip_); - EXPECT_NE(nullptr, option->initial_policy_); + EXPECT_NE(nullptr, option->initial_policy_.lock()); EXPECT_EQ(2, option->ingress_source_identity_); // Check that Ingress security ID is used in the socket mark EXPECT_TRUE((option->mark_ & 0xffff) == 0x0B00 && (option->mark_ >> 16) == 8); // Expect policy does not accept security ID 2 on ingress on port 80 - auto port_policy = option->initial_policy_->findPortPolicy(true, 80); + auto port_policy = option->getPolicy()->findPortPolicy(true, 80); EXPECT_FALSE(port_policy.allowed(2, "")); } @@ -387,7 +387,7 @@ TEST_F(MetadataConfigTest, EastWestL7LbMetadata) { EXPECT_EQ("[face::1:1:1]:41234", option->ipv6_source_address_->asString()); EXPECT_EQ(80, option->port_); EXPECT_EQ("10.1.1.1", option->pod_ip_); - EXPECT_NE(nullptr, option->initial_policy_); + EXPECT_NE(nullptr, option->initial_policy_.lock()); // Check that Endpoint's ID is used in the socket mark EXPECT_TRUE((option->mark_ & 0xffff) == 0x0900 && (option->mark_ >> 16) == 2048); @@ -417,7 +417,7 @@ TEST_F(MetadataConfigTest, EastWestL7LbMetadataNoOriginalSource) { EXPECT_EQ("[face::42]:0", option->ipv6_source_address_->asString()); EXPECT_EQ(80, option->port_); EXPECT_EQ("10.1.1.42", option->pod_ip_); - EXPECT_NE(nullptr, option->initial_policy_); + EXPECT_NE(nullptr, option->initial_policy_.lock()); EXPECT_EQ(0, option->ingress_source_identity_); // Check that Ingress ID is used in the socket mark