Read X-Forwarded-Proto header if trustProxy flag set

[stongo]

If hapi is behind a reverse proxy that sets a X-Forwarded-Proto header, a relative redirect is unable to use it. Creates an issue when hapi i is running on http behind a https reverse proxy - redirects go to http instead of https.

This PR adds a trustProxy flag to server.settings and uses the X-Forwarded-Proto header if it exists when setting baseURI

[stongo]

any feedback on this?

[hueniverse]

I don't like the new settings. I would like to find a way to make this easier. There is already another open issue related to this.

[stongo]

How do you think the best way to handle it would be? I think a setting
somewhere to accept X-Forwarded- headers is still necessary. Let me know
if you have any ideas and I'll be glad to implement

On 2014-07-14, 6:08 PM, Eran Hammer wrote:

I don't like the new settings. I would like to find a way to make this
easier. There is already another open issue related to this.

—
Reply to this email directly or view it on GitHub
#1747 (comment).

[stongo]

Would a route setting to parse X-Forwarded-Proto headers be better?
Sorry to push on this a bit, but this is somewhat of a production
blocker without adding workarounds in business logic.

[hueniverse]

Where does your X-Forwarded-Proto come from? I'm deep down the x-forwarded rat hole...

[jonathanong]

btw @dougwilson has https://github.com/expressjs/proxy-addr, though he's in the process of refactoring it.

[hueniverse]

@jonathanong I'm working on a new module forwarded that will handle all the stupid forwarding headers around. It's a cesspool. It will only parse the headers, without any concept of security or trust. That belongs elsewhere.

[dougwilson]

lol. I actually already have a module that does this, but have been waiting for work to reassign the copyright to me (people are on vacation and little me is not a priority). I was thinking of uploading it as forwarded from koajs/koa#281 but if you want to use that name, I can upload it as something else :) there are 3 different x-forwarded-for formats alone! Haha. The trust is not part of it, just parsing; the trust is going to be a diff module. Not sure if you are interested in my work here :)

[dougwilson]

Oh, haha, @hueniverse I see you already uploaded a stub there :) I was actually just going to do that for you (because I didn't want to take the name you already wanted). I do already have copyright over a portion of the module I could potentially upload.

The part that I'm waiting copyright for is the configurations: forwarding headers are different from different providers since they are non-standard. I have support for Mashery, common nginx, common apache, IIS, varnish, haproxy, Elastic Load Balancing, NodeBalancer, Stackato, Akamai GTM, and Heroku so far. Some of those use Client-IP instead of X-Forwarded-For, for example, and some don't even erase X-Forwarded-For, so you have to know ahead of time which headers to use or you can end up with a forged header :/

Anyway, just wanted to share :) It was something I want to put under jshttp, haha. Especially RFC 7239 support.

[hueniverse]

@dougwilson If you want to do this, I am happy to hand over the name. I need an API that can:

• parse the common headers as present (the user can configure which one to use)
• append or set correctly when proxying (including option to "upgrade" to the new Forwarded header)

If this is in your plan, I rather you did it :-)

[dougwilson]

@hueniverse yes :) and since you mentioned this, i prodded legal this morning and i'm actually having a meeting soon, so i should be able to get some code up you can peruse to make sure it meets your needs :) part of jshttp itself i'm planning to provide parser/formatter modules for all the non-trivial http headers (included the forwarded family) for use by other modules (they are low-level and provide plain string-manipulation to be agnostic, so hopefully it'd work for you?).

if you want, you can add dougwilson to the npm placeholder and we can go over what i push (to a repo, not npm), which seems likely to be tomorrow \m/

[hueniverse]

@dougwilson added. Once you have your repo setup and npm switch over, I'll erase the hapijs placeholder.

[stongo]

Thanks for all this! Let me know if you need any help

[dougwilson]

As an update this is almost ready :)

[stongo]

Checking in if there has been any forward motion with the "forwarded" package?

[dougwilson]

I am actually uploading the initial version of forwarded tonight (US time). The trusting part also has an initial version tonight, as the npm module "trust".

[hueniverse]

@dougwilson I'll take a look as soon as you think the API is stable. Will only use it once it is published as 1.x.x.

[dougwilson]

Yep, I agree with the 1.0 comment. I will be first publishing as 0.x so I can reflect on what just published and dog food it, which I like to do :) At jshttp we decided we would not keep thing at 0.x for very long, since people don't like that :)

[hueniverse]

This is no longer relevant given that Location header handling is removed in v8.0 given the recent changes to the HTTP protocol that no longer mandate an absolute URI. If you still need to convert URIs to absolute ones, I suggest writing a plugin with your specific rules to accomplish it.

Sorry it too so long to resolve this.

[lock]

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.