binwiederhier / ntfy

Send push notifications to your phone or desktop using PUT/POST
https://ntfy.sh
Apache License 2.0
18.4k stars 725 forks source link

Prefer Cloudflare IP header instead of X-Forwarded-For #905

Open ChokunPlayZ opened 1 year ago

ChokunPlayZ commented 1 year ago

:bulb: Idea I think there's not many configs like mine where the setup involves multiple reverse proxy (in my case NGINX and Cloudflare) the current "behind proxy" setting cause issue because NGINX attach 2 IP in the forwarded for header, ntfy always prefer the wrong one (the last one) instead there's the Cloudflare's Cf-Connecting-Ip which is more accurate than the X-Forwarded-For header it would be great if this could be added

or an alternative fix, if there is multiple IP in the X-Forwarded-For header always prefer the public IP over private ones because in my case here's how the header looks like: X-Forwarded-For: <mypublicip>, 172.17.0.1(docker)

:computer: Target components ntfy server

binwiederhier commented 12 months ago

I understand that the X-Forwarded-For handling is rudimentary. ntfy picks the rightmost address, since that is the most secure way to pick a trustworthy address. See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For for details.

To implement a proper X-Forwarded-For handling, we'd need to be able to configure a list of trustworthy proxies, e.g. proxies: 172.17.0.1. These proxies would be ignored in the selection of the IP address.

Adding a vendor-specific header like Cf-Connecting-Ip doesn't seem like a great idea, IMHO.

hipur commented 4 days ago

Maybe X-Real-IP could be beneficial, since it's easier to overwrite without reverse proxies adding another IP to the right. e.g.: traefik