Make forwarded scheme handle comma separated schemes

[kgann]

When using HAProxy to add X-Forwarded-Proto via reqadd X-Forwarded-Proto ... and the header already exists, the value will be comma separated. This causes an assertion error to be thrown.

The real issue is with the HAProxy configuration as it should detect the presence of the header and behave accordingly. I'm not sure if this should be a concern of ring-ssl. Thoughts?

[jcf]

The draft spec for x-forwarded-proto suggests the header is supposed to inform a client which protocol is being used. The idea being you can modify your response to provide the correct protocol in links etc.

I'm not sure http, https makes sense in this context, and wonder how an application would behave with a confusing value like this.

Can HAProxy be configured to pass http or https based on which is actually being used?

[jcf]

A quick Google seems to suggest HAProxy can be configured to do the 'right' thing.

• https://www.digitalocean.com/community/tutorials/how-to-implement-ssl-termination-with-haproxy-on-ubuntu-14-04
• http://serverfault.com/questions/302282/how-can-i-use-haproxy-with-ssl-and-get-x-forwarded-for-headers-and-tell-php-that

[kgann]

This issue is 100% due to a poorly configured HAProxy.

I'm not sure http, https makes sense in this context, and wonder how an application would behave with a confusing value like this.

Perhaps the test is confusing - the real header value that broke the application was "https,https".

Originally HAProxy was configured correctly and the application worked fine. Someone made a change (duplicating: reqadd X-Forwarded-Proto:\ https) that caused the app to throw an assertion error.

The fix is to configure HAProxy correctly but I fear that in the future, a similar change could bring down the application again. I decided to open a PR and get some input.

At any rate, I'm going to have to duplicate this middleware to prevent the errors in the future. I'm fine with closing this PR since you can't be expected to accommodate all possible incorrect header values.

[jcf]

https,https isnt a valid value according to the spec either…

A few ideas spring to mind:

1. Change the code to accept any valid scheme rather than just HTTP and HTTPS
2. Return a client error rather than server error when we can't parse the header
3. Expect clients to be well behaved (which is perhaps naive)

What do you think @weavejester?

[weavejester]

In HTTP, all header values can be concatenated together with a comma, so my guess is that it is technically correct, but it doesn't make any practical sense. For security purposes the proxy should replace the whole header, since otherwise a HTTP client could make the server believe that it's actually being served over HTTPS.

Maybe we should have a more detailed error message, but I think this should throw an exception, because of the security implications.

[kgann]

I think the assertion did its job in alerting me to the issue. Thanks for the feedback.

[rwilson]

Could reopening this issue be considered?

The Forwarded HTTP Extension spec considers a chain of proxy servers and specifies an order for the chain. Further, AWS ELB behavior appends to the chain of x-forwarded-* headers, and given the wide usage of that service, it might be deemed close to standard.

For context:
We encountered this while debugging issues with using Readme.io's ability to test API calls from the browser. Their system proxies the request through a chain, each of which appends the previous client to the x-forwarded-* headers, before arriving at our ELB, which appends the final proxy from Readme.io. By the time it reaches us, the header value is "https,https,http,http,http,http,http,http".

Bottom line: there are situations where you can't control all hops, and ring-ssl could be made flexible enough to handle those in a non-breaking manner.

A final note, with regard to:

For security purposes the proxy should replace the whole header, since otherwise a HTTP client could make the server believe that it's actually being served over HTTPS.

The opposite is also true: by replacing the whole header, a proxy could make a server believe the request is HTTPS end-to-end, when it may not be. Ultimately, any proxy outside your network (and maybe those inside?) can't be trusted.

[weavejester]

Further, AWS ELB behavior appends to the chain of x-forwarded-* headers, and given the wide usage of that service, it might be deemed close to standard.

Do you have a reference for that? It seems reasonable to support the behavior if that's how AWS behaves, but I'd like a "paper trail" justifying the decision.

The opposite is also true: by replacing the whole header, a proxy could make a server believe the request is HTTPS end-to-end, when it may not be.

Sure, but if you have a proxy inside your network then that should take priority, and if you don't, then you shouldn't be using the middleware. By using the middleware you're saying: "I know a trustworthy proxy will set this value."

[rwilson]

Regarding the paper trail, the AWS documentation doesn't specify the behavior of ELBs if x-forwarded-* headers are present on an incoming request, so I have reached out to support for clarification. I'll follow-up when I've heard back.

[rwilson]

Finally got sufficient context on this; it depends on the ELB listener mode.

If configured as HTTP/HTTPS, the behavior is, from AWS:

"X-Forwarded-For” – ELB will append to existing value if this header existed in request header, otherwise it will be added
"X-Forwarded-Proto” – ELB will add this header. If this header existed, ALB will replace it.
"X-Forwarded-Port” – ELB will add this header. If this header existed, ALB will replace it.

However, if configured as TCP/SSL (e.g. w/ proxy protocol to support WebSockets), the behavior is to preserve the client headers and append-only. Quoting AWS:

When you use TCP (layer 4) for both front-end and back-end connections, the load balancer forwards the request to the back-end instances without modifying the headers. In that case, you should see the headers as they are sent by the client.

[weavejester]

Do you have references for those quotations? Or is that just what support told you?

[rwilson]

Unfortunately not, that was from the support response.