diff --git a/WORKSPACE b/WORKSPACE index 9bd631e81..baf025564 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -37,13 +37,14 @@ git_repository( patch_args = ["apply"], patch_tool = "git", patches = [ - "@//patches:0001-network-Add-callback-for-upstream-authorization.patch", - "@//patches:0002-listener-add-socket-options.patch", - "@//patches:0003-original_dst_cluster-Avoid-multiple-hosts-for-the-sa.patch", - "@//patches:0004-tcp_proxy-Check-for-nullptr-in-watermark-ASSERTs.patch", - "@//patches:0005-thread_local-reset-slot-in-worker-threads-first.patch", - "@//patches:0006-http-header-expose-attribute.patch", - "@//patches:0007-liburing-arm-build.patch", + "@//patches:0001-listener-add-socket-options.patch", + "@//patches:0002-original_dst_cluster-Avoid-multiple-hosts-for-the-sa.patch", + "@//patches:0003-tcp_proxy-Check-for-nullptr-in-watermark-ASSERTs.patch", + "@//patches:0004-thread_local-reset-slot-in-worker-threads-first.patch", + "@//patches:0005-Expose-HTTP-Header-matcher-attribute.patch", + "@//patches:0006-build-Fix-arm-build-for-liburing.patch", + "@//patches:0007-network-Add-filter-callback-onDestinationSelected.patch", + "@//patches:0008-network-Compat-for-missing-upstream-filter.patch", ], # // clang-format off: Envoy's format check: Only repository_locations.bzl may contains URL references remote = "https://github.com/envoyproxy/envoy.git", diff --git a/cilium/filter_state_cilium_policy.cc b/cilium/filter_state_cilium_policy.cc index ca292597c..142fafcda 100644 --- a/cilium/filter_state_cilium_policy.cc +++ b/cilium/filter_state_cilium_policy.cc @@ -10,6 +10,7 @@ #include "source/common/common/macros.h" #include "absl/strings/string_view.h" +#include "absl/types/optional.h" #include "cilium/accesslog.h" namespace Envoy { @@ -19,64 +20,75 @@ const std::string& CiliumPolicyFilterState::key() { CONSTRUCT_ON_FIRST_USE(std::string, "cilium.policy"); } -bool CiliumPolicyFilterState::enforceNetworkPolicy(const Network::Connection& conn, - uint32_t destination_identity, - uint16_t destination_port, - const absl::string_view sni, - /* OUT */ bool& use_proxy_lib, - /* OUT */ std::string& l7_proto, - /* INOUT */ AccessLog::Entry& log_entry) const { +bool CiliumPolicyFilterState::enforcePodNetworkPolicy(const Network::Connection& conn, + uint32_t destination_identity, + uint16_t destination_port, + const absl::string_view sni, + /* OUT */ bool& use_proxy_lib, + /* OUT */ std::string& l7_proto) const { + auto remote_id = ingress_ ? source_identity_ : destination_identity; + const auto& policy = policy_resolver_->getPolicy(pod_ip_); + auto port = ingress_ ? port_ : destination_port; + auto port_policy = policy.findPortPolicy(ingress_, port); + use_proxy_lib = false; l7_proto = ""; - // enforce pod policy first, if any - if (pod_ip_.length() > 0) { - const auto& policy = policy_resolver_->getPolicy(pod_ip_); - auto remote_id = ingress_ ? source_identity_ : destination_identity; - auto port = ingress_ ? port_ : destination_port; - - auto port_policy = policy.findPortPolicy(ingress_, port); - - if (!port_policy.allowed(proxy_id_, remote_id, sni)) { - ENVOY_CONN_LOG(debug, "Pod policy DENY on proxy_id: {} id: {} port: {} sni: \"{}\"", conn, - proxy_id_, remote_id, port, sni); - return false; - } - - // populate l7proto_ if available - use_proxy_lib = port_policy.useProxylib(proxy_id_, remote_id, l7_proto); + if (!port_policy.allowed(proxy_id_, remote_id, sni)) { + ENVOY_CONN_LOG(debug, + "cilium.network: Pod {} network {} policy DENY on proxy_id: {} id: {} port: {} " + "sni: \"{}\"", + conn, pod_ip_, ingress_ ? "ingress" : "egress", proxy_id_, remote_id, + destination_port, sni); + return false; } - // enforce Ingress policy 2nd, if any - if (ingress_policy_name_.length() > 0) { - log_entry.entry_.set_policy_name(ingress_policy_name_); - const auto& policy = policy_resolver_->getPolicy(ingress_policy_name_); + // populate l7proto_ if available + use_proxy_lib = port_policy.useProxylib(proxy_id_, remote_id, l7_proto); - // 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; - } - } + ENVOY_CONN_LOG(debug, + "cilium.network: Pod {} network {} policy ALLOW on proxy_id: {} id: {} port: {} " + "sni: \"{}\"", + conn, pod_ip_, ingress_ ? "ingress" : "egress", proxy_id_, remote_id, + destination_port, sni); + return true; +} - // Enforce egress policy for Ingress - auto egress_port_policy = policy.findPortPolicy(false, destination_port); - if (!egress_port_policy.allowed(proxy_id_, destination_identity, sni)) { - ENVOY_CONN_LOG(debug, - "Egress network policy {} DROP for reserved ingress identity and destination " - "identity: {} proxy_id: {} port: {} sni: \"{}\"", - conn, ingress_policy_name_, destination_identity, proxy_id_, destination_port, - sni); +bool CiliumPolicyFilterState::enforceIngressNetworkPolicy(const Network::Connection& conn, + uint32_t destination_identity, + uint16_t destination_port, + const absl::string_view sni) const { + 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, + "cilium.network: Ingress {} network ingress policy DENY on proxy_id: {} id: {} " + "port: {} sni: \"{}\"", + conn, ingress_policy_name_, proxy_id_, ingress_source_identity_, port_, sni); return false; } } - // Connection allowed by policy + // Enforce egress policy for Ingress + auto egress_port_policy = policy.findPortPolicy(false, destination_port); + if (!egress_port_policy.allowed(proxy_id_, destination_identity, sni)) { + ENVOY_CONN_LOG(debug, + "cilium.network: Ingress {} network egress policy DENY on proxy_id: {} " + "id: {} port: {} sni: \"{}\"", + conn, ingress_policy_name_, proxy_id_, destination_identity, destination_port, + sni); + return false; + } + + ENVOY_CONN_LOG(debug, + "cilium.network: Ingress {} network policy ALLOW on proxy_id: {} id: {} port: {} " + "sni: \"{}\"", + conn, ingress_policy_name_, proxy_id_, destination_identity, destination_port, + sni); return true; } @@ -84,55 +96,94 @@ bool CiliumPolicyFilterState::enforcePodHTTPPolicy(const Network::Connection& co uint32_t destination_identity, uint16_t destination_port, /* INOUT */ Http::RequestHeaderMap& headers, + /* INOUT */ LastNetworkPolicyCache& policy_cache, /* INOUT */ AccessLog::Entry& log_entry) const { const auto& policy = policy_resolver_->getPolicy(pod_ip_); auto remote_id = ingress_ ? source_identity_ : destination_identity; auto port = ingress_ ? port_ : destination_port; + uint64_t version = policy.version(); const auto port_policy = policy.findPortPolicy(ingress_, port); - if (!port_policy.hasHttpRules()) { - ENVOY_CONN_LOG(debug, - "cilium.l7policy: Pod {} HTTP {} policy enforcement skipped (no HTTP rules) on " - "proxy_id: {} id: {} port: {}", - conn, pod_ip_, ingress_ ? "ingress" : "egress", proxy_id_, remote_id, port); - return true; + bool has_http_rules = port_policy.hasHttpRules(); + + if (!has_http_rules) { + absl::optional opt = policy_cache.previousVerdict(version, remote_id, port); + if (opt.has_value()) { + bool verdict = opt.value(); + ENVOY_CONN_LOG(debug, + "cilium.l7policy: Pod {} HTTP {} policy enforcement using cached verdict {} " + "(no HTTP rules) on " + "proxy_id: {} id: {} port: {}", + conn, pod_ip_, ingress_ ? "ingress" : "egress", verdict ? "ALLOW" : "DENY", + proxy_id_, remote_id, port); + return verdict; + } else { + ENVOY_CONN_LOG(debug, + "cilium.l7policy: Pod {} HTTP {} policy cache MISS " + "(version: {}/{}, id: {}/{}, port: {}/{}), not skipping.", + conn, pod_ip_, ingress_ ? "ingress" : "egress", version, policy_cache.version_, + remote_id, policy_cache.identity_, port, policy_cache.port_); + } + } else { + ENVOY_CONN_LOG(debug, "cilium.l7policy: Pod {} HTTP {} policy has HTTP rules, not skipping.", + conn, pod_ip_, ingress_ ? "ingress" : "egress"); } - if (!port_policy.allowed(proxy_id_, remote_id, headers, log_entry)) { + bool verdict = port_policy.allowed(proxy_id_, remote_id, headers, log_entry); + if (!has_http_rules) { + policy_cache.update(version, remote_id, port, verdict); + } + if (verdict == false) { ENVOY_CONN_LOG(debug, "cilium.l7policy: Pod {} HTTP {} policy DENY on proxy_id: {} id: {} port: {}", conn, pod_ip_, ingress_ ? "ingress" : "egress", proxy_id_, remote_id, port); - return false; + } else { + ENVOY_CONN_LOG(debug, + "cilium.l7policy: Pod {} HTTP {} policy ALLOW on proxy_id: {} id: {} port: {}", + conn, pod_ip_, ingress_ ? "ingress" : "egress", proxy_id_, remote_id, port); } - - // Connection allowed by policy - ENVOY_CONN_LOG(debug, - "cilium.l7policy: Pod {} HTTP {} policy ALLOW on proxy_id: {} id: {} port: {}", - conn, pod_ip_, ingress_ ? "ingress" : "egress", proxy_id_, remote_id, port); - return true; + return verdict; } bool CiliumPolicyFilterState::enforceIngressHTTPPolicy( const Network::Connection& conn, uint32_t destination_identity, uint16_t destination_port, /* INOUT */ Http::RequestHeaderMap& headers, + /* INOUT */ LastNetworkPolicyCache policy_cache[2], /* INOUT */ AccessLog::Entry& log_entry) const { log_entry.entry_.set_policy_name(ingress_policy_name_); log_entry.request_logged_ = false; // we reuse the same entry we used for the pod policy const auto& policy = policy_resolver_->getPolicy(ingress_policy_name_); + uint64_t version = policy.version(); // 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; + bool has_http_rules = port_policy.hasHttpRules(); + + bool have_verdict = false; + bool verdict; + if (!has_http_rules) { + absl::optional opt = + policy_cache[0].previousVerdict(version, ingress_source_identity_, port_); + if (opt.has_value()) { + have_verdict = true; + verdict = opt.value(); + ENVOY_CONN_LOG(debug, + "cilium.l7policy: Ingress {} HTTP ingress policy enforcement using using " + "cached verdict (no HTTP rules)", + conn, ingress_policy_name_); + } + } + + if (!have_verdict) { + verdict = port_policy.allowed(proxy_id_, ingress_source_identity_, headers, log_entry); + if (!has_http_rules) { + policy_cache[0].update(version, ingress_source_identity_, port_, verdict); + } } - if (!port_policy.allowed(proxy_id_, ingress_source_identity_, headers, log_entry)) { + if (verdict == false) { ENVOY_CONN_LOG( debug, "cilium.l7policy: Ingress {} HTTP ingress policy DROP on proxy_id: {} id: {} port: {}", @@ -143,15 +194,31 @@ bool CiliumPolicyFilterState::enforceIngressHTTPPolicy( // 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()) { - ENVOY_CONN_LOG(debug, - "cilium.l7policy: Ingress {} HTTP egress policy enforcement skipped (no HTTP " - "rules) on proxy_id: {} id: {} port: {}", - conn, ingress_policy_name_, proxy_id_, destination_identity, destination_port); - return true; + bool has_http_rules = port_policy.hasHttpRules(); + + bool have_verdict = false; + bool verdict; + if (!has_http_rules) { + absl::optional opt = + policy_cache[1].previousVerdict(version, destination_identity, destination_port); + if (opt.has_value()) { + have_verdict = true; + verdict = opt.value(); + ENVOY_CONN_LOG(debug, + "cilium.l7policy: Ingress {} HTTP egress policy enforcement using using " + "cached verdict (no HTTP rules)", + conn, ingress_policy_name_); + } + } + + if (!have_verdict) { + verdict = port_policy.allowed(proxy_id_, destination_identity, headers, log_entry); + if (!has_http_rules) { + policy_cache[1].update(version, destination_identity, destination_port, verdict); + } } - if (!port_policy.allowed(proxy_id_, destination_identity, headers, log_entry)) { + if (verdict == false) { 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..01c8e3729 100644 --- a/cilium/filter_state_cilium_policy.h +++ b/cilium/filter_state_cilium_policy.h @@ -14,6 +14,7 @@ #include "source/common/common/logger.h" #include "absl/strings/string_view.h" +#include "absl/types/optional.h" #include "cilium/accesslog.h" #include "cilium/network_policy.h" @@ -29,6 +30,29 @@ class PolicyResolver { }; using PolicyResolverSharedPtr = std::shared_ptr; +// local cache for policy verdicts, which is only used when the policy has no HTTP rules. +// This eliminates policy lookups when the destination identity and port remain the same +struct LastNetworkPolicyCache { + absl::optional previousVerdict(uint64_t version, uint32_t identity, uint16_t port) const { + if (version == version_ && identity == identity_ && port == port_) { + return verdict_; + } + return absl::nullopt; + } + + void update(uint64_t version, uint32_t identity, uint16_t port, bool verdict) { + version_ = version; + identity_ = identity; + port_ = port; + verdict_ = verdict; + } + + uint64_t version_ = 0; + uint32_t identity_ = 0; + uint16_t port_ = 0; + bool verdict_ = false; +}; + // FilterState that holds relevant connection & policy information that can be retrieved // by the Cilium network- and HTTP policy filters via filter state. class CiliumPolicyFilterState : public StreamInfo::FilterState::Object, @@ -56,20 +80,24 @@ class CiliumPolicyFilterState : public StreamInfo::FilterState::Object, const PolicyInstance& getPolicy() const { return policy_resolver_->getPolicy(pod_ip_); } - bool enforceNetworkPolicy(const Network::Connection& conn, uint32_t destination_identity, - uint16_t destination_port, const absl::string_view sni, - /* OUT */ bool& use_proxy_lib, - /* OUT */ std::string& l7_proto, - /* INOUT */ AccessLog::Entry& log_entry) const; + bool enforcePodNetworkPolicy(const Network::Connection& conn, uint32_t destination_identity, + uint16_t destination_port, const absl::string_view sni, + /* OUT */ bool& use_proxy_lib, + /* OUT */ std::string& l7_proto) const; + + bool enforceIngressNetworkPolicy(const Network::Connection& conn, uint32_t destination_identity, + uint16_t destination_port, const absl::string_view sni) const; bool enforcePodHTTPPolicy(const Network::Connection& conn, uint32_t destination_identity, uint16_t destination_port, /* INOUT */ Http::RequestHeaderMap& headers, + /* INOUT */ LastNetworkPolicyCache& policy_cache, /* INOUT */ AccessLog::Entry& log_entry) const; bool enforceIngressHTTPPolicy(const Network::Connection& conn, uint32_t destination_identity, uint16_t destination_port, /* INOUT */ Http::RequestHeaderMap& headers, + /* INOUT */ LastNetworkPolicyCache ingress_cache[2], /* INOUT */ AccessLog::Entry& log_entry) const; // policyUseUpstreamDestinationAddress returns 'true' if policy enforcement should be done on the diff --git a/cilium/ipcache.h b/cilium/ipcache.h index ef86a0a11..5a9d24bd7 100644 --- a/cilium/ipcache.h +++ b/cilium/ipcache.h @@ -41,7 +41,7 @@ PACKED_STRUCT(struct IpCacheKey { std::string asString() const { if (family == ENDPOINT_KEY_IPV4) { auto ip = ntohl(ip4); - return fmt::format("%d.%d.%d.%d/%d", uint8_t(ip >> 24), uint8_t(ip >> 16), uint8_t(ip >> 8), + return fmt::format("{}.{}.{}.{}/{}", uint8_t(ip >> 24), uint8_t(ip >> 16), uint8_t(ip >> 8), uint8_t(ip), lpm_key.prefixlen - 32); } else if (family == ENDPOINT_KEY_IPV6) { return fmt::format("{:x}:{:x}:{:x}:{:x}/{}", ntohl(ip6[0]), ntohl(ip6[1]), ntohl(ip6[2]), diff --git a/cilium/l7policy.cc b/cilium/l7policy.cc index dc2e115fb..de591a58e 100644 --- a/cilium/l7policy.cc +++ b/cilium/l7policy.cc @@ -26,6 +26,7 @@ #include "source/common/common/assert.h" #include "source/common/common/logger.h" #include "source/common/common/utility.h" +#include "source/common/http/headers.h" #include "source/extensions/filters/http/common/factory_base.h" #include "absl/status/statusor.h" @@ -71,16 +72,17 @@ Config::Config(const std::string& access_log_path, const std::string& denied_403 TimeSource& time_source, Stats::Scope& scope, bool is_upstream) : time_source_(time_source), stats_{ALL_CILIUM_STATS(POOL_COUNTER_PREFIX(scope, "cilium"))}, denied_403_body_(denied_403_body), is_upstream_(is_upstream), access_log_(nullptr) { - if (access_log_path.length()) { + if (!access_log_path.empty()) { access_log_ = AccessLog::open(access_log_path, time_source); } - if (denied_403_body_.length() == 0) { + if (denied_403_body_.empty()) { denied_403_body_ = "Access denied"; } size_t len = denied_403_body_.length(); if (len < 2 || denied_403_body_[len - 2] != '\r' || denied_403_body_[len - 1] != '\n') { denied_403_body_.append("\r\n"); } + ENVOY_LOG(debug, "cilium.l7policy: Config created"); } Config::Config(const ::cilium::L7Policy& config, TimeSource& time_source, Stats::Scope& scope, @@ -182,9 +184,10 @@ Http::FilterHeadersStatus AccessFilter::decodeHeaders(Http::RequestHeaderMap& he dst_address, stream_info, headers); // Enforce pod policy only for local pods without L7 LB - if (!policy_fs->policyUseUpstreamDestinationAddress() && policy_fs->pod_ip_.length() > 0) { - bool allowed = policy_fs->enforcePodHTTPPolicy(conn.ref(), destination_identity, - destination_port, headers, *log_entry_); + if (!policy_fs->policyUseUpstreamDestinationAddress() && !policy_fs->pod_ip_.empty()) { + bool allowed = + policy_fs->enforcePodHTTPPolicy(conn.ref(), destination_identity, destination_port, + headers, config_->pod_policy_cache_, *log_entry_); // Update the log entry with current headers, as the policy may have altered them. log_entry_->updateFromRequest(destination_identity, dst_address, headers); @@ -219,7 +222,7 @@ Http::FilterHeadersStatus AccessFilter::decodeHeaders(Http::RequestHeaderMap& he // must have a policy configured // This is safe as the upstream filter was introduced at Cilium 1.16 and // bpf_metadata config has had 'enforce_policy_on_l7lb' set since Cilium 1.15. - if (policy_fs->pod_ip_.length() == 0 && policy_fs->ingress_policy_name_.length() == 0) { + if (policy_fs->pod_ip_.empty() && policy_fs->ingress_policy_name_.empty()) { ENVOY_CONN_LOG(warn, "cilium.network: no policy configured", conn.ref()); return Http::FilterHeadersStatus::StopIteration; } @@ -252,9 +255,9 @@ Http::FilterHeadersStatus AccessFilter::decodeHeaders(Http::RequestHeaderMap& he bool allowed; // Is there a pod egress policy? - if (policy_fs->pod_ip_.length() > 0) { + if (!policy_fs->pod_ip_.empty()) { allowed = policy_fs->enforcePodHTTPPolicy(conn.ref(), destination_identity, destination_port, - headers, *log_entry_); + headers, config_->pod_policy_cache_, *log_entry_); // Update the log entry with current headers, as the policy may have altered them. log_entry_->updateFromRequest(destination_identity, dst_address, headers); @@ -268,9 +271,10 @@ Http::FilterHeadersStatus AccessFilter::decodeHeaders(Http::RequestHeaderMap& he } // Is there an Ingress policy? - if (policy_fs->ingress_policy_name_.length() > 0) { - allowed = policy_fs->enforceIngressHTTPPolicy(conn.ref(), destination_identity, - destination_port, headers, *log_entry_); + if (!policy_fs->ingress_policy_name_.empty()) { + allowed = + policy_fs->enforceIngressHTTPPolicy(conn.ref(), destination_identity, destination_port, + headers, config_->ingress_policy_cache_, *log_entry_); // Update the log entry with current headers, as the policy may have altered them. log_entry_->updateFromRequest(destination_identity, dst_address, headers); @@ -296,8 +300,10 @@ void AccessFilter::onStreamComplete() { } Http::FilterHeadersStatus AccessFilter::encodeHeaders(Http::ResponseHeaderMap& headers, bool) { + const auto& stream_info = callbacks_->streamInfo(); + // Skip enforcement or logging on shadows - if (callbacks_->streamInfo().isShadow()) { + if (stream_info.isShadow()) { return Http::FilterHeadersStatus::Continue; } @@ -309,6 +315,33 @@ Http::FilterHeadersStatus AccessFilter::encodeHeaders(Http::ResponseHeaderMap& h return Http::FilterHeadersStatus::Continue; } + // Propagate connection: close from upstream to downstream (if original source is preserved) + if (callbacks_->connection().has_value() && stream_info.upstreamInfo().has_value() && + stream_info.hasResponseFlag(StreamInfo::UpstreamConnectionTermination)) { + // check if upstream and downstream connections have the same source and destination + // addresses, respectively + const auto& conn = callbacks_->connection().ref(); + const auto& upstream_info = stream_info.upstreamInfo().ref(); + + if (upstream_info.upstreamRemoteAddress() == conn.connectionInfoProvider().localAddress() && + upstream_info.upstreamLocalAddress() == conn.connectionInfoProvider().remoteAddress()) { + ENVOY_CONN_LOG(debug, + "Upstream connection with same 5-tuple closed, passing connection close to " + "downstream response", + conn); + + headers.setReferenceConnection(Http::Headers::get().ConnectionValues.Close); + } else { + ENVOY_CONN_LOG(debug, + "Upstream connection closed, but it has different 5-tuple, downstream " + "connection not closed (src: {}/{}, dst: {}/{})", + conn, conn.connectionInfoProvider().remoteAddress()->asStringView(), + upstream_info.upstreamLocalAddress()->asStringView(), + conn.connectionInfoProvider().localAddress()->asStringView(), + upstream_info.upstreamRemoteAddress()->asStringView()); + } + } + if (log_entry_ == nullptr) { return Http::FilterHeadersStatus::Continue; } diff --git a/cilium/l7policy.h b/cilium/l7policy.h index c3ff812d0..351bf5d85 100644 --- a/cilium/l7policy.h +++ b/cilium/l7policy.h @@ -19,6 +19,7 @@ #include "cilium/accesslog.h" #include "cilium/api/accesslog.pb.h" #include "cilium/api/l7policy.pb.h" +#include "cilium/filter_state_cilium_policy.h" namespace Envoy { namespace Cilium { @@ -54,7 +55,12 @@ class Config : public Logger::Loggable { TimeSource& time_source_; FilterStats stats_; std::string denied_403_body_; - bool is_upstream_; + const bool is_upstream_; + +protected: + friend class AccessFilter; + LastNetworkPolicyCache pod_policy_cache_; + LastNetworkPolicyCache ingress_policy_cache_[2]; private: Cilium::AccessLogSharedPtr access_log_; diff --git a/cilium/network_filter.cc b/cilium/network_filter.cc index 69a21079c..1cbf1d19a 100644 --- a/cilium/network_filter.cc +++ b/cilium/network_filter.cc @@ -14,7 +14,6 @@ #include "envoy/server/filter_config.h" #include "envoy/stream_info/filter_state.h" #include "envoy/stream_info/stream_info.h" -#include "envoy/upstream/host_description.h" #include "source/common/buffer/buffer_impl.h" #include "source/common/common/logger.h" @@ -24,6 +23,8 @@ #include "source/common/protobuf/utility.h" #include "absl/status/statusor.h" +#include "absl/strings/string_view.h" +#include "absl/types/optional.h" #include "cilium/accesslog.h" #include "cilium/api/accesslog.pb.h" #include "cilium/api/network_filter.pb.h" @@ -38,10 +39,10 @@ namespace Server { namespace Configuration { /** - * Config registration for the bpf metadata filter. @see + * Config registration for the cilium downstream network filter. @see * NamedNetworkFilterConfigFactory. */ -class CiliumNetworkConfigFactory : public NamedNetworkFilterConfigFactory { +class CiliumDownstreamNetworkConfigFactory : public NamedNetworkFilterConfigFactory { public: // NamedNetworkFilterConfigFactory absl::StatusOr @@ -50,7 +51,7 @@ class CiliumNetworkConfigFactory : public NamedNetworkFilterConfigFactory { auto config = std::make_shared( MessageUtil::downcastAndValidate( proto_config, context.messageValidationVisitor()), - context); + false, context.serverFactoryContext()); return [config](Network::FilterManager& filter_manager) mutable -> void { filter_manager.addFilter(std::make_shared(config)); }; @@ -66,7 +67,37 @@ class CiliumNetworkConfigFactory : public NamedNetworkFilterConfigFactory { /** * Static registration for the bpf metadata filter. @see RegisterFactory. */ -REGISTER_FACTORY(CiliumNetworkConfigFactory, NamedNetworkFilterConfigFactory); +REGISTER_FACTORY(CiliumDownstreamNetworkConfigFactory, NamedNetworkFilterConfigFactory); + +/** + * Config registration for the cilium filter. @see + * NamedNetworkFilterConfigFactory. + */ +class CiliumUpstreamNetworkConfigFactory : public NamedUpstreamNetworkFilterConfigFactory { +public: + // NamedNetworkFilterConfigFactory + Network::FilterFactoryCb createFilterFactoryFromProto(const Protobuf::Message& proto_config, + UpstreamFactoryContext& context) override { + auto config = std::make_shared( + MessageUtil::downcastAndValidate( + proto_config, context.serverFactoryContext().messageValidationVisitor()), + true, context.serverFactoryContext()); + return [config](Network::FilterManager& filter_manager) mutable -> void { + filter_manager.addReadFilter(std::make_shared(config)); + }; + } + + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique<::cilium::NetworkFilter>(); + } + + std::string name() const override { return "cilium.network"; } +}; + +/** + * Static registration for the bpf metadata filter. @see RegisterFactory. + */ +REGISTER_FACTORY(CiliumUpstreamNetworkConfigFactory, NamedUpstreamNetworkFilterConfigFactory); } // namespace Configuration } // namespace Server @@ -74,9 +105,9 @@ REGISTER_FACTORY(CiliumNetworkConfigFactory, NamedNetworkFilterConfigFactory); namespace Filter { namespace CiliumL3 { -Config::Config(const ::cilium::NetworkFilter& config, - Server::Configuration::FactoryContext& context) - : time_source_(context.serverFactoryContext().timeSource()), access_log_(nullptr) { +Config::Config(const ::cilium::NetworkFilter& config, bool is_upstream, + Server::Configuration::ServerFactoryContext& context) + : is_upstream_(is_upstream), time_source_(context.timeSource()) { const auto& access_log_path = config.access_log_path(); if (access_log_path.length()) { access_log_ = Cilium::AccessLog::open(access_log_path, time_source_); @@ -105,26 +136,35 @@ bool Instance::enforceNetworkPolicy(const Cilium::CiliumPolicyFilterState* polic ENVOY_CONN_LOG(debug, "cilium.network: destination address: {}", conn, dst_address->asString()); dest_fs->setDestinationAddress(dst_address); - log_entry_.initFromConnection(policy_fs->pod_ip_, policy_fs->proxy_id_, policy_fs->ingress_, - policy_fs->source_identity_, - stream_info.downstreamAddressProvider().remoteAddress(), - destination_identity, dst_address, &config_->time_source_); - - bool use_proxy_lib; - if (!policy_fs->enforceNetworkPolicy(conn, remote_id_, destination_port_, sni, use_proxy_lib, - l7proto_, log_entry_)) { - ENVOY_CONN_LOG(debug, "cilium.network: policy DENY on id: {} port: {} sni: \"{}\"", conn, - remote_id_, destination_port_, sni); - config_->log(log_entry_, ::cilium::EntryType::Denied); - return false; + // Is there a pod egress policy? + bool use_proxy_lib = false; + if (policy_fs->pod_ip_.length() > 0) { + if (!policy_fs->enforcePodNetworkPolicy(conn, destination_identity, destination_port_, sni, + use_proxy_lib, l7proto_)) { + log_entry_.initFromConnection(policy_fs->pod_ip_, policy_fs->proxy_id_, false, + policy_fs->source_identity_, + stream_info.downstreamAddressProvider().remoteAddress(), + destination_identity, dst_address, &config_->time_source_); + config_->log(log_entry_, ::cilium::EntryType::Denied); + return false; + } + // TODO: access log allow for an SNI policy without HTTP rules? } - // Emit accesslog if north/south l7 lb, as in that case the traffic is not going back to bpf - // datapath for policy enforcement - if (log_entry_.entry_.policy_name() != policy_fs->pod_ip_) { + + // Is there an Ingress policy? + if (policy_fs->ingress_policy_name_.length() > 0) { + log_entry_.initFromConnection(policy_fs->ingress_policy_name_, policy_fs->proxy_id_, false, + policy_fs->source_identity_, + stream_info.downstreamAddressProvider().remoteAddress(), + destination_identity, dst_address, &config_->time_source_); + + if (!policy_fs->enforceIngressNetworkPolicy(conn, destination_identity, destination_port_, + sni)) { + config_->log(log_entry_, ::cilium::EntryType::Denied); + return false; + } config_->log(log_entry_, ::cilium::EntryType::Request); } - ENVOY_LOG(debug, "cilium.network: policy ALLOW on id: {} port: {} sni: \"{}\"", remote_id_, - destination_port_, sni); if (use_proxy_lib) { const std::string& policy_name = policy_fs->pod_ip_; @@ -146,16 +186,25 @@ bool Instance::enforceNetworkPolicy(const Cilium::CiliumPolicyFilterState* polic } Network::FilterStatus Instance::onNewConnection() { + // Upstream handling happens in onDestinationSelected() below. + if (config_->is_upstream_) { + return Network::FilterStatus::Continue; + } + + // If there is no upstream filter, onDestinationSelected for the upstream connection + // will be called on the downstream filter instead, but after this call. + auto& conn = callbacks_->connection(); - ENVOY_CONN_LOG(debug, "cilium.network: onNewConnection", conn); + ENVOY_CONN_LOG(debug, "cilium.network: onNewConnection (downstream)", conn); // Buffer data until proxylib policy is available, if configured with proxylib if (config_->proxylib_.get() != nullptr) { should_buffer_ = true; } + auto& stream_info = conn.streamInfo(); const auto policy_fs = - conn.streamInfo().filterState()->getDataReadOnly( + stream_info.filterState()->getDataReadOnly( Cilium::CiliumPolicyFilterState::key()); if (!policy_fs) { @@ -163,29 +212,15 @@ Network::FilterStatus Instance::onNewConnection() { return Network::FilterStatus::StopIteration; } - // Default to incoming destination port, may be changed for L7 LB - destination_port_ = policy_fs->port_; - - const auto dest_fs = - conn.streamInfo().filterState()->getDataMutable( - Cilium::CiliumDestinationFilterState::key()); - - if (!dest_fs) { - ENVOY_CONN_LOG(warn, "cilium.network: Cilium destination filter state not found", conn); - return Network::FilterStatus::StopIteration; - } - - // Pass SNI before the upstream callback so that it is available when upstream connection is - // initialized. const auto sni = conn.requestedServerName(); - if (!sni.empty()) { - ENVOY_CONN_LOG(trace, "cilium.network: SNI: {}", conn, sni); - } // Pass metadata from tls_inspector to the filterstate, if any & not already // set via upstream cluster config. + // TODO: Figure out if this can be left out if auto_sni and auto_san_validation are configured? if (!sni.empty()) { - auto filter_state = conn.streamInfo().filterState(); + ENVOY_CONN_LOG(trace, "cilium.network: SNI: {}", conn, sni); + + auto& filter_state = conn.streamInfo().filterState(); auto have_sni = filter_state->hasData(Network::UpstreamServerName::key()); auto have_san = filter_state->hasData( @@ -201,74 +236,118 @@ Network::FilterStatus Instance::onNewConnection() { } } - // use upstream callback only if required, otherwise enforce policy before returning + // Add DownstreamConnection filter state for compat with legacy configurations without upstream + // filter + stream_info.filterState()->setData( + Network::Cilium::DownstreamConnection::key(), + std::make_shared(&conn), + StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::Connection, + StreamInfo::StreamSharingMayImpactPooling::SharedWithUpstreamConnection); + + // Leave L7 LB policy enforcement to the upstream filter, or to the onDestinationSelected callback if (policy_fs->policyUseUpstreamDestinationAddress()) { - callbacks_->addUpstreamCallback( - [this, policy_fs, dest_fs, sni](Upstream::HostDescriptionConstSharedPtr host, - StreamInfo::StreamInfo& stream_info) -> bool { - // Skip enforcement or logging on shadows - if (stream_info.isShadow()) { - return true; - } - - auto& conn = callbacks_->connection(); - ENVOY_CONN_LOG(trace, "cilium.network: in upstream callback", conn); - - // Resolve the destination security ID and port - uint32_t destination_identity = 0; - - Network::Address::InstanceConstSharedPtr dst_address = host->address(); - if (nullptr == dst_address) { - ENVOY_CONN_LOG(warn, "cilium.network (egress): No destination address", conn); - return false; - } - - const auto dip = dst_address->ip(); - if (!dip) { - ENVOY_CONN_LOG(warn, "cilium.network: Non-IP destination address: {}", conn, - dst_address->asString()); - return false; - } - - // Set the destination address in the filter state, so that we can use it later when - // the socket option is set for local address - ENVOY_CONN_LOG(debug, "cilium.network (egress): destination address: {}", conn, - dst_address->asString()); - dest_fs->setDestinationAddress(dst_address); - - destination_port_ = dip->port(); - destination_identity = policy_fs->resolvePolicyId(dip); - remote_id_ = destination_identity; - - return enforceNetworkPolicy(policy_fs, dest_fs, destination_identity, dst_address, sni, - stream_info); - }); + return Network::FilterStatus::Continue; + } + + const auto dest_fs = + stream_info.filterState()->getDataMutable( + Cilium::CiliumDestinationFilterState::key()); + if (!dest_fs) { + ENVOY_CONN_LOG( + warn, "cilium.network::onDestinationSelected: Cilium destination filter state not found", + conn); + return Network::FilterStatus::StopIteration; + } + + Network::Address::InstanceConstSharedPtr dst_address = + stream_info.downstreamAddressProvider().localAddress(); + const auto dip = dst_address->ip(); + + // Resolve the destination security ID and port + uint32_t destination_identity = 0; // left as 0 for an ingress policy + + if (policy_fs->ingress_) { + destination_port_ = policy_fs->port_; + remote_id_ = policy_fs->source_identity_; } else { - auto& stream_info = conn.streamInfo(); + destination_port_ = dip->port(); + destination_identity = policy_fs->resolvePolicyId(dip); + remote_id_ = destination_identity; + } - // Resolve the destination security ID and port - uint32_t destination_identity = 0; + if (!enforceNetworkPolicy(policy_fs, dest_fs, destination_identity, dst_address, sni, + stream_info)) { + stream_info.setResponseFlag(StreamInfo::CoreResponseFlag::UnauthorizedExternalService); + conn.close(Network::ConnectionCloseType::AbortReset, "access denied"); + return Network::FilterStatus::StopIteration; + } - Network::Address::InstanceConstSharedPtr dst_address = - stream_info.downstreamAddressProvider().localAddress(); - const auto dip = dst_address->ip(); + return Network::FilterStatus::Continue; +} - if (policy_fs->ingress_) { - remote_id_ = policy_fs->source_identity_; - } else { - destination_port_ = dip->port(); - destination_identity = policy_fs->resolvePolicyId(dip); - remote_id_ = destination_identity; +// onDestinationSelected is called before an upstream connection is attempted. +// Called on the downstream filter only if none of the upstream network filter implements this and +// returns a FilterStatus. +absl::optional +Instance::onDestinationSelected(const Network::Address::InstanceConstSharedPtr& dst_address, + StreamInfo::StreamInfo& stream_info) { + // Skip enforcement and access logging on shadows + if (stream_info.isShadow()) { + return Network::FilterStatus::Continue; + } + + auto& conn = callbacks_->connection(); + ENVOY_CONN_LOG(info, "cilium.network: onDestinationSelected ({})", conn, + config_->is_upstream_ ? "upstream" : "downstream"); + + const auto policy_fs = + stream_info.filterState()->getDataReadOnly( + Cilium::CiliumPolicyFilterState::key()); + if (!policy_fs) { + ENVOY_CONN_LOG( + warn, "cilium.network::onDestinationSelected Cilium policy filter state not found", conn); + return Network::FilterStatus::StopIteration; + } + + const auto dest_fs = + stream_info.filterState()->getDataMutable( + Cilium::CiliumDestinationFilterState::key()); + if (!dest_fs) { + ENVOY_CONN_LOG( + warn, "cilium.network::onDestinationSelected: Cilium destination filter state not found", + conn); + return Network::FilterStatus::StopIteration; + } + + // Set the destination address in the filter state, so that we can use it later when + // the socket option is set for local address + ENVOY_CONN_LOG(debug, "cilium.network::onDestinationSelected: destination address: {}", conn, + dst_address->asString()); + dest_fs->setDestinationAddress(dst_address); + + // Only enforce L7 LB policy here, non-L7 LB policy has already been enforced on the downstream + // onNewConnection callback. + if (policy_fs->policyUseUpstreamDestinationAddress()) { + if (policy_fs->pod_ip_.length() == 0 && policy_fs->ingress_policy_name_.length() == 0) { + ENVOY_CONN_LOG(warn, "cilium.network::onDestinationSelected no policy configured", conn); + return Network::FilterStatus::StopIteration; } + // upstream connection sni can be different from the downstream connection due to HTTP routing + const auto sni = conn.requestedServerName(); + + // Resolve destination security ID + const Network::Address::Ip* dip = dst_address->ip(); + uint32_t destination_identity = policy_fs->resolvePolicyId(dip); + destination_port_ = dip->port(); + + remote_id_ = destination_identity; + if (!enforceNetworkPolicy(policy_fs, dest_fs, destination_identity, dst_address, sni, stream_info)) { - stream_info.setResponseFlag(StreamInfo::CoreResponseFlag::UnauthorizedExternalService); - conn.close(Network::ConnectionCloseType::AbortReset, "access denied"); return Network::FilterStatus::StopIteration; } } - return Network::FilterStatus::Continue; } diff --git a/cilium/network_filter.h b/cilium/network_filter.h index 1655e5890..3689d9fb4 100644 --- a/cilium/network_filter.h +++ b/cilium/network_filter.h @@ -6,13 +6,16 @@ #include "envoy/buffer/buffer.h" #include "envoy/common/time.h" -#include "envoy/json/json_object.h" +#include "envoy/network/address.h" #include "envoy/network/filter.h" #include "envoy/server/factory_context.h" +#include "envoy/stream_info/stream_info.h" #include "source/common/buffer/buffer_impl.h" #include "source/common/common/logger.h" +#include "absl/strings/string_view.h" +#include "absl/types/optional.h" #include "cilium/accesslog.h" #include "cilium/api/accesslog.pb.h" #include "cilium/api/network_filter.pb.h" @@ -32,16 +35,17 @@ namespace CiliumL3 { */ class Config : Logger::Loggable { public: - Config(const ::cilium::NetworkFilter& config, Server::Configuration::FactoryContext& context); - Config(const Json::Object& config, Server::Configuration::FactoryContext& context); + Config(const ::cilium::NetworkFilter& config, bool is_upstream, + Server::Configuration::ServerFactoryContext& context); void log(Cilium::AccessLog::Entry&, ::cilium::EntryType); Cilium::GoFilterSharedPtr proxylib_; + bool is_upstream_; TimeSource& time_source_; private: - Cilium::AccessLogSharedPtr access_log_; + Cilium::AccessLogSharedPtr access_log_{}; }; using ConfigSharedPtr = std::shared_ptr; @@ -59,6 +63,9 @@ class Instance : public Network::Filter, Logger::Loggable { void initializeReadFilterCallbacks(Network::ReadFilterCallbacks& callbacks) override { callbacks_ = &callbacks; } + absl::optional + onDestinationSelected(const Network::Address::InstanceConstSharedPtr& destination_address, + StreamInfo::StreamInfo& stream_info) override; // Network::WriteFilter Network::FilterStatus onWrite(Buffer::Instance&, bool end_stream) override; diff --git a/cilium/network_policy.cc b/cilium/network_policy.cc index 6c2b774b7..0eaa8b6c0 100644 --- a/cilium/network_policy.cc +++ b/cilium/network_policy.cc @@ -1148,12 +1148,14 @@ class PortNetworkPolicy : public Logger::Loggable { class PolicyInstanceImpl : public PolicyInstance { public: PolicyInstanceImpl(const NetworkPolicyMapImpl& parent, uint64_t hash, - const cilium::NetworkPolicy& proto) + const cilium::NetworkPolicy& proto, uint64_t version) : conntrack_map_name_(proto.conntrack_map_name()), endpoint_id_(proto.endpoint_id()), - hash_(hash), policy_proto_(proto), endpoint_ips_(proto), parent_(parent), + hash_(hash), policy_proto_(proto), endpoint_ips_(proto), version_(version), parent_(parent), ingress_(parent, policy_proto_.ingress_per_port_policies()), egress_(parent, policy_proto_.egress_per_port_policies()) {} + uint64_t version() const override { return version_; } + bool allowed(bool ingress, uint32_t proxy_id, uint32_t remote_id, uint16_t port, Envoy::Http::RequestHeaderMap& headers, Cilium::AccessLog::Entry& log_entry) const override { @@ -1205,6 +1207,7 @@ class PolicyInstanceImpl : public PolicyInstance { const IpAddressPair endpoint_ips_; private: + const uint64_t version_; const NetworkPolicyMapImpl& parent_; const PortNetworkPolicy ingress_; const PortNetworkPolicy egress_; @@ -1347,7 +1350,11 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate( ipcache->open(); } } - + uint64_t version; + if (!absl::SimpleAtoi(version_info, &version)) { + throw EnvoyException( + fmt::format("Network Policy has invalid version_info format: {}", version_info)); + } std::string version_name = fmt::format("NetworkPolicyMap version {}", version_info); Init::ManagerImpl version_init_manager(version_name); // Set the init manager to use via the transport factory context @@ -1390,7 +1397,8 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate( } // May throw - auto new_policy = std::make_shared(*this, new_hash, config); + auto new_policy = + std::make_shared(*this, new_hash, config, version); for (const auto& endpoint_ip : config.endpoint_ips()) { ENVOY_LOG(trace, "Cilium updating network policy for endpoint {}", endpoint_ip); diff --git a/cilium/network_policy.h b/cilium/network_policy.h index a56ebb9da..d83c9439e 100644 --- a/cilium/network_policy.h +++ b/cilium/network_policy.h @@ -153,6 +153,8 @@ class PolicyInstance { } }; + virtual uint64_t version() const { return 1; } + virtual bool allowed(bool ingress, uint32_t proxy_id, uint32_t remote_id, uint16_t port, Envoy::Http::RequestHeaderMap& headers, Cilium::AccessLog::Entry& log_entry) const PURE; diff --git a/patches/0002-listener-add-socket-options.patch b/patches/0001-listener-add-socket-options.patch similarity index 97% rename from patches/0002-listener-add-socket-options.patch rename to patches/0001-listener-add-socket-options.patch index 35f19020e..4f86a993d 100644 --- a/patches/0002-listener-add-socket-options.patch +++ b/patches/0001-listener-add-socket-options.patch @@ -1,7 +1,7 @@ -From 0cf16105f43982ac22eac548d94ac5a16d0970d2 Mon Sep 17 00:00:00 2001 +From 4abcc4d89e7278acdc864afb3d810a6d1990e339 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Mon, 14 Aug 2023 10:01:21 +0300 -Subject: [PATCH 2/6] listener: add socket options +Subject: [PATCH 1/8] listener: add socket options This reverts commit 170c89eb0b2afb7a39d44d0f8dfb77444ffc038f. @@ -30,7 +30,7 @@ index dd2dcee071..c216cb4fe3 100644 + */ + virtual void addListenSocketOptions(const Network::Socket::OptionsSharedPtr& options) PURE; +}; - + /** * FactoryContext for ProtocolOptionsFactory. diff --git a/source/common/listener_manager/listener_impl.cc b/source/common/listener_manager/listener_impl.cc @@ -45,25 +45,25 @@ index 8744009357..47dff3ccab 100644 + listener_impl_.addListenSocketOptions(options); +} Init::Manager& PerListenerFactoryContextImpl::initManager() { return listener_impl_.initManager(); } - + bool ListenerImpl::createNetworkFilterChain( diff --git a/source/common/listener_manager/listener_impl.h b/source/common/listener_manager/listener_impl.h index 5f24acc9c1..683352577d 100644 --- a/source/common/listener_manager/listener_impl.h +++ b/source/common/listener_manager/listener_impl.h @@ -186,6 +186,8 @@ public: - + Stats::Scope& listenerScope() override; - + + void addListenSocketOptions(const Network::Socket::OptionsSharedPtr& options) override; + ListenerFactoryContextBaseImpl& parentFactoryContext() { return *listener_factory_context_base_; } friend class ListenerImpl; - + @@ -327,6 +329,13 @@ public: return listener_factory_context_->listener_factory_context_base_->listener_info_; } - + + void addListenSocketOptions(const Network::Socket::OptionsSharedPtr& append_options) { + for (std::vector::size_type i = 0; + i < addresses_.size(); i++) { @@ -83,7 +83,7 @@ index dfbdef7725..617da75c0b 100644 MOCK_METHOD(Stats::Scope&, listenerScope, ()); MOCK_METHOD(const Network::ListenerInfo&, listenerInfo, (), (const)); + MOCK_METHOD(void, addListenSocketOptions, (const Network::Socket::OptionsSharedPtr&)); - + testing::NiceMock server_factory_context_; testing::NiceMock transport_socket_factory_context_; diff --git a/test/mocks/server/listener_factory_context.h b/test/mocks/server/listener_factory_context.h @@ -93,11 +93,11 @@ index 3d5b6197f0..cbf6dcf6dd 100644 @@ -21,6 +21,7 @@ public: MockListenerFactoryContext(); ~MockListenerFactoryContext() override; - + + MOCK_METHOD(void, addListenSocketOptions, (const Network::Socket::OptionsSharedPtr&)); MOCK_METHOD(ServerFactoryContext&, serverFactoryContext, (), (const)); MOCK_METHOD(TransportSocketFactoryContext&, getTransportSocketFactoryContext, (), (const)); MOCK_METHOD(const Network::DrainDecision&, drainDecision, ()); --- -2.34.1 +-- +2.49.0 diff --git a/patches/0001-network-Add-callback-for-upstream-authorization.patch b/patches/0001-network-Add-callback-for-upstream-authorization.patch deleted file mode 100644 index ac8cc327e..000000000 --- a/patches/0001-network-Add-callback-for-upstream-authorization.patch +++ /dev/null @@ -1,376 +0,0 @@ -From 612b4abeb4a2d46b4b665f988f7c1db4c34b3fd6 Mon Sep 17 00:00:00 2001 -From: Jarno Rajahalme -Date: Mon, 5 May 2025 11:15:52 +1000 -Subject: [PATCH 1/6] network: Add callback for upstream authorization - -Add new ReadFilterCallbacks addUpstreamCallback() and -iterateUpstreamCallbacks(). Network filters can add callbacks using -addUpstreamCallback(), which will then get called after an upstream -host has been selected, but before the upstream connection is -established. If any of the callbacks returns 'false', the connection -is not established. For HTTP the router will issue a 403 local -response. - -iterateUpstreamCallbacks() is also added to -StreamDecoderFilterCallbacks so that the HTTP router filter can invoke -the added callbacks before a new connection is established. - -These additions allow network read filters to perform network level -policy enforcement based on the selected upstream host. - -Callbacks can safely refer to memory held by the filter instance -adding the callback, as the calls to the callbacks are only ever be -done from the tcp_proxy or router filter in the same filter chain. - -Signed-off-by: Jarno Rajahalme ---- - envoy/http/filter.h | 7 ++++++ - envoy/network/filter.h | 28 +++++++++++++++++++++ - envoy/tcp/upstream.h | 5 ++++ - source/common/http/async_client_impl.h | 5 ++++ - source/common/http/conn_manager_impl.h | 6 +++++ - source/common/http/filter_manager.cc | 6 +++++ - source/common/http/filter_manager.h | 8 ++++++ - source/common/network/filter_manager_impl.h | 21 ++++++++++++++++ - source/common/router/router.cc | 8 ++++++ - source/common/router/upstream_request.h | 5 ++++ - source/common/tcp_proxy/tcp_proxy.cc | 7 ++++++ - source/common/tcp_proxy/tcp_proxy.h | 5 ++++ - source/common/tcp_proxy/upstream.cc | 8 ++++++ - source/common/tcp_proxy/upstream.h | 2 ++ - source/server/api_listener_impl.h | 3 +++ - 15 files changed, 124 insertions(+) - -diff --git a/envoy/http/filter.h b/envoy/http/filter.h -index f1283df286..88fa85c2f0 100644 ---- a/envoy/http/filter.h -+++ b/envoy/http/filter.h -@@ -816,6 +816,13 @@ public: - virtual absl::optional - upstreamOverrideHost() const PURE; - -+ /** -+ * Invokes all the added network level callbacks before establishing a connection to the -+ * selected upstream host. -+ * Returns 'false' if any of the callbacks rejects the connection, 'true' otherwise. -+ */ -+ virtual bool iterateUpstreamCallbacks(Upstream::HostDescriptionConstSharedPtr, -+ StreamInfo::StreamInfo&) PURE; - /** - * @return true if the filter should shed load based on the system pressure, typically memory. - */ -diff --git a/envoy/network/filter.h b/envoy/network/filter.h -index 2ad394ffeb..c843f41c05 100644 ---- a/envoy/network/filter.h -+++ b/envoy/network/filter.h -@@ -147,6 +147,22 @@ public: - - using WriteFilterSharedPtr = std::shared_ptr; - -+/** -+ * UpstreamCallback can be used to reject upstream host selection made by the TCP proxy filter. -+ * This callback is passed the Upstream::HostDescriptionConstSharedPtr, and StreamInfo. -+ * -+ * The callback is called just after the upstream host has been picked, but before a connection is -+ * established. Here the callback can reject the selected upstream host and cause the be dropped. -+ -+ * UpstreamCallback may not be called if the connection is dropped for another reason, such as -+ * no route, cluster is not found, etc. -+ * -+ * Returning 'true' allows the connection to be established. Returning 'false' prevents the -+ * connection to the selected host from being established. -+ */ -+using UpstreamCallback = std::function; -+ - /** - * Callbacks used by individual read filter instances to communicate with the filter manager. - */ -@@ -206,6 +222,18 @@ public: - */ - virtual bool startUpstreamSecureTransport() PURE; - -+ /* -+ * Adds the given callback to be executed later via iterateUpstreamCallbacks(). -+ */ -+ virtual void addUpstreamCallback(const UpstreamCallback& cb) PURE; -+ -+ /** -+ * Invokes all the added callbacks before connecting to the selected upstream host. -+ * Returns 'false' if any of the callbacks rejects the connection, 'true' otherwise. -+ */ -+ virtual bool iterateUpstreamCallbacks(Upstream::HostDescriptionConstSharedPtr, -+ StreamInfo::StreamInfo&) PURE; -+ - /** - * Control the filter close status for read filters. - * -diff --git a/envoy/tcp/upstream.h b/envoy/tcp/upstream.h -index b201d2e153..9aece0fc6c 100644 ---- a/envoy/tcp/upstream.h -+++ b/envoy/tcp/upstream.h -@@ -72,6 +72,11 @@ public: - * @param callbacks callbacks to communicate stream failure or creation on. - */ - virtual void newStream(GenericConnectionPoolCallbacks& callbacks) PURE; -+ -+ /** -+ * @return Upstream::HostDescriptionConstSharedPtr the host for which connections are pooled. -+ */ -+ virtual Upstream::HostDescriptionConstSharedPtr host() const PURE; - }; - - // An API for the UpstreamRequest to get callbacks from either an HTTP or TCP -diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h -index ead93183f0..b9f1315afa 100644 ---- a/source/common/http/async_client_impl.h -+++ b/source/common/http/async_client_impl.h -@@ -265,6 +265,11 @@ private: - ResponseHeaderMapOptRef responseHeaders() override { return {}; } - ResponseTrailerMapOptRef responseTrailers() override { return {}; } - -+ bool iterateUpstreamCallbacks(Upstream::HostDescriptionConstSharedPtr, -+ StreamInfo::StreamInfo&) override { -+ return true; -+ } -+ - // ScopeTrackedObject - void dumpState(std::ostream& os, int indent_level) const override { - const char* spaces = spacesForLevel(indent_level); -diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h -index 6afed5a356..d2e32e2ba8 100644 ---- a/source/common/http/conn_manager_impl.h -+++ b/source/common/http/conn_manager_impl.h -@@ -327,6 +327,12 @@ private: - } - - absl::optional routeConfig(); -+ -+ bool iterateUpstreamCallbacks(Upstream::HostDescriptionConstSharedPtr host, -+ StreamInfo::StreamInfo& stream_info) const override { -+ return connection_manager_.read_callbacks_->iterateUpstreamCallbacks(host, stream_info); -+ } -+ - void traceRequest(); - - // Updates the snapped_route_config_ (by reselecting scoped route configuration), if a scope is -diff --git a/source/common/http/filter_manager.cc b/source/common/http/filter_manager.cc -index 665c0572a9..566b1c13cb 100644 ---- a/source/common/http/filter_manager.cc -+++ b/source/common/http/filter_manager.cc -@@ -1954,5 +1954,11 @@ ActiveStreamDecoderFilter::upstreamOverrideHost() const { - return parent_.upstream_override_host_; - } - -+bool ActiveStreamDecoderFilter::iterateUpstreamCallbacks(Upstream::HostDescriptionConstSharedPtr host, -+ StreamInfo::StreamInfo& stream_info) { -+ return parent_.filter_manager_callbacks_.iterateUpstreamCallbacks(host, stream_info); -+ -+} -+ - } // namespace Http - } // namespace Envoy -diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h -index 2a2a8b3178..be6a33ad42 100644 ---- a/source/common/http/filter_manager.h -+++ b/source/common/http/filter_manager.h -@@ -305,6 +305,8 @@ struct ActiveStreamDecoderFilter : public ActiveStreamFilterBase, - void setUpstreamOverrideHost(Upstream::LoadBalancerContext::OverrideHost) override; - absl::optional upstreamOverrideHost() const override; - bool shouldLoadShed() const override; -+ bool iterateUpstreamCallbacks(Upstream::HostDescriptionConstSharedPtr host, -+ StreamInfo::StreamInfo& stream_info) override; - void sendGoAwayAndClose() override; - - // Each decoder filter instance checks if the request passed to the filter is gRPC -@@ -591,6 +593,12 @@ public: - * This is used for HTTP/1.1 codec. - */ - virtual bool isHalfCloseEnabled() PURE; -+ -+ /* -+ * Returns whether connection to the selected upstream host is allowed. -+ */ -+ virtual bool iterateUpstreamCallbacks(Upstream::HostDescriptionConstSharedPtr, -+ StreamInfo::StreamInfo&) const PURE; - }; - - /** -diff --git a/source/common/network/filter_manager_impl.h b/source/common/network/filter_manager_impl.h -index 6453048610..d4132a33ca 100644 ---- a/source/common/network/filter_manager_impl.h -+++ b/source/common/network/filter_manager_impl.h -@@ -156,6 +156,13 @@ private: - parent_.host_description_ = host; - } - bool startUpstreamSecureTransport() override { return parent_.startUpstreamSecureTransport(); } -+ void addUpstreamCallback(const UpstreamCallback& cb) override { -+ parent_.addUpstreamCallback(cb); -+ } -+ bool iterateUpstreamCallbacks(Upstream::HostDescriptionConstSharedPtr host, -+ StreamInfo::StreamInfo& stream_info) override { -+ return parent_.iterateUpstreamCallbacks(host, stream_info); -+ } - - FilterManagerImpl& parent_; - ReadFilterSharedPtr filter_; -@@ -190,6 +197,20 @@ private: - FilterStatus onWrite(ActiveWriteFilter* filter, WriteBufferSource& buffer_source); - void onResumeWriting(ActiveWriteFilter* filter, WriteBufferSource& buffer_source); - -+ void addUpstreamCallback(const UpstreamCallback& cb) { -+ decoder_filter_upstream_cbs_.emplace_back(cb); -+ } -+ -+ bool iterateUpstreamCallbacks(Upstream::HostDescriptionConstSharedPtr host, -+ StreamInfo::StreamInfo& stream_info) { -+ bool accept = true; -+ for (const auto& cb : decoder_filter_upstream_cbs_) { -+ accept = accept && cb(host, stream_info); -+ } -+ return accept; -+ } -+ -+ std::vector decoder_filter_upstream_cbs_{}; - FilterManagerConnection& connection_; - const ConnectionSocket& socket_; - Upstream::HostDescriptionConstSharedPtr host_description_; -diff --git a/source/common/router/router.cc b/source/common/router/router.cc -index 46b4aac132..5affb17366 100644 ---- a/source/common/router/router.cc -+++ b/source/common/router/router.cc -@@ -761,6 +761,14 @@ Http::FilterHeadersStatus Filter::continueDecodeHeaders( - return Http::FilterHeadersStatus::StopIteration; - } - -+ bool accepted = callbacks_->iterateUpstreamCallbacks(host, callbacks_->streamInfo()); -+ if (!accepted) { -+ callbacks_->streamInfo().setResponseFlag(StreamInfo::CoreResponseFlag::UnauthorizedExternalService); -+ callbacks_->sendLocalReply(Http::Code::Forbidden, "Access denied\r\n", -+ nullptr, absl::nullopt, absl::string_view()); -+ return Http::FilterHeadersStatus::StopIteration; -+ } -+ - hedging_params_ = FilterUtility::finalHedgingParams(*route_entry_, headers); - - timeout_ = FilterUtility::finalTimeout(*route_entry_, headers, !config_->suppress_envoy_headers_, -diff --git a/source/common/router/upstream_request.h b/source/common/router/upstream_request.h -index 755e4e2fad..8842169be2 100644 ---- a/source/common/router/upstream_request.h -+++ b/source/common/router/upstream_request.h -@@ -352,6 +352,11 @@ public: - } - OptRef upstreamCallbacks() override { return {*this}; } - -+ bool iterateUpstreamCallbacks(Upstream::HostDescriptionConstSharedPtr, -+ StreamInfo::StreamInfo&) const override { -+ return true; -+ } -+ - // Http::UpstreamStreamFilterCallbacks - StreamInfo::StreamInfo& upstreamStreamInfo() override { return upstream_request_.streamInfo(); } - OptRef upstream() override { -diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc -index c0bf591cab..a5210788da 100644 ---- a/source/common/tcp_proxy/tcp_proxy.cc -+++ b/source/common/tcp_proxy/tcp_proxy.cc -@@ -645,6 +645,13 @@ bool Filter::maybeTunnel(Upstream::ThreadLocalCluster& cluster) { - upstream_decoder_filter_callbacks_, getStreamInfo()); - } - if (generic_conn_pool_) { -+ bool accepted = read_callbacks_->iterateUpstreamCallbacks(generic_conn_pool_->host(), getStreamInfo()); -+ if (!accepted) { -+ getStreamInfo().setResponseFlag(StreamInfo::CoreResponseFlag::UnauthorizedExternalService); -+ onInitFailure(UpstreamFailureReason::UnauthorizedExternalService); -+ return true; -+ } -+ - connecting_ = true; - connect_attempts_++; - getStreamInfo().setAttemptCount(connect_attempts_); -diff --git a/source/common/tcp_proxy/tcp_proxy.h b/source/common/tcp_proxy/tcp_proxy.h -index 355dbdded0..4db9ee8778 100644 ---- a/source/common/tcp_proxy/tcp_proxy.h -+++ b/source/common/tcp_proxy/tcp_proxy.h -@@ -573,6 +573,10 @@ public: - return absl::nullopt; - } - bool shouldLoadShed() const override { return false; } -+ bool iterateUpstreamCallbacks(Upstream::HostDescriptionConstSharedPtr host, -+ StreamInfo::StreamInfo& stream_info) override { -+ return parent_->upstream_decoder_filter_callbacks_.iterateUpstreamCallbacks(host, stream_info); -+ } - void restoreContextOnContinue(ScopeTrackedObjectStack& tracked_object_stack) override { - tracked_object_stack.add(*this); - } -@@ -616,6 +620,7 @@ protected: - NoHealthyUpstream, - ResourceLimitExceeded, - NoRoute, -+ UnauthorizedExternalService, - }; - - // Callbacks for different error and success states during connection establishment -diff --git a/source/common/tcp_proxy/upstream.cc b/source/common/tcp_proxy/upstream.cc -index 05e590a744..d765e415c3 100644 ---- a/source/common/tcp_proxy/upstream.cc -+++ b/source/common/tcp_proxy/upstream.cc -@@ -247,6 +247,10 @@ void TcpConnPool::newStream(GenericConnectionPoolCallbacks& callbacks) { - } - } - -+Upstream::HostDescriptionConstSharedPtr TcpConnPool::host() const { -+ return conn_pool_data_.value().host(); -+} -+ - void TcpConnPool::onPoolFailure(ConnectionPool::PoolFailureReason reason, - absl::string_view failure_reason, - Upstream::HostDescriptionConstSharedPtr host) { -@@ -353,6 +357,10 @@ void HttpConnPool::newStream(GenericConnectionPoolCallbacks& callbacks) { - } - } - -+Upstream::HostDescriptionConstSharedPtr HttpConnPool::host() const { -+ return conn_pool_data_.value().host(); -+} -+ - void HttpConnPool::onPoolFailure(ConnectionPool::PoolFailureReason reason, - absl::string_view failure_reason, - Upstream::HostDescriptionConstSharedPtr host) { -diff --git a/source/common/tcp_proxy/upstream.h b/source/common/tcp_proxy/upstream.h -index 507f966719..1b8b07c492 100644 ---- a/source/common/tcp_proxy/upstream.h -+++ b/source/common/tcp_proxy/upstream.h -@@ -40,6 +40,7 @@ public: - - // GenericConnPool - void newStream(GenericConnectionPoolCallbacks& callbacks) override; -+ Upstream::HostDescriptionConstSharedPtr host() const override; - - // Tcp::ConnectionPool::Callbacks - void onPoolFailure(ConnectionPool::PoolFailureReason reason, -@@ -97,6 +98,7 @@ public: - - // GenericConnPool - void newStream(GenericConnectionPoolCallbacks& callbacks) override; -+ Upstream::HostDescriptionConstSharedPtr host() const override; - - // Http::ConnectionPool::Callbacks, - void onPoolFailure(ConnectionPool::PoolFailureReason reason, -diff --git a/source/server/api_listener_impl.h b/source/server/api_listener_impl.h -index 867a8847ba..5e2feb6a31 100644 ---- a/source/server/api_listener_impl.h -+++ b/source/server/api_listener_impl.h -@@ -79,6 +79,9 @@ protected: - } - Network::Connection& connection() override { return connection_; } - const Network::ConnectionSocket& socket() override { PANIC("not implemented"); } -+ void addUpstreamCallback(const Network::UpstreamCallback&) override {} -+ bool iterateUpstreamCallbacks(Upstream::HostDescriptionConstSharedPtr, -+ StreamInfo::StreamInfo&) override { return true; } - - // Synthetic class that acts as a stub for the connection backing the - // Network::ReadFilterCallbacks. --- -2.34.1 - diff --git a/patches/0003-original_dst_cluster-Avoid-multiple-hosts-for-the-sa.patch b/patches/0002-original_dst_cluster-Avoid-multiple-hosts-for-the-sa.patch similarity index 98% rename from patches/0003-original_dst_cluster-Avoid-multiple-hosts-for-the-sa.patch rename to patches/0002-original_dst_cluster-Avoid-multiple-hosts-for-the-sa.patch index 3a3ac30fa..49c561bfd 100644 --- a/patches/0003-original_dst_cluster-Avoid-multiple-hosts-for-the-sa.patch +++ b/patches/0002-original_dst_cluster-Avoid-multiple-hosts-for-the-sa.patch @@ -1,7 +1,7 @@ -From cd2352dcfd3ee27fbf115330524e5ecef59abfc1 Mon Sep 17 00:00:00 2001 +From 832d30991de452c513806f74f7d8f9934559005b Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Fri, 24 May 2024 18:27:28 +0200 -Subject: [PATCH 3/6] original_dst_cluster: Avoid multiple hosts for the same +Subject: [PATCH 2/8] original_dst_cluster: Avoid multiple hosts for the same address Connection pool containers use HostSharedPtr as map keys, rather than the @@ -25,9 +25,9 @@ map updates. Signed-off-by: Jarno Rajahalme --- - .../original_dst/original_dst_cluster.cc | 261 +++++++++++------- + .../original_dst/original_dst_cluster.cc | 259 +++++++++++------- .../original_dst/original_dst_cluster.h | 43 +-- - 2 files changed, 190 insertions(+), 114 deletions(-) + 2 files changed, 189 insertions(+), 113 deletions(-) diff --git a/source/extensions/clusters/original_dst/original_dst_cluster.cc b/source/extensions/clusters/original_dst/original_dst_cluster.cc index 1e8b9ce0a5..01e739c6fb 100644 @@ -36,7 +36,7 @@ index 1e8b9ce0a5..01e739c6fb 100644 @@ -29,6 +29,19 @@ OriginalDstClusterHandle::~OriginalDstClusterHandle() { dispatcher.post([cluster = std::move(cluster)]() mutable { cluster.reset(); }); } - + +namespace { +HostConstSharedPtr findHost(const HostUseMap& map, const std::string& address) { + auto it = map.find(address); @@ -111,7 +111,7 @@ index 1e8b9ce0a5..01e739c6fb 100644 @@ -216,47 +196,146 @@ OriginalDstCluster::OriginalDstCluster(const envoy::config::cluster::v3::Cluster cleanup_timer_->enableTimer(cleanup_interval_ms_); } - + -void OriginalDstCluster::addHost(HostSharedPtr& host) { - std::string address = host->address()->asString(); - HostMultiMapSharedPtr new_host_map = std::make_shared(*getCurrentHostMap()); @@ -127,9 +127,6 @@ index 1e8b9ce0a5..01e739c6fb 100644 - // The first worker that creates a host for the address defines the primary - // host structure. - new_host_map->emplace(address, std::make_shared(host)); -- } -- ENVOY_LOG(debug, "addHost() adding {} {}.", *host, address); -- setHostMap(new_host_map); +// getHost returns the host for the address. A new host is created when needed. +// Called from the worker threads. +// When multiple worker threads call this at the same time the updates of the @@ -243,8 +240,10 @@ index 1e8b9ce0a5..01e739c6fb 100644 + // Make available for load balancers + host_map_ = new_host_map; + updates_map_.swap(empty_map); -+ } - + } +- ENVOY_LOG(debug, "addHost() adding {} {}.", *host, address); +- setHostMap(new_host_map); + - // Given the current config, only EDS clusters support multiple priorities. ASSERT(priority_set_.hostSetsPerPriority().size() == 1); const auto& first_host_set = priority_set_.getOrCreateHostSet(0); @@ -258,7 +257,7 @@ index 1e8b9ce0a5..01e739c6fb 100644 - {std::move(host)}, {}, random_.random(), absl::nullopt, absl::nullopt); + {std::move(new_hosts)}, {}, random_.random(), absl::nullopt, absl::nullopt); } - + void OriginalDstCluster::cleanup() { - HostVectorSharedPtr keeping_hosts(new HostVector); - HostVector to_be_removed; @@ -353,7 +352,7 @@ index 1e8b9ce0a5..01e739c6fb 100644 - 0, HostSetImpl::partitionHosts(keeping_hosts, HostsPerLocalityImpl::empty()), {}, {}, - to_be_removed, false, absl::nullopt); } - + cleanup_timer_->enableTimer(cleanup_interval_ms_); diff --git a/source/extensions/clusters/original_dst/original_dst_cluster.h b/source/extensions/clusters/original_dst/original_dst_cluster.h index df12c06b41..3152af8664 100644 @@ -362,14 +361,14 @@ index df12c06b41..3152af8664 100644 @@ -22,25 +22,21 @@ namespace Upstream { class OriginalDstClusterFactory; class OriginalDstClusterTest; - + -struct HostsForAddress { - HostsForAddress(HostSharedPtr& host) : host_(host), used_(true) {} +// HostUse tracks the recent use of a host to avoid clearing out a host +// which is not recorded as used in any connection pool. +struct HostUse { + HostUse(HostSharedPtr& host) : host_(host), used_(true) {} - + - // Primary host for the address. This is set by the first worker that posts - // to the main to add a host. The field is read by all workers. + // The host for an address. @@ -382,7 +381,7 @@ index df12c06b41..3152af8664 100644 // Marks as recently used by load balancers. std::atomic used_; }; - + -using HostsForAddressSharedPtr = std::shared_ptr; -using HostMultiMap = absl::flat_hash_map; -using HostMultiMapSharedPtr = std::shared_ptr; @@ -391,9 +390,9 @@ index df12c06b41..3152af8664 100644 +using HostUseMap = absl::flat_hash_map; +using HostUseMapUniquePtr = std::unique_ptr; +using HostUseMapConstSharedPtr = std::shared_ptr; - + class OriginalDstCluster; - + @@ -65,7 +61,8 @@ using OriginalDstClusterHandleSharedPtr = std::shared_ptr& httpHeaderName() { return http_header_name_; } @@ -158,17 +155,23 @@ private: const OriginalDstClusterHandleSharedPtr cluster_; }; - + - HostMultiMapConstSharedPtr getCurrentHostMap() { + const HostUseMap* getHostMap() { + absl::ReaderMutexLock lock(&host_map_lock_); @@ -427,22 +426,22 @@ index df12c06b41..3152af8664 100644 absl::ReaderMutexLock lock(&host_map_lock_); return host_map_; } - + - void setHostMap(const HostMultiMapConstSharedPtr& new_host_map) { + void setHostMap(const HostUseMapConstSharedPtr& new_host_map) { absl::WriterMutexLock lock(&host_map_lock_); host_map_ = new_host_map; } - + - void addHost(HostSharedPtr&); + HostConstSharedPtr getHost(const Network::Address::Instance&); + void updateHosts(); void cleanup(); - + // ClusterImplBase @@ -179,7 +182,9 @@ private: Event::TimerPtr cleanup_timer_; - + absl::Mutex host_map_lock_; - HostMultiMapConstSharedPtr host_map_ ABSL_GUARDED_BY(host_map_lock_); + HostUseMapConstSharedPtr host_map_ ABSL_GUARDED_BY(host_map_lock_); @@ -451,6 +450,6 @@ index df12c06b41..3152af8664 100644 absl::optional http_header_name_; absl::optional metadata_key_; absl::optional port_override_; --- -2.34.1 +-- +2.49.0 diff --git a/patches/0004-tcp_proxy-Check-for-nullptr-in-watermark-ASSERTs.patch b/patches/0003-tcp_proxy-Check-for-nullptr-in-watermark-ASSERTs.patch similarity index 88% rename from patches/0004-tcp_proxy-Check-for-nullptr-in-watermark-ASSERTs.patch rename to patches/0003-tcp_proxy-Check-for-nullptr-in-watermark-ASSERTs.patch index bfc2b11ec..5b9d02e2b 100644 --- a/patches/0004-tcp_proxy-Check-for-nullptr-in-watermark-ASSERTs.patch +++ b/patches/0003-tcp_proxy-Check-for-nullptr-in-watermark-ASSERTs.patch @@ -1,36 +1,35 @@ -From 1e054ee1e266386fc53026c327ff915232f76ece Mon Sep 17 00:00:00 2001 +From 65f021303ba550fb1f53b2815c043a0c2ef8d2d0 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Mon, 2 Dec 2024 08:58:54 +0100 -Subject: [PATCH 7/8] tcp_proxy: Check for nullptr in watermark ASSERTs +Subject: [PATCH 3/8] tcp_proxy: Check for nullptr in watermark ASSERTs Signed-off-by: Jarno Rajahalme - --- source/common/tcp_proxy/tcp_proxy.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc -index a5210788da..532056606f 100644 +index c0bf591cab..1a0f6c4699 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -437,7 +437,7 @@ void Filter::UpstreamCallbacks::onEvent(Network::ConnectionEvent event) { - + void Filter::UpstreamCallbacks::onAboveWriteBufferHighWatermark() { // TCP Tunneling may call on high/low watermark multiple times. - ASSERT(parent_->config_->tunnelingConfigHelper() || !on_high_watermark_called_); + ASSERT(parent_ == nullptr || parent_->config_->tunnelingConfigHelper() || !on_high_watermark_called_); on_high_watermark_called_ = true; - + if (parent_ != nullptr) { @@ -448,7 +448,7 @@ void Filter::UpstreamCallbacks::onAboveWriteBufferHighWatermark() { - + void Filter::UpstreamCallbacks::onBelowWriteBufferLowWatermark() { // TCP Tunneling may call on high/low watermark multiple times. - ASSERT(parent_->config_->tunnelingConfigHelper() || on_high_watermark_called_); + ASSERT(parent_ == nullptr || parent_->config_->tunnelingConfigHelper() || on_high_watermark_called_); on_high_watermark_called_ = false; - + if (parent_ != nullptr) { --- -2.34.1 +-- +2.49.0 diff --git a/patches/0005-thread_local-reset-slot-in-worker-threads-first.patch b/patches/0004-thread_local-reset-slot-in-worker-threads-first.patch similarity index 97% rename from patches/0005-thread_local-reset-slot-in-worker-threads-first.patch rename to patches/0004-thread_local-reset-slot-in-worker-threads-first.patch index f5408eaa7..9c73f2b58 100644 --- a/patches/0005-thread_local-reset-slot-in-worker-threads-first.patch +++ b/patches/0004-thread_local-reset-slot-in-worker-threads-first.patch @@ -1,7 +1,7 @@ -From b13c64877b55c1a645d3b6b1d18a566930ef519c Mon Sep 17 00:00:00 2001 +From e669f3962e4bea69ad5c4deda0b2c6345555a729 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Mon, 23 Dec 2024 22:43:15 +0100 -Subject: [PATCH 8/8] thread_local: reset slot in worker threads first +Subject: [PATCH 4/8] thread_local: reset slot in worker threads first Thread local slots refer to their data via shared pointers. Reset the shared pointer first in the worker threads, and last in the main thread @@ -61,12 +61,12 @@ index 2a49789a09..e57b2fd70d 100644 + // worker thread. + runOnAllWorkerThreads(cb, cb); } - + void InstanceImpl::runOnAllThreads(std::function cb) { @@ -208,6 +214,22 @@ void InstanceImpl::runOnAllThreads(std::function cb, } } - + +void InstanceImpl::runOnAllWorkerThreads(std::function cb, + std::function worker_threads_complete_cb) const { + ASSERT_IS_MAIN_OR_TEST_THREAD(); @@ -95,7 +95,7 @@ index 719418991e..685457afe5 100644 Event::Dispatcher& dispatcher() override; bool isShutdown() const override { return shutdown_; } + void runOnAllWorkerThreads(std::function worker_cb, std::function main_cb) const override; - + private: // On destruction returns the slot index to the deferred delete queue (detaches it). This allows diff --git a/test/mocks/thread_local/mocks.h b/test/mocks/thread_local/mocks.h @@ -110,9 +110,9 @@ index 09dff23777..88d7cea1a9 100644 + worker_cb(); + main_cb(); + } - + SlotPtr allocateSlotMock() { return SlotPtr{new SlotImpl(*this, current_slot_++)}; } void runOnAllThreads1(std::function cb) { cb(); } --- -2.34.1 +-- +2.49.0 diff --git a/patches/0006-http-header-expose-attribute.patch b/patches/0005-Expose-HTTP-Header-matcher-attribute.patch similarity index 97% rename from patches/0006-http-header-expose-attribute.patch rename to patches/0005-Expose-HTTP-Header-matcher-attribute.patch index ffd448105..1cc34664f 100644 --- a/patches/0006-http-header-expose-attribute.patch +++ b/patches/0005-Expose-HTTP-Header-matcher-attribute.patch @@ -1,7 +1,7 @@ -From 024de602cae722d7f738c31d3b69fb99042ddb41 Mon Sep 17 00:00:00 2001 +From c6ce25ebd22141afe4dea330943bb3eceaf6aae5 Mon Sep 17 00:00:00 2001 From: Tam Mach Date: Wed, 19 Mar 2025 21:07:05 +1100 -Subject: [PATCH] Expose HTTP Header matcher attribute +Subject: [PATCH 5/8] Expose HTTP Header matcher attribute Signed-off-by: Tam Mach --- @@ -15,7 +15,7 @@ index 00cb00d163..e479529e20 100644 @@ -92,7 +92,6 @@ public: return present_ != invert_match_; }; - + - private: const LowerCaseString name_; const bool invert_match_; @@ -27,7 +27,7 @@ index 00cb00d163..e479529e20 100644 + const LowerCaseString name_; + const bool invert_match_; + const bool treat_missing_as_empty_; - + protected: // A matcher specific implementation to match the given header_value. virtual bool specificMatchesHeaders(absl::string_view header_value) const PURE; @@ -35,12 +35,12 @@ index 00cb00d163..e479529e20 100644 - const bool invert_match_; - const bool treat_missing_as_empty_; }; - + // Corresponds to the exact_match from the HeaderMatchSpecifier proto in the RDS API. @@ -139,11 +138,12 @@ public: HeaderDataExactMatch(const envoy::config::route::v3::HeaderMatcher& config) : HeaderDataBaseImpl(config), expected_value_(config.exact_match()) {} - + + const std::string expected_value_; + private: @@ -49,14 +49,14 @@ index 00cb00d163..e479529e20 100644 }; - const std::string expected_value_; }; - + // Corresponds to the safe_regex_match from the HeaderMatchSpecifier proto in the RDS API. @@ -158,6 +158,7 @@ public: return std::unique_ptr( new HeaderDataRegexMatch(config, std::move(*regex_or_error))); } + const Regex::CompiledMatcherPtr regex_; - + protected: HeaderDataRegexMatch(const envoy::config::route::v3::HeaderMatcher& config, @@ -168,7 +169,6 @@ public: @@ -65,7 +65,7 @@ index 00cb00d163..e479529e20 100644 }; - const Regex::CompiledMatcherPtr regex_; }; - + // Corresponds to the range_match from the HeaderMatchSpecifier proto in the RDS API. @@ -177,6 +177,8 @@ public: HeaderDataRangeMatch(const envoy::config::route::v3::HeaderMatcher& config) @@ -73,7 +73,7 @@ index 00cb00d163..e479529e20 100644 range_end_(config.range_match().end()) {} + const int64_t range_start_; + const int64_t range_end_; - + private: bool specificMatchesHeaders(absl::string_view header_value) const override { @@ -184,9 +186,6 @@ public: @@ -84,26 +84,26 @@ index 00cb00d163..e479529e20 100644 - const int64_t range_start_; - const int64_t range_end_; }; - + // Corresponds to the prefix_match from the HeaderMatchSpecifier proto in the RDS API. @@ -194,12 +193,12 @@ public: public: HeaderDataPrefixMatch(const envoy::config::route::v3::HeaderMatcher& config) : HeaderDataBaseImpl(config), prefix_(config.prefix_match()) {} + const std::string prefix_; - + private: bool specificMatchesHeaders(absl::string_view header_value) const override { return absl::StartsWith(header_value, prefix_); }; - const std::string prefix_; }; - + // Corresponds to the suffix_match from the HeaderMatchSpecifier proto in the RDS API. @@ -208,11 +207,12 @@ public: HeaderDataSuffixMatch(const envoy::config::route::v3::HeaderMatcher& config) : HeaderDataBaseImpl(config), suffix_(config.suffix_match()) {} - + + const std::string suffix_; + private: @@ -112,36 +112,36 @@ index 00cb00d163..e479529e20 100644 }; - const std::string suffix_; }; - + // Corresponds to the contains_match from the HeaderMatchSpecifier proto in the RDS API. @@ -220,12 +220,12 @@ public: public: HeaderDataContainsMatch(const envoy::config::route::v3::HeaderMatcher& config) : HeaderDataBaseImpl(config), expected_substr_(config.contains_match()) {} + const std::string expected_substr_; - + private: bool specificMatchesHeaders(absl::string_view header_value) const override { return absl::StrContains(header_value, expected_substr_); }; - const std::string expected_substr_; }; - + // Corresponds to the string_match from the HeaderMatchSpecifier proto in the RDS API. @@ -235,12 +235,12 @@ public: Server::Configuration::CommonFactoryContext& factory_context) : HeaderDataBaseImpl(config), string_match_(std::make_unique( config.string_match(), factory_context)) {} + const Matchers::StringMatcherPtr string_match_; - + private: bool specificMatchesHeaders(absl::string_view header_value) const override { return string_match_->match(header_value); }; - const Matchers::StringMatcherPtr string_match_; }; - + using HeaderDataPtr = std::unique_ptr; --- -2.34.1 +-- +2.49.0 diff --git a/patches/0007-liburing-arm-build.patch b/patches/0006-build-Fix-arm-build-for-liburing.patch similarity index 91% rename from patches/0007-liburing-arm-build.patch rename to patches/0006-build-Fix-arm-build-for-liburing.patch index 6e478e7e1..d39389564 100644 --- a/patches/0007-liburing-arm-build.patch +++ b/patches/0006-build-Fix-arm-build-for-liburing.patch @@ -1,7 +1,7 @@ -From 6d8d0c7dba595676d249a3c7fbba4ede3ebd19cd Mon Sep 17 00:00:00 2001 +From f09bb19055c30acadd887e3f844ab3eb20c8cb7e Mon Sep 17 00:00:00 2001 From: Tam Mach Date: Wed, 14 May 2025 11:27:14 +1000 -Subject: [PATCH 7/7] build: Fix arm build for liburing +Subject: [PATCH 6/8] build: Fix arm build for liburing --- bazel/foreign_cc/BUILD | 14 ++++++++++++++ @@ -32,6 +32,6 @@ index f97ff45b55..96b07e3c83 100644 lib_source = "@com_github_axboe_liburing//:all", tags = [ "nocompdb", --- -2.34.1 +-- +2.49.0 diff --git a/patches/0007-network-Add-filter-callback-onDestinationSelected.patch b/patches/0007-network-Add-filter-callback-onDestinationSelected.patch new file mode 100644 index 000000000..d3f9ae641 --- /dev/null +++ b/patches/0007-network-Add-filter-callback-onDestinationSelected.patch @@ -0,0 +1,375 @@ +From b965363ca76dbbc740d1ca92950b5c5891f98081 Mon Sep 17 00:00:00 2001 +From: Jarno Rajahalme +Date: Tue, 24 Jun 2025 22:15:24 +0200 +Subject: [PATCH 7/8] network: Add filter callback 'onDestinationSelected' + +Add a new upstream read filter callback 'onDestinationSelected' that is +called after the upstream host has been selected, but before the network +connection is attempted. Only called on upstream filters, as downstream +filter already have a connected connection when created. + +Any upstream network filter returning 'StopIteration' from this callback +causes the connection to not be made and a local reset event be +raised. Http codec client translates this event to a new +'StreamResetReason::LocalAccessDenied', which in turn results into +'Http::Code::Forbidden' ('403') be returned. + +Signed-off-by: Jarno Rajahalme +--- + envoy/http/stream_reset_handler.h | 2 ++ + envoy/network/filter.h | 17 +++++++++++ + envoy/stream_info/stream_info.h | 1 + + envoy/upstream/upstream.h | 1 + + source/common/http/codec_client.cc | 9 ++++-- + source/common/http/conn_pool_base.cc | 3 ++ + source/common/http/utility.cc | 2 ++ + source/common/network/connection_impl.cc | 32 ++++++++++++++++++++ + source/common/network/connection_impl.h | 2 ++ + source/common/network/filter_manager_impl.cc | 18 +++++++++++ + source/common/network/filter_manager_impl.h | 14 +++++++++ + source/common/quic/envoy_quic_utils.cc | 1 + + source/common/router/retry_state_impl.cc | 5 +-- + source/common/router/router.cc | 22 +++++++++----- + source/common/router/upstream_request.cc | 9 +++++- + 15 files changed, 124 insertions(+), 14 deletions(-) + +diff --git a/envoy/http/stream_reset_handler.h b/envoy/http/stream_reset_handler.h +index 09e3b1dd80..55b9094834 100644 +--- a/envoy/http/stream_reset_handler.h ++++ b/envoy/http/stream_reset_handler.h +@@ -39,6 +39,8 @@ enum class StreamResetReason { + OverloadManager, + // If stream was locally reset due to HTTP/1 upstream half closing before downstream. + Http1PrematureUpstreamHalfClose, ++ // If the connection was denied by an upstream filter (no retries allowed) ++ LocalAccessDenied, + }; + + /** +diff --git a/envoy/network/filter.h b/envoy/network/filter.h +index 2ad394ffeb..156e461b62 100644 +--- a/envoy/network/filter.h ++++ b/envoy/network/filter.h +@@ -250,6 +250,23 @@ public: + */ + virtual FilterStatus onData(Buffer::Instance& data, bool end_stream) PURE; + ++ /** ++ * Called before an upstream connection is connected, so before onNewConnection is called. ++ * At the time of the callback the upstream socket options have not been applied yet, nor ++ * has the source address been bound. ++ * Not called for downstream filters, which are created only after the connection has already ++ * been accepted. ++ * @param destination_address is the address the connection will be made to, if allowed. ++ * @param stream_info is the StreamInfo of the client connection. ++ * @return optional status used by the filter manager to manage further filter iteration. If any ++ * filter returns StopIteration, the connection is not allowed and the iteration is ++ * stopped without calling any callbacks on the remaining filters. ++ */ ++ virtual absl::optional onDestinationSelected(const Address::InstanceConstSharedPtr&, ++ StreamInfo::StreamInfo&) { ++ return absl::nullopt; ++ } ++ + /** + * Called when a connection is first established. Filters should do one time long term processing + * that needs to be done when a connection is established. Filter chain iteration can be stopped +diff --git a/envoy/stream_info/stream_info.h b/envoy/stream_info/stream_info.h +index 6cb68573d2..fff7022203 100644 +--- a/envoy/stream_info/stream_info.h ++++ b/envoy/stream_info/stream_info.h +@@ -280,6 +280,7 @@ struct LocalCloseReasonValues { + "closing_upstream_tcp_connection_due_to_downstream_reset_close"; + const std::string NonPooledTcpConnectionHostHealthFailure = + "non_pooled_tcp_connection_host_health_failure"; ++ const std::string AccessDenied = "access_denied"; + }; + + using LocalCloseReasons = ConstSingleton; +diff --git a/envoy/upstream/upstream.h b/envoy/upstream/upstream.h +index 5212183a5a..e9978df460 100644 +--- a/envoy/upstream/upstream.h ++++ b/envoy/upstream/upstream.h +@@ -739,6 +739,7 @@ public: + COUNTER(upstream_rq_retry_success) \ + COUNTER(upstream_rq_rx_reset) \ + COUNTER(upstream_rq_timeout) \ ++ COUNTER(upstream_rq_local_access_denied) \ + COUNTER(upstream_rq_total) \ + COUNTER(upstream_rq_tx_reset) \ + COUNTER(upstream_http3_broken) \ +diff --git a/source/common/http/codec_client.cc b/source/common/http/codec_client.cc +index 4e437da158..3be7242246 100644 +--- a/source/common/http/codec_client.cc ++++ b/source/common/http/codec_client.cc +@@ -107,9 +107,12 @@ void CodecClient::onEvent(Network::ConnectionEvent event) { + active_requests_.size()); + disableIdleTimer(); + idle_timer_.reset(); +- StreamResetReason reason = event == Network::ConnectionEvent::RemoteClose +- ? StreamResetReason::RemoteConnectionFailure +- : StreamResetReason::LocalConnectionFailure; ++ StreamResetReason reason = ++ event == Network::ConnectionEvent::RemoteClose ++ ? StreamResetReason::RemoteConnectionFailure ++ : (connection_->localCloseReason() == StreamInfo::LocalCloseReasons::get().AccessDenied ++ ? StreamResetReason::LocalAccessDenied ++ : StreamResetReason::LocalConnectionFailure); + if (connected_) { + reason = StreamResetReason::ConnectionTermination; + if (protocol_error_) { +diff --git a/source/common/http/conn_pool_base.cc b/source/common/http/conn_pool_base.cc +index f65427fbcc..4c2505eea1 100644 +--- a/source/common/http/conn_pool_base.cc ++++ b/source/common/http/conn_pool_base.cc +@@ -179,6 +179,9 @@ void MultiplexedActiveClientBase::onStreamReset(Http::StreamResetReason reason) + case StreamResetReason::ConnectError: + case StreamResetReason::Http1PrematureUpstreamHalfClose: + break; ++ case StreamResetReason::LocalAccessDenied: ++ parent_.host()->cluster().trafficStats()->upstream_rq_local_access_denied_.inc(); ++ break; + } + } + +diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc +index 747bd5d699..988a4643ee 100644 +--- a/source/common/http/utility.cc ++++ b/source/common/http/utility.cc +@@ -1148,6 +1148,8 @@ const std::string Utility::resetReasonToString(const Http::StreamResetReason res + return "overload manager reset"; + case Http::StreamResetReason::Http1PrematureUpstreamHalfClose: + return "HTTP/1 premature upstream half close"; ++ case Http::StreamResetReason::LocalAccessDenied: ++ return "local access denied"; + } + + return ""; +diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc +index b1670c0fe8..b591e8ae28 100644 +--- a/source/common/network/connection_impl.cc ++++ b/source/common/network/connection_impl.cc +@@ -1054,6 +1054,26 @@ ClientConnectionImpl::ClientConnectionImpl( + if (socket_->connectionInfoProviderSharedPtr()->remoteAddress()->ip() == nullptr) { + return; + } ++ ++ // Check if any filter denies connect to the selected destination IP address. ++ // This is done at the end of construction so that the client connection is fully set up ++ // when calling the filters' onDestinationSelected() callback. ++ if (!isConnectAllowed(stream_info_)) { ++ setFailureReason(StreamInfo::LocalCloseReasons::get().AccessDenied); ++ ENVOY_CONN_LOG_EVENT(debug, "connect_denied", "{}", *this, failureReason()); ++ ++ // Set the reason for the local close ++ setLocalCloseReason(StreamInfo::LocalCloseReasons::get().AccessDenied); ++ ++ // Set a special error state to ensure asynchronous close to give the owner of the ++ // ConnectionImpl a chance to add callbacks and detect the "disconnect". ++ immediate_error_event_ = ConnectionEvent::LocalClose; ++ ++ // Trigger a write event to close this connection out-of-band. ++ ioHandle().activateFileEvents(Event::FileReadyType::Write); ++ return; ++ } ++ + if (!Network::Socket::applyOptions(options, *socket_, + envoy::config::core::v3::SocketOption::STATE_PREBIND)) { + // Set a special error state to ensure asynchronous close to give the owner of the +@@ -1087,6 +1107,18 @@ ClientConnectionImpl::ClientConnectionImpl( + } + } + ++bool ConnectionImpl::isConnectAllowed(StreamInfo::StreamInfo& stream_info) const { ++ const auto& dst = socket_->connectionInfoProvider().remoteAddress(); ++ if (dst->ip() == nullptr) { ++ return true; ++ } ++ ++ // Ask upstream filters if the connection should be denied ++ auto status = filter_manager_.onDestinationSelected(dst, stream_info); ++ ++ return !status.has_value() || status.value() == FilterStatus::Continue; ++} ++ + void ClientConnectionImpl::connect() { + ENVOY_CONN_LOG_EVENT(debug, "client_connection", "connecting to {}", *this, + socket_->connectionInfoProvider().remoteAddress()->asString()); +diff --git a/source/common/network/connection_impl.h b/source/common/network/connection_impl.h +index cef15aea42..9e84a93feb 100644 +--- a/source/common/network/connection_impl.h ++++ b/source/common/network/connection_impl.h +@@ -163,6 +163,8 @@ public: + + DetectedCloseType detectedCloseType() const override { return detected_close_type_; } + ++ bool isConnectAllowed(StreamInfo::StreamInfo& stream_info) const; ++ + protected: + // A convenience function which returns true if + // 1) The read disable count is zero or +diff --git a/source/common/network/filter_manager_impl.cc b/source/common/network/filter_manager_impl.cc +index a1cf193961..0239eecec5 100644 +--- a/source/common/network/filter_manager_impl.cc ++++ b/source/common/network/filter_manager_impl.cc +@@ -59,6 +59,24 @@ bool FilterManagerImpl::initializeReadFilters() { + return true; + } + ++absl::optional ++FilterManagerImpl::onDestinationSelected(const Address::InstanceConstSharedPtr& destination_address, ++ StreamInfo::StreamInfo& stream_info) const { ++ absl::optional status; ++ for (auto& entry : upstream_filters_) { ++ if (entry->filter_) { ++ auto opt = entry->filter_->onDestinationSelected(destination_address, stream_info); ++ if (opt.has_value()) { ++ status = opt; ++ if (status.value() == FilterStatus::StopIteration) { ++ return status; ++ } ++ } ++ } ++ } ++ return status; ++} ++ + void FilterManagerImpl::onContinueReading(ActiveReadFilter* filter, + ReadBufferSource& buffer_source) { + // Filter could return status == FilterStatus::StopIteration immediately, close the connection and +diff --git a/source/common/network/filter_manager_impl.h b/source/common/network/filter_manager_impl.h +index 6453048610..aeba69876d 100644 +--- a/source/common/network/filter_manager_impl.h ++++ b/source/common/network/filter_manager_impl.h +@@ -102,6 +102,8 @@ public: + virtual void closeConnection(ConnectionCloseAction action) PURE; + }; + ++class ConnectionImpl; ++ + /** + * This is a filter manager for TCP (L4) filters. It is split out for ease of testing. + */ +@@ -134,6 +136,18 @@ protected: + bool local_close_pending_{false}; + }; + ++ // onDestinationSelected calls onDestinationSelected callback on all read filters during ++ // ClientConnectionImpl construction, after the client connection has been set up, but before ++ // the socket options have been applied, source address is bound, or connect call is made. ++ // @param destination_address is already validated to be an IP address ++ // @return optional FilterStatus which is StopIteration if any upstream filter returned ++ // StopIteration, Continue if any upstream filter returned Continue, but not ++ // StopIteration, and absl::nullopt otherwise. ++ friend ConnectionImpl; ++ absl::optional ++ onDestinationSelected(const Address::InstanceConstSharedPtr& destination_address, ++ StreamInfo::StreamInfo& stream_info) const; ++ + private: + struct ActiveReadFilter : public ReadFilterCallbacks, LinkedObject { + ActiveReadFilter(FilterManagerImpl& parent, ReadFilterSharedPtr filter) +diff --git a/source/common/quic/envoy_quic_utils.cc b/source/common/quic/envoy_quic_utils.cc +index d4ade8773f..f62a26cdef 100644 +--- a/source/common/quic/envoy_quic_utils.cc ++++ b/source/common/quic/envoy_quic_utils.cc +@@ -93,6 +93,7 @@ quic::QuicRstStreamErrorCode envoyResetReasonToQuicRstError(Http::StreamResetRea + case Http::StreamResetReason::ConnectError: + return quic::QUIC_STREAM_CONNECT_ERROR; + case Http::StreamResetReason::LocalReset: ++ case Http::StreamResetReason::LocalAccessDenied: + return quic::QUIC_STREAM_REQUEST_REJECTED; + case Http::StreamResetReason::OverloadManager: + return quic::QUIC_STREAM_EXCESSIVE_LOAD; +diff --git a/source/common/router/retry_state_impl.cc b/source/common/router/retry_state_impl.cc +index 1dc0f96f17..48b1594b93 100644 +--- a/source/common/router/retry_state_impl.cc ++++ b/source/common/router/retry_state_impl.cc +@@ -466,8 +466,9 @@ RetryStateImpl::wouldRetryFromReset(const Http::StreamResetReason reset_reason, + bool upstream_request_started) { + ASSERT(!disable_http3); + // First check "never retry" conditions so we can short circuit (we never +- // retry if the reset reason is overflow). +- if (reset_reason == Http::StreamResetReason::Overflow) { ++ // retry if the reset reason is overflow or local access denied). ++ if (reset_reason == Http::StreamResetReason::Overflow || ++ reset_reason == Http::StreamResetReason::LocalAccessDenied) { + return RetryDecision::NoRetry; + } + +diff --git a/source/common/router/router.cc b/source/common/router/router.cc +index 46b4aac132..fd94e46d42 100644 +--- a/source/common/router/router.cc ++++ b/source/common/router/router.cc +@@ -1499,9 +1499,11 @@ void Filter::onUpstreamReset(Http::StreamResetReason reset_reason, + return; + } + +- const Http::Code error_code = (reset_reason == Http::StreamResetReason::ProtocolError) +- ? Http::Code::BadGateway +- : Http::Code::ServiceUnavailable; ++ const Http::Code error_code = (reset_reason == Http::StreamResetReason::LocalAccessDenied) ++ ? Http::Code::Forbidden ++ : (reset_reason == Http::StreamResetReason::ProtocolError ++ ? Http::Code::BadGateway ++ : Http::Code::ServiceUnavailable); + chargeUpstreamAbort(error_code, dropped, upstream_request); + auto request_ptr = upstream_request.removeFromList(upstream_requests_); + callbacks_->dispatcher().deferredDelete(std::move(request_ptr)); +@@ -1515,11 +1517,13 @@ void Filter::onUpstreamReset(Http::StreamResetReason reset_reason, + const StreamInfo::CoreResponseFlag response_flags = streamResetReasonToResponseFlag(reset_reason); + + const std::string body = +- absl::StrCat("upstream connect error or disconnect/reset before headers. ", +- (is_retry_ ? "retried and the latest " : ""), +- "reset reason: ", Http::Utility::resetReasonToString(reset_reason), +- !transport_failure_reason.empty() ? ", transport failure reason: " : "", +- transport_failure_reason); ++ (error_code == Http::Code::Forbidden) ++ ? "access denied\r\n" ++ : absl::StrCat("upstream connect error or disconnect/reset before headers. ", ++ (is_retry_ ? "retried and the latest " : ""), ++ "reset reason: ", Http::Utility::resetReasonToString(reset_reason), ++ !transport_failure_reason.empty() ? ", transport failure reason: " : "", ++ transport_failure_reason); + const std::string& basic_details = + downstream_response_started_ ? StreamInfo::ResponseCodeDetails::get().LateUpstreamReset + : StreamInfo::ResponseCodeDetails::get().EarlyUpstreamReset; +@@ -1576,6 +1580,8 @@ Filter::streamResetReasonToResponseFlag(Http::StreamResetReason reset_reason) { + return StreamInfo::CoreResponseFlag::UpstreamProtocolError; + case Http::StreamResetReason::OverloadManager: + return StreamInfo::CoreResponseFlag::OverloadManager; ++ case Http::StreamResetReason::LocalAccessDenied: ++ return StreamInfo::CoreResponseFlag::UnauthorizedExternalService; + } + + PANIC_DUE_TO_CORRUPT_ENUM; +diff --git a/source/common/router/upstream_request.cc b/source/common/router/upstream_request.cc +index c5de7a3789..e6ca24dc4d 100644 +--- a/source/common/router/upstream_request.cc ++++ b/source/common/router/upstream_request.cc +@@ -560,13 +560,20 @@ void UpstreamRequest::onPoolFailure(ConnectionPool::PoolFailureReason reason, + absl::string_view transport_failure_reason, + Upstream::HostDescriptionConstSharedPtr host) { + recordConnectionPoolCallbackLatency(); +- Http::StreamResetReason reset_reason = [](ConnectionPool::PoolFailureReason reason) { ++ Http::StreamResetReason reset_reason = [transport_failure_reason](ConnectionPool::PoolFailureReason reason) { + switch (reason) { + case ConnectionPool::PoolFailureReason::Overflow: + return Http::StreamResetReason::Overflow; + case ConnectionPool::PoolFailureReason::RemoteConnectionFailure: + return Http::StreamResetReason::RemoteConnectionFailure; + case ConnectionPool::PoolFailureReason::LocalConnectionFailure: ++ // Set LocalAccessDenied if transport failure reason is "access_denied". ++ // It would be more elegant to have a dedicated "LocalAccessDenied" PoolFailureReason, but ++ // that would have to be reflected on all switch statements on PoolFailureReason and is left ++ // for future enhancement. ++ if (transport_failure_reason == StreamInfo::LocalCloseReasons::get().AccessDenied) { ++ return Http::StreamResetReason::LocalAccessDenied; ++ } + return Http::StreamResetReason::LocalConnectionFailure; + case ConnectionPool::PoolFailureReason::Timeout: + return Http::StreamResetReason::ConnectionTimeout; +-- +2.49.0 + diff --git a/patches/0008-network-Compat-for-missing-upstream-filter.patch b/patches/0008-network-Compat-for-missing-upstream-filter.patch new file mode 100644 index 000000000..9a0616dd5 --- /dev/null +++ b/patches/0008-network-Compat-for-missing-upstream-filter.patch @@ -0,0 +1,90 @@ +From 05750d1510cdfe1a8ea23e460d974b8328704301 Mon Sep 17 00:00:00 2001 +From: Jarno Rajahalme +Date: Mon, 23 Jun 2025 14:08:09 +0200 +Subject: [PATCH 8/8] network: Compat for missing upstream filter + +Call the new 'onDestinationSelected' read filter callback on downstream +filter if no upstream filters implement it. This is only done if +Network::Cilium::DownstreamConnection filter state is present. Currently +this is set up by the cilium.network filter. + +This is done for backwards compatibility with current Cilium agent +releases that do not yet set up the upstream network filter. Once the +oldest supported Cilium release does that, this patch can be removed. + +Signed-off-by: Jarno Rajahalme +--- + envoy/network/connection.h | 16 ++++++++++++++++ + source/common/network/connection_impl.cc | 22 ++++++++++++++++++++++ + 2 files changed, 38 insertions(+) + +diff --git a/envoy/network/connection.h b/envoy/network/connection.h +index c7a93e8327..ac43af2a13 100644 +--- a/envoy/network/connection.h ++++ b/envoy/network/connection.h +@@ -23,6 +23,22 @@ class Dispatcher; + + namespace Network { + ++// Cilium backwards compat. Remove when Cilium 1.18 is the oldest supported release. ++class ConnectionImpl; ++ ++namespace Cilium { ++ ++class DownstreamConnection : public StreamInfo::FilterState::Object { ++public: ++ DownstreamConnection(Connection* connection) : connection_(connection) {} ++ ++ static const std::string& key(); ++ ++ Connection* connection_; ++}; ++ ++} // namespace Cilium ++ + /** + * Events that occur on a connection. + */ +diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc +index b591e8ae28..23137ea0a8 100644 +--- a/source/common/network/connection_impl.cc ++++ b/source/common/network/connection_impl.cc +@@ -1107,6 +1107,14 @@ ClientConnectionImpl::ClientConnectionImpl( + } + } + ++namespace Cilium { ++ ++const std::string& DownstreamConnection::key() { ++ CONSTRUCT_ON_FIRST_USE(std::string, "cilium.downstream_connection"); ++} ++ ++} // namespace Cilium ++ + bool ConnectionImpl::isConnectAllowed(StreamInfo::StreamInfo& stream_info) const { + const auto& dst = socket_->connectionInfoProvider().remoteAddress(); + if (dst->ip() == nullptr) { +@@ -1116,6 +1124,20 @@ bool ConnectionImpl::isConnectAllowed(StreamInfo::StreamInfo& stream_info) const + // Ask upstream filters if the connection should be denied + auto status = filter_manager_.onDestinationSelected(dst, stream_info); + ++ if (!status.has_value()) { ++ // No upstream filter returned a value, check also downstream filters, if available. ++ const auto downstream_connection_fs = ++ stream_info_.filterState()->getDataReadOnly( ++ Cilium::DownstreamConnection::key()); ++ if (downstream_connection_fs) { ++ const ConnectionImpl* conn = dynamic_cast( ++ downstream_connection_fs->connection_); ++ if (conn) { ++ status = conn->filter_manager_.onDestinationSelected(dst, stream_info); ++ } ++ } ++ } ++ + return !status.has_value() || status.value() == FilterStatus::Continue; + } + +-- +2.49.0 + diff --git a/tests/metadata_config_test.cc b/tests/metadata_config_test.cc index dbf464936..2055ba073 100644 --- a/tests/metadata_config_test.cc +++ b/tests/metadata_config_test.cc @@ -3,7 +3,6 @@ #include #include -#include #include #include #include @@ -36,9 +35,9 @@ #include "absl/status/status.h" #include "absl/strings/string_view.h" -#include "cilium/accesslog.h" #include "cilium/api/bpf_metadata.pb.h" #include "cilium/bpf_metadata.h" +#include "cilium/ipcache.h" #include "gtest/gtest.h" #include "tests/bpf_metadata.h" @@ -348,17 +347,8 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbIngressEnforcedMetadata) { 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"); - // Expect policy accepts security ID 12345678 on ingress on port 80 - bool use_proxy_lib; - std::string l7_proto; - EXPECT_TRUE( - policy_fs->enforceNetworkPolicy(conn_, 12345678, 80, "", use_proxy_lib, l7_proto, log_entry)); - EXPECT_FALSE(use_proxy_lib); - EXPECT_EQ("", l7_proto); - EXPECT_NE("pod", log_entry.entry_.policy_name()); + EXPECT_TRUE(policy_fs->enforceIngressNetworkPolicy(conn_, 12345678, 80, "")); auto source_addresses_socket_option = socket_metadata->buildSourceAddressSocketOption(-1); EXPECT_NE(nullptr, source_addresses_socket_option); @@ -418,35 +408,29 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbPodAndIngressEnforcedMetadata) { 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"); - // Expect pod policy denies security ID 12345678 on port 80 (only 222 allowed) bool use_proxy_lib; std::string l7_proto; EXPECT_FALSE( - policy_fs->enforceNetworkPolicy(conn_, 12345678, 80, "", use_proxy_lib, l7_proto, log_entry)); + policy_fs->enforcePodNetworkPolicy(conn_, 12345678, 80, "", use_proxy_lib, l7_proto)); EXPECT_FALSE(use_proxy_lib); EXPECT_EQ("", l7_proto); - EXPECT_EQ("pod", log_entry.entry_.policy_name()); // Expect pod policy allows egress to security ID 222 on port 80 // Ingress policy allows ingress from 9999 (pod's security ID) // Ingress policy allows 222 egress - EXPECT_TRUE( - policy_fs->enforceNetworkPolicy(conn_, 222, 80, "", use_proxy_lib, l7_proto, log_entry)); + EXPECT_TRUE(policy_fs->enforcePodNetworkPolicy(conn_, 222, 80, "", use_proxy_lib, l7_proto)); EXPECT_FALSE(use_proxy_lib); EXPECT_EQ("", l7_proto); - EXPECT_NE("pod", log_entry.entry_.policy_name()); + EXPECT_TRUE(policy_fs->enforceIngressNetworkPolicy(conn_, 222, 80, "")); // Expect pod policy allows egress to security ID 333 on port 80 // Ingress policy allows ingress from 9999 // Ingress policy denies 333 egress - EXPECT_FALSE( - policy_fs->enforceNetworkPolicy(conn_, 333, 80, "", use_proxy_lib, l7_proto, log_entry)); + EXPECT_TRUE(policy_fs->enforcePodNetworkPolicy(conn_, 333, 80, "", use_proxy_lib, l7_proto)); EXPECT_FALSE(use_proxy_lib); EXPECT_EQ("", l7_proto); - EXPECT_NE("pod", log_entry.entry_.policy_name()); + EXPECT_FALSE(policy_fs->enforceIngressNetworkPolicy(conn_, 333, 80, "")); auto source_addresses_socket_option = socket_metadata->buildSourceAddressSocketOption(-1); EXPECT_NE(nullptr, source_addresses_socket_option); @@ -489,17 +473,8 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbIngressEnforcedCIDRMetadata) { 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"); - // Expect policy does not accept security ID 2 on port 80 - bool use_proxy_lib; - std::string l7_proto; - EXPECT_FALSE( - policy_fs->enforceNetworkPolicy(conn_, 2, 80, "", use_proxy_lib, l7_proto, log_entry)); - EXPECT_FALSE(use_proxy_lib); - EXPECT_EQ("", l7_proto); - EXPECT_NE("pod", log_entry.entry_.policy_name()); + EXPECT_FALSE(policy_fs->enforceIngressNetworkPolicy(conn_, 2, 80, "")); auto source_addresses_socket_option = socket_metadata->buildSourceAddressSocketOption(-1); EXPECT_NE(nullptr, source_addresses_socket_option);