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

Enclose IPv6 address in X-Forwarded-Host in brackets #1213

Closed
wants to merge 2 commits into from

Conversation

mwpastore
Copy link
Contributor

Prevent Rack::Request#host from stripping off the last hextet of an IPv6 address contained in X-Forwarded-Host (and returned by Rack::Request#host_with_port) by enclosing the address in brackets.

IPv6 addresses in the HTTP_HOST, SERVER_NAME, and SERVER_ADDR CGI variables will (should) always be enclosed in brackets.

Prevent Rack::Request#host from stripping off last hextet of IPv6
address in X-Forwarded-Host returned by Rack::Request#host_with_port
by enclosing the address in brackets.

IPv6 addresses in the HTTP_HOST, SERVER_NAME, and SERVER_ADDR CGI
variables will (should) always be enclosed in brackets.
@matthewd
Copy link
Contributor

matthewd commented Nov 1, 2017

IPv6 addresses in the HTTP_HOST, SERVER_NAME, and SERVER_ADDR CGI variables will (should) always be enclosed in brackets.

Indeed. And X-Forwarded-Host is a forwarded value for HTTP_HOST.

This sounds rather more like a hack around an upstream misconfiguration than a necessary correction to me. Could you give some more detail on how you came to encounter such a value?

@mwpastore
Copy link
Contributor Author

Yes, and the best reverse proxies I've seen don't even use the X-Forwarded-Host header, they just pass the Host value along. RFC 7239 says that IPv6 addresses in this header should be enclosed in square brackets.

I haven't seen it it in the wild in person, just in forum posts and the like, so this may be a case of overly-defensive programming. I agree that if the reverse proxy is sending an invalid value, it should be fixed there. It's just such a subtle, silent issue—and the X- headers seem to be especially prone to errors and misconfiguration—that a guard here makes sense to me.

Take it or leave it!

@HoneyryderChuck
Copy link

Take a look at this, there's quite a lot of sketchy IPv6 support in ruby's core libs.

mwpastore added a commit to mwpastore/rack-protection-maximum_cookie that referenced this pull request Nov 6, 2017
@jeremyevans
Copy link
Contributor

RFC 7239 Section 7.4:

   Note that IPv6 addresses may not be quoted in
   X-Forwarded-For and may not be enclosed by square brackets, but they
   are quoted and enclosed in square brackets in "Forwarded".

       X-Forwarded-For: 192.0.2.43, 2001:db8:cafe::17

   becomes:

       Forwarded: for=192.0.2.43, for="[2001:db8:cafe::17]"

So I think we should handle this for X-Forwarded-*. but not Forwarded (when support is added for Forwarded #1423).

Could you please rebase against master?

@ioquatix
Copy link
Member

ioquatix commented Feb 5, 2020

This makes perfect sense to me, thanks for the specs. I'll merge it.

@ioquatix
Copy link
Member

ioquatix commented Feb 5, 2020

I reviewed the PR and it looks like we recently merged #1538 which prefered the non-square-brackets representation. However, this has not been released yet. So we could adjust it.

Based on the defintion of URI#host, I think we should probably prefer the square brackets representation... but I'm not sure what is canonical. I need to do some research.

@ioquatix ioquatix closed this Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants