"trust proxy" callback called multiple times on single request

[cullylarson]

I set up something like this to troubleshoot an issue with "trust proxy":

app.set('trust proxy', ip => {
    console.log({ip})
    return true
})

I noticed that each time I access req.ip from somewhere in the app, {ip} from the "trust proxy" callback was being logged twice. The first was 127.0.0.1 and the second was my load balancer's IP address.

My express app is behind a load balancer and an nginx reverse proxy. So there are two IP addresses in the x-forwarded-for header: the first is my browser's IP, making the request, and the second is the IP of my load balancer. Perhaps that's why it's being called twice?

The issue is that if I test for 127.0.0.1 or just set "trust proxy" to "loopback", "trust proxy" will not be enabled. This is because the first call returns true as expected, but the second call returns false (the load balancer is not a loopback IP).

I also noticed that there's an undocumented second parameter to the "trust proxy" callback. It appears to be set to the return value from the previous call to the "trust proxy" callback (i.e. if the first call returns true, this will be 1; if the first call returns false, this will be 0).

What I'm trying to do is trust traffic that comes through my nginx reverse proxy (i.e. when ip is 127.0.0.1), but that doesn't work as explained above. Is it reliable to use the second parameter and just return true if it is already true?

Also, why is it being called twice? Why is the value of ip for each call to the callback not the same as the values from x-forwarded-for? Would it be beneficial to update the documentation for "trust proxy" to describe this behavior and the second parameter?

[dougwilson]

I am mobile, so the best I can put into a quick response until perhaps tomorrow is that yes, you are seeing the expected behavior, and yes, using a custom function is underdocumented on our site.

Most use cases (including yours) do not need the usage of a custom function, which is likely why the underdocumentation has not been raised before. In your case, you need to pass two IPs as the config: the outbound IP of your loadbalancer and loopback.

[cullylarson]

I'm trying to come up with a solution that doesn't require configuration. I could provide the IP of the loadbalancer but I would have to add extra config for that. Also, if the IP ever changes, I'll have to change the config. I think I'm safe in assuming that if one of the IPs is localhost, I'm behind a proxy.

Is it safe to use that second parameter to the callback (i.e. the interface won't change in the future)?

[dougwilson]

We don't change API without a major version bump. If something is accidentally undocumented or untested, I would encourage you to contribute tests and docs around untested or undocumented features you are care about. That would prevent another contributor from accidentally changing the behavior, if it is untested.

As for your issue, it depends on how your loadbalancer is set up. If it replaces the entire contents of the header, so it guarantees the entire contents are trusted, then just use "true"; if it does not, then your solution may have untrusted data.

I cannot help make definitive statements around how you should set up security settings in your environment, as I cannot put myself in that liability situation, so ultimately it is something you will need to determine what is right based on your particular setup.

Just to note that our configuration also accepts subnets, if that makes it easier for your particular set up. You can also pass a number for number of hops to trust, which sounds like 2 in your case.