express icon indicating copy to clipboard operation
express copied to clipboard

Add CloudFront-Forwarded-Proto support

Open touzoku opened this issue 2 years ago • 6 comments

Hi,

I know it is not a very standard header to support to, but it has been very hard to patch expressjs every time we need to use CloudFront in front of a bare express app. I am not sure why AWS does not stick to the standard headers in the first place.

Would it be possible to add support for the CloudFront-Forwarded-Proto header as I have demonstrated in this pull request?

touzoku avatar Nov 15 '21 07:11 touzoku

Hi, sorry you are having trouble. I don't think we can just add this as a default as existing folks using this feature will not have that header stripped at their edge, possibly resulting in a security issue due to client spoofing.

dougwilson avatar Nov 15 '21 07:11 dougwilson

@dougwilson Thank you for a prompt response.

X-Forwarded-Proto unlike security-critical headers such as X-Forwarded-For does not enable any attacks other than potential self-exploitation. Please let me know if I am missing anything here.

touzoku avatar Nov 15 '21 07:11 touzoku

We do not know what applications are deciding to do with req.protocol, but Express does use it to determine if secure cookies are safe to send or not, which I'm sure would raise a security issue.

We can possibly add this header as an opt-in flag that you can see in your app if you need to change the header, is that an option for you?

dougwilson avatar Nov 15 '21 07:11 dougwilson

@dougwilson yes, an opt-in configuration would help. Actually, it makes more sense, as it is a non-standard header.

As for security implication (not for the sake of argument), I do understand secure cookies concept. There is no way to force a victim's browser to send this header. So the only risk is that attacker will receive her own cookies via an unencrypted connection (aka self-exploitation). If the application does something else based on req.secure (e.g., dumps all the passwords if certain header is present), it would be as vulnerable with the original X-Forwarded-Proto header.

touzoku avatar Nov 15 '21 08:11 touzoku

So I took a look at other frameworks that mention this header and it seems they document how to use this would result in adding the following line to your Express app. Does this work for you? I cannot find any framework that supports this header out of the box and they all document solutions that would look like the following in Express

app.use((req, res, next) => { req.headers['x-forwarded-proto'] = req.headers['cloudfront-forwarded-proto']; next() })

Please let me know if that works for you or not (sorry I don't have cloudfront to test with). If it does not work, please if possible elaborate why.

dougwilson avatar Nov 15 '21 08:11 dougwilson

It looks like there is actually a module on npm for this: https://www.npmjs.com/package/@0xc/forward-cloudfront-proto

dougwilson avatar Nov 15 '21 08:11 dougwilson