search

m-lab / ndt-server

docker native ndt5 and ndt7 server with prometheus integration
https://www.measurementlab.net/
Apache License 2.0
101 stars 41 forks source link

feat(ndt7): Use X-Forwarded-For hint for the client's IP address #404

Closed auvik-bheesham closed 6 months ago

auvik-bheesham commented 6 months ago

When running behind a proxy the RemoteAddr in various places won't return the real client's IP. This commit uses various hints to get it, falling back to using the default.

cc @pthankappan

This change is Reviewable

robertodauria commented 6 months ago

Hello, thank you for submitting this PR.

Unfortunately, for ndt-server to function properly, it requires an unterminated end-to-end TCP connection. The use of a reverse proxy, which would terminate the connection, is not supported. This will lead to inaccuracies in the TCPInfo, BBRInfo, and other network-related data, as they would be associated with the connection between the server and the reverse proxy instead of the server and the client.

Also note that if you are running the fullstack ndt-server, which includes sidecar services, all of them will still record the proxy's IP rather than the client's.

Additionally, data transmission to or from the client would be handled by the proxy's network stack, rather than the NDT server's network stack, resulting in inaccurate application-level metrics.

For these reasons, we recommend avoiding the use of ndt-server behind a reverse proxy and not overwriting the ClientIP field.

auvik-bheesham commented 6 months ago

Hey! Thanks for the detailed response, I let the team know. Lots of new info here for me.

We'll reconsider our approach. We might end up finding some middle ground here, unsure. We've already got a bunch of infra set up and some requirements along with that, so there's going to be some acrobatics required to fit this in.

Either way, we learned a lot. Thanks again!