Skip to content

Commit a5d9d3f

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 e9b4aad commit a5d9d3f

File tree

7 files changed

+88
-23
lines changed

7 files changed

+88
-23
lines changed

cilium/bpf_metadata.cc

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

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

@@ -368,6 +368,10 @@ const PolicyInstance& Config::getPolicy(const std::string& pod_ip) const {
368368
return npmap_->getPolicyInstance(pod_ip, allow_egress);
369369
}
370370

371+
bool Config::exists(const std::string& pod_ip) const {
372+
return npmap_ != nullptr && npmap_->exists(pod_ip);
373+
}
374+
371375
absl::optional<Cilium::BpfMetadata::SocketMetadata>
372376
Config::extractSocketMetadata(Network::ConnectionSocket& socket) {
373377
Network::Address::InstanceConstSharedPtr src_address =
@@ -473,7 +477,7 @@ Config::extractSocketMetadata(Network::ConnectionSocket& socket) {
473477

474478
// Resolve source identity for the Ingress address
475479
source_identity = resolvePolicyId(ingress_ip);
476-
if (source_identity == Cilium::ID::WORLD) {
480+
if (source_identity == Cilium::ID::World) {
477481
// No security ID available for the configured source IP
478482
ENVOY_LOG(warn,
479483
"cilium.bpf_metadata (north/south L7 LB): Unknown local Ingress IP source address "
@@ -484,7 +488,8 @@ Config::extractSocketMetadata(Network::ConnectionSocket& socket) {
484488

485489
// Original source address is never used for north/south LB
486490
src_address = nullptr;
487-
} else if (!use_original_source_address_ || (npmap_ != nullptr && npmap_->exists(other_ip))) {
491+
} else if (!use_original_source_address_ || destination_identity == Cilium::ID::Host ||
492+
(npmap_ != nullptr && npmap_->exists(other_ip))) {
488493
// Otherwise only use the original source address if permitted and the destination is not
489494
// in the same node.
490495
//

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: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ class PolicyResolver {
2727

2828
virtual uint32_t resolvePolicyId(const Network::Address::Ip*) const PURE;
2929
virtual const PolicyInstance& getPolicy(const std::string&) const PURE;
30+
virtual bool exists(const std::string&) const PURE;
3031
};
3132

3233
// FilterState that holds relevant connection & policy information that can be retrieved
@@ -55,7 +56,7 @@ class CiliumPolicyFilterState : public StreamInfo::FilterState::Object,
5556
if (resolver) {
5657
return resolver->resolvePolicyId(ip);
5758
}
58-
return Cilium::ID::WORLD; // default to WORLD policy ID if resolver is no longer available
59+
return Cilium::ID::World; // default to WORLD policy ID if resolver is no longer available
5960
}
6061

6162
const PolicyInstance& getPolicy() const {

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: 50 additions & 10 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 std::weak_ptr<PolicyResolver>& 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,51 @@ 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+
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+
const auto resolver = policy_resolver_.lock();
93+
if (!resolver) {
94+
// No policy resolver
95+
ENVOY_LOG(debug, "No policy resolver");
96+
return false;
97+
}
98+
99+
auto destination_identity = resolver->resolvePolicyId(destination_ip);
77100
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;
101+
"Socket {} destination address {} has security identity {}",
102+
socket.ioHandle().fdDoNotUse(), destination_ip_str, destination_identity);
103+
104+
if (destination_identity == Cilium::ID::Host) {
105+
ENVOY_LOG(
106+
trace,
107+
"Skipping restore of local address on socket: {} - destination is the local host {}",
108+
socket.ioHandle().fdDoNotUse(), destination_ip_str);
109+
return true;
110+
}
111+
112+
if (resolver->exists(destination_ip_str)) {
113+
ENVOY_LOG(trace,
114+
"Skipping restore of local address on socket: {} - destination is a local pod {}",
115+
socket.ioHandle().fdDoNotUse(), destination_ip_str);
116+
return true;
117+
}
118+
} else if (source_address->ip()) {
119+
ENVOY_LOG(debug,
120+
"Destination address filter state for socket {} is not available",
121+
socket.ioHandle().fdDoNotUse());
82122
}
83123

84124
// 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 std::weak_ptr<PolicyResolver>& 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 std::weak_ptr<PolicyResolver> policy_resolver_;
54+
4855
int linger_time_;
4956

5057
Network::Address::InstanceConstSharedPtr original_source_address_;

0 commit comments

Comments
 (0)