Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hubble node rate limits trigger when running behind proxy (NGINX) #1721

Open
obo20 opened this issue Feb 21, 2024 · 4 comments
Open

Hubble node rate limits trigger when running behind proxy (NGINX) #1721

obo20 opened this issue Feb 21, 2024 · 4 comments
Labels
s-ready Ready to be picked up

Comments

@obo20
Copy link

obo20 commented Feb 21, 2024

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:

  • Potential side effects on the hubble peer's gRPC advertised endpoint at the p2p layer.

tagging @varunsrin as requested in last week's dev call.

@github-actions github-actions bot added the s-triage Needs to be reviewed, designed and prioritized label Feb 21, 2024
@sanjayprabhu
Copy link
Contributor

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)

@sds
Copy link
Member

sds commented Feb 23, 2024

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.

@obo20
Copy link
Author

obo20 commented Feb 29, 2024

@sds fully agree, operators that are not running behind a reverse proxy would not want to trust that header.

@BlinkyStitt
Copy link
Contributor

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.

@sds sds added s-ready Ready to be picked up and removed s-triage Needs to be reviewed, designed and prioritized labels Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s-ready Ready to be picked up
Projects
None yet
Development

No branches or pull requests

4 participants