feat(api): Add global IP rate limiting framework (ADR-0022 phase 1)#846
Draft
ymh1874 wants to merge 1 commit into
Draft
feat(api): Add global IP rate limiting framework (ADR-0022 phase 1)#846ymh1874 wants to merge 1 commit into
ymh1874 wants to merge 1 commit into
Conversation
5c825d0 to
346b29c
Compare
Implements ADR-0022 phase 1: a handler-level rate-limiting framework backed by the `governor` crate. Wires a global per-IP bucket on the `POST /v3/auth/tokens` handler, checking the limit before the CPU-intensive password-hash path (Invariant 4). Framework design: - `crates/config/src/rate_limit.rs`: reusable `RateLimitSection` struct (enabled/burst_size/replenish_rate_per_second) shared by all future buckets (per-user, per-domain, per-IdP). - `crates/core/src/rate_limit.rs`: `RateLimitState` with one `Option<Arc<DefaultKeyedRateLimiter<String>>>` per bucket. Disabled buckets cost only an `Option` discriminant. Includes `check_ip`, `retain_recent`, and IPv6 /64 prefix aggregation. - Fail-hard init (Invariant 2): `RateLimitState::from_config` returns `KeystoneError::RateLimitConfig` when `enabled=true` with zero burst or replenish rate, aborting startup rather than silently mis-configuring. - `KeystoneApiError::TooManyRequests` → HTTP 429 with `Retry-After` header. - SPIFFE bypass: `Option<Extension<ConnectInfo<SocketAddr>>>` as the first handler argument gives `None` on internal/admin mTLS interfaces (which don't populate `ConnectInfo`), so rate limiting applies only to the public TCP listener. - Background eviction task (60 s interval) calls `retain_recent()` on all keyed stores, preventing unbounded memory growth under adversarial unique-key flooding. Handler tests cover: - burst_size=1 with `ConnectInfo` injected: first request passes rate limit (auth error → non-429), second is rejected with 429 + Retry-After. - No `ConnectInfo` (SPIFFE bypass): both requests reach identity even after the burst is exhausted, and neither returns 429. Deferred: per-user, per-domain, and per-IdP buckets require keying on a confirmed user/domain ID after DB lookup but before password hashing — an invasive driver refactor tracked as a follow-up to this framework PR. Partially implements openstack-experimental#843. Note: This commit was done with the help of AI. Signed-off-by: Yousef Hussein <ymh1874@gmail.com>
346b29c to
c98c7cb
Compare
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.
Summary
governor-backed rate-limiting framework wired onPOST /v3/auth/tokens.RateLimitSectionconfig struct andRateLimitStateservice field so future buckets (per-user, per-domain, per-IdP) are a one-field addition to the framework.429 Too Many RequestswithRetry-After.ADR-0022 invariants addressed
[rate_limit_global_ip]inkeystone.confRateLimitState::from_configreturnsKeystoneError::RateLimitConfigand aborts startup whenenabled=truewith zero burst or replenish rateRetry-Afterheader; no key-identifying information exposedauthenticate_request(password hash)Arc<DefaultKeyedRateLimiter<String>>per bucket; deferred buckets areNonefieldsgovernor::DefaultClock=QuantaClockon std targets (TSC-backed, always monotonic)Invariants 7 (username normalization) and 8 (post-lookup per-user throttle) are deferred — they require keying on a confirmed user ID after DB lookup but before hashing, tracked as a follow-up driver refactor.
SPIFFE bypass
Internal (mTLS/TCP) and admin (mTLS/UDS) interfaces do not populate
ConnectInfo<SocketAddr>; the handler receivesNoneand skips IP limiting. Only the public TCP listener is subject to this check.Note:
Option<Extension<ConnectInfo<SocketAddr>>>is used (notOption<ConnectInfo<SocketAddr>>) because in axum 0.8,Option<T>: FromRequestParts<S>requiresT: OptionalFromRequestParts<S>, whichConnectInfo<T>does not implement butExtension<T>does.Files changed
Cargo.tomlgovernor = "0.10"workspace depcrates/core/Cargo.tomlgovernor.workspace = truecrates/config/src/rate_limit.rs(new)RateLimitSection— reusable config struct for any bucketcrates/config/src/lib.rsrate_limit_global_ip: RateLimitSectiontoConfigcrates/core-types/src/error.rsKeystoneError::RateLimitConfigcrates/core/src/rate_limit.rs(new)RateLimitState,check_ip,retain_recent, IPv6 /64 key derivation, unit testscrates/core/src/keystone.rsrate_limiters: RateLimitStatetoServicecrates/api-types/src/error.rsKeystoneApiError::TooManyRequests { retry_after }crates/api-types/src/error_conv.rsTooManyRequests→ 429 +Retry-Afterheadercrates/keystone/src/api/v3/auth/token/create.rscreatehandlercrates/keystone/src/bin/keystone.rstools/keystone.conf[rate_limit_global_ip]with default valuesTest plan
cargo fmt --checkcleancargo clippy --lib --testsclean on all modified crates[rate_limit_global_ip]with small burst, loopcurl -X POST /v3/auth/tokens, confirm401 → 429flip withRetry-AfterheaderPartially implements #843.
🤖 Generated with Claude Code