Respect X-Forwarded-Proto header

[jellybob]

This is an attempt to resolve #86

[coveralls]

Coverage increased (+0.02%) to 82.627% when pulling 9f1501b on neos-uk:no-forwarded-proto-overwrite into 52e57c0 on socketry:master.

[jellybob]

All three of these commits actually have the same effect (I was thrown off by #88), so take your pick which one you'd like and I'll rebase things.

[ioquatix]

Did we set RACK_URL_SCHEME anywhere else or did I just completely miss it?

Also, is this checked by rack linter?

[jellybob]

RACK_URL_SCHEME defaults to request.scheme, however that's not accurate in cases where we're listening on HTTP but have SSL terminated upstream.

[ioquatix]

If the user sent the X-Forwarded-Proto: https could that break the application that's running on http? Just wondering if there are security implications to this.

[bryanp]

For context, here's how puma sets RACK_URL_SCHEME:

env[RACK_URL_SCHEME] = default_server_port(env) == PORT_443 ? HTTPS : HTTP

Here's default_server_port:

def default_server_port(env)
  if ['on', HTTPS].include?(env[HTTPS_KEY]) || env[HTTP_X_FORWARDED_PROTO].to_s[0...5] == HTTPS || env[HTTP_X_FORWARDED_SCHEME] == HTTPS || env[HTTP_X_FORWARDED_SSL] == "on"
    PORT_443
  else
    PORT_80
  end
end

In theory if the application was exposed directly, you could bypass middleware to force ssl by sending X-Forwarded-Proto: https. Whether or not that has security implications I don't know, since it could only affect the current user's session.

I'm not certain on this, but I would think it's up to nginx, haproxy, or whatever is used in front of the application to set or reject these headers. Here's how Heroku handles it:

• https://devcenter.heroku.com/articles/http-routing#heroku-headers

[sfnelson]

@ioquatix the purpose of the scheme check is to protect the user from sending information over unencrypted connections. If the user provides the X-Forwarded-Proto: https it will only impact them. I don't think this is a security concern.

[ioquatix]

Thanks @bryanp - X-Forwarded-Proto is also not standardised... there is another header which is specified by the RFC... I need to review it.

The security issues are less of a concern, but I do want to hear more about it if anyone has any other concerns. Using the user facing headers... should probably be something explicit in the app... as it stands, the protocol reported by falcon is the last hop... but in this case we want the first hop...

[jellybob]

Typically this header will get explicitly set by the server doing SSL termination. I've tested this on Heroku and even if I send X-Forwarded-Proto: https on an HTTP request by the time it gets to my application server its been rewritten to http by their load balancer. In other places I've injected the header in nginx with similar behaviour.

[ioquatix]

Sorry, I have not come back to this and it's been sitting for a while.

There are a few options:

• This PR, which is great.
• Prefer Forwarded header.
• Something more elaborate like Puma.

There is one other issue.

The HTTP/2 pseudo header :scheme should probably be preferred which comes via request.scheme.

I don't really have a strong feeling on this right now, except that what we are doing is already the simplest and most straight forward approach.

Should RACK_URL_SCHEME reflect the pseudo header? Or should we allow other headers to override it?

What's stopping applications from parsing out the X-Forwarded-Proto if that's what they want? Should we do this by default?

I'm all for making those headers easier to work with, and I don't even mind if we should use a more elaborate method for setting the default RACK_URL_SCHEME (e.g. this PR). I'm also wary of imposing more complex logic into the field, which then becomes a bit of "technical debt" going forward, and will only get more complex when we need to deal with Forwarded: (multi-hop) as well as X-Forwarded-Proto.

Thoughts?

[ioquatix]

Bearing in mind, that if the client connection is HTTP, but the internal connection is HTTPS, that's also confusing.

What is RACK_URL_SCHEME used for? Determining if the connection is secure from end to end? Generating URLs?