Rate-limit the /v1/rotate endpoint#1971
Open
0xVijay wants to merge 1 commit into
Open
Conversation
The /v1/rotate handler was registered without the throttled rate limiter that /v1/verify uses. Since rotate performs a full RSA-OAEP decrypt + re-encrypt on every request, an authenticated cluster user who can reach the endpoint via the kube-apiserver proxy can drive the controller's CPU to 100% and stall SealedSecret reconciliation cluster-wide. Wraps the rotate handler in the same httpRateLimiter.RateLimit(...) chain used for verify. Updates the help text on --rate-limit and --rate-limit-burst to reflect that both endpoints are now covered. Reported in GHSA-jpp5-6cf2-prqh.
Contributor
|
This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thank you for your contribution. |
Contributor
|
Due to the lack of activity in the last 7 days since it was marked as "stale", we proceed to close this Pull Request. Do not hesitate to reopen it later if necessary. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Wraps the
/v1/rotatehandler in the samehttpRateLimiter.RateLimit(...)chain that/v1/verifyalready uses, and clarifies the help text on--rate-limit/--rate-limit-burstto reflect that both endpoints are covered.Why
/v1/rotatedecrypts a SealedSecret with the controller's private key and re-encrypts it with the latest sealing key — a full RSA-OAEP decrypt + re-encrypt on every request. The default Helm chart bindssystem:authenticatedto the proxier role, so any user with a valid cluster credential can reach the endpoint via the kube-apiserver proxy and flood it. The CPU goes to 100% and SealedSecret reconciliation stalls across the whole cluster./v1/verifywas already rate-limited atpkg/controller/server.go:49. Rotate just needs the same wrapper.This was discussed in advisory GHSA-jpp5-6cf2-prqh — @alvneiayu accepted it as a genuine improvement and suggested opening a regular issue / PR. Sending the PR directly.
Change
Plus a one-word help-text update on the two existing rate-limit flags so the docs match the behaviour.
Notes
--rate-limit/--rate-limit-burstquotas so deployments don't need configuration changes.429 Too Many Requestsonce the per-source GCRA bucket is exhausted.