Skip to content

filter chain match: support source CIDRs and ports #7064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattklein123 merged 12 commits into from
May 31, 2019
Merged

filter chain match: support source CIDRs and ports #7064

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
43744eb
filter chain match: support source CIDRs and ports
mattklein123 May 23, 2019
5c29732
Merge remote-tracking branch 'origin/master' into source_ip_match
mattklein123 May 24, 2019
f8af6d7
comments
mattklein123 May 24, 2019
3b0c106
Merge remote-tracking branch 'origin/master' into source_ip_match
mattklein123 May 29, 2019
f8af833
IPv6 test
mattklein123 May 29, 2019
f57bba3
Merge remote-tracking branch 'origin/master' into source_ip_match
mattklein123 May 29, 2019
73cdf30
comment
mattklein123 May 29, 2019
2460514
Merge remote-tracking branch 'origin/master' into source_ip_match
mattklein123 May 29, 2019
e4a6b29
Merge remote-tracking branch 'origin/master' into source_ip_match
mattklein123 May 31, 2019
764d800
comment
mattklein123 May 31, 2019
aea58fe
Merge remote-tracking branch 'origin/master' into source_ip_match
mattklein123 May 31, 2019
1c8a379
comments
mattklein123 May 31, 2019
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
6 changes: 3 additions & 3 deletions api/envoy/api/v2/listener/listener.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Copy Markdown
Member

@htuch htuch May 28, 2019

Choose a reason for hiding this comment

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

Can you comment on the rationale behind the ordering here? Is it arbitrary? Is there a practical reason to select source IP/port as the LSBs?

Copy link
Copy Markdown
Member Author

@mattklein123 mattklein123 May 29, 2019

Choose a reason for hiding this comment

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

I talked to @PiotrSikora about this offline. Effectively, it's arbitrary, and since we are adding new ones it seems best to add them at the end? IMO this needs to be configurable but that's out of scope for this change. @PiotrSikora any additional thoughts here?

Copy link
Copy Markdown
Contributor

@PiotrSikora PiotrSikora May 31, 2019

Choose a reason for hiding this comment

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

It's not really arbitrary. The order is trying to more and more precisely describe destination (what service are you connecting to) with source selectors added at the end for the "split horizon".

This order allows filter chains to effectively act as virtual servers with fancy selectors (transport protocols, application protocols, etc.), while still being able to configure "split horizon" on a per virtual server basis for those that need it.

If source selectors had priority, then "split horizon" would be at the top of the decision tree, requiring all virtual servers to be configured for each source.

The only thing that I'm not 100% sure about is the order between source IPs and ports... Thinking about this now, perhaps it should be reversed (source port with higher priority) to allow matching connections from port X from any IP, but I honestly cannot think of a "real" use case for source port matching anyway.

There is an open issue for making the order configurable (#3411), but in the year+ that this order has been in place, I don't recall anyone complaining that the existing order doesn't work for their use case.

Copy link
Copy Markdown
Member Author

@mattklein123 mattklein123 May 31, 2019

Choose a reason for hiding this comment

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

Yeah I don't have a strong opinion on the order of IP vs. port. Perhaps ship and iterate? I'm fine either way.

Copy link
Copy Markdown
Member

@htuch htuch May 31, 2019

Choose a reason for hiding this comment

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

Thanks for the explanation @PiotrSikora. I wonder if we should capture this somewhere, it's useful design rationale.

// 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
Expand Down Expand Up @@ -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
Expand Down
61 changes: 4 additions & 57 deletions api/envoy/config/filter/network/tcp_proxy/v2/tcp_proxy.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
// <envoy_api_field_config.filter.network.tcp_proxy.v2.TcpProxy.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
// <envoy_api_field_config.filter.network.tcp_proxy.v2.TcpProxy.deprecated_v1>`
// configuration is set.
WeightedCluster weighted_clusters = 10;
}

Expand Down Expand Up @@ -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 <envoy_api_msg_listener.FilterChainMatch>`).
// [#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
Expand Down Expand Up @@ -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<source/common/json/config_schemas.cc>`.
//
// .. 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.
Expand Down
3 changes: 3 additions & 0 deletions docs/root/intro/deprecated.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <envoy_api_field_config.filter.network.redis_proxy.v2.RedisProxy.cluster>` in :ref:`redis_proxy.proto <envoy_api_file_envoy/config/filter/network/redis_proxy/v2/redis_proxy.proto>` is deprecated. Set a :ref:`catch_all_cluster <envoy_api_field_config.filter.network.redis_proxy.v2.RedisProxy.PrefixRoutes.catch_all_cluster>` instead.
* Use of json based schema in router check tool tests. The tests should follow validation :repo:`schema<test/tools/router_check/validation.proto>`.
* Use of the v1 style route configuration for the :ref:`TCP proxy filter <config_network_filters_tcp_proxy>`
is now fully replaced with listener :ref:`filter chain matching <envoy_api_msg_listener.FilterChainMatch>`.
Use this instead.
* Use of :ref:`runtime <envoy_api_field_config.bootstrap.v2.Bootstrap.runtime>` in :ref:`Bootstrap
<envoy_api_msg_config.bootstrap.v2.Bootstrap>`. Use :ref:`layered_runtime
<envoy_api_field_config.bootstrap.v2.Bootstrap.layered_runtime>` instead.
Expand Down
3 changes: 3 additions & 0 deletions docs/root/intro/version_history.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ Version history
* http: mitigated a race condition with the :ref:`delayed_close_timeout<envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.delayed_close_timeout>` where it could trigger while actively flushing a pending write buffer for a downstream connection.
* http: changed `sendLocalReply` to send percent-encoded `GrpcMessage`.
* 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 <envoy_api_field_listener.FilterChainMatch.source_prefix_ranges>`
and :ref:`source port <envoy_api_field_listener.FilterChainMatch.source_ports>` filter
chain matching.
* original_src filter: added the :ref:`filter<config_http_filters_original_src>`.
* rbac: migrated from v2alpha to v2.
* redis: add support for Redis cluster custom cluster type.
Expand Down
27 changes: 15 additions & 12 deletions source/common/network/utility.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,27 +332,30 @@ 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<uint32_t>(0)));
return any;
CONSTRUCT_ON_FIRST_USE(Address::InstanceConstSharedPtr,
new Address::Ipv4Instance(static_cast<uint32_t>(0)));
}

Address::InstanceConstSharedPtr Utility::getIpv6AnyAddress() {
// Initialized on first call in a thread-safe manner.
static Address::InstanceConstSharedPtr any(new Address::Ipv6Instance(static_cast<uint32_t>(0)));
return any;
CONSTRUCT_ON_FIRST_USE(Address::InstanceConstSharedPtr,
new Address::Ipv6Instance(static_cast<uint32_t>(0)));
}

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,
Expand Down
10 changes: 10 additions & 0 deletions source/common/network/utility.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading