Skip to content

Commit

Permalink
tls_wrapper: Use raw socket with non-TLS policy
Browse files Browse the repository at this point in the history
Cilium tls_wrapper should allow raw socket to be used if policy allows
without TLS context.

Also check for SNI when getting TLS context so that the one matching the
SNI is used if multiple are available.

Add policy tests validating the policy functionality.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
  • Loading branch information
jrajahalme authored and sayboras committed Nov 12, 2024
1 parent cd22d9f commit c35cf76
Show file tree
Hide file tree
Showing 9 changed files with 462 additions and 78 deletions.
12 changes: 7 additions & 5 deletions cilium/bpf_metadata.cc
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ Cilium::SocketOptionSharedPtr Config::getMetadata(Network::ConnectionSocket& soc
const auto sip = src_address->ip();
const auto dst_address = socket.ioHandle().localAddress();
const auto dip = dst_address->ip();
auto sni = socket.requestedServerName();

if (!sip || !dip) {
ENVOY_LOG_MISC(debug, "Non-IP addresses: src: {} dst: {}", src_address->asString(),
Expand All @@ -296,11 +297,12 @@ Cilium::SocketOptionSharedPtr Config::getMetadata(Network::ConnectionSocket& soc
if (is_ingress_) {
pod_ip = dip->addressAsString();
other_ip = sip->addressAsString();
ENVOY_LOG_MISC(debug, "INGRESS POD IP: {}, source IP: {}", pod_ip, other_ip);
ENVOY_LOG_MISC(debug, "INGRESS POD IP: {}, source IP: {}, sni: \"{}\"", pod_ip, other_ip, sni);
} else {
pod_ip = sip->addressAsString();
other_ip = dip->addressAsString();
ENVOY_LOG_MISC(debug, "EGRESS POD IP: {}, destination IP: {}", pod_ip, other_ip);
ENVOY_LOG_MISC(debug, "EGRESS POD IP: {}, destination IP: {} sni: \"{}\"", pod_ip, other_ip,
sni);
}

// Load the policy for the Pod that sends or receives traffic.
Expand Down Expand Up @@ -402,8 +404,8 @@ Cilium::SocketOptionSharedPtr Config::getMetadata(Network::ConnectionSocket& soc

// policy must exist at this point
if (policy == nullptr) {
ENVOY_LOG(warn, "cilium.bpf_metadata ({}): No policy found for {}",
is_ingress_ ? "ingress" : "egress", pod_ip);
ENVOY_LOG(warn, "cilium.bpf_metadata ({}): No policy found for {} sni: \"{}\"",
is_ingress_ ? "ingress" : "egress", pod_ip, sni);
return nullptr;
}

Expand Down Expand Up @@ -443,7 +445,7 @@ Cilium::SocketOptionSharedPtr Config::getMetadata(Network::ConnectionSocket& soc
return std::make_shared<Cilium::SocketOption>(
policy, mark, ingress_source_identity, source_identity, is_ingress_, is_l7lb_, dip->port(),
std::move(pod_ip), std::move(src_address), std::move(source_addresses.ipv4_),
std::move(source_addresses.ipv6_), shared_from_this(), proxy_id_);
std::move(source_addresses.ipv6_), shared_from_this(), proxy_id_, sni);
}

Network::FilterStatus Instance::onAccept(Network::ListenerFilterCallbacks& cb) {
Expand Down
15 changes: 10 additions & 5 deletions cilium/network_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,10 @@ Network::FilterStatus Instance::onNewConnection() {
if (option->ingress_source_identity_ != 0) {
auto ingress_port_policy = option->initial_policy_->findPortPolicy(true, destination_port_);
if (!ingress_port_policy.allowed(option->ingress_source_identity_, sni)) {
ENVOY_CONN_LOG(debug,
"cilium.network: ingress policy drop for source identity: {} port: {}",
conn, option->ingress_source_identity_, destination_port_);
ENVOY_CONN_LOG(
debug,
"cilium.network: ingress policy DROP for source identity: {} port: {} sni: \"{}\"",
conn, option->ingress_source_identity_, destination_port_, sni);
return false;
}
}
Expand All @@ -161,9 +162,13 @@ Network::FilterStatus Instance::onNewConnection() {
remote_id_ = option->ingress_ ? option->identity_ : destination_identity;
if (!port_policy.allowed(remote_id_, sni)) {
// Connection not allowed by policy
ENVOY_CONN_LOG(warn, "cilium.network: Policy DENY on id: {} port: {}", conn, remote_id_,
destination_port_);
ENVOY_CONN_LOG(debug, "cilium.network: Policy DENY on id: {} port: {} sni: \"{}\"", conn,
remote_id_, destination_port_, sni);
return false;
} else {
// Connection allowed by policy
ENVOY_CONN_LOG(debug, "cilium.network: Policy ALLOW on id: {} port: {} sni: \"{}\"", conn,
remote_id_, destination_port_, sni);
}

const std::string& policy_name = option->pod_ip_;
Expand Down
65 changes: 42 additions & 23 deletions cilium/network_policy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -489,22 +489,30 @@ class PortNetworkPolicyRule : public Logger::Loggable<Logger::Id::config> {
return true; // allowed by default
}

Ssl::ContextSharedPtr getServerTlsContext(uint32_t remote_id,
const Ssl::ContextConfig** config) const {
Ssl::ContextSharedPtr getServerTlsContext(uint32_t remote_id, absl::string_view sni,
const Ssl::ContextConfig** config,
bool& raw_socket_allowed) const {
bool denied = false;
if (server_context_ && allowed(remote_id, denied)) {
*config = &server_context_->getTlsContextConfig();
return server_context_->getTlsContext();
if (allowed(remote_id, sni, denied)) {
if (server_context_) {
*config = &server_context_->getTlsContextConfig();
return server_context_->getTlsContext();
}
raw_socket_allowed = true;
}
return nullptr;
}

Ssl::ContextSharedPtr getClientTlsContext(uint32_t remote_id,
const Ssl::ContextConfig** config) const {
Ssl::ContextSharedPtr getClientTlsContext(uint32_t remote_id, absl::string_view sni,
const Ssl::ContextConfig** config,
bool& raw_socket_allowed) const {
bool denied = false;
if (client_context_ && allowed(remote_id, denied)) {
*config = &client_context_->getTlsContextConfig();
return client_context_->getTlsContext();
if (allowed(remote_id, sni, denied)) {
if (client_context_) {
*config = &client_context_->getTlsContextConfig();
return client_context_->getTlsContext();
}
raw_socket_allowed = true;
}
return nullptr;
}
Expand Down Expand Up @@ -671,20 +679,24 @@ class PortNetworkPolicyRules : public Logger::Loggable<Logger::Id::config> {
return allowed && !denied;
}

Ssl::ContextSharedPtr getServerTlsContext(uint32_t remote_id,
const Ssl::ContextConfig** config) const {
Ssl::ContextSharedPtr getServerTlsContext(uint32_t remote_id, absl::string_view sni,
const Ssl::ContextConfig** config,
bool& raw_socket_allowed) const {
for (const auto& rule : rules_) {
Ssl::ContextSharedPtr server_context = rule->getServerTlsContext(remote_id, config);
Ssl::ContextSharedPtr server_context =
rule->getServerTlsContext(remote_id, sni, config, raw_socket_allowed);
if (server_context)
return server_context;
}
return nullptr;
}

Ssl::ContextSharedPtr getClientTlsContext(uint32_t remote_id,
const Ssl::ContextConfig** config) const {
Ssl::ContextSharedPtr getClientTlsContext(uint32_t remote_id, absl::string_view sni,
const Ssl::ContextConfig** config,
bool& raw_socket_allowed) const {
for (const auto& rule : rules_) {
Ssl::ContextSharedPtr client_context = rule->getClientTlsContext(remote_id, config);
Ssl::ContextSharedPtr client_context =
rule->getClientTlsContext(remote_id, sni, config, raw_socket_allowed);
if (client_context)
return client_context;
}
Expand Down Expand Up @@ -787,21 +799,23 @@ bool PortPolicy::allowed(uint32_t remote_id,
});
}

Ssl::ContextSharedPtr PortPolicy::getServerTlsContext(uint32_t remote_id,
const Ssl::ContextConfig** config) const {
Ssl::ContextSharedPtr PortPolicy::getServerTlsContext(uint32_t remote_id, absl::string_view sni,
const Ssl::ContextConfig** config,
bool& raw_socket_allowed) const {
Ssl::ContextSharedPtr ret;
for_first_range([&](const PortNetworkPolicyRules& rules) -> bool {
ret = rules.getServerTlsContext(remote_id, config);
ret = rules.getServerTlsContext(remote_id, sni, config, raw_socket_allowed);
return ret != nullptr;
});
return ret;
}

Ssl::ContextSharedPtr PortPolicy::getClientTlsContext(uint32_t remote_id,
const Ssl::ContextConfig** config) const {
Ssl::ContextSharedPtr PortPolicy::getClientTlsContext(uint32_t remote_id, absl::string_view sni,
const Ssl::ContextConfig** config,
bool& raw_socket_allowed) const {
Ssl::ContextSharedPtr ret;
for_first_range([&](const PortNetworkPolicyRules& rules) -> bool {
ret = rules.getClientTlsContext(remote_id, config);
ret = rules.getClientTlsContext(remote_id, sni, config, raw_socket_allowed);
return ret != nullptr;
});
return ret;
Expand Down Expand Up @@ -1020,7 +1034,7 @@ class PolicyInstanceImpl : public PolicyInstance {
PolicyInstanceImpl(const NetworkPolicyMap& parent, uint64_t hash,
const cilium::NetworkPolicy& proto)
: conntrack_map_name_(proto.conntrack_map_name()), endpoint_id_(proto.endpoint_id()),
hash_(hash), policy_proto_(proto), endpoint_ips_(proto),
hash_(hash), policy_proto_(proto), endpoint_ips_(proto), parent_(parent),
ingress_(parent, policy_proto_.ingress_per_port_policies()),
egress_(parent, policy_proto_.egress_per_port_policies()) {}

Expand Down Expand Up @@ -1062,6 +1076,8 @@ class PolicyInstanceImpl : public PolicyInstance {
return res;
}

void tlsWrapperMissingPolicyInc() const override { parent_.tlsWrapperMissingPolicyInc(); }

public:
std::string conntrack_map_name_;
uint32_t endpoint_id_;
Expand All @@ -1070,6 +1086,7 @@ class PolicyInstanceImpl : public PolicyInstance {
const IPAddressPair endpoint_ips_;

private:
const NetworkPolicyMap& parent_;
const PortNetworkPolicy ingress_;
const PortNetworkPolicy egress_;
};
Expand Down Expand Up @@ -1447,6 +1464,8 @@ class AllowAllEgressPolicyInstanceImpl : public PolicyInstance {

std::string String() const override { return "AllowAllEgressPolicyInstanceImpl"; }

void tlsWrapperMissingPolicyInc() const override {}

private:
PolicyMap empty_map_;
static const std::string empty_string;
Expand Down
21 changes: 16 additions & 5 deletions cilium/network_policy.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,18 @@ class PortPolicy : public Logger::Loggable<Logger::Id::config> {
bool allowed(uint32_t remote_id, const envoy::config::core::v3::Metadata& metadata) const;
// getServerTlsContext returns the server TLS context, if any. If a non-null pointer is returned,
// then also the config pointer '*config' is set.
Ssl::ContextSharedPtr getServerTlsContext(uint32_t remote_id,
const Ssl::ContextConfig** config) const;
// If '*config' is nullptr and 'raw_socket_allowed' is 'true' on return then the policy
// allows the connection without TLS and a raw socket should be used.
Ssl::ContextSharedPtr getServerTlsContext(uint32_t remote_id, absl::string_view sni,
const Ssl::ContextConfig** config,
bool& raw_socket_allowed) const;
// getClientTlsContext returns the client TLS context, if any. If a non-null pointer is returned,
// then also the config pointer '*config' is set.
Ssl::ContextSharedPtr getClientTlsContext(uint32_t remote_id,
const Ssl::ContextConfig** config) const;
// If '*config' is nullptr and 'raw_socket_allowed' is 'true' on return then the policy
// allows the connection without TLS and a raw socket should be used.
Ssl::ContextSharedPtr getClientTlsContext(uint32_t remote_id, absl::string_view sni,
const Ssl::ContextConfig** config,
bool& raw_socket_allowed) const;

private:
bool for_range(std::function<bool(const PortNetworkPolicyRules&, bool& denied)> allowed) const;
Expand Down Expand Up @@ -127,6 +133,8 @@ class PolicyInstance {
virtual const IPAddressPair& getEndpointIPs() const PURE;

virtual std::string String() const PURE;

virtual void tlsWrapperMissingPolicyInc() const PURE;
};
using PolicyInstanceConstSharedPtr = std::shared_ptr<const PolicyInstance>;

Expand Down Expand Up @@ -173,7 +181,8 @@ class NetworkPolicyDecoder : public Envoy::Config::OpaqueResourceDecoder {
COUNTER(updates) \
COUNTER(updates_rejected) \
COUNTER(updates_timed_out) \
HISTOGRAM(update_latency_ms, Milliseconds)
HISTOGRAM(update_latency_ms, Milliseconds) \
COUNTER(tls_wrapper_missing_policy)
// clang-format on

/**
Expand Down Expand Up @@ -237,6 +246,8 @@ class NetworkPolicyMap : public Singleton::Instance,
return *transport_factory_context_;
}

void tlsWrapperMissingPolicyInc() const { stats_.tls_wrapper_missing_policy_.inc(); }

private:
const std::shared_ptr<const PolicyInstanceImpl>&
GetPolicyInstanceImpl(const std::string& endpoint_policy_name) const;
Expand Down
10 changes: 6 additions & 4 deletions cilium/socket_option.h
Original file line number Diff line number Diff line change
Expand Up @@ -228,21 +228,22 @@ class SocketOption : public SocketMarkOption {
Network::Address::InstanceConstSharedPtr original_source_address,
Network::Address::InstanceConstSharedPtr ipv4_source_address,
Network::Address::InstanceConstSharedPtr ipv6_source_address,
const std::shared_ptr<PolicyResolver>& policy_id_resolver, uint32_t proxy_id)
const std::shared_ptr<PolicyResolver>& policy_id_resolver, uint32_t proxy_id,
absl::string_view sni)
: SocketMarkOption(mark, source_identity, original_source_address, ipv4_source_address,
ipv6_source_address),
ingress_source_identity_(ingress_source_identity), initial_policy_(policy),
ingress_(ingress), is_l7lb_(l7lb), port_(port), pod_ip_(std::move(pod_ip)),
proxy_id_(proxy_id), policy_id_resolver_(policy_id_resolver) {
proxy_id_(proxy_id), sni_(sni), policy_id_resolver_(policy_id_resolver) {
ENVOY_LOG(debug,
"Cilium SocketOption(): source_identity: {}, "
"ingress: {}, port: {}, pod_ip: {}, source_addresses: {}/{}/{}, mark: {:x} (magic "
"mark: {:x}, cluster: {}, ID: {}), proxy_id: {}",
"mark: {:x}, cluster: {}, ID: {}), proxy_id: {}, sni: \"{}\"",
identity_, ingress_, port_, pod_ip_,
original_source_address_ ? original_source_address_->asString() : "",
ipv4_source_address_ ? ipv4_source_address_->asString() : "",
ipv6_source_address_ ? ipv6_source_address_->asString() : "", mark_, mark & 0xff00,
mark & 0xff, mark >> 16, proxy_id_);
mark & 0xff, mark >> 16, proxy_id_, sni_);
ASSERT(initial_policy_ != nullptr);
}

Expand All @@ -266,6 +267,7 @@ class SocketOption : public SocketMarkOption {
uint16_t port_;
std::string pod_ip_;
uint32_t proxy_id_;
std::string sni_;

private:
const std::shared_ptr<PolicyResolver> policy_id_resolver_;
Expand Down
32 changes: 24 additions & 8 deletions cilium/tls_wrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ class SslSocketWrapper : public Network::TransportSocket {
void setTransportSocketCallbacks(Network::TransportSocketCallbacks& callbacks) override {
// Get the Cilium socket option from the callbacks in order to get the TLS
// configuration
// Cilium socket option is only created if the (intial) policy for the local pod exists.
// If the policy requires TLS then a TLS socket is used, but if the policy does not require
// TLS a raw socket is used instead,
const auto option = Cilium::GetSocketOption(callbacks.connection().socketOptions());
if (option) {
// Resolve the destination security ID and port
Expand All @@ -58,13 +61,17 @@ class SslSocketWrapper : public Network::TransportSocket {
}
}

// get the requested server name from the connection, if any
const auto& sni = option->sni_;

auto remote_id = option->ingress_ ? option->identity_ : destination_identity;
auto port_policy =
option->initial_policy_->findPortPolicy(option->ingress_, destination_port);
const Envoy::Ssl::ContextConfig* config;
Envoy::Ssl::ContextSharedPtr ctx = is_client
? port_policy.getClientTlsContext(remote_id, &config)
: port_policy.getServerTlsContext(remote_id, &config);
const Envoy::Ssl::ContextConfig* config = nullptr;
bool raw_socket_allowed = false;
Envoy::Ssl::ContextSharedPtr ctx =
is_client ? port_policy.getClientTlsContext(remote_id, sni, &config, raw_socket_allowed)
: port_policy.getServerTlsContext(remote_id, sni, &config, raw_socket_allowed);
if (ctx) {
// create the underlying SslSocket
auto status_or_socket = Extensions::TransportSockets::Tls::SslSocket::create(
Expand All @@ -77,7 +84,15 @@ class SslSocketWrapper : public Network::TransportSocket {
ENVOY_LOG_MISC(error, "Unable to create ssl socket {}",
status_or_socket.status().message());
}
} else if (config == nullptr && raw_socket_allowed) {
// Use RawBufferSocket when policy allows without TLS.
// If policy has TLS context config then a raw socket must NOT be used.
socket_ = std::make_unique<Network::RawBufferSocket>();
// Set the callbacks
socket_->setTransportSocketCallbacks(callbacks);
} else {
option->initial_policy_->tlsWrapperMissingPolicyInc();

std::string ipStr("<none>");
if (option->ingress_) {
Network::Address::InstanceConstSharedPtr src_address =
Expand All @@ -94,10 +109,11 @@ class SslSocketWrapper : public Network::TransportSocket {
ipStr = dip->addressAsString();
}
}
ENVOY_LOG_MISC(
warn, "cilium.tls_wrapper: Could not get {} TLS context for {} IP {} (id {}) port {}",
is_client ? "client" : "server", option->ingress_ ? "source" : "destination", ipStr,
remote_id, destination_port);
ENVOY_LOG_MISC(warn,
"cilium.tls_wrapper: Could not get {} TLS context for {} IP {} (id {}) port "
"{} sni \"{}\" and raw socket is not allowed",
is_client ? "client" : "server", option->ingress_ ? "source" : "destination",
ipStr, remote_id, destination_port, sni);
}
} else {
ENVOY_LOG_MISC(warn, "cilium.tls_wrapper: Can not correlate connection with Cilium Network "
Expand Down
16 changes: 0 additions & 16 deletions cilium/tls_wrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,10 @@

#include "envoy/registry/registry.h"
#include "envoy/server/transport_socket_config.h"
#include "envoy/stats/scope.h"
#include "envoy/stats/stats_macros.h"

namespace Envoy {
namespace Cilium {

// clang-format off
#define ALL_SSL_SOCKET_FACTORY_STATS(COUNTER) \
COUNTER(ssl_context_update_by_sds) \
COUNTER(upstream_context_secrets_not_ready) \
COUNTER(downstream_context_secrets_not_ready)
// clang-format on

/**
* Wrapper struct for SSL socket factory stats. @see stats_macros.h
*/
struct SslSocketFactoryStats {
ALL_SSL_SOCKET_FACTORY_STATS(GENERATE_COUNTER_STRUCT)
};

/**
* Config registration for the Cilium TLS wrapper transport socket factory.
* @see TransportSocketConfigFactory.
Expand Down
2 changes: 1 addition & 1 deletion tests/bpf_metadata.cc
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ Cilium::SocketOptionSharedPtr TestConfig::getMetadata(Network::ConnectionSocket&

return std::make_shared<Cilium::SocketOption>(policy, 0, 0, source_identity, is_ingress_,
is_l7lb_, port, std::move(pod_ip), nullptr, nullptr,
nullptr, shared_from_this(), 0);
nullptr, shared_from_this(), 0, "");
}

} // namespace BpfMetadata
Expand Down
Loading

0 comments on commit c35cf76

Please sign in to comment.