Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement proper hop-to-hop + forwarded headers #2202

Closed
13 tasks done
thibaultcha opened this issue Mar 14, 2017 · 5 comments
Closed
13 tasks done

Implement proper hop-to-hop + forwarded headers #2202

thibaultcha opened this issue Mar 14, 2017 · 5 comments
Assignees
Labels
task/feature Requests for new features in Kong

Comments

@thibaultcha
Copy link
Member

thibaultcha commented Mar 14, 2017

As a reverse proxy, Kong needs to forward the appropriate headers from trusted sources.

  • X-Real-IP with the value from $realip_remote_addr.
  • X-Forwarded-For: the value from $proxy_add_x_forwarded_for.
  • X-Forwarded-Proto: forwarded if present, use $scheme otherwise.
  • X-Forwarded-Host: forward if present, use request Host header otherwise.
  • X-Forwarded-Port: forward if present, use $server_port otherwise.
  • Forward upstream Date: header back to client with proxy_pass_header Date;.
  • Ability to set trusted IP blocks from the config file.
    • ability to configure set_real_ip_from directive in nginx_kong.conf template from kong.conf.
    • ability to configure real_ip_from from kong.conf.
    • ability to configure real_ip_recursive directive in nginx_kong.conf template from kong.conf.
    • validate given IPs from this new configuration value.
    • only forward the X-Forwarded-Proto header if the client IP is part of the set_real_ip_from CIDR blocks. Use lua-resty-iputils for this if we don't find a way to do it from the Nginx config level.
  • tests

I have pushed my current work on feat/real-hop-to-hop with work and tests already done, and checked the already-implemented features.

This gathers:
#1662
#1788
#1896
#1823
#1445
#1762
#1661

And maybe others.

@bungle has stepped forward to finish this.

@subnetmarco
Copy link
Member

Missing X-Forwarded-Path, which covers the same use-case of X-Forwarded-Host but when a path resolver is being used.

@thibaultcha
Copy link
Member Author

thibaultcha commented Mar 20, 2017

X-Forwarded-Path doesn't really belong to this scope. This issue is about standard forwarded headers for any HTTP reverse-proxy out there that Kong does not necessarily respect. The X-Forwarded-Host header is provided regardless of the routing strategy, because it is mandatory for an HTTP request to carry a Host header.

However, X-Forwarded-Path, X-Forwarded-Method (yes, since we now also resolve via HTTP method) belong to the router, and would be new features on top of it, which I am fine with adding later on as well, but it just raises a concern, described as 2. down below.


We need to keep in mind a couple things with X-Forwarded-Host:

  1. Making sure that switching the value to $http does not override the desired value when preserve_host is disabled/enabled on the API object. preserve_host handles the upstream Host header, but we still need to make sure that the upstream X-Forwarded-Host header is still right in both cases. This warrants a couple tests imho.
  2. X-Forwarded-Host is provided regardless of whether the routing matched a configured hosts value on the API or not. If we provide X-Forwarded-{Path,Method), then maybe we ought to provide a special header for routing resolved via hosts as well.

Proposed solution:

As stated before, those X-Forwarded-{Path,Method} headers are not adopted out there, and are Kong-specific and related to the router. Let's implement them later, in another PR, and let's use different names for them so that it is clear they are Kong-specific and related to the routing mechanism used for a given request. Maybe something like:

  • X-Forwarded-Kong-Host (not the same as X-Forwarded-Host)
  • X-Forwarded-Kong-Path
  • X-Forwarded-Kong-Method

bungle pushed a commit that referenced this issue Mar 22, 2017
bungle added a commit that referenced this issue Mar 22, 2017
* Introduce trusted_ips config property
* Introduce real_ip_header
* New lua-resty-mediador
* Implement logic to validate client ips as trusted
* Implement Lua logic for the X-Forwarded-For upstream header
* Related tests (renamed real_ip test suite to upstream_headers)

Fix #2202
bungle added a commit that referenced this issue Mar 23, 2017
* Introduce trusted_ips config property
* Introduce real_ip_header
* New lua-resty-mediador
* Implement logic to validate client ips as trusted
* Implement Lua logic for the X-Forwarded-For upstream header
* Related tests (renamed real_ip test suite to upstream_headers)

Fix #2202
@udangel-r7
Copy link

Thanks for the work, it would be great to see that in 0.10.2 as not forwarding X-Forwarded-Proto is somehow problematic :/

thibaultcha added a commit that referenced this issue Apr 15, 2017
thibaultcha pushed a commit that referenced this issue Apr 15, 2017
* Introduce trusted_ips config property
* Introduce real_ip_header
* New lua-resty-mediador
* Implement logic to validate client ips as trusted
* Implement Lua logic for the X-Forwarded-For upstream header
* Move the WebSocket upstream headers dance to the Lua-land instead of
  the old Nginx maps.
* Related tests (renamed real_ip test suite to upstream_headers)

Fix #2202
@thibaultcha
Copy link
Member Author

#2236 is merged, closing this now, thanks @bungle! :)

@maxtuzz
Copy link

maxtuzz commented Aug 6, 2017

Confirmed working with 0.11.0rc2 with trusted_ips 👍

Made my job easier. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
task/feature Requests for new features in Kong
Projects
None yet
Development

No branches or pull requests

5 participants