fix: use nginx realip module #2977
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found Additional details and impacted files@@ Coverage Diff @@
## master #2977 +/- ##
=======================================
Coverage 99.01% 99.01%
=======================================
Files 3 3
Lines 203 203
=======================================
Hits 201 201
Misses 2 2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
We recommend to use an external load balancer (such as nginx) in addition to self-hosted docker-compose: https://develop.sentry.dev/self-hosted/#productionalizing
Usually, a load balancer sends X-Forwarded-For request header so it will always be the case 1 (from above list), and sentry container will get the correct client's IP address.
There's a page dedicated for that, but it's missing nginx (because at the time of writing, I don't use nginx), it'll be nice if you can also take a look to fill some missing gaps (if any): https://develop.sentry.dev/self-hosted/reverse-proxy
nginx/nginx.conf
Outdated
| set_real_ip_from 10.0.0.0/8; | ||
| set_real_ip_from 172.16.0.0/12; | ||
| set_real_ip_from 192.168.0.0/16; |
There was a problem hiding this comment.
This is up for debate, but I think proper list of internal subnets should be acquired from here: https://en.wikipedia.org/wiki/Reserved_IP_addresses. Also considering that the address might come from an IPv6 address. But if this turns out to be out of scope, then these addresses are fine.
There was a problem hiding this comment.
These should be docker default subnets which are:
172.17.0.0/16
172.18.0.0/16
172.19.0.0/16
172.20.0.0/14
172.24.0.0/14
172.28.0.0/14
192.168.0.0/16
10.0.0.0/8
There was a problem hiding this comment.
@aminvakil Thank you for the reference! That seems the most correct subnet list.
| # Remove the Connection header if the client sends it, | ||
| # it could be "close" to close a keepalive connection | ||
| proxy_set_header Connection ''; | ||
| proxy_set_header Host $host; | ||
| proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; | ||
| proxy_set_header X-Forwarded-For $remote_addr; |
There was a problem hiding this comment.
Note: this might break some installations that have multiple hops before this Nginx, e.g. when the self-hosted installation is behind Google/AWS load balancers that normally add a bunch of records to X-Forwarded-For header automatically.
That means that we should highlight this in the release notes, and point to recommendations around "how to pass the trusted client IP to the Sentry backend".
I'm adding it here: getsentry/develop#1257 |
By default, Sentry gets the client's IP address from the
X-Forwarded-Forheader:https://github.com/getsentry/sentry/blob/master/src/sentry/conf/server.py#L3080
However, this header can be spoofed. The more correct approach adhering to standard is suggested in getsentry/sentry#68884
However, before going forward with it, we need to make sure that we are not breaking self-hosted installations.
This PR makes sure that the
X-Forwarded-Forheader that is sent along the request to thesentrycontainer, contains the real IP address of the client, and it's the only one, no other values in the list.We do so by using nginx realip module in the
nginxcontainer of self-hosted installation:X-Forwarded-Forheader coming from internal subnets (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16), then we trust it and use the rightmost IP address that does not belong to the said subnets. E.g in the case ofSPOOFED_IP, MY_REAL_IP, 172.17.0.0it will beMY_REAL_IP;X-Forwarded-Forheader coming from the other untrusted subnet, then the rightmost (untrustworthy) value will be used;X-Forwarded-Forheader in a request to thenginxcontainer, then the IP address of the connection will be used (as usual).We recommend to use an external load balancer (such as nginx) in addition to self-hosted docker-compose: https://develop.sentry.dev/self-hosted/#productionalizing
Usually, a load balancer sends
X-Forwarded-Forrequest header so it will always be the case 1 (from above list), andsentrycontainer will get the correct client's IP address.