Support RFC 7239: HTTP Forwarded header

[fatkodima]

This is an updated version of #1017 with addressed Jeremy's comments and slightly changed implementation, but the logic remains the same as described in original PR description.

@ioquatix

Closes #1017
Closes #1310

[ioquatix]

This looks good, can you check failing tests?

[fatkodima]

Yes, I have checked that. Works on my machine with that ruby version and test seed. Can you trigger rebuild just in case? After that will look more closely.

[jeremyevans]

One issue with supporting Forwarded and trying to remain backwards compatible with X-Forwarded-* is that it can introduce security issues when used with older servers that only support X-Forwarded-*. For example, an attacker can specify the Forwarded header, and if the server sets X-Forwarded-For but rack prefers Forwarded if set, that would result in using attacker specified data. If you are trying to enforce that only certain IP addresses can access a resource through a trusted proxy, this opens up a security issue.

To be safe, if both headers are present, we should only use Forwarded in preference to X-Forwarded-* if the server specifies support for it. This would require an update to SPEC, allowing an entry such as rack.use_forwarded. If the entry is present, the Forwarded header is used and X-Forwarded-* is ignored. Otherwise, X-Forwarded-* is used and Forwarded is ignored.

@tenderlove, @ioquatix Does my analysis here make sense? If so, what are your thoughts on my recommended approach?

Other than that issue, I reviewed this and I'm OK with the changes. The conflict is only due to documentation I added recently, and easily resolved. There are some unrelated changes that would better have been submitted in separate PRs, but I don't think would cause us not to merge this once the security issue is addressed.

[matthewd]

@jeremyevans I don't see what support the server implementation would need to have -- its opinions are in REMOTE_ADDR and friends. The (IMO near-insurmountable without explicit configuration) challenge is that we need to know which header was supplied by the immediate-upstream proxy... and we have no means of direct communication there that can be trusted.*

Perhaps, as it's a relatively new header in the real world, it wouldn't be unreasonable for us to declare that an upstream that sets Forwarded is only compliant [and thus supported] if it also guarantees to strip the others -- leaving us free to either treat a mixed header set as we do now, or refuse the request a la https://github.com/rails/rails/blob/7b29bc2179702a04613068e6e6efd6cf7f343440/actionpack/lib/action_dispatch/middleware/remote_ip.rb#L136

---

* ... unless we can trust all reverse proxies to swallow an incoming Connection header per spec, in which case we could rely on Connection: forwarded. It's my understanding that proxies can be pretty loose with spec behaviour, though -- reverse proxies especially.

[jeremyevans]

@matthewd I think you are correct. This probably cannot be handled by SPEC since it isn't handled by the server itself. Still, we should probably do something to prevent a security issue being introduced. If we assume that proxies that support Forwarded will correctly convert X-Forwarded-* to Forwarded as recommended by the RFC, then the appearance of both could be considered an error. However, as @ioquatix mentioned in a related issue, some proxies may set both to the same values, which also seems reasonable. Maybe the logic should be:

if forwarded_set
  if x_forwarded_set
    if forwarded_set_has_same_info_as_x_forwarded_set
       use either
    else
      raise exception
    end
  else
    use forwarded_set
  end
elsif x_forwarded_set
  use x_forwarded_set
else
  not forwarded, use other header
end

[ioquatix]

@jeremyevans can you assign this to a milestone, either 2.2 or 3.0 depending on what you think makes the most sense?

If servers need to be on board with the change, I'd say 3.0 wouldn't be a mistake.

[lunich]

It's possible that get_header(HTTP_X_FORWARDED_HOST) == '' in some cases. For instance:

curl http://example.com -H 'X_FORWARDED_HOST;' -H 'HOST: example.com'

or

telnet example.com 80
Trying 127.0.0.1...
Connected to example.com.
Escape character is '^]'.
GET / HTTP/1.0
HOST: example.com
X_FORWARDED_HOST:

[ioquatix]

@fatkodima do you mind rebasing on master?

[fatkodima]

@ioquatix I'm sorry, I'm no longer in the context. Feel free to do with this PR whatever you want.

[ioquatix]

@jeremyevans has created a new PR for this feature/functionality.