Open obo20 opened 6 months ago
Good point, we could trust the x-forwarded-for by default.
This won't affect the advertised grpc endpoint (I don't think the rate limits apply to the sync endpoints used by other hubs)
I don't think we should trust X-Forwarded-For
by default (allows an attacker to avoid rate limiting by spoofing a different IP each time) but agree a configurable option to trust X-Forwarded-For
, and then have the reverse proxy responsible for parsing X-Forwarded-For
and converting appropriately if we're in a chain of load balancers.
@sds fully agree, operators that are not running behind a reverse proxy would not want to trust that header.
We are running a hub behind an amazon load balancer and ran into this. Amazon sets X-Forwarded-For for us, so an option on the hubble to opt-into trusting it would be much appreciated.
What is the bug? When operating behind a reverse proxy, hubble currently views all grpc requests as coming from a singular internal IP, which quickly triggers the hubble internal rate limiting defined by the rpc-subscribe-per-ip-limit variables.
Ideally hubble would be be able to trust the x-forwarded-for header (metadata for grpc) and utilize that IP for rate limiting. This could be an optional setting turned on if desired, similarly to how frameworks like expressJS have a trust-proxy setting you can turn on.
Current unknowns that it would be useful to get further context on before we submit a PR:
tagging @varunsrin as requested in last week's dev call.