-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Implement rate-limit by IP address for critical endpoints #1970
Comments
Slightly related to #1941 |
I think it'd be better to move the healthz endpoint to the telemetry server instead. It shouldn't be public in the first place. As for other rate limits: I'm not a huge fan of tackling this issue from applications. Although Dex is not technically HA at the moment, it would perfectly make sense to run it in a HA setup. Then you need a distributed storage to store the bucket for rate limiting. IMHO Dex should rarely be running on it's own. There is usually a proxy/load balancer more equipped to handle rate limiting. |
@sagikazarmark thanks for the quick answer. I have two questions:
|
It's more of a theory than actual issues. The gc job inside Dex can technically make instances step on each others toes.
That's exactly what I was aiming at. Not necessarily concrete solutions, but overall definition of the problem with some general suggestions. Either from an attack vector or an operational best practices point of view. Documenting the related use cases (public vs internal Dex instances) might also be a good approach. |
Is your feature request related to a problem?
On every request to auth/healthz endpoint, dex creates an authrequest entity in storage. An intruder with any HTTP load generator can cause an outage.
Describe the solution you'd like to see
It would be marvelous to add middleware to limit requests on certain endpoints using the token/bucket algorithm.
Describe alternatives you've considered
We can add a doc with suggestions to configure some external tool, e.g., proxy, load balancer. Users need to know which endpoints they need to secure and how.
However, with this variant, we have no option using just dex (and limit all requests in one place).
Additional context
It causes a lot of pain in environments without proxy in front of a dex. I have faced that problem myself.
There is a comment suggesting the same.
The text was updated successfully, but these errors were encountered: