Skip to content

Commit a808711

Browse files
committed
bpf_metadata: Skip if destination is local host
Original source address need not and can not be used if the destination is a local pod, or a local host. Add the local host case to bpf_metadata filter, and add handling of both cases to socket_option_source_address, as at that time the chosen destination is known. In principle, we should not need these checks in bpf_metadata any more, but tests/ currently depends on them. Signed-off-by: Jarno Rajahalme <[email protected]>
1 parent 5e7bc73 commit a808711

File tree

7 files changed

+79
-23
lines changed

7 files changed

+79
-23
lines changed

cilium/bpf_metadata.cc

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ uint32_t Config::resolvePolicyId(const Network::Address::Ip* ip) const {
273273

274274
// default destination identity to the world if needed
275275
if (id == 0) {
276-
id = Cilium::ID::WORLD;
276+
id = Cilium::ID::World;
277277
ENVOY_LOG(trace, "bpf_metadata: Identity for IP defaults to WORLD", ip->addressAsString());
278278
}
279279

@@ -365,6 +365,10 @@ const PolicyInstance& Config::getPolicy(const std::string& pod_ip) const {
365365
return npmap_->getPolicyInstance(pod_ip, allow_egress);
366366
}
367367

368+
bool Config::exists(const std::string& pod_ip) const {
369+
return npmap_ != nullptr && npmap_->exists(pod_ip);
370+
}
371+
368372
absl::optional<Cilium::BpfMetadata::SocketMetadata>
369373
Config::extractSocketMetadata(Network::ConnectionSocket& socket) {
370374
Network::Address::InstanceConstSharedPtr src_address =
@@ -470,7 +474,7 @@ Config::extractSocketMetadata(Network::ConnectionSocket& socket) {
470474

471475
// Resolve source identity for the Ingress address
472476
source_identity = resolvePolicyId(ingress_ip);
473-
if (source_identity == Cilium::ID::WORLD) {
477+
if (source_identity == Cilium::ID::World) {
474478
// No security ID available for the configured source IP
475479
ENVOY_LOG(warn,
476480
"cilium.bpf_metadata (north/south L7 LB): Unknown local Ingress IP source address "
@@ -481,7 +485,8 @@ Config::extractSocketMetadata(Network::ConnectionSocket& socket) {
481485

482486
// Original source address is never used for north/south LB
483487
src_address = nullptr;
484-
} else if (!use_original_source_address_ || (npmap_ != nullptr && npmap_->exists(other_ip))) {
488+
} else if (!use_original_source_address_ || destination_identity == Cilium::ID::Host ||
489+
(npmap_ != nullptr && npmap_->exists(other_ip))) {
485490
// Otherwise only use the original source address if permitted and the destination is not
486491
// in the same node.
487492
//

cilium/bpf_metadata.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ struct SocketMetadata : public Logger::Loggable<Logger::Id::filter> {
6868
std::shared_ptr<Envoy::Cilium::SourceAddressSocketOption> buildSourceAddressSocketOption(
6969
int linger_time, const std::shared_ptr<CiliumDestinationFilterState>& dest_fs = nullptr) {
7070
return std::make_shared<Envoy::Cilium::SourceAddressSocketOption>(
71-
source_identity_, linger_time, original_source_address_, source_address_ipv4_,
72-
source_address_ipv6_, dest_fs);
71+
source_identity_, policy_resolver_, linger_time, original_source_address_,
72+
source_address_ipv4_, source_address_ipv6_, dest_fs);
7373
};
7474

7575
// Add ProxyLib L7 protocol as requested application protocol on the socket.
@@ -144,6 +144,7 @@ class Config : public Cilium::PolicyResolver,
144144
// PolicyResolver
145145
uint32_t resolvePolicyId(const Network::Address::Ip*) const override;
146146
const PolicyInstance& getPolicy(const std::string&) const override;
147+
bool exists(const std::string&) const override;
147148

148149
virtual absl::optional<SocketMetadata> extractSocketMetadata(Network::ConnectionSocket& socket);
149150

cilium/filter_state_cilium_policy.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ class PolicyResolver {
2626

2727
virtual uint32_t resolvePolicyId(const Network::Address::Ip*) const PURE;
2828
virtual const PolicyInstance& getPolicy(const std::string&) const PURE;
29+
virtual bool exists(const std::string&) const PURE;
2930
};
3031
using PolicyResolverSharedPtr = std::shared_ptr<PolicyResolver>;
3132

cilium/host_map.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ class PolicyHostMap : public Singleton::Instance,
154154
return it->second;
155155
}
156156
}
157-
return ID::UNKNOWN;
157+
return ID::Unknown;
158158
}
159159

160160
uint64_t resolve(absl::uint128 addr6) const {
@@ -164,7 +164,7 @@ class PolicyHostMap : public Singleton::Instance,
164164
return it->second;
165165
}
166166
}
167-
return ID::UNKNOWN;
167+
return ID::Unknown;
168168
}
169169

170170
uint64_t resolve(const Network::Address::Ip* addr) const {
@@ -176,7 +176,7 @@ class PolicyHostMap : public Singleton::Instance,
176176
if (ipv6) {
177177
return resolve(ipv6->address());
178178
}
179-
return ID::WORLD;
179+
return ID::World;
180180
}
181181

182182
protected:
@@ -195,7 +195,7 @@ class PolicyHostMap : public Singleton::Instance,
195195

196196
uint64_t resolve(const Network::Address::Ip* addr) const {
197197
const ThreadLocalHostMap* hostmap = getHostMap();
198-
return (hostmap != nullptr) ? hostmap->resolve(addr) : ID::UNKNOWN;
198+
return (hostmap != nullptr) ? hostmap->resolve(addr) : ID::Unknown;
199199
}
200200

201201
void logmaps(const std::string& msg) {

cilium/policy_id.h

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,19 @@ namespace Envoy {
66
namespace Cilium {
77

88
enum ID : uint64_t {
9-
UNKNOWN = 0,
10-
WORLD = 2,
9+
Unknown = 0,
10+
Host = 1,
11+
World = 2,
12+
Unmanaged = 3,
13+
Health = 4,
14+
Init = 5,
15+
RemoteNode = 6,
16+
KubeApiServer = 7,
17+
Ingress = 8,
18+
WorldIPv4 = 9,
19+
WorldIPv6 = 10,
20+
EncryptedOverlay = 11,
21+
1122
// LocalIdentityFlag is the bit in the numeric identity that identifies
1223
// a numeric identity to have local scope
1324
LocalIdentityFlag = 1 << 24,

cilium/socket_option_source_address.cc

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include <sys/socket.h>
44

55
#include <cstdint>
6+
#include <memory>
67
#include <utility>
78
#include <vector>
89

@@ -17,18 +18,20 @@
1718

1819
#include "absl/numeric/int128.h"
1920
#include "cilium/filter_state_cilium_destination.h"
21+
#include "cilium/filter_state_cilium_policy.h"
22+
#include "cilium/policy_id.h"
2023

2124
namespace Envoy {
2225
namespace Cilium {
2326

2427
SourceAddressSocketOption::SourceAddressSocketOption(
25-
uint32_t source_identity, int linger_time,
28+
uint32_t source_identity, const PolicyResolverSharedPtr& policy_resolver, int linger_time,
2629
Network::Address::InstanceConstSharedPtr original_source_address,
2730
Network::Address::InstanceConstSharedPtr ipv4_source_address,
2831
Network::Address::InstanceConstSharedPtr ipv6_source_address,
2932
std::shared_ptr<CiliumDestinationFilterState> dest_fs)
30-
: source_identity_(source_identity), linger_time_(linger_time),
31-
original_source_address_(std::move(original_source_address)),
33+
: source_identity_(source_identity), policy_resolver_(policy_resolver),
34+
linger_time_(linger_time), original_source_address_(std::move(original_source_address)),
3235
ipv4_source_address_(std::move(ipv4_source_address)),
3336
ipv6_source_address_(std::move(ipv6_source_address)), dest_fs_(std::move(dest_fs)) {
3437
ENVOY_LOG(debug,
@@ -71,14 +74,42 @@ bool SourceAddressSocketOption::setOption(
7174
}
7275

7376
if (source_address->ip() && dest_fs_->getDestinationAddress() &&
74-
dest_fs_->getDestinationAddress()->ip() &&
75-
source_address->ip()->addressAsString() ==
76-
dest_fs_->getDestinationAddress()->ip()->addressAsString()) {
77-
ENVOY_LOG(trace,
78-
"Skipping restore of local address on socket: {} - source address is same as "
79-
"destination address {}",
80-
socket.ioHandle().fdDoNotUse(), source_address->ip()->addressAsString());
81-
return true;
77+
dest_fs_->getDestinationAddress()->ip()) {
78+
// Skip using original source if hairpinning back to the source, as otherwise Linux would
79+
// drop the packet
80+
auto destination_ip = dest_fs_->getDestinationAddress()->ip();
81+
const auto& destination_ip_str = destination_ip->addressAsString();
82+
if (source_address->ip()->addressAsString() == destination_ip_str) {
83+
ENVOY_LOG(trace,
84+
"Skipping restore of local address on socket: {} - source address is same as "
85+
"destination address {}",
86+
socket.ioHandle().fdDoNotUse(), destination_ip_str);
87+
return true;
88+
}
89+
// Also skip using original source if destination is a local pod or the local host,
90+
// as otherwise there could be 5-tuple collisions, and the local host may not be able to
91+
// send replies back to the proxy otherwise.
92+
auto destination_identity = policy_resolver_->resolvePolicyId(destination_ip);
93+
ENVOY_LOG(trace, "Socket {} destination address {} has security identity {}",
94+
socket.ioHandle().fdDoNotUse(), destination_ip_str, destination_identity);
95+
96+
if (destination_identity == Cilium::ID::Host) {
97+
ENVOY_LOG(
98+
trace,
99+
"Skipping restore of local address on socket: {} - destination is the local host {}",
100+
socket.ioHandle().fdDoNotUse(), destination_ip_str);
101+
return true;
102+
}
103+
104+
if (policy_resolver_->exists(destination_ip_str)) {
105+
ENVOY_LOG(trace,
106+
"Skipping restore of local address on socket: {} - destination is a local pod {}",
107+
socket.ioHandle().fdDoNotUse(), destination_ip_str);
108+
return true;
109+
}
110+
} else if (source_address->ip()) {
111+
ENVOY_LOG(debug, "Destination address filter state for socket {} is not available",
112+
socket.ioHandle().fdDoNotUse());
82113
}
83114

84115
// Note: SO_LINGER option is set on the socket of the upstream connection.

cilium/socket_option_source_address.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#pragma once
22

33
#include <cstdint>
4+
#include <memory>
45
#include <vector>
56

67
#include "envoy/config/core/v3/socket_option.pb.h"
@@ -25,7 +26,8 @@ class SourceAddressSocketOption : public Network::Socket::Option,
2526
public Logger::Loggable<Logger::Id::filter> {
2627
public:
2728
SourceAddressSocketOption(
28-
uint32_t source_identity, int linger_time = -1,
29+
uint32_t source_identity, const PolicyResolverSharedPtr& policy_resolver,
30+
int linger_time = -1,
2931
Network::Address::InstanceConstSharedPtr original_source_address = nullptr,
3032
Network::Address::InstanceConstSharedPtr ipv4_source_address = nullptr,
3133
Network::Address::InstanceConstSharedPtr ipv6_source_address = nullptr,
@@ -45,6 +47,11 @@ class SourceAddressSocketOption : public Network::Socket::Option,
4547
bool isSupported() const override { return true; }
4648

4749
uint32_t source_identity_;
50+
51+
// need information about the destination policy/identity to decide if original source address can
52+
// be used or not.
53+
const PolicyResolverSharedPtr policy_resolver_;
54+
4855
int linger_time_;
4956

5057
Network::Address::InstanceConstSharedPtr original_source_address_;

0 commit comments

Comments
 (0)