envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
25k stars 4.81k forks source link

Dynamic setting value of TcpProxy.tunneling_config.hostname #19612

Closed jewertow closed 2 years ago

jewertow commented 2 years ago

Description: I have a case where applications running in Kubernetes communicate with external services (outside Kubernetes and in another network) through a dedicated forward proxy. Apps send HTTP CONNECT request and the proxy opens a TCP tunnel. The flow is illustrated below. scenario

I would like to make forward proxy transparent for apps by routing requests from applications via Envoy to forward proxy. This would allow to avoid sending HTTP CONNECT requests from apps. The desired solution is illustrated below. solution

It's quite easy to achieve the desired flow by creating the following listener:

static_resources:
  listeners:
  - name: egress_gateway_listener
    address:
      socket_address:
        address: 0.0.0.0
        port_value: 443
    listener_filters:
    - name: envoy.filters.listener.tls_inspector
    filter_chains:
    - filters:
      - name: tcp
        typed_config:
          "@type": type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
          cluster: external_proxy_cluster
          tunneling_config:
            hostname: example.com:443
  clusters:
  - name: external_proxy_cluster
    connect_timeout: 0.25s
    type: strict_dns
    lb_policy: round_robin
    load_assignment:
      cluster_name: proxy_cluster
      endpoints:
      - lb_endpoints:
        - endpoint:
            address:
              socket_address:
                address: external-proxy
                port_value: 3128

The problem is that the tunneling_config.hostname can only be statically defined, so I can't tunnel traffic to multiple destinations having a single listener. One of the potential solutions would be to set hostname based on SNI. TLS Inspector filter provides SNI value in %REQUESTED_SERVER_NAME%, but it's not possible to overwrite Host header.

tunneling_config:
  hostname: example.com:443
  headers_to_add:
  - header:
      key: Host
      value: "%REQUESTED_SERVER_NAME%"
    append_action: OVERWRITE_IF_EXISTS_OR_ADD

So what do you think about to make Host header mutable or to figure out another way to dynamically set tunneling_config.hostname?

Related issues

13809 #18838

ramaraochavali commented 2 years ago

+1 for this. @lambdai mentioned that should be supported already but it seems it is not. Host header in this case I think should be mutable and we should relax this restriction.

@lambdai @alyssawilk WDYT?

jewertow commented 2 years ago

Another solution could be to to use "%REQUESTED_SERVER_NAME%" placeholder as a value of hostname:

tunneling_config:
  hostname: "%REQUESTED_SERVER_NAME%:443"

WDYT?

alyssawilk commented 2 years ago

yeah 100% this would be useful

ramaraochavali commented 2 years ago

Another solution could be to to use "%REQUESTED_SERVER_NAME%" placeholder as a value of hostname:

Yes. That would also be a good option.

ramaraochavali commented 2 years ago

@jewertow are you working on this?

jewertow commented 2 years ago

@ramaraochavali I didn't start yet, but I'm going to work on this the next week. But first I would like to dive into the implementation of TcpProxy and SNI inspector filter to figure out a better solution than just overwriting the host header. It's a hack rather than a good and long term solution, so I would like to suggest some more elegant API that will somehow inject SNI automatically when SNI inspector is enabled.

What's more, it's also a potential feature for Istio egress gateway, so I have to discuss this idea with the Istio community. But don't worry, it's a top priority for me, so I will not delay it.

ramaraochavali commented 2 years ago

@jewertow Sure. thanks. This is also high priority for us - so wanted to know :-)

lambdai commented 2 years ago

There is an config verification that header name "host" or ":authority" can not be added or removed.

I comment that line and it works.

So I guess the only thing we need here is to keep maintain the restriction in http router but allow the mutation in tcp proxy tunnel setting

ramaraochavali commented 2 years ago

@lambdai Yes. That is what my original idea for the fix and it makes sense to me.

@jewertow Can we do that instead of thinking about changing/adding additional behaviours to hostname field?

jewertow commented 2 years ago

I know that I suggested this workaround, but to be honest, I really don't like this idea. Please take a look at this config:

tunneling_config:
  hostname: example.com:443
  headers_to_add:
  - header:
      key: Host
      value: "%REQUESTED_SERVER_NAME%"
    append_action: OVERWRITE_IF_EXISTS_OR_ADD

This configuration is really bad, because:

  1. if filter_chain_match.server_names is set, then hostname will never be used, because %REQUESTED_SERVER_NAME% is always present, so setting this value is redundant and completely useless;
  2. if filter_chain_match.server_names is not set, then %REQUESTED_SERVER_NAME% may be empty in case of no SNI and it will cause failures, due to empty hostname.

Both cases are tricky and error prone. I don't think that it's a satisfying solution.

I would suggest the following changes in this API:

message TunnelingConfig {
  oneof hostname_config {
    // one of below configs is required

    // Static request-target for CONNECT request
    string hostname = 1;

    // Automatically sets request-target for CONNECT request using SNI specified in a downstream connection
    // Supports only TLS connections and requires to enable envoy.filters.listener.tls_inspector.
    SniHostname sni_hostname = 2;
  }

  // Sets hostname for new upstream connection based on the SNI extracted by TLS Inspector filter from the downstream connection
  // This option assumes that the downstream connection is TLS. In case of plain TCP inbound connection, filter does not establishes connection.
  message SniHostname {

    // Optional port used to create the request-target for all CONNECT request, i.e. CONNECT <hostname_from_sni>:<default_port>
    // If default_port is not specified, 443 is used.
    uint32 default_port = 1;

    // Optional map of hostnames and ports that specifies which port to use for a specific hostname
    repeated HostnamePortConfig custom_hostname_and_port_configs = 2;
  }

  message HostnamePortConfig {
    string hostname = 1;
    uint32 port = 2;
  }
}

Such an API would be backward compatible and cover most cases for tunneling TLS connections.

Examples how would this API be used:

  1. Plain TCP with static request-target (already supported):
    filter_chains:
    - filters:
    - name: envoy.filters.network.tcp_proxy
    typed_config:
      "@type": type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
      cluster: proxy_cluster
      tunneling_config:
        hostname: www.example.com:80
  2. TLS with static request-target (already supported):
    filter_chains:
    - filter_chain_match:
    server_names:
    - "www.example.com"
    filters:
    - name: envoy.filters.network.tcp_proxy
    typed_config:
      "@type": type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
      cluster: proxy_cluster
      tunneling_config:
        hostname: www.example.com:443
    listener_filters:
    - name: envoy.filters.listener.tls_inspector
    typed_config:
      "@type": "type.googleapis.com/envoy.extensions.filters.listener.tls_inspector.v3.TlsInspector"
  3. TLS with automatically injected hostname and default port 443:
    filter_chains:
    - filter_chain_match:
    server_names:
    - "www.example.com"
    - "www.google.com"
    filters:
    - name: envoy.filters.network.tcp_proxy
    typed_config:
      "@type": type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
      cluster: proxy_cluster
      tunneling_config:
        sni_hostname: {}
    listener_filters:
    - name: envoy.filters.listener.tls_inspector
    typed_config:
      "@type": "type.googleapis.com/envoy.extensions.filters.listener.tls_inspector.v3.TlsInspector"
  4. TLS with automatically injected hostname and custom port for a specific hostname:
    filter_chains:
    - filter_chain_match:
    server_names:
    - "www.example.com"
    - "www.google.com"
    filters:
    - name: envoy.filters.network.tcp_proxy
    typed_config:
      "@type": type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
      cluster: proxy_cluster
      tunneling_config:
        sni_hostname:
          custom_hostname_and_port_configs:
          - hostname: www.example.com
            port: 8443
    listener_filters:
    - name: envoy.filters.listener.tls_inspector
    typed_config:
      "@type": "type.googleapis.com/envoy.extensions.filters.listener.tls_inspector.v3.TlsInspector"

What do you think?

ramaraochavali commented 2 years ago

@jewertow TBH, We do not need that much of complexity for our usecase. - we know SNI will be there and we setup that config that @lambdai shared for such cases only. But I do not have much knowledge on the different possible usecases with CONNECT - So may be @alyssawilk can weigh in here?

hmettendorf commented 2 years ago

Hello guys, I have a question about the use case in this issue. There may be similarities to our current problem.

As I understand it, the gateway should dynamically accept several (instead of a single static) hostnames and forward them accordingly via the TCP proxy.

My question now is whether completely different target hosts (different IP addresses) can be addressed here, or whether it is a single host with several "vhosts".

We would like to do following:

Sorry in advance if this is absolutely unrelated!

alyssawilk commented 2 years ago

I think we'd want to replace the old fixed hostname with a oneof, where one of the options could be downstream hostname (from SNI, or some pre-populated field), one of the options could be auto_sni (for CONNECT for DFP) and one of them could be static config. But I'd be interested in the thoughts of some of the @envoyproxy/api-shepherds , specifically @htuch and @mattklein123

mattklein123 commented 2 years ago

Yeah I think deprecating and replacing with a oneof would be the right way of approaching this. It's not great but you could also relax the requirement that hostname be non-empty, and then add a oneof for the new values, and then just check in code that only 1 of them is set if you want to avoid a deprecation.

htuch commented 2 years ago

+1 to https://github.com/envoyproxy/envoy/issues/19612#issuecomment-1028147274

jewertow commented 2 years ago

Thank you for your feedback. I will submit a pull request with a prototype soon.

lambdai commented 2 years ago

@jewertow is adding AutoSni as a new choice other than setting constant hostname, see below, while still restricting headers_to_add cannot have : prefixed headers.

Apart from this auto_sni, we should relax ":path"/":authority" for 2 reasons:

  1. :authority rewrite can be extended to anything in stream_info, such as DYNAMIC_METADATA and FILTERSTATE
  2. We are seeing picky server that doesn't like the default ":protocol" and ":path" headers which are relatively newly defined in rfc8441 #20378

What do you think we relax the headers_to_add? @alyssawilk @mattklein123

message TcpProxy {
     string hostname = 1;  
     AutoSni auto_sni = 4;
     // Neither *:-prefixed* pseudo-headers nor the Host: header can be overridden.
    repeated config.core.v3.HeaderValueOption headers_to_add = 3
        [(validate.rules).repeated = {max_items: 1000}];
}
alyssawilk commented 2 years ago

we've been pretty strict about headers to add not including HTTP/2 "control" headers so folks vetting the control plane have clear knobs for setting host. I'd be mildly averse to doing that in one place and not others but cc @yanavlasov @htuch for thoughts

htuch commented 2 years ago

I agree. The only reason I can think you might treat this differently is that in this case (I think) we're specifying the initial request as a client, rather than some filter randomly mutating in a HTTP filter pipeline. That said, I think it would be clearer to use explicit fields as needed for override and limit to only those that we have a use case for.

ramaraochavali commented 2 years ago

@jewertow just a ping, you are working on this? We have been waiting for this. It seems there is a consensus on auto_sni?

jewertow commented 2 years ago

@ramaraochavali I'm sorry that it's not completed yet. I'm going to continue working on the implementation this week and I hope it will be ready to review by the end of this or next week.

pxpnetworks commented 2 years ago

hi guys, i understand there will be an auto_sni option in tunneling_config - will it be bounded only to requestedServerName from the downstream connection info or we can use other things like downstreamDirectRemoteAddress ? my use case for this is to encode source workload ips in either :authority or SNI when i proxy tcp connections upstream using tunneling_config.

ramaraochavali commented 2 years ago

@jewertow no worries. thank you

emanuelioan commented 2 years ago

Hi! Any updates on this feature? I'm also tracking this for an application.

jewertow commented 2 years ago

@pxpnetworks I submitted another pull request and explained why not auto_sni. But yes, the general idea is to provide SNI using requestedServerName. If you want to provide workload source IP, it's rather a question if you can overwrite :authority header. As far as I remember it's not allowed... I am not sure what do you mean by providing SNI, could you explain in more detail?

jewertow commented 2 years ago

@emanuelioan yes, there is a new PR.

kyessenov commented 2 years ago

Somewhat high-level comment: per MASQUE proposal, I think it's necessary to support some URL template generation, but it really should be in :path not :authority for extended CONNECT.

scrocquesel commented 2 years ago

I have similar needs but envoy is also doing TLS upstream to the final service. Which means the downstream request has no sni information. You can look at this example https://github.com/scrocquesel/envoy_examples/tree/main/tls_origination_via_tls_l2_proxy.

The downside is that a listener and a cluster must be replicated for each destination.

scrocquesel commented 2 years ago

@jewertow many thanks for this contribution (and to anyone else who did contribute in one way or another). I was able to update my sample above to make it dynamic.

jewertow commented 2 years ago

@scrocquesel I'm glad I could help you.

batchamalick commented 1 year ago

Just in case if anyone is looking for a working config ,I'm sharing which worked for me for curl -x http://ip:10000 https://google.com

admin:
  address:
   socket_address:
    protocol: TCP
    address: 127.0.0.1
    port_value: 9902
static_resources:
  listeners:
  - name: listener_0
    address:
     socket_address:
      protocol: TCP
      address: 127.0.0.1
      port_value: 10000
    filter_chains:
    - filters:
      - name: envoy.filters.network.http_connection_manager
        typed_config:
          "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
          stat_prefix: ingress_http
          route_config:
           name: dynamic_route
           virtual_hosts:
           - name: dynamic_service
             domains:
             - "*"
             routes:
             - match:
                connect_matcher:
                 {}
               route:
                cluster: dynamic_forward_proxy_cluster
                upgrade_configs:
                - upgrade_type: CONNECT
                  connect_config:
                   {}
          http_filters:
          - name: envoy.filters.http.dynamic_forward_proxy
            typed_config:
             "@type": type.googleapis.com/envoy.extensions.filters.http.dynamic_forward_proxy.v3.FilterConfig
             dns_cache_config:
              name: dynamic_forward_proxy_cache_config
              dns_lookup_family: V4_ONLY
              typed_dns_resolver_config:
               name: envoy.network.dns_resolver.cares
               typed_config:
                "@type": type.googleapis.com/envoy.extensions.network.dns_resolver.cares.v3.CaresDnsResolverConfig
                resolvers:
                - socket_address:
                    address: "8.8.8.8"
                    port_value: 53
          - name: envoy.filters.http.router
            typed_config:
             "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
  clusters:
  - name: dynamic_forward_proxy_cluster
    lb_policy: CLUSTER_PROVIDED
    cluster_type:
     name: envoy.clusters.dynamic_forward_proxy
     typed_config:
      "@type": type.googleapis.com/envoy.extensions.clusters.dynamic_forward_proxy.v3.ClusterConfig
      dns_cache_config:
       name: dynamic_forward_proxy_cache_config
       dns_lookup_family: V4_ONLY
       typed_dns_resolver_config:
        name: envoy.network.dns_resolver.cares
        typed_config:
         "@type": type.googleapis.com/envoy.extensions.network.dns_resolver.cares.v3.CaresDnsResolverConfig
         resolvers:
         - socket_address:
             address: "8.8.8.8"
             port_value: 53
ggmm-0 commented 1 year ago

The documentation says that you can set hostname dynamically using e.g. dynamic metadata: https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/network/tcp_proxy/v3/tcp_proxy.proto#envoy-v3-api-msg-extensions-filters-network-tcp-proxy-v3-tcpproxy-tunnelingconfig

But how can e.g. %DYNAMIC_METADATA(tunnel:address)% be set before TcpProxy?

kyessenov commented 1 year ago

It assumes the usage of internal upstream transport socket to copy metadata from the endpoint metadata.

ggmm-0 commented 1 year ago

@kyessenov, thank you. Is there anything else required beside this when routing to internal_listener?

  clusters:
  - name: cluster_0
    load_assignment:
      cluster_name: cluster_0
      endpoints:
      - lb_endpoints:
        - endpoint:
            address:
              envoy_internal_address:
                server_listener_name: internal_listener
    transport_socket:
      name: envoy.transport_sockets.internal_upstream
      typed_config:
        "@type": type.googleapis.com/envoy.extensions.transport_sockets.internal_upstream.v3.InternalUpstreamTransport
        passthrough_metadata:
        - name: tunnel
          kind: { host: {}}
        transport_socket:
          name: envoy.transport_sockets.raw_buffer
          typed_config:
            "@type": type.googleapis.com/envoy.extensions.transport_sockets.raw_buffer.v3.RawBuffer

Because this setting doesn't seem to be working (%DYNAMIC_METADATA(tunnel:address)% returns empty string).

kyessenov commented 1 year ago

Yeah, you'd need actual metadata on the endpoint. https://github.com/istio/proxy/blob/master/testdata/cluster/internal_outbound.yaml.tmpl#L11.

ggmm-0 commented 1 year ago

@kyessenov, do you mean something like this?

  clusters:
  - name: cluster_0
    load_assignment:
      cluster_name: cluster_0
      endpoints:
      - lb_endpoints:
        - endpoint:
            address:
              envoy_internal_address:
                server_listener_name: internal_listener
    metadata:
      filter_metadata:
        tunnel:
          address:  "my.host.com:80"
    transport_socket:
      name: envoy.transport_sockets.internal_upstream
      typed_config:
        "@type": type.googleapis.com/envoy.extensions.transport_sockets.internal_upstream.v3.InternalUpstreamTransport
        passthrough_metadata:
        - name: tunnel
          kind: { host: {}}
        transport_socket:
          name: envoy.transport_sockets.raw_buffer
          typed_config:
            "@type": type.googleapis.com/envoy.extensions.transport_sockets.raw_buffer.v3.RawBuffer

It doesn't seem to be working. Am I missing something? Also, I thought that this tunnel:address would be set dynamically (e.g. based on Host header if HTTP).

kyessenov commented 1 year ago

I think you got indentation wrong - metadata is on endpoints, not cluster. tunnel:address is not something hard-coded, it's really just a metadata reference.

You can do a dynamic version. You'd need a couple of filters, one to capture address, and another to set the destination, communicating via filter state. You can see how we do it in Istio:

ggmm-0 commented 1 year ago

You're right - but surprisingly, metadata can also be provided on cluster level. Thank you very much, that's clear for me now.

littlejiancc commented 9 months ago

@kyessenov hi, can you help take a look https://github.com/envoyproxy/envoy/issues/32103