better detection of forwarded / proxied HTTP protocol

[ahmadnassri]

while X-Forwarded-Proto is the defacto-standard, it is not actually a standard, and there has been many other alternative headers used that serve the same purpose, from the common (e.g. X-Real-Proto) to the obscure (e.g. cf-visitor) etc ...

forwarded-http handles all the commen headers as well as the less common ones, in addition to (and most importantly) defaulting to the RFC 7239 standard.

[dougwilson]

Hi! Please make which headers are checked configurable; many people will not have blacklisted things like Forwarded and people will now be able to spoof things. In addition, this really should be contributed directly to our dependency: forwarded instead of us requiring two different forwarded header parsing modules.

[dougwilson]

many people will not have blacklisted things like Forwarded and people will now be able to spoof things

scratch that, as I see it's only being used for the proto and it really isn't that critical if that gets spoofed.

[ahmadnassri]

Hi! Please make which headers are checked configurable; many people will not have blacklisted things like Forwarded

fair enough, can be easily configured through express's application settings, however, what would the default behavior be?

• backward compatibility: X-Forwarded-Proto only, however, I find that limiting, and as Express is a high level library, it should default to the HTTP standards.
• light backward compatibility: X-Forwarded-Proto + overwrite with Forwarded when present. (my preference)

happy to add that logic in.

and people will now be able to spoof things.

as per your comment, and everything is spoof-able with headers :)

In addition, this really should be contributed directly to our dependency: forwarded instead of us requiring two different forwarded header parsing modules.

I was studying proxy-addr to better understand what it does, as there might be some overlap with what forwarded-http does with IPs...

I reviewed forwarded and the reason I did NOT make changes there is because forwarded only deals with IPs, whereas forwarded-http looks for IPs, Protocol, Host, By as per RFC 7239 across many of the different implementations.

this means that proxy-addr could / should use forwarded-http in-place of forwarded .. unless you wish to keep a separation of modules for ip, proto, headers, etc ... (but lets discuss that further separately? I'd be more than happy to restructure and put under jshttp)

[dougwilson]

I reviewed forwarded and the reason I did NOT make changes there is because forwarded only deals with IPs, whereas forwarded-http looks for IPs, Protocol, Host, By as per RFC 7239 across many of the different implementations.

Correct, but there is no reason it cannot be expanded. The reason it's called forwarded is to match the HTTP Forwared header.

[ahmadnassri]

Correct, but there is no reason it cannot be expanded. The reason it's called forwarded is to match the HTTP Forwared header.

👍 PR coming up :)

[dougwilson]

as per your comment, and everything is spoof-able with headers :)

absolutely not true; reading of the headers is disabled in express by default and enabling the feature means you are saying you definitely trust the X-Forwarded-* incoming headers (i.e. saying you have a proxy that strips them and/or updates them). if your proxy updates them, then you really need to specify a specific depth or a proper IP trust chain, which is what proxy-addr; this is standard prod-ops configuration.

ALSO I have seen your avatar around (and even spoke with Mashape a few times), and if you are interested in joining jshttp, let me know offline and we can discuss :)

Typically, to keep the codebase of Express from becoming a giant monolith, we divide functionality into the hierarchy express -> pillarjs -> jshttp, and so typically desire contributions to head to relevant sub-modules in these organizations, if one exists, and then can be incorporated up the chain ;)

[dougwilson]

because forwarded only deals with IPs

P.S. the forwarded module has nothing to do with IPs; the X-Forwarded-For header is just a comma-separated list of proxy servers: there is no IP requirement in the de-facto spec (just like in Forwarded) ;)

[ahmadnassri]

absolutely not true; reading of the headers is disabled in express

right, I meant on the HTTP/network layer spoofing occurs, we're still using trust proxy setting before processing those values here. we're on the same page.

you really need to specify a specific depth or a proper IP trust chain

absolutely, I had added a simple regex-based filtering of ips in http-forwarded for this very reason, but I prefer the way proxy-addr does it actually.

monolith, jhttp, etc ...

👍 fully agreed, its good to get a conversation like this started, even if a PR gets closed / discarded, then move on to make the appropriate changes elsewhere as needed.

be happy to contribute my time to jshttp, and further any discussion offline, as it is already, I live in the HTTP world in Mashape daily (and in Node.js) what's the appropriate channel for further discussion? email, IRC?

[dougwilson]

I live in the HTTP world in Mashape daily (and in Node.js)

haha, that's what I figured ;)

what's the appropriate channel for further discussion? email, IRC?

Just email me and I would love to set something up to speak with you and really want to connect to grow these types of modules in Node.js :)

[ahmadnassri]

P.S. the forwarded module has nothing to do with IPs; the X-Forwarded-For header is just a comma-separated list of proxy servers: there is no IP requirement in the de-facto spec (just like in Forwarded) ;)

s/IPs/Addresses/g

another quick typing issue, i was thinking of "addresses" but defaulted to just typing "ips" :)

[ahmadnassri]

(FYI for future readers) as per discussion above, a PR to update the functionality of forwarded is now where this conversation will continue