istio / istio

Connect, secure, control, and observe services.
https://istio.io
Apache License 2.0
35.72k stars 7.7k forks source link

Need ability to set use_remote_address to false in ingressgateway envoy config #12549

Closed gdhagger closed 5 years ago

gdhagger commented 5 years ago

Describe the feature request Need to be able to tell an ingressgateway that it's not the absolute edge proxy. Currently the value for it's envoy http connection filter 'use_remote_address' configuration is hardcoded to true. This causes it to ignore values set for x-forwarded-for, x-forwarded-proto etc. In turn, this breaks the behavior of the httpsRedirect flag in a gateway configuration when TLS is terminated prior to the ingressgateway.

Describe alternatives you've considered I've experimented with using Lua EnvoyFilters to inspect headers and provide a workaround http->https redirect, however the filter sees the headers after they've already been mangled. Potentially I could run yet another proxy between the loadbalancer and the ingressgateway, but this is something I really want to avoid. As far as I can tell it's not possible to patch an automatically generated envoy filter.

Additional context My exact scenario is this:

Related to:

sangwa commented 5 years ago

@gdhagger For the particular case of using a Google HTTPS balancer over GKE, we've been able to make it work with a bunch of workarounds:

(The drawback here is that if a node pool is recreated then the ports should be mapped again for the new instance group, but we have the whole sequence scripted in Terraform so it helps a bit.)

With this configuration a common Gateway resource with httpsRedirect for port 80 and TLS mode: SIMPLE termination on port 443 is working as expected. Hope it helps.

gdhagger commented 5 years ago

@sangwa These workarounds shouldn't be necessary, and even with them I don't believe it actually solves the problem.

Also, from the testing I've done, even with externalTrafficPolicy: Local I don't believe the correct IP will be used by envoy as the 'real' IP of the client unless we are able to tune the use_remote_address parameter. (Also, for the record I'm using Network Endpoint Groups rather than Instance Groups to hit my pods)

For now I've solved the redirect portion by putting AWS CloudFront in front of my LB for any subdomains that need to be redirected from HTTP->HTTPS.

sangwa commented 5 years ago

Two loadbalancers = two IPs

Not necessarily, we have both balancers on the same global IP, just different ports (80 vs 443).

gdhagger commented 5 years ago

My apologies, you're absolutely right! For some reason I was completely convinced I could only attach a global IP to one LB at a time.

On Tue, Mar 26, 2019 at 2:35 AM Yaroslav Zhavoronkov < notifications@github.com> wrote:

Two loadbalancers = two IPs

Not necessarily, we have both balancers on the same global IP, just different ports (80 vs 443).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/istio/istio/issues/12549#issuecomment-476494010, or mute the thread https://github.com/notifications/unsubscribe-auth/AAbeH-sZTG76IEagLGFLJHJJgWLTgAA2ks5vab-ygaJpZM4b4HhW .

gdhagger commented 5 years ago

Note for anyone who comes across this issue:

When trying to create an ingress assigned to a global IP already attached to another loadbalancer NOT managed by GKE ingress, I received the following error:

Error during sync: Error running load balancer syncing routine: googleapi: Error 400: Invalid value for field 'resource.IPAddress': 'IP.RED.ACT.ED'. Specified IP address is in-use and would result in a conflict., invalid

However, using another IP address I was able to create two GKE ingresses using the same global IP.

MartinLG commented 5 years ago

I have the exact same problem... I think Istio should trust headers comming from known upper LB and avoidi to strip them. I understand the security issues related to these headers, but this is a blocking point for us to adopt Istio.

cagataygurturk commented 5 years ago

Same issue here.

nicktrav commented 5 years ago

Can someone that works on Istio help triage this?

Maybe @rshriram? I've seen some comments of yours on some related PRs / issues, such as #13157.

nicktrav commented 5 years ago

@howardjohn - are there any updates on this? cc: @PiotrSikora, who was also active in the conversation over on #7679, which is related.

howardjohn commented 5 years ago

Yes. Someone added this and we found later it broke existing users so we set it behind a feature flag. For now it can be enabled with --set pilot.env.PILOT_SIDECAR_USE_REMOTE_ADDRESS=true. I think someone is working on adding a proper api for this

rshriram commented 5 years ago

Also, with the newer EnvoyFilter API, you can tweak a whole lot of envoy settings like this without clumsy workarounds like env vars. https://github.com/istio/api/blob/master/networking/v1alpha3/envoy_filter.proto

nicktrav commented 5 years ago

@rshriram - that api looks like exactly what we want. Nice. Is going to be in 1.3.x, or later?

@howardjohn - thanks! I'll check that out in the meantime.

rshriram commented 5 years ago

1.3.x .. its almost done

howardjohn commented 5 years ago

This seems completed by both Envoyfilter and the short term solution of environment variable. Is there any other work to be done here?

fcantournet commented 4 years ago

Yes. Someone added this and we found later it broke existing users so we set it behind a feature flag. For now it can be enabled with --set pilot.env.PILOT_SIDECAR_USE_REMOTE_ADDRESS=true. I think someone is working on adding a proper api for this

I might be missing something but I don't see how this is applied to the ingress-gateway ? this seems hardcoded here : https://github.com/istio/istio/blob/f772ceea87ae58896e1eabe2cf83ebfd8c54bf6d/pilot/pkg/networking/core/v1alpha3/gateway.go#L358

fcantournet commented 4 years ago

@howardjohn I think we should re-open this issue : The issues specifically is about setting use_remote_address to false on the ingress-gateway. Currently I have not been able to do that, except by forking pilot.

[
    {
        "name": "0.0.0.0_80",
        "address": {
            "socketAddress": {
                "address": "0.0.0.0",
                "portValue": 80
            }
        },
        "filterChains": [
            {
                "filters": [
                    {
                        "name": "envoy.http_connection_manager",
                        "typedConfig": {
                            "@type": "type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager",
                            "statPrefix": "0.0.0.0_80",
                            "rds": {
  ...
                            },
                            "httpFilters": [
...
],
                            "useRemoteAddress": true,
                            "xffNumTrustedHops": 2,
...
    }
]
hzxuzhonghu commented 4 years ago

Maybe related use_remote_address is not a bool type. it is a struct

type BoolValue struct {
    // The bool value.
    Value                bool     `protobuf:"varint,1,opt,name=value,proto3" json:"value,omitempty"`
    XXX_NoUnkeyedLiteral struct{} `json:"-"`
    XXX_unrecognized     []byte   `json:"-"`
    XXX_sizecache        int32    `json:"-"`
}
cdmurph32 commented 4 years ago

You can preserve the external request id with the following EnvoyFilter. I'm running Istio 1.3.0.

apiVersion: networking.istio.io/v1alpha3
kind: EnvoyFilter
metadata:
  name: ingressgateway-settings
  namespace: istio-system
spec:
  configPatches:
  - applyTo: NETWORK_FILTER
    match:
      context: GATEWAY
      listener:
        filterChain:
          filter:
            name: envoy.http_connection_manager
    patch:
      operation: MERGE
      value:
        typed_config:
          '@type': type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager
          preserve_external_request_id: true

Thanks to https://github.com/envoyproxy/envoy/pull/7140

fcantournet commented 4 years ago

Maybe related use_remote_address is not a bool type. it is a struct

type BoolValue struct {
  // The bool value.
  Value                bool     `protobuf:"varint,1,opt,name=value,proto3" json:"value,omitempty"`
  XXX_NoUnkeyedLiteral struct{} `json:"-"`
  XXX_unrecognized     []byte   `json:"-"`
  XXX_sizecache        int32    `json:"-"`
}

Maybe you're onto something @hzxuzhonghu use_remote_address is BoolType whereas preserve_external_request_id is indeed bool.

However my understanding is that the representation in json is a bool in both case : https://developers.google.com/protocol-buffers/docs/reference/google.protobuf#boolvalue

jh-sz commented 4 years ago

@fcantournet have you got a solution for your issue for 1.3.x version, i have created a EnvoyFilter similar to yours, and confirmed that it works on 1.4.0, but there're intermittent 503 issues. Changing it to 1.3.5 doesn't seem to work

bbenlazreg commented 4 years ago

@jh-sz I am facing the same issue on 1.3.5, setting xff_num_trusted_hops works on 1.4.0 but not in 1.3.5, did you managed to make it work ?

jh-sz commented 4 years ago

@bbenlazreg don't think it works for 1.3.5

ngtuna commented 3 years ago

don't think it's solved in 1.10. I can not set use_remote_address: false via EnvoyFilter

ashleywang1 commented 1 year ago

This is still true as of Istio 1.17.2. I've tried using this EnvoyFilter:

apiVersion: networking.istio.io/v1alpha3
kind: EnvoyFilter
metadata:
  name: ingressgateway-settings
  namespace: istio-ingressgateway
spec:
  configPatches:
  - applyTo: NETWORK_FILTER
    match:
      context: GATEWAY
      listener:
        filterChain:
          filter:
            name: envoy.filters.network.http_connection_manager
    patch:
      operation: MERGE
      value:
        typed_config:
          '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
          use_remote_address: false
          xff_num_trusted_hops: 2
  workloadSelector:
    labels:
      istio: ingressgateway-ns

And saw the same thing:

use_remote_address: true,
...
xff_num_trusted_hops: 2,

This is a duplicate of https://github.com/istio/istio/issues/18169