From 43744eb1b42d2826c439f178edf27150c13d83f4 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Thu, 23 May 2019 13:23:58 -0600 Subject: [PATCH 1/6] filter chain match: support source CIDRs and ports This PR also fully deprecates the tcp_proxy v1 configuration. This will be deleted following the standard deprecation cycle. All new uses should use filter chain matching. Fixes https://github.com/envoyproxy/envoy/issues/4457 Signed-off-by: Matt Klein --- api/envoy/api/v2/listener/listener.proto | 6 +- .../network/tcp_proxy/v2/tcp_proxy.proto | 61 +--- .../best_practices/best_practices.rst | 7 + docs/root/configuration/configuration.rst | 2 +- docs/root/intro/deprecated.rst | 3 + docs/root/intro/version_history.rst | 3 + source/server/listener_manager_impl.cc | 230 ++++++++++--- source/server/listener_manager_impl.h | 51 ++- test/server/listener_manager_impl_test.cc | 322 +++++++++--------- 9 files changed, 400 insertions(+), 285 deletions(-) create mode 100644 docs/root/configuration/best_practices/best_practices.rst diff --git a/api/envoy/api/v2/listener/listener.proto b/api/envoy/api/v2/listener/listener.proto index b43bcd8d509d4..9c931cd51ae64 100644 --- a/api/envoy/api/v2/listener/listener.proto +++ b/api/envoy/api/v2/listener/listener.proto @@ -64,6 +64,8 @@ message Filter { // 4. Transport protocol. // 5. Application protocols (e.g. ALPN for TLS protocol). // 6. Source type (e.g. any, local or external network). +// 7. Source IP address. +// 8. Source port. // // For criteria that allow ranges or wildcards, the most specific value in any // of the configured filter chains that matches the incoming connection is going @@ -108,14 +110,12 @@ message FilterChainMatch { // connection is contained in at least one of the specified subnets. If the // parameter is not specified or the list is empty, the source IP address is // ignored. - // [#not-implemented-hide:] repeated core.CidrRange source_prefix_ranges = 6; // The criteria is satisfied if the source port of the downstream connection // is contained in at least one of the specified ports. If the parameter is // not specified, the source port is ignored. - // [#not-implemented-hide:] - repeated google.protobuf.UInt32Value source_ports = 7; + repeated uint32 source_ports = 7 [(validate.rules).repeated .items.uint32 = {gte: 1, lte: 65535}]; // If non-empty, a list of server names (e.g. SNI for TLS protocol) to consider when determining // a filter chain match. Those values will be compared against the server names of a new diff --git a/api/envoy/config/filter/network/tcp_proxy/v2/tcp_proxy.proto b/api/envoy/config/filter/network/tcp_proxy/v2/tcp_proxy.proto index 9eb8f4f078173..62874fe1d45db 100644 --- a/api/envoy/config/filter/network/tcp_proxy/v2/tcp_proxy.proto +++ b/api/envoy/config/filter/network/tcp_proxy/v2/tcp_proxy.proto @@ -30,25 +30,11 @@ message TcpProxy { // The upstream cluster to connect to. // - // .. note:: - // - // Complex routing (based on connection properties) is being implemented in listeners. Once - // fully implemented, this field (or `weighted_clusters`) will be the only way to configure - // the target cluster. In the interim, complex routing requires using a :ref:`deprecated_v1 - // ` configuration. - // This field is ignored if a `deprecated_v1` configuration is set. - // string cluster = 2; // Multiple upstream clusters can be specified for a given route. The // request is routed to one of the upstream clusters based on weights // assigned to each cluster. - // - // .. note:: - // - // This field is ignored if the :ref:`deprecated_v1 - // ` - // configuration is set. WeightedCluster weighted_clusters = 10; } @@ -79,9 +65,8 @@ message TcpProxy { // emitted by the this tcp_proxy. repeated envoy.config.filter.accesslog.v2.AccessLog access_log = 5; - // TCP Proxy filter configuration using V1 format, until Envoy gets the - // ability to match source/destination at the listener level (called - // :ref:`filter chain match `). + // [#not-implemented-hide:] Deprecated. + // TCP Proxy filter configuration using V1 format. message DeprecatedV1 { // A TCP proxy route consists of a set of optional L4 criteria and the // name of a cluster. If a downstream connection matches all the @@ -134,46 +119,8 @@ message TcpProxy { repeated TCPRoute routes = 1 [(validate.rules).repeated .min_items = 1]; } - // TCP Proxy filter configuration using deprecated V1 format. This is required for complex - // routing until filter chain matching in the listener is implemented. - // - // Example: - // - // .. code-block:: yaml - // - // - name: "envoy.tcp_proxy" - // config: - // deprecated_v1: true - // value: - // stat_prefix: "prefix" - // access_log: - // - ... - // route_config: - // routes: - // - cluster: "cluster" - // destination_ip_list: - // - "10.1.0.0/8" - // destination_ports: "8080" - // source_ip_list: - // - "10.1.0.0/16" - // - "2001:db8::/32" - // source_ports: "8000,9000-9999" - // - // .. attention:: - // - // Using the deprecated V1 configuration excludes the use of any V2 configuration options. Only - // the V1 configuration is used. All available fields are shown in the example, although the - // access log configuration is omitted for simplicity. The access log configuration uses the - // :repo:`deprecated V1 access log configuration`. - // - // .. attention:: - // - // In the deprecated V1 configuration, source and destination CIDR ranges are specified as a - // list of strings with each string in CIDR notation. Source and destination ports are - // specified as single strings containing a comma-separated list of ports and/or port ranges. - // - // Deprecation pending https://github.com/envoyproxy/envoy/issues/4457 - DeprecatedV1 deprecated_v1 = 6; + // [#not-implemented-hide:] Deprecated. + DeprecatedV1 deprecated_v1 = 6 [deprecated = true]; // The maximum number of unsuccessful connection attempts that will be made before // giving up. If the parameter is not specified, 1 connection attempt will be made. diff --git a/docs/root/configuration/best_practices/best_practices.rst b/docs/root/configuration/best_practices/best_practices.rst new file mode 100644 index 0000000000000..259e6aefbd8ea --- /dev/null +++ b/docs/root/configuration/best_practices/best_practices.rst @@ -0,0 +1,7 @@ +Configuration best practices +============================ + +.. toctree:: + :maxdepth: 2 + + edge diff --git a/docs/root/configuration/configuration.rst b/docs/root/configuration/configuration.rst index 3effeaa8e8554..d004bd250ce72 100644 --- a/docs/root/configuration/configuration.rst +++ b/docs/root/configuration/configuration.rst @@ -5,7 +5,6 @@ Configuration reference .. toctree:: :maxdepth: 2 - :includehidden: overview/v2_overview listeners/listeners @@ -25,3 +24,4 @@ Configuration reference overload_manager/overload_manager secret well_known_dynamic_metadata + best_practices/best_practices diff --git a/docs/root/intro/deprecated.rst b/docs/root/intro/deprecated.rst index 722825bd50d91..95d520667b245 100644 --- a/docs/root/intro/deprecated.rst +++ b/docs/root/intro/deprecated.rst @@ -15,6 +15,9 @@ Version 1.11.0 (Pending) * The --max-stats and --max-obj-name-len flags no longer has any effect. * Use of :ref:`cluster ` in :ref:`redis_proxy.proto ` is deprecated. Set a :ref:`catch_all_cluster ` instead. * Use of json based schema in router check tool tests. The tests should follow validation :repo:`schema`. +* Use of the v1 style route configuration for the :ref:`TCP proxy filter ` + is now fully replaced with listener :ref:`filter chain matching `. + Use this instead. Version 1.10.0 (Apr 5, 2019) ============================ diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 4c72e0bd481fe..2fdd25d4d0d40 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -24,6 +24,9 @@ Version history * http: fixed a crashing bug where gRPC local replies would cause segfaults when upstream access logging was on. * http: mitigated a race condition with the :ref:`delayed_close_timeout` where it could trigger while actively flushing a pending write buffer for a downstream connection. * jwt_authn: make filter's parsing of JWT more flexible, allowing syntax like ``jwt=eyJhbGciOiJS...ZFnFIw,extra=7,realm=123`` +* listener: added :ref:`source IP ` + and :ref:`source port ` filter + chain matching. * rbac: migrated from v2alpha to v2. * redis: add support for Redis cluster custom cluster type. * redis: added :ref:`prefix routing ` to enable routing commands based on their key's prefix to different upstream. diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index c877132033da5..4fd9726ff005e 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -220,12 +220,6 @@ ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, const std::st for (const auto& filter_chain : config.filter_chains()) { const auto& filter_chain_match = filter_chain.filter_chain_match(); - if (!filter_chain_match.address_suffix().empty() || filter_chain_match.has_suffix_len() || - filter_chain_match.source_prefix_ranges_size() || filter_chain_match.source_ports_size()) { - throw EnvoyException(fmt::format("error adding listener '{}': contains filter chains with " - "unimplemented fields", - address_->asString())); - } if (filter_chains.find(filter_chain_match) != filter_chains.end()) { throw EnvoyException(fmt::format("error adding listener '{}': multiple filter chains with " "the same matching rules are defined", @@ -272,6 +266,12 @@ ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, const std::st } } + std::vector source_ips; + for (const auto& source_ip : filter_chain_match.source_prefix_ranges()) { + const auto& cidr_range = Network::Address::CidrRange::create(source_ip); + source_ips.push_back(cidr_range.asString()); + } + std::vector application_protocols( filter_chain_match.application_protocols().begin(), filter_chain_match.application_protocols().end()); @@ -284,7 +284,7 @@ ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, const std::st addFilterChain( PROTOBUF_GET_WRAPPED_OR_DEFAULT(filter_chain_match, destination_port, 0), destination_ips, server_names, filter_chain_match.transport_protocol(), application_protocols, - filter_chain_match.source_type(), + filter_chain_match.source_type(), source_ips, filter_chain_match.source_ports(), config_factory.createTransportSocketFactory(*message, factory_context, server_names), parent_.factory_.createNetworkFilterFactoryList(filter_chain.filters(), *this)); @@ -293,8 +293,8 @@ ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, const std::st (!server_names.empty() || !application_protocols.empty())); } - // Convert DestinationIPsMap to DestinationIPsTrie for faster lookups. - convertDestinationIPsMapToTrie(); + // Convert both destination and source IP CIDRs to tries for faster lookups. + convertIPsToTries(); // Automatically inject TLS Inspector if it wasn't configured explicitly and it's needed. if (need_tls_inspector) { @@ -340,13 +340,15 @@ void ListenerImpl::addFilterChain( const std::vector& server_names, const std::string& transport_protocol, const std::vector& application_protocols, const envoy::api::v2::listener::FilterChainMatch_ConnectionSourceType source_type, + const std::vector& source_ips, + const Protobuf::RepeatedField& source_ports, Network::TransportSocketFactoryPtr&& transport_socket_factory, std::vector filters_factory) { const auto filter_chain = std::make_shared(std::move(transport_socket_factory), std::move(filters_factory)); addFilterChainForDestinationPorts(destination_ports_map_, destination_port, destination_ips, server_names, transport_protocol, application_protocols, - source_type, filter_chain); + source_type, source_ips, source_ports, filter_chain); } void ListenerImpl::addFilterChainForDestinationPorts( @@ -354,6 +356,8 @@ void ListenerImpl::addFilterChainForDestinationPorts( const std::vector& destination_ips, const std::vector& server_names, const std::string& transport_protocol, const std::vector& application_protocols, const envoy::api::v2::listener::FilterChainMatch_ConnectionSourceType source_type, + const std::vector& source_ips, + const Protobuf::RepeatedField& source_ports, const Network::FilterChainSharedPtr& filter_chain) { if (destination_ports_map.find(destination_port) == destination_ports_map.end()) { destination_ports_map[destination_port] = @@ -361,7 +365,7 @@ void ListenerImpl::addFilterChainForDestinationPorts( } addFilterChainForDestinationIPs(destination_ports_map[destination_port].first, destination_ips, server_names, transport_protocol, application_protocols, - source_type, filter_chain); + source_type, source_ips, source_ports, filter_chain); } void ListenerImpl::addFilterChainForDestinationIPs( @@ -369,38 +373,49 @@ void ListenerImpl::addFilterChainForDestinationIPs( const std::vector& server_names, const std::string& transport_protocol, const std::vector& application_protocols, const envoy::api::v2::listener::FilterChainMatch_ConnectionSourceType source_type, + const std::vector& source_ips, + const Protobuf::RepeatedField& source_ports, const Network::FilterChainSharedPtr& filter_chain) { if (destination_ips.empty()) { addFilterChainForServerNames(destination_ips_map[EMPTY_STRING], server_names, - transport_protocol, application_protocols, source_type, - filter_chain); + transport_protocol, application_protocols, source_type, source_ips, + source_ports, filter_chain); } else { for (const auto& destination_ip : destination_ips) { addFilterChainForServerNames(destination_ips_map[destination_ip], server_names, transport_protocol, application_protocols, source_type, - filter_chain); + source_ips, source_ports, filter_chain); } } } void ListenerImpl::addFilterChainForServerNames( - ServerNamesMap& server_names_map, const std::vector& server_names, + ServerNamesMapSharedPtr& server_names_map_ptr, const std::vector& server_names, const std::string& transport_protocol, const std::vector& application_protocols, const envoy::api::v2::listener::FilterChainMatch_ConnectionSourceType source_type, + const std::vector& source_ips, + const Protobuf::RepeatedField& source_ports, const Network::FilterChainSharedPtr& filter_chain) { + if (server_names_map_ptr == nullptr) { + server_names_map_ptr = std::make_shared(); + } + auto& server_names_map = *server_names_map_ptr; + if (server_names.empty()) { addFilterChainForApplicationProtocols(server_names_map[EMPTY_STRING][transport_protocol], - application_protocols, source_type, filter_chain); + application_protocols, source_type, source_ips, + source_ports, filter_chain); } else { for (const auto& server_name : server_names) { if (isWildcardServerName(server_name)) { // Add mapping for the wildcard domain, i.e. ".example.com" for "*.example.com". addFilterChainForApplicationProtocols( server_names_map[server_name.substr(1)][transport_protocol], application_protocols, - source_type, filter_chain); + source_type, source_ips, source_ports, filter_chain); } else { addFilterChainForApplicationProtocols(server_names_map[server_name][transport_protocol], - application_protocols, source_type, filter_chain); + application_protocols, source_type, source_ips, + source_ports, filter_chain); } } } @@ -410,14 +425,16 @@ void ListenerImpl::addFilterChainForApplicationProtocols( ApplicationProtocolsMap& application_protocols_map, const std::vector& application_protocols, const envoy::api::v2::listener::FilterChainMatch_ConnectionSourceType source_type, + const std::vector& source_ips, + const Protobuf::RepeatedField& source_ports, const Network::FilterChainSharedPtr& filter_chain) { if (application_protocols.empty()) { - addFilterChainForSourceTypes(application_protocols_map[EMPTY_STRING], source_type, - filter_chain); + addFilterChainForSourceTypes(application_protocols_map[EMPTY_STRING], source_type, source_ips, + source_ports, filter_chain); } else { for (const auto& application_protocol : application_protocols) { addFilterChainForSourceTypes(application_protocols_map[application_protocol], source_type, - filter_chain); + source_ips, source_ports, filter_chain); } } } @@ -425,8 +442,42 @@ void ListenerImpl::addFilterChainForApplicationProtocols( void ListenerImpl::addFilterChainForSourceTypes( SourceTypesArray& source_types_array, const envoy::api::v2::listener::FilterChainMatch_ConnectionSourceType source_type, + const std::vector& source_ips, + const Protobuf::RepeatedField& source_ports, + const Network::FilterChainSharedPtr& filter_chain) { + if (source_ips.empty()) { + addFilterChainForSourceIPs(source_types_array[source_type].first, EMPTY_STRING, source_ports, + filter_chain); + } else { + for (const auto& source_ip : source_ips) { + addFilterChainForSourceIPs(source_types_array[source_type].first, source_ip, source_ports, + filter_chain); + } + } +} + +void ListenerImpl::addFilterChainForSourceIPs( + SourceIPsMap& source_ips_map, const std::string& source_ip, + const Protobuf::RepeatedField& source_ports, const Network::FilterChainSharedPtr& filter_chain) { - if (source_types_array[source_type] != nullptr) { + if (source_ports.empty()) { + addFilterChainForSourcePorts(source_ips_map[source_ip], 0, filter_chain); + } else { + for (auto source_port : source_ports) { + addFilterChainForSourcePorts(source_ips_map[source_ip], source_port, filter_chain); + } + } +} + +void ListenerImpl::addFilterChainForSourcePorts(SourcePortsMapSharedPtr& source_ports_map_ptr, + uint32_t source_port, + const Network::FilterChainSharedPtr& filter_chain) { + if (source_ports_map_ptr == nullptr) { + source_ports_map_ptr = std::make_shared(); + } + auto& source_ports_map = *source_ports_map_ptr; + + if (source_ports_map.find(source_port) != source_ports_map.end()) { // If we got here and found already configured branch, then it means that this FilterChainMatch // is a duplicate, and that there is some overlap in the repeated fields with already processed // FilterChainMatches. @@ -434,32 +485,66 @@ void ListenerImpl::addFilterChainForSourceTypes( "overlapping matching rules are defined", address_->asString())); } - source_types_array[source_type] = filter_chain; + source_ports_map[source_port] = filter_chain; +} + +namespace { + +// Template function for creating a CIDR list entry for either source or destination address. +template +std::pair> makeCidrListEntry(const std::string& cidr, + const T& data) { + std::vector subnets; + if (cidr == EMPTY_STRING) { + if (Network::Address::ipFamilySupported(AF_INET)) { + subnets.push_back(Network::Address::CidrRange::create("0.0.0.0/0")); + } + if (Network::Address::ipFamilySupported(AF_INET6)) { + subnets.push_back(Network::Address::CidrRange::create("::/0")); + } + } else { + subnets.push_back(Network::Address::CidrRange::create(cidr)); + } + return std::make_pair>(T(data), std::move(subnets)); } -void ListenerImpl::convertDestinationIPsMapToTrie() { +}; // namespace + +void ListenerImpl::convertIPsToTries() { for (auto& port : destination_ports_map_) { + // These variables are used as we build up the destination CIDRs used for the trie. auto& destination_ips_pair = port.second; auto& destination_ips_map = destination_ips_pair.first; - std::vector>> list; + std::vector>> + destination_ips_list; + for (const auto& entry : destination_ips_map) { - std::vector subnets; - if (entry.first == EMPTY_STRING) { - if (Network::Address::ipFamilySupported(AF_INET)) { - subnets.push_back(Network::Address::CidrRange::create("0.0.0.0/0")); - } - if (Network::Address::ipFamilySupported(AF_INET6)) { - subnets.push_back(Network::Address::CidrRange::create("::/0")); + destination_ips_list.push_back(makeCidrListEntry(entry.first, entry.second)); + + // 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_names_entry : *entry.second) { + for (auto& transport_protocols_entry : server_names_entry.second) { + for (auto& application_protocols_entry : transport_protocols_entry.second) { + for (auto& source_array_entry : application_protocols_entry.second) { + auto& source_ips_map = source_array_entry.first; + std::vector< + std::pair>> + source_ips_list; + + for (auto& source_ip : source_ips_map) { + source_ips_list.push_back(makeCidrListEntry(source_ip.first, source_ip.second)); + } + + source_array_entry.second = std::make_unique(source_ips_list, true); + } + } } - } else { - subnets.push_back(Network::Address::CidrRange::create(entry.first)); } - list.push_back( - std::make_pair>( - std::make_shared(entry.second), - std::vector(subnets))); } - destination_ips_pair.second = std::make_unique(list, true); + + destination_ips_pair.second = std::make_unique(destination_ips_list, true); } } @@ -580,34 +665,77 @@ const Network::FilterChain* ListenerImpl::findFilterChainForSourceTypes(const SourceTypesArray& source_types, const Network::ConnectionSocket& socket) const { - auto filter_chain_local = + const auto& filter_chain_local = source_types[envoy::api::v2::listener::FilterChainMatch_ConnectionSourceType:: FilterChainMatch_ConnectionSourceType_LOCAL]; - auto filter_chain_external = + const auto& filter_chain_external = source_types[envoy::api::v2::listener::FilterChainMatch_ConnectionSourceType:: FilterChainMatch_ConnectionSourceType_EXTERNAL]; - // isLocalConnection can be expensive. Call it only if LOCAL or EXTERNAL are defined. - const bool is_local_connection = (filter_chain_local || filter_chain_external) - ? Network::Utility::isLocalConnection(socket) - : false; + // isLocalConnection can be expensive. Call it only if LOCAL or EXTERNAL have entries. + const bool is_local_connection = + (!filter_chain_local.first.empty() || !filter_chain_external.first.empty()) + ? Network::Utility::isLocalConnection(socket) + : false; if (is_local_connection) { - if (filter_chain_local) { - return filter_chain_local.get(); + if (!filter_chain_local.first.empty()) { + return findFilterChainForSourceIpAndPort(*filter_chain_local.second, socket); } } else { - if (filter_chain_external) { - return filter_chain_external.get(); + if (!filter_chain_external.first.empty()) { + return findFilterChainForSourceIpAndPort(*filter_chain_external.second, socket); } } - auto filter_chain_any = + const auto& filter_chain_any = source_types[envoy::api::v2::listener::FilterChainMatch_ConnectionSourceType:: FilterChainMatch_ConnectionSourceType_ANY]; - return filter_chain_any.get(); + if (!filter_chain_any.first.empty()) { + return findFilterChainForSourceIpAndPort(*filter_chain_any.second, socket); + } else { + return nullptr; + } +} + +const Network::FilterChain* +ListenerImpl::findFilterChainForSourceIpAndPort(const SourceIPsTrie& source_ips_trie, + const Network::ConnectionSocket& socket) const { + // Use invalid IP address (matching only filter chains without IP requirements) for UDS. + static const auto& fake_address = Network::Utility::parseInternetAddress("255.255.255.255"); + + auto address = socket.remoteAddress(); + if (address->type() != Network::Address::Type::Ip) { + address = fake_address; + } + + // Match on both: exact IP and wider CIDR ranges using LcTrie. + const auto& data = source_ips_trie.getData(address); + if (!data.empty()) { + ASSERT(data.size() == 1); + const auto& source_ports_map = *data.back(); + const uint32_t source_port = address->ip()->port(); + const auto port_match = source_ports_map.find(source_port); + + // Did we get a direct hit on port. + if (port_match != source_ports_map.end()) { + return port_match->second.get(); + } + + // Try port 0 if we didn't already try it (UDS). + if (source_port != 0) { + const auto any_match = source_ports_map.find(0); + if (any_match != source_ports_map.end()) { + return any_match->second.get(); + } + } + + return nullptr; + } + + return nullptr; } bool ListenerImpl::createNetworkFilterChain( diff --git a/source/server/listener_manager_impl.h b/source/server/listener_manager_impl.h index f90641e6ea10b..24dd7b0baa61a 100644 --- a/source/server/listener_manager_impl.h +++ b/source/server/listener_manager_impl.h @@ -319,19 +319,24 @@ class ListenerImpl : public Network::ListenerConfig, SystemTime last_updated_; private: - typedef std::array SourceTypesArray; - typedef std::unordered_map ApplicationProtocolsMap; - typedef std::unordered_map TransportProtocolsMap; + using SourcePortsMap = absl::flat_hash_map; + using SourcePortsMapSharedPtr = std::shared_ptr; + using SourceIPsMap = absl::flat_hash_map; + using SourceIPsTrie = Network::LcTrie::LcTrie; + using SourceIPsTriePtr = std::unique_ptr; + using SourceTypesArray = std::array, 3>; + using ApplicationProtocolsMap = absl::flat_hash_map; + using TransportProtocolsMap = absl::flat_hash_map; // Both exact server names and wildcard domains are part of the same map, in which wildcard // domains are prefixed with "." (i.e. ".example.com" for "*.example.com") to differentiate // between exact and wildcard entries. - typedef std::unordered_map ServerNamesMap; - typedef std::unordered_map DestinationIPsMap; - typedef std::shared_ptr ServerNamesMapSharedPtr; - typedef Network::LcTrie::LcTrie DestinationIPsTrie; - typedef std::unique_ptr DestinationIPsTriePtr; - typedef std::unordered_map> - DestinationPortsMap; + using ServerNamesMap = absl::flat_hash_map; + using ServerNamesMapSharedPtr = std::shared_ptr; + using DestinationIPsMap = absl::flat_hash_map; + using DestinationIPsTrie = Network::LcTrie::LcTrie; + using DestinationIPsTriePtr = std::unique_ptr; + using DestinationPortsMap = + absl::flat_hash_map>; void addFilterChain(uint16_t destination_port, const std::vector& destination_ips, @@ -339,6 +344,8 @@ class ListenerImpl : public Network::ListenerConfig, const std::string& transport_protocol, const std::vector& application_protocols, const envoy::api::v2::listener::FilterChainMatch_ConnectionSourceType source_type, + const std::vector& source_ips, + const Protobuf::RepeatedField& source_ports, Network::TransportSocketFactoryPtr&& transport_socket_factory, std::vector filters_factory); void addFilterChainForDestinationPorts( @@ -346,29 +353,45 @@ class ListenerImpl : public Network::ListenerConfig, const std::vector& destination_ips, const std::vector& server_names, const std::string& transport_protocol, const std::vector& application_protocols, const envoy::api::v2::listener::FilterChainMatch_ConnectionSourceType source_type, + const std::vector& source_ips, + const Protobuf::RepeatedField& source_ports, const Network::FilterChainSharedPtr& filter_chain); void addFilterChainForDestinationIPs( DestinationIPsMap& destination_ips_map, const std::vector& destination_ips, const std::vector& server_names, const std::string& transport_protocol, const std::vector& application_protocols, const envoy::api::v2::listener::FilterChainMatch_ConnectionSourceType source_type, + const std::vector& source_ips, + const Protobuf::RepeatedField& source_ports, const Network::FilterChainSharedPtr& filter_chain); void addFilterChainForServerNames( - ServerNamesMap& server_names_map, const std::vector& server_names, + ServerNamesMapSharedPtr& server_names_map_ptr, const std::vector& server_names, const std::string& transport_protocol, const std::vector& application_protocols, const envoy::api::v2::listener::FilterChainMatch_ConnectionSourceType source_type, + const std::vector& source_ips, + const Protobuf::RepeatedField& source_ports, const Network::FilterChainSharedPtr& filter_chain); void addFilterChainForApplicationProtocols( ApplicationProtocolsMap& application_protocol_map, const std::vector& application_protocols, const envoy::api::v2::listener::FilterChainMatch_ConnectionSourceType source_type, + const std::vector& source_ips, + const Protobuf::RepeatedField& source_ports, const Network::FilterChainSharedPtr& filter_chain); void addFilterChainForSourceTypes( SourceTypesArray& source_types_array, const envoy::api::v2::listener::FilterChainMatch_ConnectionSourceType source_type, + const std::vector& source_ips, + const Protobuf::RepeatedField& source_ports, const Network::FilterChainSharedPtr& filter_chain); + void addFilterChainForSourceIPs(SourceIPsMap& source_ips_map, const std::string& source_ip, + const Protobuf::RepeatedField& source_ports, + const Network::FilterChainSharedPtr& filter_chain); + void addFilterChainForSourcePorts(SourcePortsMapSharedPtr& source_ports_map_ptr, + uint32_t source_port, + const Network::FilterChainSharedPtr& filter_chain); - void convertDestinationIPsMapToTrie(); + void convertIPsToTries(); const Network::FilterChain* findFilterChainForDestinationIP(const DestinationIPsTrie& destination_ips_trie, @@ -386,6 +409,10 @@ class ListenerImpl : public Network::ListenerConfig, findFilterChainForSourceTypes(const SourceTypesArray& source_types, const Network::ConnectionSocket& socket) const; + const Network::FilterChain* + findFilterChainForSourceIpAndPort(const SourceIPsTrie& source_ips_trie, + const Network::ConnectionSocket& socket) const; + static bool isWildcardServerName(const std::string& name); // Mapping of FilterChain's configured destination ports, IPs, server names, transport protocols diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index 1a316fd387294..f7a6d556f6bc7 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -156,68 +156,32 @@ class ListenerManagerImplWithRealFiltersTest : public ListenerManagerImplTest { } const Network::FilterChain* - findFilterChain(uint16_t destination_port, bool expect_destination_port_match, - const std::string& destination_address, bool expect_destination_address_match, - const std::string& server_name, bool expect_server_name_match, - const std::string& transport_protocol, bool expect_transport_protocol_match, + findFilterChain(uint16_t destination_port, const std::string& destination_address, + const std::string& server_name, const std::string& transport_protocol, const std::vector& application_protocols, - bool expect_application_protocol_match, const std::string& source_address, - bool expect_source_type_test, bool expect_source_type_match) { - int local_addr_calls = expect_destination_port_match ? 2 : 1; + const std::string& source_address, uint16_t source_port) { if (absl::StartsWith(destination_address, "/")) { local_address_.reset(new Network::Address::PipeInstance(destination_address)); } else { - if (expect_source_type_test) { - local_addr_calls += 1; - } local_address_.reset( new Network::Address::Ipv4Instance(destination_address, destination_port)); } - EXPECT_CALL(*socket_, localAddress()) - .Times(local_addr_calls) - .WillRepeatedly(ReturnRef(local_address_)); + ON_CALL(*socket_, localAddress()).WillByDefault(ReturnRef(local_address_)); - if (expect_destination_address_match) { - EXPECT_CALL(*socket_, requestedServerName()).WillOnce(Return(absl::string_view(server_name))); - } else { - EXPECT_CALL(*socket_, requestedServerName()).Times(0); - } + ON_CALL(*socket_, requestedServerName()).WillByDefault(Return(absl::string_view(server_name))); + ON_CALL(*socket_, detectedTransportProtocol()) + .WillByDefault(Return(absl::string_view(transport_protocol))); + ON_CALL(*socket_, requestedApplicationProtocols()) + .WillByDefault(ReturnRef(application_protocols)); - if (expect_server_name_match) { - EXPECT_CALL(*socket_, detectedTransportProtocol()) - .WillOnce(Return(absl::string_view(transport_protocol))); + if (absl::StartsWith(source_address, "/")) { + remote_address_.reset(new Network::Address::PipeInstance(source_address)); } else { - EXPECT_CALL(*socket_, detectedTransportProtocol()).Times(0); + remote_address_.reset(new Network::Address::Ipv4Instance(source_address, source_port)); } + ON_CALL(*socket_, remoteAddress()).WillByDefault(ReturnRef(remote_address_)); - if (expect_transport_protocol_match) { - EXPECT_CALL(*socket_, requestedApplicationProtocols()) - .WillOnce(ReturnRef(application_protocols)); - } else { - EXPECT_CALL(*socket_, requestedApplicationProtocols()).Times(0); - } - - if (expect_application_protocol_match && expect_source_type_test) { - if (absl::StartsWith(source_address, "/")) { - remote_address_.reset(new Network::Address::PipeInstance(source_address)); - } else { - remote_address_.reset(new Network::Address::Ipv4Instance(source_address, 111)); - } - EXPECT_CALL(*socket_, remoteAddress()).Times(1).WillRepeatedly(ReturnRef(remote_address_)); - } else { - EXPECT_CALL(*socket_, remoteAddress()).Times(0); - } - - const Network::FilterChain* result = - manager_->listeners().back().get().filterChainManager().findFilterChain(*socket_); - if (expect_destination_port_match && expect_destination_address_match && - expect_server_name_match && expect_transport_protocol_match && - expect_application_protocol_match && expect_source_type_match) { - EXPECT_NE(result, nullptr); - } else { - EXPECT_EQ(result, nullptr); - } - return result; + return manager_->listeners().back().get().filterChainManager().findFilterChain(*socket_); } /** @@ -381,8 +345,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SslContext) { manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); - auto filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, {}, - true, "8.8.8.8", false, true); + auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "8.8.8.8", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); } @@ -594,7 +557,7 @@ drain_type: modify_only } // Make sure that a listener creation does not fail on IPv4 only setups when FilterChainMatch is not -// specified and we try to create default CidrRange. See convertDestinationIPsMapToTrie function for +// specified and we try to create default CidrRange. See makeCidrListEntry function for // more details. TEST_F(ListenerManagerImplTest, AddListenerOnIpv4OnlySetups) { InSequence s; @@ -615,8 +578,8 @@ drain_type: default ListenerHandle* listener_foo = expectListenerCreate(false); - EXPECT_CALL(os_sys_calls, socket(AF_INET, _, 0)).WillOnce(Return(Api::SysCallIntResult{5, 0})); - EXPECT_CALL(os_sys_calls, socket(AF_INET6, _, 0)).WillOnce(Return(Api::SysCallIntResult{-1, 0})); + ON_CALL(os_sys_calls, socket(AF_INET, _, 0)).WillByDefault(Return(Api::SysCallIntResult{5, 0})); + ON_CALL(os_sys_calls, socket(AF_INET6, _, 0)).WillByDefault(Return(Api::SysCallIntResult{-1, 0})); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); @@ -626,7 +589,7 @@ drain_type: default } // Make sure that a listener creation does not fail on IPv6 only setups when FilterChainMatch is not -// specified and we try to create default CidrRange. See convertDestinationIPsMapToTrie function for +// specified and we try to create default CidrRange. See makeCidrListEntry function for // more details. TEST_F(ListenerManagerImplTest, AddListenerOnIpv6OnlySetups) { InSequence s; @@ -647,8 +610,8 @@ drain_type: default ListenerHandle* listener_foo = expectListenerCreate(false); - EXPECT_CALL(os_sys_calls, socket(AF_INET, _, 0)).WillOnce(Return(Api::SysCallIntResult{-1, 0})); - EXPECT_CALL(os_sys_calls, socket(AF_INET6, _, 0)).WillOnce(Return(Api::SysCallIntResult{5, 0})); + ON_CALL(os_sys_calls, socket(AF_INET, _, 0)).WillByDefault(Return(Api::SysCallIntResult{-1, 0})); + ON_CALL(os_sys_calls, socket(AF_INET6, _, 0)).WillByDefault(Return(Api::SysCallIntResult{5, 0})); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); @@ -1346,13 +1309,11 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithDestinationP EXPECT_EQ(1U, manager_->listeners().size()); // IPv4 client connects to unknown port - no match. - auto filter_chain = findFilterChain(1234, false, "127.0.0.1", false, "", false, "tls", false, {}, - false, "8.8.8.8", false, false); + auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "8.8.8.8", 111); EXPECT_EQ(filter_chain, nullptr); // IPv4 client connects to valid port - using 1st filter chain. - filter_chain = findFilterChain(8080, true, "127.0.0.1", true, "", true, "tls", true, {}, true, - "8.8.8.8", false, true); + filter_chain = findFilterChain(8080, "127.0.0.1", "", "tls", {}, "8.8.8.8", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); auto transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1363,8 +1324,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithDestinationP EXPECT_EQ(server_names.front(), "server1.example.com"); // UDS client - no match. - filter_chain = findFilterChain(0, false, "/tmp/test.sock", false, "", false, "tls", false, {}, - false, "/tmp/test.sock", false, false); + filter_chain = findFilterChain(0, "/tmp/test.sock", "", "tls", {}, "/tmp/test.sock", 111); EXPECT_EQ(filter_chain, nullptr); } @@ -1392,13 +1352,11 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithDestinationI EXPECT_EQ(1U, manager_->listeners().size()); // IPv4 client connects to unknown IP - no match. - auto filter_chain = findFilterChain(1234, true, "1.2.3.4", false, "", false, "tls", false, {}, - false, "8.8.8.8", false, false); + auto filter_chain = findFilterChain(1234, "1.2.3.4", "", "tls", {}, "8.8.8.8", 111); EXPECT_EQ(filter_chain, nullptr); // IPv4 client connects to valid IP - using 1st filter chain. - filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, {}, true, - "8.8.8.8", false, true); + filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "8.8.8.8", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); auto transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1409,8 +1367,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithDestinationI EXPECT_EQ(server_names.front(), "server1.example.com"); // UDS client - no match. - filter_chain = findFilterChain(0, true, "/tmp/test.sock", false, "", false, "tls", false, {}, - false, "/tmp/test.sock", false, false); + filter_chain = findFilterChain(0, "/tmp/test.sock", "", "tls", {}, "/tmp/test.sock", 111); EXPECT_EQ(filter_chain, nullptr); } @@ -1438,18 +1395,16 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithServerNamesM EXPECT_EQ(1U, manager_->listeners().size()); // TLS client without SNI - no match. - auto filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", false, "tls", false, {}, - false, "8.8.8.8", false, false); + auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "8.8.8.8", 111); EXPECT_EQ(filter_chain, nullptr); // TLS client without matching SNI - no match. - filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "www.example.com", false, "tls", - false, {}, false, "8.8.8.8", false, false); + filter_chain = findFilterChain(1234, "127.0.0.1", "www.example.com", "tls", {}, "8.8.8.8", 111); EXPECT_EQ(filter_chain, nullptr); // TLS client with matching SNI - using 1st filter chain. - filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "server1.example.com", true, "tls", - true, {}, true, "8.8.8.8", false, true); + filter_chain = + findFilterChain(1234, "127.0.0.1", "server1.example.com", "tls", {}, "8.8.8.8", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); auto transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1484,13 +1439,11 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithTransportPro EXPECT_EQ(1U, manager_->listeners().size()); // TCP client - no match. - auto filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "raw_buffer", false, - {}, false, "8.8.8.8", false, false); + auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "raw_buffer", {}, "8.8.8.8", 111); EXPECT_EQ(filter_chain, nullptr); // TLS client - using 1st filter chain. - filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, {}, true, - "8.8.8.8", false, true); + filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "8.8.8.8", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); auto transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1526,13 +1479,11 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithApplicationP EXPECT_EQ(1U, manager_->listeners().size()); // TLS client without ALPN - no match. - auto filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, {}, - false, "8.8.8.8", false, false); + auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "8.8.8.8", 111); EXPECT_EQ(filter_chain, nullptr); // TLS client with "http/1.1" ALPN - using 1st filter chain. - filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, - {"h2", "http/1.1"}, true, "8.8.8.8", false, true); + filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {"h2", "http/1.1"}, "8.8.8.8", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); auto transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1568,13 +1519,12 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithSourceTypeMa EXPECT_EQ(1U, manager_->listeners().size()); // EXTERNAL IPv4 client without "http/1.1" ALPN - no match. - auto filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, {}, - true, "8.8.8.8", true, false); + auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "8.8.8.8", 111); EXPECT_EQ(filter_chain, nullptr); // LOCAL IPv4 client with "http/1.1" ALPN - using 1st filter chain. - filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, - {"h2", "http/1.1"}, true, "127.0.0.1", true, true); + filter_chain = + findFilterChain(1234, "127.0.0.1", "", "tls", {"h2", "http/1.1"}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); auto transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1585,8 +1535,8 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithSourceTypeMa EXPECT_EQ(server_names.front(), "server1.example.com"); // LOCAL UDS client with "http/1.1" ALPN - using 1st filter chain. - filter_chain = findFilterChain(0, true, "/tmp/test.sock", true, "", true, "tls", true, - {"h2", "http/1.1"}, true, "/tmp/test.sock", true, true); + filter_chain = + findFilterChain(0, "/tmp/test.sock", "", "tls", {"h2", "http/1.1"}, "/tmp/test.sock", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1596,6 +1546,95 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithSourceTypeMa EXPECT_EQ(server_names.front(), "server1.example.com"); } +// Verify source IP matches. +TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithSourceIpMatch) { + const std::string yaml = TestEnvironment::substitute(R"EOF( + address: + socket_address: { address: 127.0.0.1, port_value: 1234 } + listener_filters: + - name: "envoy.listener.tls_inspector" + config: {} + filter_chains: + - filter_chain_match: + source_prefix_ranges: + - address_prefix: 10.0.0.1 + prefix_len: 24 + tls_context: + common_tls_context: + tls_certificates: + - certificate_chain: { filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns_cert.pem" } + private_key: { filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns_key.pem" } + )EOF", + Network::Address::IpVersion::v4); + + EXPECT_CALL(server_.random_, uuid()); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); + manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); + EXPECT_EQ(1U, manager_->listeners().size()); + + // IPv4 client with source 10.0.1.1. No match. + auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "10.0.1.1", 111); + EXPECT_EQ(filter_chain, nullptr); + + // IPv4 client with source 10.0.0.10, Match + filter_chain = + findFilterChain(1234, "127.0.0.1", "", "tls", {"h2", "http/1.1"}, "10.0.0.10", 111); + ASSERT_NE(filter_chain, nullptr); + EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); + auto transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); + auto ssl_socket = + dynamic_cast(transport_socket.get()); + auto server_names = ssl_socket->dnsSansLocalCertificate(); + EXPECT_EQ(server_names.size(), 1); + EXPECT_EQ(server_names.front(), "server1.example.com"); + + // UDS client. No match. + filter_chain = + findFilterChain(0, "/tmp/test.sock", "", "tls", {"h2", "http/1.1"}, "/tmp/test.sock", 0); + ASSERT_EQ(filter_chain, nullptr); +} + +// Verify source port matches. +TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithSourcePortMatch) { + const std::string yaml = TestEnvironment::substitute(R"EOF( + address: + socket_address: { address: 127.0.0.1, port_value: 1234 } + listener_filters: + - name: "envoy.listener.tls_inspector" + config: {} + filter_chains: + - filter_chain_match: + source_ports: + - 100 + tls_context: + common_tls_context: + tls_certificates: + - certificate_chain: { filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns_cert.pem" } + private_key: { filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns_key.pem" } + )EOF", + Network::Address::IpVersion::v4); + + EXPECT_CALL(server_.random_, uuid()); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); + manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); + EXPECT_EQ(1U, manager_->listeners().size()); + + // Client with source port 100. Match. + auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "127.0.0.1", 100); + ASSERT_NE(filter_chain, nullptr); + EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); + auto transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); + auto ssl_socket = + dynamic_cast(transport_socket.get()); + auto server_names = ssl_socket->dnsSansLocalCertificate(); + EXPECT_EQ(server_names.size(), 1); + EXPECT_EQ(server_names.front(), "server1.example.com"); + + // Client with source port 101. No match. + filter_chain = findFilterChain(1234, "8.8.8.8", "", "tls", {"h2", "http/1.1"}, "4.4.4.4", 101); + ASSERT_EQ(filter_chain, nullptr); +} + // Define multiple source_type filter chain matches and test against them. TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainWithSourceTypeMatch) { const std::string yaml = TestEnvironment::substitute(R"EOF( @@ -1636,13 +1675,12 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainWithSourceType EXPECT_EQ(1U, manager_->listeners().size()); // LOCAL TLS client with "http/1.1" ALPN - no match. - auto filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, - {"h2", "http/1.1"}, true, "127.0.0.1", true, false); + auto filter_chain = + findFilterChain(1234, "127.0.0.1", "", "tls", {"h2", "http/1.1"}, "127.0.0.1", 111); EXPECT_EQ(filter_chain, nullptr); // LOCAL TLS client without "http/1.1" ALPN - using 1st filter chain. - filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, {}, true, - "127.0.0.1", true, true); + filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); auto transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1653,8 +1691,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainWithSourceType EXPECT_EQ(server_names.front(), "server1.example.com"); // EXTERNAL TLS client with "http/1.1" ALPN - using 2nd filter chain. - filter_chain = findFilterChain(1234, true, "8.8.8.8", true, "", true, "tls", true, - {"h2", "http/1.1"}, true, "4.4.4.4", true, true); + filter_chain = findFilterChain(1234, "8.8.8.8", "", "tls", {"h2", "http/1.1"}, "4.4.4.4", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1663,8 +1700,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainWithSourceType EXPECT_EQ(uri[0], "spiffe://lyft.com/test-team"); // EXTERNAL TLS client without "http/1.1" ALPN - using 3nd filter chain. - filter_chain = findFilterChain(1234, true, "8.8.8.8", true, "", true, "tls", true, {}, true, - "4.4.4.4", true, true); + filter_chain = findFilterChain(1234, "8.8.8.8", "", "tls", {}, "4.4.4.4", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1712,8 +1748,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithDestinati EXPECT_EQ(1U, manager_->listeners().size()); // IPv4 client connects to default port - using 1st filter chain. - auto filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, {}, - true, "127.0.0.1", false, true); + auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); auto transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1723,8 +1758,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithDestinati EXPECT_EQ(uri[0], "spiffe://lyft.com/test-team"); // IPv4 client connects to port 8080 - using 2nd filter chain. - filter_chain = findFilterChain(8080, true, "127.0.0.1", true, "", true, "tls", true, {}, true, - "127.0.0.1", false, true); + filter_chain = findFilterChain(8080, "127.0.0.1", "", "tls", {}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1734,8 +1768,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithDestinati EXPECT_EQ(server_names.front(), "server1.example.com"); // IPv4 client connects to port 8081 - using 3nd filter chain. - filter_chain = findFilterChain(8081, true, "127.0.0.1", true, "", true, "tls", true, {}, true, - "127.0.0.1", false, true); + filter_chain = findFilterChain(8081, "127.0.0.1", "", "tls", {}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1745,8 +1778,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithDestinati EXPECT_EQ(server_names.front(), "*.example.com"); // UDS client - using 1st filter chain. - filter_chain = findFilterChain(0, true, "/tmp/test.sock", true, "", true, "tls", true, {}, true, - "127.0.0.1", false, true); + filter_chain = findFilterChain(0, "/tmp/test.sock", "", "tls", {}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1793,8 +1825,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithDestinati EXPECT_EQ(1U, manager_->listeners().size()); // IPv4 client connects to default IP - using 1st filter chain. - auto filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, {}, - true, "127.0.0.1", false, true); + auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); auto transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1804,8 +1835,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithDestinati EXPECT_EQ(uri[0], "spiffe://lyft.com/test-team"); // IPv4 client connects to exact IP match - using 2nd filter chain. - filter_chain = findFilterChain(1234, true, "192.168.0.1", true, "", true, "tls", true, {}, true, - "127.0.0.1", false, true); + filter_chain = findFilterChain(1234, "192.168.0.1", "", "tls", {}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1815,8 +1845,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithDestinati EXPECT_EQ(server_names.front(), "server1.example.com"); // IPv4 client connects to wildcard IP match - using 3nd filter chain. - filter_chain = findFilterChain(1234, true, "192.168.1.1", true, "", true, "tls", true, {}, true, - "192.168.1.1", false, true); + filter_chain = findFilterChain(1234, "192.168.1.1", "", "tls", {}, "192.168.1.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1826,8 +1855,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithDestinati EXPECT_EQ(server_names.front(), "*.example.com"); // UDS client - using 1st filter chain. - filter_chain = findFilterChain(0, true, "/tmp/test.sock", true, "", true, "tls", true, {}, true, - "/tmp/test.sock", false, true); + filter_chain = findFilterChain(0, "/tmp/test.sock", "", "tls", {}, "/tmp/test.sock", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1883,8 +1911,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithServerNam EXPECT_EQ(1U, manager_->listeners().size()); // TLS client without SNI - using 1st filter chain. - auto filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, {}, - true, "127.0.0.1", false, true); + auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); auto transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1894,8 +1921,8 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithServerNam EXPECT_EQ(uri[0], "spiffe://lyft.com/test-team"); // TLS client with exact SNI match - using 2nd filter chain. - filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "server1.example.com", true, "tls", - true, {}, true, "127.0.0.1", false, true); + filter_chain = + findFilterChain(1234, "127.0.0.1", "server1.example.com", "tls", {}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1905,8 +1932,8 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithServerNam EXPECT_EQ(server_names.front(), "server1.example.com"); // TLS client with wildcard SNI match - using 3nd filter chain. - filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "server2.example.com", true, "tls", - true, {}, true, "127.0.0.1", false, true); + filter_chain = + findFilterChain(1234, "127.0.0.1", "server2.example.com", "tls", {}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1916,8 +1943,8 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithServerNam EXPECT_EQ(server_names.front(), "*.example.com"); // TLS client with wildcard SNI match - using 3nd filter chain. - filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "www.wildcard.com", true, "tls", - true, {}, true, "127.0.0.1", false, true); + filter_chain = + findFilterChain(1234, "127.0.0.1", "www.wildcard.com", "tls", {}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1953,14 +1980,12 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithTransport EXPECT_EQ(1U, manager_->listeners().size()); // TCP client - using 1st filter chain. - auto filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "raw_buffer", true, - {}, true, "127.0.0.1", false, true); + auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "raw_buffer", {}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_FALSE(filter_chain->transportSocketFactory().implementsSecureTransport()); // TLS client - using 2nd filter chain. - filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, {}, true, - "127.0.0.1", false, true); + filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); auto transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -1997,14 +2022,13 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithApplicati EXPECT_EQ(1U, manager_->listeners().size()); // TLS client without ALPN - using 1st filter chain. - auto filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, {}, - true, "127.0.0.1", false, true); + auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_FALSE(filter_chain->transportSocketFactory().implementsSecureTransport()); // TLS client with "h2,http/1.1" ALPN - using 2nd filter chain. - filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, - {"h2", "http/1.1"}, true, "127.0.0.1", false, true); + filter_chain = + findFilterChain(1234, "127.0.0.1", "", "tls", {"h2", "http/1.1"}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); auto transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -2043,25 +2067,24 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithMultipleR EXPECT_EQ(1U, manager_->listeners().size()); // TLS client without SNI and ALPN - using 1st filter chain. - auto filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, {}, - true, "127.0.0.1", false, true); + auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_FALSE(filter_chain->transportSocketFactory().implementsSecureTransport()); // TLS client with exact SNI match but without ALPN - no match (SNI blackholed by configuration). - filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "server1.example.com", true, "tls", - true, {}, false, "127.0.0.1", false, false); + filter_chain = + findFilterChain(1234, "127.0.0.1", "server1.example.com", "tls", {}, "127.0.0.1", 111); EXPECT_EQ(filter_chain, nullptr); // TLS client with ALPN match but without SNI - using 1st filter chain. - filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "", true, "tls", true, - {"h2", "http/1.1"}, true, "127.0.0.1", false, true); + filter_chain = + findFilterChain(1234, "127.0.0.1", "", "tls", {"h2", "http/1.1"}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_FALSE(filter_chain->transportSocketFactory().implementsSecureTransport()); // TLS client with exact SNI match and ALPN match - using 2nd filter chain. - filter_chain = findFilterChain(1234, true, "127.0.0.1", true, "server1.example.com", true, "tls", - true, {"h2", "http/1.1"}, true, "127.0.0.1", false, true); + filter_chain = findFilterChain(1234, "127.0.0.1", "server1.example.com", "tls", + {"h2", "http/1.1"}, "127.0.0.1", 111); ASSERT_NE(filter_chain, nullptr); EXPECT_TRUE(filter_chain->transportSocketFactory().implementsSecureTransport()); auto transport_socket = filter_chain->transportSocketFactory().createTransportSocket(nullptr); @@ -2201,29 +2224,6 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithSameMatch "the same matching rules are defined"); } -TEST_F(ListenerManagerImplWithRealFiltersTest, - MultipleFilterChainsWithSameMatchPlusUnimplementedFields) { - const std::string yaml = TestEnvironment::substitute(R"EOF( - address: - socket_address: { address: 127.0.0.1, port_value: 1234 } - listener_filters: - - name: "envoy.listener.tls_inspector" - config: {} - filter_chains: - - filter_chain_match: - transport_protocol: "tls" - - filter_chain_match: - transport_protocol: "tls" - source_prefix_ranges: { address_prefix: 127.0.0.0, prefix_len: 8 } - )EOF", - Network::Address::IpVersion::v4); - - EXPECT_THROW_WITH_MESSAGE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true), - EnvoyException, - "error adding listener '127.0.0.1:1234': contains filter chains with " - "unimplemented fields"); -} - TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithOverlappingRules) { const std::string yaml = TestEnvironment::substitute(R"EOF( address: From f8af6d74ace4aa8b5e0bd2526943339b7f8012b9 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Fri, 24 May 2019 08:42:17 -0600 Subject: [PATCH 2/6] comments Signed-off-by: Matt Klein --- source/server/listener_manager_impl.cc | 47 ++++++++++++++------------ 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index 4fd9726ff005e..cf6a8b9074e59 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -248,6 +248,7 @@ ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, const std::st // Validate IP addresses. std::vector destination_ips; + destination_ips.reserve(filter_chain_match.prefix_ranges().size()); for (const auto& destination_ip : filter_chain_match.prefix_ranges()) { const auto& cidr_range = Network::Address::CidrRange::create(destination_ip); destination_ips.push_back(cidr_range.asString()); @@ -267,6 +268,7 @@ ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, const std::st } std::vector source_ips; + source_ips.reserve(filter_chain_match.source_prefix_ranges().size()); for (const auto& source_ip : filter_chain_match.source_prefix_ranges()) { const auto& cidr_range = Network::Address::CidrRange::create(source_ip); source_ips.push_back(cidr_range.asString()); @@ -477,7 +479,7 @@ void ListenerImpl::addFilterChainForSourcePorts(SourcePortsMapSharedPtr& source_ } auto& source_ports_map = *source_ports_map_ptr; - if (source_ports_map.find(source_port) != source_ports_map.end()) { + if (!source_ports_map.try_emplace(source_port, filter_chain).second) { // If we got here and found already configured branch, then it means that this FilterChainMatch // is a duplicate, and that there is some overlap in the repeated fields with already processed // FilterChainMatches. @@ -485,7 +487,6 @@ void ListenerImpl::addFilterChainForSourcePorts(SourcePortsMapSharedPtr& source_ "overlapping matching rules are defined", address_->asString())); } - source_ports_map[source_port] = filter_chain; } namespace { @@ -497,10 +498,12 @@ std::pair> makeCidrListEntry(const s std::vector subnets; if (cidr == EMPTY_STRING) { if (Network::Address::ipFamilySupported(AF_INET)) { - subnets.push_back(Network::Address::CidrRange::create("0.0.0.0/0")); + static const std::string catch_all_ipv4 = "0.0.0.0/0"; + subnets.push_back(Network::Address::CidrRange::create(catch_all_ipv4)); } if (Network::Address::ipFamilySupported(AF_INET6)) { - subnets.push_back(Network::Address::CidrRange::create("::/0")); + static const std::string catch_all_ipv6 = "::/0"; + subnets.push_back(Network::Address::CidrRange::create(catch_all_ipv6)); } } else { subnets.push_back(Network::Address::CidrRange::create(cidr)); @@ -517,6 +520,7 @@ void ListenerImpl::convertIPsToTries() { auto& destination_ips_map = destination_ips_pair.first; std::vector>> destination_ips_list; + destination_ips_list.reserve(destination_ips_map.size()); for (const auto& entry : destination_ips_map) { destination_ips_list.push_back(makeCidrListEntry(entry.first, entry.second)); @@ -532,6 +536,7 @@ void ListenerImpl::convertIPsToTries() { std::vector< std::pair>> source_ips_list; + source_ips_list.reserve(source_ips_map.size()); for (auto& source_ip : source_ips_map) { source_ips_list.push_back(makeCidrListEntry(source_ip.first, source_ip.second)); @@ -713,26 +718,26 @@ ListenerImpl::findFilterChainForSourceIpAndPort(const SourceIPsTrie& source_ips_ // Match on both: exact IP and wider CIDR ranges using LcTrie. const auto& data = source_ips_trie.getData(address); - if (!data.empty()) { - ASSERT(data.size() == 1); - const auto& source_ports_map = *data.back(); - const uint32_t source_port = address->ip()->port(); - const auto port_match = source_ports_map.find(source_port); + if (data.empty()) { + return nullptr; + } - // Did we get a direct hit on port. - if (port_match != source_ports_map.end()) { - return port_match->second.get(); - } + ASSERT(data.size() == 1); + const auto& source_ports_map = *data.back(); + const uint32_t source_port = address->ip()->port(); + const auto port_match = source_ports_map.find(source_port); - // Try port 0 if we didn't already try it (UDS). - if (source_port != 0) { - const auto any_match = source_ports_map.find(0); - if (any_match != source_ports_map.end()) { - return any_match->second.get(); - } - } + // Did we get a direct hit on port. + if (port_match != source_ports_map.end()) { + return port_match->second.get(); + } - return nullptr; + // Try port 0 if we didn't already try it (UDS). + if (source_port != 0) { + const auto any_match = source_ports_map.find(0); + if (any_match != source_ports_map.end()) { + return any_match->second.get(); + } } return nullptr; From f8af8335d1ccdff9115c465d43350437c9e4eade Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Tue, 28 May 2019 21:02:58 -0600 Subject: [PATCH 3/6] IPv6 test Signed-off-by: Matt Klein --- test/server/listener_manager_impl_test.cc | 48 +++++++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index f7a6d556f6bc7..649d25548ec52 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -163,8 +163,8 @@ class ListenerManagerImplWithRealFiltersTest : public ListenerManagerImplTest { if (absl::StartsWith(destination_address, "/")) { local_address_.reset(new Network::Address::PipeInstance(destination_address)); } else { - local_address_.reset( - new Network::Address::Ipv4Instance(destination_address, destination_port)); + local_address_ = + Network::Utility::parseInternetAddress(destination_address, destination_port); } ON_CALL(*socket_, localAddress()).WillByDefault(ReturnRef(local_address_)); @@ -177,7 +177,7 @@ class ListenerManagerImplWithRealFiltersTest : public ListenerManagerImplTest { if (absl::StartsWith(source_address, "/")) { remote_address_.reset(new Network::Address::PipeInstance(source_address)); } else { - remote_address_.reset(new Network::Address::Ipv4Instance(source_address, source_port)); + remote_address_ = Network::Utility::parseInternetAddress(source_address, source_port); } ON_CALL(*socket_, remoteAddress()).WillByDefault(ReturnRef(remote_address_)); @@ -1588,12 +1588,54 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithSourceIpMatc EXPECT_EQ(server_names.size(), 1); EXPECT_EQ(server_names.front(), "server1.example.com"); + // IPv6 client. No match. + filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, + "2001:0db8:85a3:0000:0000:8a2e:0370:7334", 111); + EXPECT_EQ(filter_chain, nullptr); + // UDS client. No match. filter_chain = findFilterChain(0, "/tmp/test.sock", "", "tls", {"h2", "http/1.1"}, "/tmp/test.sock", 0); ASSERT_EQ(filter_chain, nullptr); } +// Verify source IPv6 matches. +TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithSourceIpv6Match) { + const std::string yaml = TestEnvironment::substitute(R"EOF( + address: + socket_address: { address: 127.0.0.1, port_value: 1234 } + listener_filters: + - name: "envoy.listener.tls_inspector" + config: {} + filter_chains: + - filter_chain_match: + source_prefix_ranges: + - address_prefix: 2001:0db8:85a3:0000:0000:0000:0000:0000 + prefix_len: 64 + tls_context: + common_tls_context: + tls_certificates: + - certificate_chain: { filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns_cert.pem" } + private_key: { filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns_key.pem" } + )EOF", + Network::Address::IpVersion::v4); + + EXPECT_CALL(server_.random_, uuid()); + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); + manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); + EXPECT_EQ(1U, manager_->listeners().size()); + + // IPv6 client. match. + auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, + "2001:0db8:85a3:0000:0000:8a2e:0370:7334", 111); + EXPECT_NE(filter_chain, nullptr); + + // IPv6 client. No match. + filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, + "2001:0db8:85a3:0001:0000:8a2e:0370:7334", 111); + EXPECT_EQ(filter_chain, nullptr); +} + // Verify source port matches. TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithSourcePortMatch) { const std::string yaml = TestEnvironment::substitute(R"EOF( From 73cdf303bedacc5aa24d18f6bd903d2cd85801f4 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Wed, 29 May 2019 07:58:51 -0600 Subject: [PATCH 4/6] comment Signed-off-by: Matt Klein --- source/common/network/utility.cc | 8 +++++++ source/common/network/utility.h | 10 +++++++++ source/server/listener_manager_impl.cc | 29 +++++++++++++++----------- source/server/server.cc | 6 +++--- 4 files changed, 38 insertions(+), 15 deletions(-) diff --git a/source/common/network/utility.cc b/source/common/network/utility.cc index e7f7039e9a405..935ba8dda9b72 100644 --- a/source/common/network/utility.cc +++ b/source/common/network/utility.cc @@ -355,6 +355,14 @@ Address::InstanceConstSharedPtr Utility::getIpv6AnyAddress() { return any; } +const std::string& Utility::getIpv4CidrCatchAllAddress() { + CONSTRUCT_ON_FIRST_USE(std::string, "0.0.0.0/0"); +} + +const std::string& Utility::getIpv6CidrCatchAllAddress() { + CONSTRUCT_ON_FIRST_USE(std::string, "::/0"); +} + Address::InstanceConstSharedPtr Utility::getAddressWithPort(const Address::Instance& address, uint32_t port) { switch (address.ip()->version()) { diff --git a/source/common/network/utility.h b/source/common/network/utility.h index 68f56d7ec5491..2eceaaf7f24a1 100644 --- a/source/common/network/utility.h +++ b/source/common/network/utility.h @@ -179,6 +179,16 @@ class Utility { */ static Address::InstanceConstSharedPtr getIpv6AnyAddress(); + /** + * @return the IPv4 CIDR catch-all address (0.0.0.0/0). + */ + static const std::string& getIpv4CidrCatchAllAddress(); + + /** + * @return the IPv6 CIDR catch-all address (::/0). + */ + static const std::string& getIpv6CidrCatchAllAddress(); + /** * @param address IP address instance. * @param port to update. diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index cf6a8b9074e59..91ccffdf625a9 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -498,12 +498,12 @@ std::pair> makeCidrListEntry(const s std::vector subnets; if (cidr == EMPTY_STRING) { if (Network::Address::ipFamilySupported(AF_INET)) { - static const std::string catch_all_ipv4 = "0.0.0.0/0"; - subnets.push_back(Network::Address::CidrRange::create(catch_all_ipv4)); + subnets.push_back( + Network::Address::CidrRange::create(Network::Utility::getIpv4CidrCatchAllAddress())); } if (Network::Address::ipFamilySupported(AF_INET6)) { - static const std::string catch_all_ipv6 = "::/0"; - subnets.push_back(Network::Address::CidrRange::create(catch_all_ipv6)); + subnets.push_back( + Network::Address::CidrRange::create(Network::Utility::getIpv6CidrCatchAllAddress())); } } else { subnets.push_back(Network::Address::CidrRange::create(cidr)); @@ -574,15 +574,23 @@ ListenerImpl::findFilterChain(const Network::ConnectionSocket& socket) const { return nullptr; } +namespace { + +// Return a fake address for use when either the source or destination is UDS. +Network::Address::InstanceConstSharedPtr fakeAddress() { + static const Network::Address::InstanceConstSharedPtr address = + Network::Utility::parseInternetAddress("255.255.255.255"); + return address; +} + +} // namespace + const Network::FilterChain* ListenerImpl::findFilterChainForDestinationIP(const DestinationIPsTrie& destination_ips_trie, const Network::ConnectionSocket& socket) const { - // Use invalid IP address (matching only filter chains without IP requirements) for UDS. - static const auto& fake_address = Network::Utility::parseInternetAddress("255.255.255.255"); - auto address = socket.localAddress(); if (address->type() != Network::Address::Type::Ip) { - address = fake_address; + address = fakeAddress(); } // Match on both: exact IP and wider CIDR ranges using LcTrie. @@ -708,12 +716,9 @@ ListenerImpl::findFilterChainForSourceTypes(const SourceTypesArray& source_types const Network::FilterChain* ListenerImpl::findFilterChainForSourceIpAndPort(const SourceIPsTrie& source_ips_trie, const Network::ConnectionSocket& socket) const { - // Use invalid IP address (matching only filter chains without IP requirements) for UDS. - static const auto& fake_address = Network::Utility::parseInternetAddress("255.255.255.255"); - auto address = socket.remoteAddress(); if (address->type() != Network::Address::Type::Ip) { - address = fake_address; + address = fakeAddress(); } // Match on both: exact IP and wider CIDR ranges using LcTrie. diff --git a/source/server/server.cc b/source/server/server.cc index 70a763e558a98..15d41ccc07bfe 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -594,15 +594,15 @@ void InstanceImpl::shutdownAdmin() { ServerLifecycleNotifier::HandlePtr InstanceImpl::registerCallback(Stage stage, StageCallback callback) { auto& callbacks = stage_callbacks_[stage]; - return absl::make_unique>(callbacks, callback); + return std::make_unique>(callbacks, callback); } ServerLifecycleNotifier::HandlePtr InstanceImpl::registerCallback(Stage stage, StageCallbackWithCompletion callback) { ASSERT(stage == Stage::ShutdownExit); auto& callbacks = stage_completable_callbacks_[stage]; - return absl::make_unique>(callbacks, - callback); + return std::make_unique>(callbacks, + callback); } void InstanceImpl::notifyCallbacksForStage(Stage stage, Event::PostCb completion_cb) { From 764d8008211db3230146487877c443fd3ca7a2a9 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Thu, 30 May 2019 18:21:29 -0600 Subject: [PATCH 5/6] comment Signed-off-by: Matt Klein --- source/common/network/utility.cc | 19 +++++++------------ source/server/listener_manager_impl.cc | 5 ++--- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/source/common/network/utility.cc b/source/common/network/utility.cc index 935ba8dda9b72..00bea45974cf6 100644 --- a/source/common/network/utility.cc +++ b/source/common/network/utility.cc @@ -332,27 +332,22 @@ bool Utility::isLoopbackAddress(const Address::Instance& address) { } Address::InstanceConstSharedPtr Utility::getCanonicalIpv4LoopbackAddress() { - // Initialized on first call in a thread-safe manner. - static Address::InstanceConstSharedPtr loopback(new Address::Ipv4Instance("127.0.0.1", 0)); - return loopback; + CONSTRUCT_ON_FIRST_USE(Address::InstanceConstSharedPtr, + new Address::Ipv4Instance("127.0.0.1", 0)); } Address::InstanceConstSharedPtr Utility::getIpv6LoopbackAddress() { - // Initialized on first call in a thread-safe manner. - static Address::InstanceConstSharedPtr loopback(new Address::Ipv6Instance("::1", 0)); - return loopback; + CONSTRUCT_ON_FIRST_USE(Address::InstanceConstSharedPtr, new Address::Ipv6Instance("::1", 0)); } Address::InstanceConstSharedPtr Utility::getIpv4AnyAddress() { - // Initialized on first call in a thread-safe manner. - static Address::InstanceConstSharedPtr any(new Address::Ipv4Instance(static_cast(0))); - return any; + CONSTRUCT_ON_FIRST_USE(Address::InstanceConstSharedPtr, + new Address::Ipv4Instance(static_cast(0))); } Address::InstanceConstSharedPtr Utility::getIpv6AnyAddress() { - // Initialized on first call in a thread-safe manner. - static Address::InstanceConstSharedPtr any(new Address::Ipv6Instance(static_cast(0))); - return any; + CONSTRUCT_ON_FIRST_USE(Address::InstanceConstSharedPtr, + new Address::Ipv6Instance(static_cast(0))); } const std::string& Utility::getIpv4CidrCatchAllAddress() { diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index 6c6da9cac9703..29f3e4f12c76b 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -627,9 +627,8 @@ namespace { // Return a fake address for use when either the source or destination is UDS. Network::Address::InstanceConstSharedPtr fakeAddress() { - static const Network::Address::InstanceConstSharedPtr address = - Network::Utility::parseInternetAddress("255.255.255.255"); - return address; + CONSTRUCT_ON_FIRST_USE(Network::Address::InstanceConstSharedPtr, + Network::Utility::parseInternetAddress("255.255.255.255")); } } // namespace From 1c8a3790601d991a31b3dfe7b2e677a3165b99c7 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Fri, 31 May 2019 08:13:08 -0600 Subject: [PATCH 6/6] comments Signed-off-by: Matt Klein --- source/server/listener_manager_impl.cc | 5 ++++ test/server/listener_manager_impl_test.cc | 29 ++++++++++++++++++++--- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index 29f3e4f12c76b..460c8bc7551b2 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -269,6 +269,11 @@ ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, const std::st for (const auto& filter_chain : config.filter_chains()) { const auto& filter_chain_match = filter_chain.filter_chain_match(); + if (!filter_chain_match.address_suffix().empty() || filter_chain_match.has_suffix_len()) { + throw EnvoyException(fmt::format("error adding listener '{}': contains filter chains with " + "unimplemented fields", + address_->asString())); + } if (filter_chains.find(filter_chain_match) != filter_chains.end()) { throw EnvoyException(fmt::format("error adding listener '{}': multiple filter chains with " "the same matching rules are defined", diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index e7693a97c6090..7ce9a38fb3690 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -1613,7 +1613,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithSourceIpMatc auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "10.0.1.1", 111); EXPECT_EQ(filter_chain, nullptr); - // IPv4 client with source 10.0.0.10, Match + // IPv4 client with source 10.0.0.10, Match. filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {"h2", "http/1.1"}, "10.0.0.10", 111); ASSERT_NE(filter_chain, nullptr); @@ -1662,12 +1662,12 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SingleFilterChainWithSourceIpv6Ma manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true); EXPECT_EQ(1U, manager_->listeners().size()); - // IPv6 client. match. + // IPv6 client with matching subnet. Match. auto filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "2001:0db8:85a3:0000:0000:8a2e:0370:7334", 111); EXPECT_NE(filter_chain, nullptr); - // IPv6 client. No match. + // IPv6 client with non-matching subnet. No match. filter_chain = findFilterChain(1234, "127.0.0.1", "", "tls", {}, "2001:0db8:85a3:0001:0000:8a2e:0370:7334", 111); EXPECT_EQ(filter_chain, nullptr); @@ -2303,6 +2303,29 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithSameMatch "the same matching rules are defined"); } +TEST_F(ListenerManagerImplWithRealFiltersTest, + MultipleFilterChainsWithSameMatchPlusUnimplementedFields) { + const std::string yaml = TestEnvironment::substitute(R"EOF( + address: + socket_address: { address: 127.0.0.1, port_value: 1234 } + listener_filters: + - name: "envoy.listener.tls_inspector" + config: {} + filter_chains: + - filter_chain_match: + transport_protocol: "tls" + - filter_chain_match: + transport_protocol: "tls" + address_suffix: 127.0.0.0 + )EOF", + Network::Address::IpVersion::v4); + + EXPECT_THROW_WITH_MESSAGE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true), + EnvoyException, + "error adding listener '127.0.0.1:1234': contains filter chains with " + "unimplemented fields"); +} + TEST_F(ListenerManagerImplWithRealFiltersTest, MultipleFilterChainsWithOverlappingRules) { const std::string yaml = TestEnvironment::substitute(R"EOF( address: