Skip to content

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

Closed
mwpastore wants to merge 2 commits into
rack:masterfrom
mwpastore:ipv6-forwarded-host
Closed

Enclose IPv6 address in X-Forwarded-Host in brackets#1213
mwpastore wants to merge 2 commits into
rack:masterfrom
mwpastore:ipv6-forwarded-host

Conversation

@mwpastore

Copy link
Copy Markdown
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

matthewd commented Nov 1, 2017

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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

ioquatix commented Feb 5, 2020

Copy link
Copy Markdown
Member

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

@ioquatix

ioquatix commented Feb 5, 2020

Copy link
Copy Markdown
Member

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.

5 participants