Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions api/envoy/config/filter/http/jwt_authn/v2alpha/config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -391,8 +391,7 @@ message FilterStateRule {

// A map of string keys to requirements. The string key is the string value
// in the FilterState with the name specified in the *name* field above.
map<string, JwtRequirement>
requires = 3;
map<string, JwtRequirement> requires = 3;
}

// This is the Envoy HTTP filter config for JWT authentication.
Expand Down
5 changes: 4 additions & 1 deletion api/envoy/config/listener/v3/listener_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ message Filter {
// listed at the end, because that's how we want to list them in the docs.
//
// [#comment:TODO(PiotrSikora): Add support for configurable precedence of the rules]
// [#next-free-field: 14]
// [#next-free-field: 101]
message FilterChainMatch {
option (udpa.annotations.versioning).previous_message_type =
"envoy.api.v2.listener.FilterChainMatch";
Expand All @@ -112,6 +112,9 @@ message FilterChainMatch {

reserved 1;

// The Connection Metadata of the connection to match against.
string connection_metadata = 100;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have a new notion of connection metadata? Doesn't the StreamInfo associated with a connection, providing both dynamic metadata and FilterState suffice?
/wait

Copy link
Copy Markdown
Author

@ssripadham ssripadham Feb 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@htuch - First of all, thanks @htuch for reviewing the code. I am not familiar with StreamInfo. Are you referring to https://www.envoyproxy.io/docs/envoy/v1.21.0/configuration/http/http_filters/lua_filter.html?highlight=streaminfo#config-http-filters-lua-stream-info-wrapper ? In any case, we will still need the changes in FilterChainMatch to match based in the dynamic metadata correct?

Copy link
Copy Markdown
Author

@ssripadham ssripadham Feb 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FilterChain matching is bound to the listener (socket) and is based on Network::ConnectionSocket object.
In this PR, we are attaching the metadata to Network::ConnectionSocket.

During filter chain match, this is referenced as below:

FilterChainManagerImpl::findFilterChain(const Network::ConnectionSocket& socket) const {
  const auto& connection_metadata = absl::AsciiStrToLower(socket.connectionInfoProvider().connectionMetadata());
  ...

However, streamInfo is on Connection itself and not on the Socket and is referenced as below connection().streamInfo().setDynamicMetadata()

How do you propose we use streamInfo to do filterChain match?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The caller should have access to this, e..g. in ActiveStreamListenerBase::newConnection. @lambdai does this work?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the change in this PR kicks in filterChainManager().findFilterChain(*socket) in the code below. So it will still require the socket to hold the metadata and the findFilterChain to select the filter chain based on the metadata.

void ActiveStreamListenerBase::newConnection(Network::ConnectionSocketPtr&& socket,
                                             std::unique_ptr<StreamInfo::StreamInfo> stream_info) {
  // Find matching filter chain.
  const auto filter_chain = config_->filterChainManager().findFilterChain(*socket);
 ...
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unified matcher has been merged #20110. We still need a metadata matcher but that would be an extension now.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kyessenov - This is great to hear. Could you please share the design for Unified matcher and how to integrate metadata matcher to unified filter?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @kyessenov - we are unable to use the unified matcher as it doesn't support custom matcher we are building for dynamic metadata matching #23503

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kyessenov - Is there a way to support dynamic metadata matching in addition to static string matching in Unified matcher?


// Optional destination port to consider when use_original_dst is set on the
// listener in determining a filter chain match.
google.protobuf.UInt32Value destination_port = 8 [(validate.rules).uint32 = {lte: 65535 gte: 1}];
Expand Down
3 changes: 1 addition & 2 deletions api/envoy/extensions/filters/http/jwt_authn/v3/config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -583,8 +583,7 @@ message FilterStateRule {

// A map of string keys to requirements. The string key is the string value
// in the FilterState with the name specified in the *name* field above.
map<string, JwtRequirement>
requires = 3;
map<string, JwtRequirement> requires = 3;
}

// This is the Envoy HTTP filter config for JWT authentication.
Expand Down
10 changes: 10 additions & 0 deletions envoy/network/listen_socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ class ConnectionSocket : public virtual Socket, public virtual ScopeTrackedObjec
*/
virtual absl::string_view ja3Hash() const PURE;

/**
* Set Connection Metadata.
*/
virtual void setConnectionMetadata(absl::string_view connection_metadata) PURE;

/**
* @return Connection Metadata.
*/
virtual absl::string_view connectionMetadata() const PURE;

/**
* @return absl::optional<std::chrono::milliseconds> An optional of the most recent round-trip
* time of the connection. If the platform does not support this, then an empty optional is
Expand Down
10 changes: 10 additions & 0 deletions envoy/network/socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ class ConnectionInfoProvider {
*/
virtual absl::string_view requestedServerName() const PURE;

/**
* @return Connection Metadatafor downstream client.
*/
virtual absl::string_view connectionMetadata() const PURE;

/**
* @return Connection ID of the downstream connection, or unset if not available.
**/
Expand Down Expand Up @@ -142,6 +147,11 @@ class ConnectionInfoSetter : public ConnectionInfoProvider {
*/
virtual void setRequestedServerName(const absl::string_view requested_server_name) PURE;

/**
* @param Connection Metadata.
*/
virtual void setConnectionMetadata(const absl::string_view connection_metadata) PURE;

/**
* @param id Connection ID of the downstream connection.
**/
Expand Down
3 changes: 3 additions & 0 deletions source/common/http/filter_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,9 @@ class OverridableRemoteConnectionInfoSetterStreamInfo : public StreamInfo::Strea
absl::string_view requestedServerName() const override {
return StreamInfoImpl::downstreamAddressProvider().requestedServerName();
}
absl::string_view connectionMetadata() const override {
return StreamInfoImpl::downstreamAddressProvider().connectionMetadata();
}
absl::optional<uint64_t> connectionID() const override {
return StreamInfoImpl::downstreamAddressProvider().connectionID();
}
Expand Down
8 changes: 8 additions & 0 deletions source/common/network/listen_socket_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,14 @@ class ConnectionSocketImpl : public SocketImpl, public ConnectionSocket {
}
absl::string_view ja3Hash() const override { return connectionInfoProvider().ja3Hash(); }

void setConnectionMetadata(absl::string_view connection_metadata) override {
// Always keep the connection_metadata as lower case.
connectionInfoProvider().setConnectionMetadata(absl::AsciiStrToLower(connection_metadata));
}
absl::string_view connectionMetadata() const override {
return connectionInfoProvider().connectionMetadata();
}

absl::optional<std::chrono::milliseconds> lastRoundTripTime() override {
return ioHandle().lastRoundTripTime();
}
Expand Down
5 changes: 5 additions & 0 deletions source/common/network/socket_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ class ConnectionInfoSetterImpl : public ConnectionInfoSetter {
void setRequestedServerName(const absl::string_view requested_server_name) override {
server_name_ = std::string(requested_server_name);
}
absl::string_view connectionMetadata() const override { return connection_metadata_; }
void setConnectionMetadata(const absl::string_view connection_metadata) override {
connection_metadata_ = std::string(connection_metadata);
}
absl::optional<uint64_t> connectionID() const override { return connection_id_; }
void setConnectionID(uint64_t id) override { connection_id_ = id; }
absl::optional<absl::string_view> interfaceName() const override { return interface_name_; }
Expand All @@ -70,6 +74,7 @@ class ConnectionInfoSetterImpl : public ConnectionInfoSetter {
Address::InstanceConstSharedPtr remote_address_;
Address::InstanceConstSharedPtr direct_remote_address_;
std::string server_name_;
std::string connection_metadata_;
absl::optional<uint64_t> connection_id_;
absl::optional<std::string> interface_name_;
Ssl::ConnectionInfoConstSharedPtr ssl_info_;
Expand Down
156 changes: 105 additions & 51 deletions source/server/filter_chain_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,8 @@ void FilterChainManagerImpl::addFilterChains(
++new_filter_chain_size;
}

addFilterChainForDestinationPorts(
destination_ports_map_,
addFilterChainForConnectionMetadata(
connection_metadata_map_, absl::AsciiStrToLower(filter_chain_match.connection_metadata()),
PROTOBUF_GET_WRAPPED_OR_DEFAULT(filter_chain_match, destination_port, 0), destination_ips,
server_names, filter_chain_match.transport_protocol(),
filter_chain_match.application_protocols(), direct_source_ips,
Expand Down Expand Up @@ -275,6 +275,25 @@ void FilterChainManagerImpl::copyOrRebuildDefaultFilterChain(
}
}

void FilterChainManagerImpl::addFilterChainForConnectionMetadata(
ConnectionMetadataMap& connection_metadata_map, const std::string& connection_metadata,
uint16_t destination_port, const std::vector<std::string>& destination_ips,
const absl::Span<const std::string> server_names, const std::string& transport_protocol,
const absl::Span<const std::string* const> application_protocols,
const std::vector<std::string>& direct_source_ips,
const envoy::config::listener::v3::FilterChainMatch::ConnectionSourceType source_type,
const std::vector<std::string>& source_ips,
const absl::Span<const Protobuf::uint32> source_ports,
const Network::FilterChainSharedPtr& filter_chain) {
if (connection_metadata_map.find(connection_metadata) == connection_metadata_map.end()) {
connection_metadata_map[connection_metadata] = DestinationPortsMap{};
}
addFilterChainForDestinationPorts(connection_metadata_map[connection_metadata], destination_port,
destination_ips, server_names, transport_protocol,
application_protocols, direct_source_ips, source_type,
source_ips, source_ports, filter_chain);
}

void FilterChainManagerImpl::addFilterChainForDestinationPorts(
DestinationPortsMap& destination_ports_map, uint16_t destination_port,
const std::vector<std::string>& destination_ips,
Expand Down Expand Up @@ -468,13 +487,46 @@ std::pair<T, std::vector<Network::Address::CidrRange>> makeCidrListEntry(const s

const Network::FilterChain*
FilterChainManagerImpl::findFilterChain(const Network::ConnectionSocket& socket) const {
const auto& connection_metadata =
absl::AsciiStrToLower(socket.connectionInfoProvider().connectionMetadata());

const Network::FilterChain* best_match_filter_chain = nullptr;
const auto connection_metadata_match = connection_metadata_map_.find(connection_metadata);
if (connection_metadata_match != connection_metadata_map_.end()) {
best_match_filter_chain =
findFilterChainForDestinationPort(connection_metadata_match->second, socket);
if (best_match_filter_chain != nullptr) {
return best_match_filter_chain;
} else {
// There is entry for specific tenant ID but none of the filter chain matches. Instead of
// matching catch-all tenant ID "", the fallback filter chain is returned.
return default_filter_chain_.get();
}
}

// Match on a filter chain without tenant ID requirements.
const auto connection_metadata_catchall_match = connection_metadata_map_.find(EMPTY_STRING);
if (connection_metadata_catchall_match != connection_metadata_map_.end()) {
best_match_filter_chain =
findFilterChainForDestinationPort(connection_metadata_catchall_match->second, socket);
}

return best_match_filter_chain != nullptr
? best_match_filter_chain
// Neither exact tenant ID nor catch-all tenant ID matches. Use fallback filter chain.
: default_filter_chain_.get();
}

const Network::FilterChain* FilterChainManagerImpl::findFilterChainForDestinationPort(
const DestinationPortsMap& destination_ports_map,
const Network::ConnectionSocket& socket) const {
const auto& address = socket.connectionInfoProvider().localAddress();

const Network::FilterChain* best_match_filter_chain = nullptr;
// Match on destination port (only for IP addresses).
if (address->type() == Network::Address::Type::Ip) {
const auto port_match = destination_ports_map_.find(address->ip()->port());
if (port_match != destination_ports_map_.end()) {
const auto port_match = destination_ports_map.find(address->ip()->port());
if (port_match != destination_ports_map.end()) {
best_match_filter_chain = findFilterChainForDestinationIP(*port_match->second.second, socket);
if (best_match_filter_chain != nullptr) {
return best_match_filter_chain;
Expand All @@ -486,8 +538,8 @@ FilterChainManagerImpl::findFilterChain(const Network::ConnectionSocket& socket)
}
}
// Match on catch-all port 0 if there is no specific port sub tree.
const auto port_match = destination_ports_map_.find(0);
if (port_match != destination_ports_map_.end()) {
const auto port_match = destination_ports_map.find(0);
if (port_match != destination_ports_map.end()) {
best_match_filter_chain = findFilterChainForDestinationIP(*port_match->second.second, socket);
}
return best_match_filter_chain != nullptr
Expand Down Expand Up @@ -670,58 +722,60 @@ const Network::FilterChain* FilterChainManagerImpl::findFilterChainForSourceIpAn
}

void FilterChainManagerImpl::convertIPsToTries() {
for (auto& [destination_port, destination_ips_pair] : destination_ports_map_) {
UNREFERENCED_PARAMETER(destination_port);
// These variables are used as we build up the destination CIDRs used for the trie.
auto& [destination_ips_map, destination_ips_trie] = destination_ips_pair;
std::vector<std::pair<ServerNamesMapSharedPtr, std::vector<Network::Address::CidrRange>>>
destination_ips_list;
destination_ips_list.reserve(destination_ips_map.size());

for (const auto& [destination_ip, server_names_map_ptr] : destination_ips_map) {
destination_ips_list.push_back(makeCidrListEntry(destination_ip, server_names_map_ptr));

// This hugely nested for loop greatly pains me, but I'm not sure how to make it better.
// We need to get access to all of the source IP strings so that we can convert them into
// a trie like we did for the destination IPs above.
for (auto& [server_name, transport_protocols_map] : *server_names_map_ptr) {
UNREFERENCED_PARAMETER(server_name);
for (auto& [transport_protocol, application_protocols_map] : transport_protocols_map) {
UNREFERENCED_PARAMETER(transport_protocol);
for (auto& [application_protocol, direct_source_ips_pair] : application_protocols_map) {
UNREFERENCED_PARAMETER(application_protocol);
auto& [direct_source_ips_map, direct_source_ips_trie] = direct_source_ips_pair;

std::vector<
std::pair<SourceTypesArraySharedPtr, std::vector<Network::Address::CidrRange>>>
direct_source_ips_list;
direct_source_ips_list.reserve(direct_source_ips_map.size());

for (auto& [direct_source_ip, source_arrays_ptr] : direct_source_ips_map) {
direct_source_ips_list.push_back(
makeCidrListEntry(direct_source_ip, source_arrays_ptr));

for (auto& [source_ips_map, source_ips_trie] : *source_arrays_ptr) {
std::vector<
std::pair<SourcePortsMapSharedPtr, std::vector<Network::Address::CidrRange>>>
source_ips_list;
source_ips_list.reserve(source_ips_map.size());

for (auto& [source_ip, source_port_map_ptr] : source_ips_map) {
source_ips_list.push_back(makeCidrListEntry(source_ip, source_port_map_ptr));
for (auto& [connection_metadata, destination_ports_map] : connection_metadata_map_) {
for (auto& [destination_port, destination_ips_pair] : destination_ports_map) {
UNREFERENCED_PARAMETER(destination_port);
// These variables are used as we build up the destination CIDRs used for the trie.
auto& [destination_ips_map, destination_ips_trie] = destination_ips_pair;
std::vector<std::pair<ServerNamesMapSharedPtr, std::vector<Network::Address::CidrRange>>>
destination_ips_list;
destination_ips_list.reserve(destination_ips_map.size());

for (const auto& [destination_ip, server_names_map_ptr] : destination_ips_map) {
destination_ips_list.push_back(makeCidrListEntry(destination_ip, server_names_map_ptr));

// This hugely nested for loop greatly pains me, but I'm not sure how to make it better.
// We need to get access to all of the source IP strings so that we can convert them into
// a trie like we did for the destination IPs above.
for (auto& [server_name, transport_protocols_map] : *server_names_map_ptr) {
UNREFERENCED_PARAMETER(server_name);
for (auto& [transport_protocol, application_protocols_map] : transport_protocols_map) {
UNREFERENCED_PARAMETER(transport_protocol);
for (auto& [application_protocol, direct_source_ips_pair] : application_protocols_map) {
UNREFERENCED_PARAMETER(application_protocol);
auto& [direct_source_ips_map, direct_source_ips_trie] = direct_source_ips_pair;

std::vector<
std::pair<SourceTypesArraySharedPtr, std::vector<Network::Address::CidrRange>>>
direct_source_ips_list;
direct_source_ips_list.reserve(direct_source_ips_map.size());

for (auto& [direct_source_ip, source_arrays_ptr] : direct_source_ips_map) {
direct_source_ips_list.push_back(
makeCidrListEntry(direct_source_ip, source_arrays_ptr));

for (auto& [source_ips_map, source_ips_trie] : *source_arrays_ptr) {
std::vector<
std::pair<SourcePortsMapSharedPtr, std::vector<Network::Address::CidrRange>>>
source_ips_list;
source_ips_list.reserve(source_ips_map.size());

for (auto& [source_ip, source_port_map_ptr] : source_ips_map) {
source_ips_list.push_back(makeCidrListEntry(source_ip, source_port_map_ptr));
}

source_ips_trie = std::make_unique<SourceIPsTrie>(source_ips_list, true);
}

source_ips_trie = std::make_unique<SourceIPsTrie>(source_ips_list, true);
}
direct_source_ips_trie =
std::make_unique<DirectSourceIPsTrie>(direct_source_ips_list, true);
}
direct_source_ips_trie =
std::make_unique<DirectSourceIPsTrie>(direct_source_ips_list, true);
}
}
}
}

destination_ips_trie = std::make_unique<DestinationIPsTrie>(destination_ips_list, true);
destination_ips_trie = std::make_unique<DestinationIPsTrie>(destination_ips_list, true);
}
}
}

Expand Down
20 changes: 19 additions & 1 deletion source/server/filter_chain_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,18 @@ class FilterChainManagerImpl : public Network::FilterChainManager,
using DestinationIPsTriePtr = std::unique_ptr<DestinationIPsTrie>;
using DestinationPortsMap =
absl::flat_hash_map<uint16_t, std::pair<DestinationIPsMap, DestinationIPsTriePtr>>;
using ConnectionMetadataMap = absl::flat_hash_map<std::string, DestinationPortsMap>;

void addFilterChainForConnectionMetadata(
ConnectionMetadataMap& connection_metadata_map, const std::string& connection_metadata,
uint16_t destination_port, const std::vector<std::string>& destination_ips,
const absl::Span<const std::string> server_names, const std::string& transport_protocol,
const absl::Span<const std::string* const> application_protocols,
const std::vector<std::string>& direct_source_ips,
const envoy::config::listener::v3::FilterChainMatch::ConnectionSourceType source_type,
const std::vector<std::string>& source_ips,
const absl::Span<const Protobuf::uint32> source_ports,
const Network::FilterChainSharedPtr& filter_chain);
void addFilterChainForDestinationPorts(
DestinationPortsMap& destination_ports_map, uint16_t destination_port,
const std::vector<std::string>& destination_ips,
Expand Down Expand Up @@ -334,6 +345,9 @@ class FilterChainManagerImpl : public Network::FilterChainManager,
const Network::FilterChainSharedPtr& filter_chain);

const Network::FilterChain*
findFilterChainForDestinationPort(const DestinationPortsMap& destination_ports_map,
const Network::ConnectionSocket& socket) const;
const Network::FilterChain*
findFilterChainForDestinationIP(const DestinationIPsTrie& destination_ips_trie,
const Network::ConnectionSocket& socket) const;
const Network::FilterChain*
Expand Down Expand Up @@ -372,7 +386,11 @@ class FilterChainManagerImpl : public Network::FilterChainManager,

// Mapping of FilterChain's configured destination ports, IPs, server names, transport protocols
// and application protocols, using structures defined above.
DestinationPortsMap destination_ports_map_;
// DestinationPortsMap destination_ports_map_;

// Mapping of FilterChain's configured Connection Metadata, destination ports, IPs, server names,
// transport protocols and application protocols, using structures defined above.
ConnectionMetadataMap connection_metadata_map_;

const Network::Address::InstanceConstSharedPtr address_;
// This is the reference to a factory context which all the generations of listener share.
Expand Down
5 changes: 2 additions & 3 deletions test/common/http/header_map_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -423,9 +423,8 @@ TEST_P(HeaderMapImplTest, AllInlineHeaders) {
INLINE_REQ_RESP_STRING_HEADERS(TEST_INLINE_STRING_HEADER_FUNCS)
}
{
// No request trailer O(1) headers.
}
{
// No request trailer O(1) headers.
} {
auto header_map = ResponseHeaderMapImpl::create();
INLINE_RESP_STRING_HEADERS(TEST_INLINE_STRING_HEADER_FUNCS)
INLINE_REQ_RESP_STRING_HEADERS(TEST_INLINE_STRING_HEADER_FUNCS)
Expand Down
2 changes: 2 additions & 0 deletions test/mocks/network/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,8 @@ class MockConnectionSocket : public ConnectionSocket {
MOCK_METHOD(absl::string_view, requestedServerName, (), (const));
MOCK_METHOD(void, setJA3Hash, (absl::string_view));
MOCK_METHOD(absl::string_view, ja3Hash, (), (const));
MOCK_METHOD(void, setConnectionMetadata, (absl::string_view));
MOCK_METHOD(absl::string_view, connectionMetadata, (), (const));
MOCK_METHOD(void, addOption_, (const Socket::OptionConstSharedPtr&));
MOCK_METHOD(void, addOptions_, (const Socket::OptionsSharedPtr&));
MOCK_METHOD(const Network::ConnectionSocket::OptionsSharedPtr&, options, (), (const));
Expand Down
Loading