Repository navigation
Conversation
✅ Deploy Preview for expressjscom-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
For reviewers' convenience, here are the rendered pages, which aren't actually linked from anywhere in the deploy preview site: |
bjohansebas
left a comment
bjohansebas
left a comment
There was a problem hiding this comment.
just a few minor comments; I’m not an expert on proxies.
| ```js | ||
| app.set('trust proxy', 1); // for an application behind a single proxy | ||
| ``` | ||
|
|
There was a problem hiding this comment.
if the application trusts two proxy hops, it expects a path like Client → Proxy A → Proxy B → Application. However, if a client can also reach it through Client → Proxy B → Application, a forged value in X-Forwarded-For could be treated as the client’s IP address
It might be a good idea to add a note explaining this and recommending trusted proxy addresses or subnets when paths can have different lengths.
| <Alert type="warning"> | |
| Only use a hop count when every possible path to your application has the expected number of trusted proxies. Blocking direct access alone is not sufficient: clients may still reach the application through a shorter proxy chain and supply a forged X-Forwarded-For value. If paths can have different lengths, configure trusted proxy addresses or subnets instead. | |
| </Alert> |
|
|
||
| Many proxies do not overwrite `X-Forwarded-Host`, so your application should not derive its own host from the request at all. A forged value pollutes anything built from `req.hostname`, such as a password reset link that now points at the attacker's domain. Put your canonical host in configuration and use that instead. | ||
|
|
||
| When using a count as your `trust proxy` setting it is important to ensure your application is only reachable through the proxy and not directly from the internet. This is always a good idea behind a proxy, but applications that use a count are more exposed than those that use a range: a range checks the connecting address against your list before trusting any forwarded header, whereas a count trusts whichever address connected as the first hop, without checking what it is. |
There was a problem hiding this comment.
| When using a count as your `trust proxy` setting it is important to ensure your application is only reachable through the proxy and not directly from the internet. This is always a good idea behind a proxy, but applications that use a count are more exposed than those that use a range: a range checks the connecting address against your list before trusting any forwarded header, whereas a count trusts whichever address connected as the first hop, without checking what it is. | |
| When using a hop count as your trust proxy setting, ensure clients cannot reach the application directly or through a shorter proxy chain than configured. Blocking direct access alone is not sufficient: for example, a client might bypass a CDN and connect directly to your load balancer. On that shorter path, Express may accept a client-supplied X-Forwarded-For value as req.ip. An address range or list checks the connecting address before trusting forwarded headers, whereas a count trusts hops based only on their position. Prefer trusted proxy addresses or subnets when paths can differ in length. |
|
|
||
| When you deploy an Express app behind a proxy, by default your application is no longer seeing the IP address in `req.ip` of the actual client who sent the request, as well as possibly seeing a different `req.hostname` or `req.protocol` than the original client used. This is a problem if your application is relying on those fields to make decisions like IP based rate limiting, including IP in access logs, or whether to set a session cookie marked `Secure`. | ||
|
|
||
| By default, Express sets `req.ip` from the socket the request arrived on, not from any header. When your application is directly reachable over the internet, that address is the client that made the request. When your application sits behind a proxy, such as a load balancer or a reverse proxy like Nginx, every request arrives from the proxy, so `req.ip` is the proxy's address, not the client's. The same is true of `req.hostname` and `req.protocol`: behind a proxy that terminates TLS, Express sees a plain HTTP request from the proxy. |
There was a problem hiding this comment.
| By default, Express sets `req.ip` from the socket the request arrived on, not from any header. When your application is directly reachable over the internet, that address is the client that made the request. When your application sits behind a proxy, such as a load balancer or a reverse proxy like Nginx, every request arrives from the proxy, so `req.ip` is the proxy's address, not the client's. The same is true of `req.hostname` and `req.protocol`: behind a proxy that terminates TLS, Express sees a plain HTTP request from the proxy. | |
| By default, Express derives `req.ip` from `req.socket.remoteAddress`, which identifies the peer connected directly to the application. When your application sits behind a proxy, such as a load balancer or a reverse proxy like Nginx, this is normally the proxy's address rather than the original client's. `req.protocol` reflects whether the connection to Express uses TLS: if the proxy terminates TLS and forwards the request over HTTP, Express reports `http` even when the client used HTTPS. `req.hostname` comes from the `Host` header, so it can still reflect the original hostname if the proxy preserves that header. |
|
There's a fair amount to unpack here, and I'm going to have to come back to this with more detailed comments. At a high level, I think we need to review and update the two other places that document
There is some duplication between these three docs, which is OK... IF there is a good reason for it:
All three of these docs use CIDR notation in examples, with no explanation or references. While many developers may be familiar with it, we should at least say something like "We're assuming you're familiar with CIDR notation used for trust proxy option values" (with xref link). Also the API ref confusingly mixes IPv4 and IPv6 CIDR notation in the explanation of loopback, linklocal, and uniquelocal. This can easily be improved. Finally, since the |
This branch has not been deployed
3 participants
This PR introduces the trust proxy spoke page for the security hub.
Suffice to say, I learned just how much I did not understand trust proxy while writing this!
Unrelated to this PR, but I do feel pretty strongly that we should rework this setting slightly in v6, specifically dropping the
trust proxy: trueoption as I am convinced now that it is a footgun. We should also push users to setting an address range and not a hop count for this setting, which I realized very late into the process of writing this documentation, and these docs now reflect that better advice.This PR also tweaks the two hub items I had previously for trust proxy, in order to better reflect what I have learned developers should ask themselves to determine if they are correctly using this setting, and if they are using it w/ a correctly configured proxy.
The structure I went for here is what I expect to do for the rest of the spokes:
Feedback on structure is highly relevant here!