Skip to content

Commit

Permalink
Revert "policy: Do not store policy reference in cilium socket option"
Browse files Browse the repository at this point in the history 
This reverts commit 2aa20ee.
  • Loading branch information
@sayboras
sayboras committed Dec 2, 2024
1 parent 17f4f4a commit c7b1578
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 35 deletions.
2 changes: 1 addition & 1 deletion cilium/bpf_metadata.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ Cilium::SocketOptionSharedPtr Config::getMetadata(Network::ConnectionSocket& soc
mark = ((is_ingress_) ? 0x0A00 : 0x0B00) | cluster_id | identity_id;
}
return std::make_shared<Cilium::SocketOption>(
mark, ingress_source_identity, source_identity, is_ingress_, is_l7lb_, dip->port(),
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_, sni);
}
Expand Down
9 changes: 2 additions & 7 deletions cilium/network_filter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,6 @@ Network::FilterStatus Instance::onNewConnection() {
ENVOY_LOG(warn, "cilium.network (egress): No destination address ");
return false;
}
const auto& policy = option->getPolicy();
if (!policy) {
ENVOY_LOG_MISC(warn, "cilium.network: No policy found for pod {}", option->pod_ip_);
return false;
}
if (!option->ingress_) {
const auto dip = dst_address->ip();
if (!dip) {
Expand All @@ -151,7 +146,7 @@ Network::FilterStatus Instance::onNewConnection() {
destination_identity = option->resolvePolicyId(dip);

if (option->ingress_source_identity_ != 0) {
auto ingress_port_policy = policy->findPortPolicy(true, destination_port_);
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,
Expand All @@ -162,7 +157,7 @@ Network::FilterStatus Instance::onNewConnection() {
}
}

auto port_policy = policy->findPortPolicy(option->ingress_, destination_port_);
auto port_policy = option->initial_policy_->findPortPolicy(option->ingress_, destination_port_);

remote_id_ = option->ingress_ ? option->identity_ : destination_identity;
if (!port_policy.allowed(remote_id_, sni)) {
Expand Down
21 changes: 12 additions & 9 deletions cilium/socket_option.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,18 +222,19 @@ class SocketMarkOption : public Network::Socket::Option,

class SocketOption : public SocketMarkOption {
public:
SocketOption(uint32_t mark, uint32_t ingress_source_identity, uint32_t source_identity,
bool ingress, bool l7lb, uint16_t port, std::string&& pod_ip,
SocketOption(PolicyInstanceConstSharedPtr policy, uint32_t mark, uint32_t ingress_source_identity,
uint32_t source_identity, bool ingress, bool l7lb, uint16_t port,
std::string&& pod_ip,
Network::Address::InstanceConstSharedPtr original_source_address,
Network::Address::InstanceConstSharedPtr ipv4_source_address,
Network::Address::InstanceConstSharedPtr ipv6_source_address,
const std::shared_ptr<PolicyResolver>& policy_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), ingress_(ingress), is_l7lb_(l7lb),
port_(port), pod_ip_(std::move(pod_ip)), proxy_id_(proxy_id), sni_(sni),
policy_resolver_(policy_resolver) {
ingress_source_identity_(ingress_source_identity), initial_policy_(policy),
ingress_(ingress), is_l7lb_(l7lb), port_(port), pod_ip_(std::move(pod_ip)),
proxy_id_(proxy_id), sni_(sni), policy_id_resolver_(policy_id_resolver) {
ENVOY_LOG(debug,
"Cilium SocketOption(): source_identity: {}, "
"ingress: {}, port: {}, pod_ip: {}, source_addresses: {}/{}/{}, mark: {:x} (magic "
Expand All @@ -243,14 +244,15 @@ class SocketOption : public SocketMarkOption {
ipv4_source_address_ ? ipv4_source_address_->asString() : "",
ipv6_source_address_ ? ipv6_source_address_->asString() : "", mark_, mark & 0xff00,
mark & 0xff, mark >> 16, proxy_id_, sni_);
ASSERT(initial_policy_ != nullptr);
}

uint32_t resolvePolicyId(const Network::Address::Ip* ip) const {
return policy_resolver_->resolvePolicyId(ip);
return policy_id_resolver_->resolvePolicyId(ip);
}

const PolicyInstanceConstSharedPtr getPolicy() const {
return policy_resolver_->getPolicy(pod_ip_);
return policy_id_resolver_->getPolicy(pod_ip_);
}

// policyUseUpstreamDestinationAddress returns 'true' if policy enforcement should be done on the
Expand All @@ -259,6 +261,7 @@ class SocketOption : public SocketMarkOption {

// Additional ingress policy enforcement is performed if ingress_source_identity is non-zero
uint32_t ingress_source_identity_;
const PolicyInstanceConstSharedPtr initial_policy_; // Never NULL
bool ingress_;
bool is_l7lb_;
uint16_t port_;
Expand All @@ -267,7 +270,7 @@ class SocketOption : public SocketMarkOption {
std::string sni_;

private:
const std::shared_ptr<PolicyResolver> policy_resolver_;
const std::shared_ptr<PolicyResolver> policy_id_resolver_;
};

using SocketOptionSharedPtr = std::shared_ptr<SocketOption>;
Expand Down
21 changes: 8 additions & 13 deletions cilium/tls_wrapper.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,6 @@ class SslSocketWrapper : public Network::TransportSocket {
// TLS a raw socket is used instead,
const auto option = Cilium::GetSocketOption(callbacks.connection().socketOptions());
if (option) {
const auto& policy = option->getPolicy();
if (!policy) {
ENVOY_LOG_MISC(warn, "cilium.tls_wrapper: No policy found for pod {}", option->pod_ip_);
return;
}
// Resolve the destination security ID and port
uint32_t destination_identity = 0;
uint32_t destination_port = option->port_;
Expand Down Expand Up @@ -70,7 +65,8 @@ class SslSocketWrapper : public Network::TransportSocket {
const auto& sni = option->sni_;

auto remote_id = option->ingress_ ? option->identity_ : destination_identity;
auto port_policy = policy->findPortPolicy(option->ingress_, destination_port);
auto port_policy =
option->initial_policy_->findPortPolicy(option->ingress_, destination_port);
const Envoy::Ssl::ContextConfig* config = nullptr;
bool raw_socket_allowed = false;
Envoy::Ssl::ContextSharedPtr ctx =
Expand All @@ -95,7 +91,7 @@ class SslSocketWrapper : public Network::TransportSocket {
// Set the callbacks
socket_->setTransportSocketCallbacks(callbacks);
} else {
policy->tlsWrapperMissingPolicyInc();
option->initial_policy_->tlsWrapperMissingPolicyInc();

std::string ipStr("<none>");
if (option->ingress_) {
Expand All @@ -113,12 +109,11 @@ class SslSocketWrapper : public Network::TransportSocket {
ipStr = dip->addressAsString();
}
}
ENVOY_LOG_MISC(
warn,
"cilium.tls_wrapper: Could not get {} TLS context for pod {} on {} IP {} (id {}) port "
"{} sni \"{}\" and raw socket is not allowed",
is_client ? "client" : "server", option->pod_ip_,
option->ingress_ ? "source" : "destination", ipStr, remote_id, destination_port, sni);
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
6 changes: 3 additions & 3 deletions tests/bpf_metadata.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,9 @@ Cilium::SocketOptionSharedPtr TestConfig::getMetadata(Network::ConnectionSocket&
ENVOY_LOG_MISC(info, "setRequestedApplicationProtocols({})", l7proto);
}

return std::make_shared<Cilium::SocketOption>(0, 0, source_identity, is_ingress_, is_l7lb_, port,
std::move(pod_ip), nullptr, nullptr, nullptr,
shared_from_this(), 0, "");
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, "");
}

} // namespace BpfMetadata
Expand Down
9 changes: 7 additions & 2 deletions tests/metadata_config_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbMetadata) {
EXPECT_EQ("[face::42]:0", option->ipv6_source_address_->asString());
EXPECT_EQ(80, option->port_);
EXPECT_EQ("10.1.1.42", option->pod_ip_);
EXPECT_NE(nullptr, option->initial_policy_);
EXPECT_EQ(0, option->ingress_source_identity_);

// Check that Ingress security ID is used in the socket mark
Expand Down Expand Up @@ -295,13 +296,14 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbIngressEnforcedMetadata) {
EXPECT_EQ("[face::42]:0", option->ipv6_source_address_->asString());
EXPECT_EQ(80, option->port_);
EXPECT_EQ("10.1.1.42", option->pod_ip_);
EXPECT_NE(nullptr, option->initial_policy_);
EXPECT_EQ(12345678, option->ingress_source_identity_);

// Check that Ingress security ID is used in the socket mark
EXPECT_TRUE((option->mark_ & 0xffff) == 0x0B00 && (option->mark_ >> 16) == 8);

// Expect policy accepts security ID 12345678 on ingress on port 80
auto port_policy = option->getPolicy()->findPortPolicy(true, 80);
auto port_policy = option->initial_policy_->findPortPolicy(true, 80);
EXPECT_TRUE(port_policy.allowed(12345678, ""));
}

Expand Down Expand Up @@ -331,13 +333,14 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbIngressEnforcedCIDRMetadata) {
EXPECT_EQ("[face::42]:0", option->ipv6_source_address_->asString());
EXPECT_EQ(80, option->port_);
EXPECT_EQ("10.1.1.42", option->pod_ip_);
EXPECT_NE(nullptr, option->initial_policy_);
EXPECT_EQ(2, option->ingress_source_identity_);

// Check that Ingress security ID is used in the socket mark
EXPECT_TRUE((option->mark_ & 0xffff) == 0x0B00 && (option->mark_ >> 16) == 8);

// Expect policy does not accept security ID 2 on ingress on port 80
auto port_policy = option->getPolicy()->findPortPolicy(true, 80);
auto port_policy = option->initial_policy_->findPortPolicy(true, 80);
EXPECT_FALSE(port_policy.allowed(2, ""));
}

Expand Down Expand Up @@ -384,6 +387,7 @@ TEST_F(MetadataConfigTest, EastWestL7LbMetadata) {
EXPECT_EQ("[face::1:1:1]:41234", option->ipv6_source_address_->asString());
EXPECT_EQ(80, option->port_);
EXPECT_EQ("10.1.1.1", option->pod_ip_);
EXPECT_NE(nullptr, option->initial_policy_);

// Check that Endpoint's ID is used in the socket mark
EXPECT_TRUE((option->mark_ & 0xffff) == 0x0900 && (option->mark_ >> 16) == 2048);
Expand Down Expand Up @@ -413,6 +417,7 @@ TEST_F(MetadataConfigTest, EastWestL7LbMetadataNoOriginalSource) {
EXPECT_EQ("[face::42]:0", option->ipv6_source_address_->asString());
EXPECT_EQ(80, option->port_);
EXPECT_EQ("10.1.1.42", option->pod_ip_);
EXPECT_NE(nullptr, option->initial_policy_);
EXPECT_EQ(0, option->ingress_source_identity_);

// Check that Ingress ID is used in the socket mark
Expand Down

0 comments on commit c7b1578

Please sign in to comment.