diff --git a/WORKSPACE b/WORKSPACE index 69548b889..a26f95246 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -39,6 +39,7 @@ git_repository( "@//patches:0002-upstream-Add-callback-for-upstream-authorization.patch", "@//patches:0003-tcp_proxy-Add-filter-state-proxy_read_before_connect.patch", "@//patches:0004-router-Do-not-set-SNI-or-SAN-due-to-auto_sni-or-auto.patch", + "@//patches:0005-listener-add-socket-options.patch", ], ) diff --git a/cilium/bpf_metadata.cc b/cilium/bpf_metadata.cc index bdee8fb2f..af860b8b5 100644 --- a/cilium/bpf_metadata.cc +++ b/cilium/bpf_metadata.cc @@ -37,6 +37,14 @@ class BpfMetadataConfigFactory : public NamedListenerFilterConfigFactory { MessageUtil::downcastAndValidate( proto_config, context.messageValidationVisitor()), context); + // Set the socket mark option for the listen socket. + // Can use identity 0 on the listen socket option, as the bpf datapath is only interested + // in whether the proxy is ingress, egress, or if there is no proxy at all. + std::shared_ptr options = std::make_shared(); + uint32_t mark = (config->is_ingress_) ? 0x0A00 : 0x0B00; + options->push_back(std::make_shared(mark, 0)); + context.addListenSocketOptions(options); + return [listener_filter_matcher, config](Network::ListenerFilterManager& filter_manager) mutable -> void { filter_manager.addAcceptFilter(listener_filter_matcher, diff --git a/cilium/network_filter.cc b/cilium/network_filter.cc index 2717b7bdc..08ebbd3bb 100644 --- a/cilium/network_filter.cc +++ b/cilium/network_filter.cc @@ -107,7 +107,7 @@ Network::FilterStatus Instance::onNewConnection() { // Pass metadata from tls_inspector to the filterstate, if any & not already // set via upstream cluster config, but not in a sidecar, which have no mark - if (sni != "" && option->mark_ != 0) { + if (sni != "" && !option->isSidecar()) { auto filterState = conn.streamInfo().filterState(); auto have_sni = filterState->hasData(Network::UpstreamServerName::key()); @@ -128,7 +128,8 @@ Network::FilterStatus Instance::onNewConnection() { StreamInfo::StreamInfo& stream_info) -> bool { ENVOY_LOG(trace, "cilium.network: in upstream callback"); auto& conn = callbacks_->connection(); - + uint32_t source_identity = option->identity_; + // Resolve the destination security ID and port uint32_t destination_identity = 0; uint32_t destination_port = option->port_; @@ -152,19 +153,20 @@ Network::FilterStatus Instance::onNewConnection() { destination_identity = option->resolvePolicyId(dip); } + uint32_t identity = option->ingress_ ? source_identity : destination_identity; + port_policy_ = option->initial_policy_->findPortPolicy(option->ingress_, destination_port, - option->ingress_ ? option->identity_ - : destination_identity); + identity); if (port_policy_ == nullptr) { ENVOY_CONN_LOG(warn, "cilium.network: Policy NOT FOUND for id: {} port: {}", conn, - option->ingress_ ? option->identity_ : destination_identity, destination_port); + identity, destination_port); return false; } - if (!port_policy_->Matches(sni, option->ingress_ ? option->identity_ : destination_identity)) { + if (!port_policy_->Matches(sni, identity)) { // Connection not allowed by policy ENVOY_CONN_LOG(warn, "cilium.network: Policy DENY on id: {} port: {}", conn, - option->ingress_ ? option->identity_ : destination_identity, destination_port); + identity, destination_port); return false; } @@ -174,7 +176,7 @@ Network::FilterStatus Instance::onNewConnection() { // Initialize Go parser if requested if (config_->proxylib_.get() != nullptr) { go_parser_ = config_->proxylib_->NewInstance( - conn, l7proto_, option->ingress_, option->identity_, destination_identity, + conn, l7proto_, option->ingress_, source_identity, destination_identity, stream_info.downstreamAddressProvider().remoteAddress()->asString(), dst_address->asString(), policy_name); if (go_parser_.get() == nullptr) { @@ -182,7 +184,7 @@ Network::FilterStatus Instance::onNewConnection() { return false; } } else { // no Go parser, initialize logging for metadata based access control - log_entry_.InitFromConnection(policy_name, option->ingress_, option->identity_, + log_entry_.InitFromConnection(policy_name, option->ingress_, source_identity, stream_info.downstreamAddressProvider().remoteAddress(), destination_identity, dst_address, &config_->time_source_); } diff --git a/cilium/socket_option.h b/cilium/socket_option.h index 71a8417a8..58faa2337 100644 --- a/cilium/socket_option.h +++ b/cilium/socket_option.h @@ -26,11 +26,11 @@ class PolicyResolver { class SocketMarkOption : public Network::Socket::Option, public Logger::Loggable { public: - SocketMarkOption(uint32_t mark, uint32_t identity, bool ingress, bool l7lb, - Network::Address::InstanceConstSharedPtr original_source_address, - Network::Address::InstanceConstSharedPtr ipv4_source_address, - Network::Address::InstanceConstSharedPtr ipv6_source_address) - : identity_(identity), mark_(mark), ingress_(ingress), is_l7lb_(l7lb), + SocketMarkOption(uint32_t mark, uint32_t identity, + Network::Address::InstanceConstSharedPtr original_source_address = nullptr, + Network::Address::InstanceConstSharedPtr ipv4_source_address = nullptr, + Network::Address::InstanceConstSharedPtr ipv6_source_address = nullptr) + : identity_(identity), mark_(mark), original_source_address_(std::move(original_source_address)), ipv4_source_address_(std::move(ipv4_source_address)), ipv6_source_address_(std::move(ipv6_source_address)) {} @@ -83,7 +83,9 @@ class SocketMarkOption : public Network::Socket::Option, source_address = ipv4_source_address_; } } - if (source_address) { + // identity is zero for the listener socket itself, set transparent and reuse options also for + // the listener socket. + if (source_address || identity_ == 0) { // Allow reuse of the original source address. This may by needed for // retries to not fail on "address already in use" when using a specific // source address and port. @@ -184,14 +186,10 @@ class SocketMarkOption : public Network::Socket::Option, bool isSupported() const override { return true; } - // policyUseUpstreamDestinationAddress returns 'true' if policy enforcement should be done on the - // basis of the upstream destination address. - bool policyUseUpstreamDestinationAddress() const { return is_l7lb_; } + bool isSidecar() const { return mark_ == 0; } uint32_t identity_; uint32_t mark_; - bool ingress_; - bool is_l7lb_; Network::Address::InstanceConstSharedPtr original_source_address_; // Version specific source addresses are only used if original source address is not used. // Selection is made based on the socket domain, which is selected based on the destination @@ -209,10 +207,10 @@ class SocketOption : public SocketMarkOption { Network::Address::InstanceConstSharedPtr ipv4_source_address, Network::Address::InstanceConstSharedPtr ipv6_source_address, const std::shared_ptr& policy_id_resolver) - : SocketMarkOption(mark, source_identity, ingress, l7lb, original_source_address, + : SocketMarkOption(mark, source_identity, original_source_address, ipv4_source_address, ipv6_source_address), - initial_policy_(policy), port_(port), pod_ip_(std::move(pod_ip)), - policy_id_resolver_(policy_id_resolver) { + initial_policy_(policy), ingress_(ingress), is_l7lb_(l7lb), port_(port), + pod_ip_(std::move(pod_ip)), policy_id_resolver_(policy_id_resolver) { ENVOY_LOG(debug, "Cilium SocketOption(): source_identity: {}, " "ingress: {}, port: {}, pod_ip: {}, source_addresses: {}/{}/{}, mark: {:x} (magic " @@ -233,7 +231,13 @@ class SocketOption : public SocketMarkOption { return policy_id_resolver_->getPolicy(pod_ip_); } + // policyUseUpstreamDestinationAddress returns 'true' if policy enforcement should be done on the + // basis of the upstream destination address. + bool policyUseUpstreamDestinationAddress() const { return is_l7lb_; } + const PolicyInstanceConstSharedPtr initial_policy_; // Never NULL + bool ingress_; + bool is_l7lb_; uint16_t port_; std::string pod_ip_; diff --git a/patches/0005-listener-add-socket-options.patch b/patches/0005-listener-add-socket-options.patch new file mode 100644 index 000000000..a763757cf --- /dev/null +++ b/patches/0005-listener-add-socket-options.patch @@ -0,0 +1,90 @@ +From 0f7c83d1a8d634b3f5909016e2a1eafd50acc639 Mon Sep 17 00:00:00 2001 +From: Jarno Rajahalme +Date: Mon, 14 Aug 2023 10:01:21 +0300 +Subject: [PATCH] Revert "listener: keep ListenerFactoryContext small (#7528)" + +This reverts commit 170c89eb0b2afb7a39d44d0f8dfb77444ffc038f. + +diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h +index ed4b946b38..efc2e49ef8 100644 +--- a/envoy/server/factory_context.h ++++ b/envoy/server/factory_context.h +@@ -309,6 +309,11 @@ public: + */ + class ListenerFactoryContext : public virtual FactoryContext { + public: ++ /** ++ * Store socket options to be set on the listen socket before listening. ++ */ ++ virtual void addListenSocketOptions(const Network::Socket::OptionsSharedPtr& options) PURE; ++ + /** + * Give access to the listener configuration + */ +diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.cc b/source/extensions/listener_managers/listener_manager/listener_impl.cc +index 94b29a189e..bb2cc17115 100644 +--- a/source/extensions/listener_managers/listener_manager/listener_impl.cc ++++ b/source/extensions/listener_managers/listener_manager/listener_impl.cc +@@ -907,6 +907,9 @@ envoy::config::core::v3::TrafficDirection PerListenerFactoryContextImpl::directi + return listener_factory_context_base_->direction(); + }; + TimeSource& PerListenerFactoryContextImpl::timeSource() { return api().timeSource(); } ++void PerListenerFactoryContextImpl::addListenSocketOptions(const Network::Socket::OptionsSharedPtr& options) { ++ listener_impl_.addListenSocketOptions(options); ++} + const Network::ListenerConfig& PerListenerFactoryContextImpl::listenerConfig() const { + return *listener_config_; + } +diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.h b/source/extensions/listener_managers/listener_manager/listener_impl.h +index 4dfbd43b5a..9da9fa4d5a 100644 +--- a/source/extensions/listener_managers/listener_manager/listener_impl.h ++++ b/source/extensions/listener_managers/listener_manager/listener_impl.h +@@ -241,6 +241,7 @@ public: + bool isQuicListener() const override; + + // ListenerFactoryContext ++ void addListenSocketOptions(const Network::Socket::OptionsSharedPtr& options) override; + const Network::ListenerConfig& listenerConfig() const override; + + ListenerFactoryContextBaseImpl& parentFactoryContext() { return *listener_factory_context_base_; } +@@ -383,6 +384,13 @@ public: + return config().traffic_direction(); + } + ++ void addListenSocketOptions(const Network::Socket::OptionsSharedPtr& append_options) { ++ for (std::vector::size_type i = 0; ++ i < addresses_.size(); i++) { ++ addListenSocketOptions(listen_socket_options_list_[i], append_options); ++ } ++ } ++ + void ensureSocketOptions(Network::Socket::OptionsSharedPtr& options) { + if (options == nullptr) { + options = std::make_shared>(); +diff --git a/test/mocks/server/factory_context.h b/test/mocks/server/factory_context.h +index e1327228eb..db110d263e 100644 +--- a/test/mocks/server/factory_context.h ++++ b/test/mocks/server/factory_context.h +@@ -46,6 +46,7 @@ public: + MOCK_METHOD(envoy::config::core::v3::TrafficDirection, direction, (), (const)); + MOCK_METHOD(TimeSource&, timeSource, ()); + ++ MOCK_METHOD(void, addListenSocketOptions, (const Network::Socket::OptionsSharedPtr&)); + MOCK_METHOD(const Network::ListenerConfig&, listenerConfig, (), (const)); + + Event::TestTimeSystem& timeSystem() { return time_system_; } +diff --git a/test/mocks/server/listener_factory_context.h b/test/mocks/server/listener_factory_context.h +index 5341b517d1..924b8cb0b1 100644 +--- a/test/mocks/server/listener_factory_context.h ++++ b/test/mocks/server/listener_factory_context.h +@@ -20,6 +20,7 @@ public: + MockListenerFactoryContext(); + ~MockListenerFactoryContext() override; + ++ MOCK_METHOD(void, addListenSocketOptions, (const Network::Socket::OptionsSharedPtr&)); + const Network::ListenerConfig& listenerConfig() const override { return listener_config_; } + MOCK_METHOD(const Network::ListenerConfig&, listenerConfig_, (), (const)); + MOCK_METHOD(ServerFactoryContext&, getServerFactoryContext, (), (const)); +-- +2.41.0 +