linkerd / linkerd2

Ultralight, security-first service mesh for Kubernetes. Main repo for Linkerd 2.x.
https://linkerd.io
Apache License 2.0
10.7k stars 1.28k forks source link

Set `Forwarded` / `X-Forwarded-For` when proxying HTTP traffic #4219

Open paultiplady opened 4 years ago

paultiplady commented 4 years ago

Feature Request

When the linkerd-proxy forwards an HTTP request, append a Forwarded or X-Forwarded-For header entry so that upstreams can tell where the forwarded request came from.

What problem are you trying to solve?

It's common for HTTP services to log or consider the source-IP of incoming requests. For example, for rate-limiting or auditing. Usually external Load Balancers will set the X-Forwarded-For header, and Linkerd doesn't interfere with that. However for intra-cluster traffic (east/west), there is currently no good way for meshed services to learn the real source-IP of HTTP traffic, since the Linkerd proxy rewrites the source IP to 127.0.0.1.

How should the problem be solved?

When the linkerd-proxy forwards an HTTP request, inject a Forwarded or X-Forwarded-For header.

This typically involves appending the IP to the list, if it already exists, like:

X-Forwarded-For: 1.1.1.1, 2.2.2.2

Any alternatives you've considered?

It might be desirable to pick one of those headers, or to support both. XFF is universally supported. Forwarded is not (e.g. it's not supported in Django: https://code.djangoproject.com/ticket/30729).

How would users interact with this feature?

Presumably this would be injected annotation config, something like

annotations:
  config.linkerd.io/http-proxy-header: "x-forwarded-for" | "forwarded"
paultiplady commented 4 years ago

Tangentially related: https://github.com/linkerd/linkerd2/issues/3403

grampelberg commented 4 years ago

This has enough moving parts, we'll want to get an RFC going for it before implementation. I'm particularly interested in the security implications, as anyone can add a XFF header themselves at any point.

joewilliams commented 3 years ago

This would be a really nice feature and is fairly standard feature in most proxy set ups. We are bumping into related x-forwarded-for issues over in this discussion as well https://github.com/linkerd/linkerd2/discussions/5683

joewilliams commented 3 years ago

Thinking about this a bit more over the weekend, I am wondering if it makes sense to have a couple configuration options. One for the behavior ignore, append vs overwrite and two for the name of the header. I could see a couple scenarios where this could come in handy. For instance, lets say I want x-forwarded-for to be untouched but want l5d-forwarded-for to be written instead. This would ignore x-forwarded-for and overwrite (or append) l5d-forwarded-for with the value linkerd would have used for x-forwarded-for.

annotations:
  config.linkerd.io/http-proxy-header: "l5d-forwarded-for"
  config.linkerd.io/http-proxy-header-behavior: "overwrite"

For the OP example:

annotations:
  config.linkerd.io/http-proxy-header: "x-forwarded-for"
  config.linkerd.io/http-proxy-header-behavior: "append"

Never touch x-forwarded-for:

annotations:
  config.linkerd.io/http-proxy-header-behavior: "ignore"

I think this would be some what equivalent to how Envoy deals with use_remote_address but a bit less confusing on how the configuration combinations actually work. 😅

kleimkuhler commented 3 years ago

@joewilliams Sorry there has been no response on this. We'll make sure to discuss and consider options to solve this issue and your linked discussion.

For your l5d-forwarded-for example, what use case do you have in mind for when both that and x-forwarded-for are set? Do you see a scenario when they would be different? It seems worth at least adding the x-forwarded-for header without configuration, but your suggestion is definitely a lot more to consider given the possible headers and valid values.

jsoref commented 3 years ago

x-forwarded-for would tell me about the external proxy, and l5d-forwarded-for would tell me where in the cluster the request came from.

I have an nginx backend pod that offers debugging information for clients, and it'd be nice for me to not mention the linkerd internal details.

sgrzemski commented 3 years ago

Any progress on that?

linuxeryt commented 2 years ago

How's it going

codemug commented 2 years ago

What's the progress on this discussion? This is a big issue if we want to access the Client IP at the service end, which gets turned to 127.0.0.1 by linkerd-proxy.

debu99 commented 2 years ago

10.30.47.26 - - [12/Oct/2022:07:30:28 +0000] "GET / HTTP/1.1" 200 615 "-" "curl/7.64.0" "-" 10.30.47.26 - - [12/Oct/2022:07:31:50 +0000] "GET / HTTP/1.1" 200 615 "-" "curl/7.64.0" "-" 2022/10/12 07:32:11 [error] 31#31: *9 open() "/usr/share/nginx/html/tttt" failed (2: No such file or directory), client: 10.30.47.26, server: localhost, request: "GET /tttt HTTP/1.1", host: "nginx" 10.30.47.26 - - [12/Oct/2022:07:32:11 +0000] "GET /tttt HTTP/1.1" 404 153 "-" "curl/7.64.0" "-" 2022/10/12 07:33:22 [error] 31#31: *10 open() "/usr/share/nginx/html/tttt" failed (2: No such file or directory), client: 10.30.47.26, server: localhost, request: "GET /tttt HTTP/1.1", host: "10.30.47.26" 10.30.47.26 - - [12/Oct/2022:07:33:22 +0000] "GET /tttt HTTP/1.1" 404 153 "-" "curl/7.64.0" "-"

I used a nginx to work as a server side, looks like the client ip is same as the host ip(10.30.47.26), is this related with above discussion, how can i get the client ip?

jeremychase commented 2 years ago

Bumping this issue because it has many thumbs up 👍

deusxanima commented 1 year ago

@olix0r spent some time testing various scenarios today and was able to confirm that setting skip-inbound-ports and skip-outbound-ports annotations on the workload does not preserve the original source-ip in the header but instead replaces it with the ingress pod ip.

deusxanima commented 1 year ago

For anyone following along, the recommended way to get around the issue of x-forwarded-for and forwarded headers being changed is to set the config.linkerd.io/skip-inbound-ports annotation on the ingress/proxy deployment.

For specific controllers such as Nginx, Traefik, and Kong, it is important to mark the actual ingress-listener ports vs. the destination ports being used by the client when sending the HTTP requests. For example, Kong listens on port 8000 for HTTP traffic and 8443 for HTTPS traffic, so rather than exempting 80/443 traffic (which the client will send to) you should exempt 8000/8443 traffic which the ingress will listen and process it on.

ihordyrman commented 1 year ago

Any updates on this issue? This is quite disappointing as the problem seems to be a common one that affects many users, including myself.

wmorgan commented 11 months ago

@ihordyrman (and others) can you expand a bit about why you want this feature? Does the l5d-client-id header capture your use case?

AlexGoris-KasparSolutions commented 8 months ago

So why I would consider this feature important, is because we've noticed that when we inject our kubernetes ingress controllers (using the Kubernetes nginx ingress controller) with a linkerd proxy sidecar, we lose the external IP at the ingress controller level and everything which comes after that.

As others have mentioned, we consider it important for some applications to have monitoring and auditing in place which records the external IP from which a certain request came.

Currently it looks like we have to resort to not injecting our ingress controllers with the linkerd proxy sidecar, which also prohibits us from enforcing strong mTLS authentication in upstream services which need to accept connections from the ingress controllers.

adleong commented 8 months ago

Hi @AlexGoris-KasparSolutions. Something you can do here is use the config.linkerd.io/skip-inbound-ports annotation on your ingress pods so that inbound connections skip the Linkerd sidecar proxy but you still get mTLS on the outbound side between your ingress controller and the upstream services.

AlexGoris-KasparSolutions commented 8 months ago

Thanks @adleong , this indeed solves the issue I faced!