envoy-gateway: Additional configuration for determining internal address

[cskarby]

Description
The http request header x-forwarded-for (XFF) is not beeing set when the request is origination from ip-ranges that envoy detects as internal. By default any request from private ip ranges (RFC1918 / RFC4193) are considered internal. Envoy's list of internal CIDR ranges can be configured with the internal address configuration in the http connection manager.

When running the envoy gateway in an internal network, where the client will connect directly to the envoy-gateway without network address translation from a private ip addresses, it would still be interesting to know the client's IP address. When the XFF header is not set, the backend will only have the ip address of the proxying gateway, and not the actual client.

Please provide guidance for how to configure the envoy-gateway with custom CIDR ranges to consider as internal. Ideally it should be possible to set this via helm values, or at least via relevant kubernetes configuration objects. In the meantime I would also be interested in ways to configure this via low level interfaces, e.g. overriding the bootstrap configuration or any other way, if that is possible?

Relevant Links

• API spec HttpConnectionManager.InternalAddressConfig
• Upstream issue envoyproxy/envoy#19844

[arkodg]

this makes sense, the API can live under ClientIPDetection

gateway/api/v1alpha1/clienttrafficpolicy_types.go

Line 220 in 0abdda7

type ClientIPDetectionSettings struct {

@cskarby the workarounds are to use EnvoyPatchPolicy or update bootstrap

cc @zhaohuabing @davidalger

[zhaohuabing]

BTW, looks like it's only configurable at the HCM level, so probably can't use update bootstrap.

[cskarby]

I'm new to this project, and not so familiar with envoy configuration, however I hope my draft could be useful. Just let me know if it is not... Probably the API should be bumped when adding new fields? And further we probably should add some tests

[arkodg]

@cskarby there may be two ways of representing this in the API

1. allow private subnet IPs to be populated in XFF - this can be represented as a bool *AllowPrivateIPs and internally EG can go ahead and configure 10.0.0.0/8,........ in InternalAddressConfig in Envoy
2. only populate IPs from these trusted CIDRs in XFF - this can be represented as []string CIDR which can be implemented once we have xff: add support for configuring a list of trusted CIDRs envoy#31831

wdyt @envoyproxy/gateway-maintainers

[cskarby]

@cskarby there may be two ways of representing this in the API

1. allow private subnet IPs to be populated in XFF - this can be represented as a bool *AllowPrivateIPs and internally EG can go ahead and configure 10.0.0.0/8,........ in InternalAddressConfig in Envoy
2. only populate IPs from these trusted CIDRs in XFF - this can be represented as []string CIDR which can be implemented once we have xff: add support for configuring a list of trusted CIDRs envoy#31831

wdyt @envoyproxy/gateway-maintainers

Thank you for your quick response! Personally I like the second option best, see proposal below, but it diverges from the defaults in vanilla envoy. The first option is already the default behavior, but actually I need a way to not treat these addresses as internally trusted sources. To override it we would need to set some other addresses, what about localhost?

A different problem

Actually my problem is different to that of envoy#31831, which is about trusting existing XFF headers. I would like envoy-gateway to set XFF headers so that I can see the client IP in a new XFF header when doing upstream processing. I have no proxy between the client and envoy-gateway, so there is no legitimate requests originating with an existing XFF header.

Envoy will only append to XFF if the use_remote_address HTTP connection manager option is set to true and the skip_xff_append is set false. This means that if use_remote_address is false (which is the default) or skip_xff_append is true, the connection manager operates in a transparent mode where it does not modify XFF.

Even when use_remote_address is true, Envoy flags incoming requests from RFC1918 ranges (10.0.0.0/8, 172.16.0.0/12 and 192.168.0.0/16) and the RFC4193 range (fc00::/7) as internal.

From A few very important notes about XFF:

2. XFF is what Envoy uses to determine whether a request is internal origin or external origin. If use_remote_address is set to true, the request is internal if and only if the request contains no XFF and the immediate downstream node’s connection to Envoy has an internal (RFC1918 or RFC4193) source address. [...]

So in my case I need to set use_remote_address =true, but then the request from any private IP range that comes without an existing XFF will be flagged as internal. This is not desired for two reasons:

1. Users on our VPN may send an XFF and make the upstream believe the request actually originated from any IP
2. Upstream is not able to see which IP address the request actually came from, as envoy will flag the request as internal and not generate XFF header for the response sent to upstream. Hence upstream will only see the address of the envoy-gateway instance forwarding the request.

Regarding the first point in your feedback: Envoy already considers connections from these addresses as internal, unless overridden by explicit configuration. The configuration API is already in envoy, but not envoy-gateway. This is why we need a way to configure it in envoy-gateway.

Proposal of an alternative to the current MR

Perhaps the default for envoy-gateway should be to treat all IP addresses as untrusted? I believe we could achive this by default setting HttpConnectionManager.InternalAddressConfig to 127.0.0.0/8 and ::1/128 - then only requests from localhost ranges are considered internal by default. I think it is more intuitive and a safer default, but it would diverge from vanilla envoy, which could cause confusion for experienced envoy admins.

Later when xff: add support for configuring a list of trusted CIDRs envoy#31831 is merged, we could extend the API to look something like this (which I believe is in line with what you write in your feedback):

apiVersion: gateway.envoyproxy.io/v1alpha2-proposal
kind: ClientTrafficPolicy
metadata:
  name: enable-tcp-keepalive-policy
  namespace: default
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: Gateway
    name: eg
    namespace: default
  clientIPDetection:
    xForwardedFor:
      xff_trusted_cidrs:
        - 10.0.10.0/24
        - 192.88.99.128/30

When we have a matching ClientTrafficPolicy, we could also add all ranges from xff_trusted_cidrs to HttpConnectionManager.InternalAddressConfig in additon to the localhost ranges? (If it is still needed after xff: add support for configuring a list of trusted CIDRs envoy#31831?)

Questions

1. Should the default for envoy-gateway be to only trust connections from localhost (although this diverges from vanilla envoy)?
2. If yes, should we split these two things into separate merge requests - one for only trusting localhost, and one for adding support for xff: add support for configuring a list of trusted CIDRs envoy#31831? If we split it there will be no way to override it and restore the vanilla envoy behavior until after their new functionallity is available to us.

[zhaohuabing]

Actually my problem is different to that of envoy#31831, which is about trusting existing XFF headers. I would like envoy-gateway to set XFF headers so that I can see the client IP in a new XFF header when doing upstream processing. I have no proxy between the client and envoy-gateway, so there is no legitimate requests originating with an existing XFF header.

@cskarby Looks like whether the IP range is internal or not doesn't impact Envoy's decision on appending XXF header. Of course I may miss something here - Envoy's configuration around XXF is a bit messy.

https://github.com/envoyproxy/envoy/blob/f0201e54683875efeecf09df7328ad374be52d2c/source/common/http/conn_manager_utility.cc#L121-L143

  if (config.useRemoteAddress()) {
     ....

    if (!config.skipXffAppend()) {
      if (Network::Utility::isLoopbackAddress(
              *connection.connectionInfoProvider().remoteAddress())) {
        Utility::appendXff(request_headers, config.localAddress());
      } else {
        Utility::appendXff(request_headers, *connection.connectionInfoProvider().remoteAddress());
      }
    }
  ...
}

[cskarby]

Indeed not so easy to navigate, so I just tested a minimal diff to see if it makes a difference

diff --git a/internal/xds/translator/listener.go b/internal/xds/translator/listener.go
index 427cca26..bff7e0eb 100644
--- a/internal/xds/translator/listener.go
+++ b/internal/xds/translator/listener.go
@@ -205,6 +205,10 @@ func (t *Translator) addXdsHTTPFilterChain(xdsListener *listenerv3.Listener, irL
 	if originalIPDetectionExtensions != nil {
 		useRemoteAddress = false
 	}
+	
+	var localhostCidrRanges []*corev3.CidrRange
+	localhostCidrRanges = append(localhostCidrRanges, &corev3.CidrRange{AddressPrefix: "127.0.0.1", PrefixLen: &wrapperspb.UInt32Value{Value:   8}})
+	localhostCidrRanges = append(localhostCidrRanges, &corev3.CidrRange{AddressPrefix: "::1",       PrefixLen: &wrapperspb.UInt32Value{Value: 128}})
 
 	mgr := &hcmv3.HttpConnectionManager{
 		AccessLog:  al,
@@ -225,6 +229,7 @@ func (t *Translator) addXdsHTTPFilterChain(xdsListener *listenerv3.Listener, irL
 		Http2ProtocolOptions: http2ProtocolOptions(),
 		// https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/headers#x-forwarded-for
 		UseRemoteAddress:              &wrappers.BoolValue{Value: useRemoteAddress},
+		InternalAddressConfig:         &hcmv3.HttpConnectionManager_InternalAddressConfig{UnixSockets: true, CidrRanges: localhostCidrRanges},
 		XffNumTrustedHops:             xffNumTrustedHops(irListener.ClientIPDetection),
 		OriginalIpDetectionExtensions: originalIPDetectionExtensions,
 		// normalize paths according to RFC 3986

and the patch actually modifies the behaviour of envoy when it comes to sending the XFF header upstream. I will investigate further...