Skip to content

Commit

Permalink
cilium: Set privileged options for listener socket as well
Browse files Browse the repository at this point in the history
Patch Envoy to support setting socket options from listener filters and
use them to set privileged options for the listener.

Minor cleanup in socket option implementation to move unused fields
from SocketMarkOption class to SocketOption where they are used.

Signed-off-by: Jarno Rajahalme <[email protected]>
  • Loading branch information
jrajahalme committed Aug 14, 2023
1 parent 4840d77 commit f2ea823
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 23 deletions.
1 change: 1 addition & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)

Expand Down
8 changes: 8 additions & 0 deletions cilium/bpf_metadata.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ class BpfMetadataConfigFactory : public NamedListenerFilterConfigFactory {
MessageUtil::downcastAndValidate<const ::cilium::BpfMetadata&>(
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<Envoy::Network::Socket::Options> options = std::make_shared<Envoy::Network::Socket::Options>();
uint32_t mark = (config->is_ingress_) ? 0x0A00 : 0x0B00;
options->push_back(std::make_shared<Cilium::SocketMarkOption>(mark, 0));
context.addListenSocketOptions(options);

return [listener_filter_matcher,
config](Network::ListenerFilterManager& filter_manager) mutable -> void {
filter_manager.addAcceptFilter(listener_filter_matcher,
Expand Down
20 changes: 11 additions & 9 deletions cilium/network_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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>(Network::UpstreamServerName::key());
Expand All @@ -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_;
Expand All @@ -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;
}

Expand All @@ -174,15 +176,15 @@ 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) {
ENVOY_CONN_LOG(warn, "cilium.network: Go parser \"{}\" not found", conn, l7proto_);
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_);
}
Expand Down
32 changes: 18 additions & 14 deletions cilium/socket_option.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ class PolicyResolver {
class SocketMarkOption : public Network::Socket::Option,
public Logger::Loggable<Logger::Id::filter> {
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)) {}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -209,10 +207,10 @@ class SocketOption : public SocketMarkOption {
Network::Address::InstanceConstSharedPtr ipv4_source_address,
Network::Address::InstanceConstSharedPtr ipv6_source_address,
const std::shared_ptr<PolicyResolver>& 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 "
Expand All @@ -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_;

Expand Down
90 changes: 90 additions & 0 deletions patches/0005-listener-add-socket-options.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
From 0f7c83d1a8d634b3f5909016e2a1eafd50acc639 Mon Sep 17 00:00:00 2001
From: Jarno Rajahalme <[email protected]>
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<Network::Address::InstanceConstSharedPtr>::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<std::vector<Network::Socket::OptionConstSharedPtr>>();
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

0 comments on commit f2ea823

Please sign in to comment.