ingress-nginx icon indicating copy to clipboard operation
ingress-nginx copied to clipboard

Support standard Forwarded header

Open PSanetra opened this issue 11 months ago • 8 comments

As far as I can see the ingess controller currently just supports the "non-standard" X-Forwarded-* headers. It would be nice to support also the standard Forwarded header.

Use case

Keycloak has added support for the Forwarded header with version 21.0.0 and advices reverse-proxies to override this header.

PSanetra avatar Aug 01 '23 09:08 PSanetra

/triage accepted /priority backlog /good-first-issue

strongjz avatar Aug 03 '23 15:08 strongjz

@strongjz: This request has been marked as suitable for new contributors.

Guidelines

Please ensure that the issue body includes answers to the following questions:

  • Why are we solving this issue?
  • To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
  • Does this issue have zero to low barrier of entry?
  • How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-good-first-issue command.

In response to this:

/triage accepted /priority backlog /good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot avatar Aug 03 '23 15:08 k8s-ci-robot

Some requirements from the rfc7239 are listed below, as the place to pay attention to for the implementation:

  • Forwarded should be turned off by default, and parameters should be configured independently.

As section 4 mentions

Due to the sensitive nature of the data passed in this header field (see Sections 8.2 and 8.3), this header field should be turned off by default. Further, each parameter should be configured individually.

  • by and for parameters should use obfuscated identifiers by default.

section 5.1

When proxies choose to use the "by" parameter, its default configuration SHOULD contain an obfuscated identifier as described in Section 6.3.

and section 5.2

When proxies choose to use the "for" parameter, its default configuration SHOULD contain an obfuscated identifier as described in Section 6.3.

The configuration of nghttp2's add-forwarded could be a reference.

--add-forwarded=<LIST> Append RFC 7239 Forwarded header field with parameters specified in comma delimited list <LIST>. The supported parameters are "by", "for", "host", and "proto". By default, the value of "by" and "for" parameters are obfuscated string. See --forwarded-by and --forwarded-for options respectively. Note that nghttpx does not translate non-standard X-Forwarded-* header fields into Forwarded header field, and vice versa.

--strip-incoming-forwarded Strip Forwarded header field from inbound client requests.

--forwarded-by=(obfuscated|ip|<VALUE>) Specify the parameter value sent out with "by" parameter of Forwarded header field. If "obfuscated" is given, the string is randomly generated at startup. If "ip" is given, the interface address of the connection, including port number, is sent with "by" parameter. In case of UNIX domain socket, "localhost" is used instead of address and port. User can also specify the static obfuscated string. The limitation is that it must start with "", and only consists of character set [A-Za-z0-9.-], as described in RFC 7239.

Default: obfuscated

--forwarded-for=(obfuscated|ip) Specify the parameter value sent out with "for" parameter of Forwarded header field. If "obfuscated" is given, the string is randomly generated for each client connection. If "ip" is given, the remote client address of the connection, without port number, is sent with "for" parameter. In case of UNIX domain socket, "localhost" is used instead of address.

Default: obfuscated

nginx forwarded doc also describes using a secret string for identifying trusted proxies.

Forwarded: for=12.34.56.78, for=23.45.67.89;secret=egah2CGj55fSJFs, for=10.1.2.3

zitudu avatar Aug 09 '23 10:08 zitudu

Since name forwarded-header has been used for x-forwarded- header, forwarded-rfc7239 may be used for standard forwarded header defined in rfc7239. We may support these configuration options:

  • enable-forwarded-rfc7239[=false] turn on Forwarded header. If false, Forwarded header will be discarded.
  • forwarded-rfc7239[=for] list of parameters separated by a comma. Supported parameters are "for", "by", "host", "proto". Empty string can be used for pass through Forwarded header to upstream. We may add support of secret(non-standard extension) as nginx forwarded doc described in the future.
  • forwarded-rfc7239-for[=ip] supports IP only. obfuscated seems not supported by nginx.
  • forwarded-rfc7239-by[=ip] supports IP only. obfuscated seems not supported by nginx.

I'm glad to implement this feature.

/assign

zitudu avatar Aug 09 '23 12:08 zitudu

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.

github-actions[bot] avatar Sep 09 '23 01:09 github-actions[bot]

/remove lifecycle/frozen

tao12345666333 avatar Nov 22 '23 05:11 tao12345666333

I like to work on this issue, but as a new person to community, a little guidance on implementation details (how to define configuration annotations, what parts to edit, etc.) would be truly appreciated (perhaps mentioning a similar PR would help), and I also would like to know if I should also update the docs related to this issue.

kmirzavaziri avatar Jan 24 '24 13:01 kmirzavaziri

@strongjz does this issue still need help? I can work on this.

bharathk005 avatar Feb 16 '24 04:02 bharathk005

@zitudu are you still willing to work on this? Otherwise, I would like to move forward with someone that may have more bandwidth for it.

Thanks!

rikatz avatar Feb 27 '24 12:02 rikatz

I have some interest in working on this issue. Is okay for everyone?

msfidelis avatar Mar 19 '24 21:03 msfidelis

Hi users and maintainers of Ingress NGINX Controller,

My team also needs this feature and we implement it using approach described at https://www.nginx.com/resources/wiki/start/topics/examples/forwarded/:

  1. http-snippet

    map $server_addr $proxy_forwarded_elem_by {
      # IPv4
      ~^[0-9.]+$        "by=$server_addr:$server_port";
      # IPv6
      ~^[0-9A-Fa-f:.]+$ "by=\"[$server_addr]:$server_port\"";
      default           "by=unknown";
    }
    map $realip_remote_addr $proxy_forwarded_elem_for {
      # IPv4
      ~^[0-9.]+$        "for=$realip_remote_addr:$realip_remote_port";
      # IPv6
      ~^[0-9A-Fa-f:.]+$ "for=\"[$realip_remote_addr]:$realip_remote_port\"";
      default           "for=unknown";
    }
    map $server_addr $proxy_forwarded_elem_host_server_addr {
      # IPv4
      ~^[0-9.]+$        "host=$server_addr:$server_port";
      # IPv6
      ~^[0-9A-Fa-f:.]+$ "host=[$server_addr]:$server_port";
    }
    map $http_host $proxy_forwarded_elem_host {
      ~^.+$             "host=$http_host";
      default           "$proxy_forwarded_elem_host_server_addr";
    }
    map $scheme $proxy_forwarded_elem_proto {
      ~^.+$             "proto=$scheme";
    }
    map $http_forwarded $proxy_add_forwarded {
      ~^.+$ "$http_forwarded, $proxy_forwarded_elem_by;$proxy_forwarded_elem_for;$proxy_forwarded_elem_host;$proxy_forwarded_elem_proto";
      default "$proxy_forwarded_elem_by;$proxy_forwarded_elem_for;$proxy_forwarded_elem_host;$proxy_forwarded_elem_proto";
    }
    
  2. location-snippet

    proxy_set_header Forwarded $proxy_add_forwarded;
    

Unfortunately, due to https://trac.nginx.org/nginx/ticket/1316 this solution works well (supports multiple incoming "Forwarded" headers) only starting from Ingress NGINX Controller 1.10.0 (the latest version of Ingress NGINX Controller for the time this comment is written).

Looking forward for https://github.com/kubernetes/ingress-nginx/pull/10322.

Thank you.

mabrarov avatar Mar 30 '24 18:03 mabrarov

2nd gentle ping @zitudu

longwuyuan avatar Mar 30 '24 19:03 longwuyuan