ActionCable: Same origin match against X-FORWARDED-HOST when proxying

[skateman]

When using allow_same_origin_as_host with a reverse proxy the HOST header is set to the proxy host which will cause the CSRF protection to forbid the request. Each time a request hits a reverse proxy, it
updated the HOST header and also prepends the host from the previous request to the X-FORWARDED-FOR header. When the request finally hits the server where ActionCable is running, the first host in the header will be the right one for us.

I am proposing to change my original idea introduced in 268c340 to test against the first element of this header when it is set.

[matthewd]

Hmmm... my first reaction is that I'm not sure this is safe, because we can't be sure the upstream will set/clear that header, even if we establish that the upstream is itself trusted. (The latter is where RFC 7239 comes in, but until it's more widely adopted we have the same issue that we can't assume it itself didn't come from somewhere unknown.)

Looking back to my reasoning in #26568 (comment), however, I think that might suggest this is still safe: a victim browser pointing at an attacker's DNS name won't send cookies that belong to the site's real DNS name, regardless of whether they're talking directly to our server or to an attacker-controlled header-modifying proxy. The danger is exclusively that an attacker can cause a victim's browser to connect to our server, using our DNS name, from an attacker-controlled Origin -- and barring [out-of-scope] DNS hijacking, I think we're still covered. (Assuming it's not possible for JS to specify a header value. Host is uniquely safe in that regard, so we'll need to be suitably sure before extending that assumption to arbitrary headers.)

I think the trust issues above do still preclude our relying on that header for other uses where knowing our hostname is actually important... we just might have weaker requirements in this particular instance.

Incidentally, I think most reverse-proxy configurations side-step this sort of problem by having the proxy pass along the original Host value unchanged.

[skateman]

@matthewd this depends on the ProxyPreserveHost setting, unfortunately, we have setups where we have this turned off 😞

[skateman]

@matthewd when we mentioned you've mentioned there's an issue with this, can you please explain it here?

[matthewd]

@skateman sorry; conference-brain. My concern was in here: https://github.com/rails/rails/pull/34716/files#diff-e7a49345fe344d173cbcd874c21a1a61R203

If HTTP_X_FORWARDED_HOST is set, it will ignore HTTP_HOST. That's probably a reasonable implementation of your goal, but it does mean a potential incompatibility for people unknowingly relying on the current behaviour. So my theory was that we should check both HTTP_X_FORWARDED_HOST and HTTP_HOST, and accept it if either one matches HTTP_ORIGIN.

However, looking back over this now, I'm worried it moves us much closer to a scenario where a malicious site can bypass the origin check entirely. Are we sure a nefarious proxy can't use this to make a victim browser successfully connect to a target ACa server? (That's basically the vector the origin check is there for, right?)

[rails-bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

[grk]

Ran into this problem recently so resurrecting this thread.

I think if we don't want to trust X-Forwarded-For there are bigger issues - Rack::Request goes through all forwarded headers by default when calling request.host or request.host_with_port and it's used in many places.

My idea for a fix that I'm currently testing in our deployment with a monkeypatch is this:

request = Rack::Request.new(env)
proto = request.ssl? ? "https" : "http"
if server.config.allow_same_origin_as_host && env["HTTP_ORIGIN"] == "#{proto}://#{request.host_with_port}"
  true
# ...