Issue#139 - security: rate-limit /oauth2/bc-authorize (per-client + per-user)

SECURITY.md

@@ -61,3 +61,48 @@
 
   We thank everyone who has reported issues responsibly. Confirmed reporters who
   opt in are listed in the published advisory and our release notes.
+
+  ## Production hardening notes
+
+  ### Backchannel (CIBA) rate limiting
+
+  `/oauth2/bc-authorize` is rate-limited per `client_id` and per
+  `login_hint` to prevent end-user notification spam,
+  `backchannel_auth_requests` table flooding, and `login_hint`
+  enumeration.
+
+  **Bucket key composition.** Buckets are keyed on `client_id` and
+  `login_hint` alone — NOT on `(client_id, tenant)` or
+  `(login_hint, tenant)`. `account_id` / `project_id` arrive in the
+  request body and are not bound to the client (clients in ZeroID are
+  global). Including them in the bucket key would let an attacker
+  bypass the cap by supplying random tenant values per request. Tenant
+  fields are still included in the WARN log event for forensics.
+
+  The default implementation is an **in-process, in-memory token bucket**.
+  This is correct for single-instance deployments. **Multi-replica
+  deployments need a shared-store backend** — without one, each replica
+  enforces the limit independently and a fleet of N replicas effectively
+  permits N× the documented cap.
+
+  - **Default config:** 10 req/min per client, 5 req/min per user. Tunable
+    via `BackchannelConfig.PerClientRateLimitPerMinute` and
+    `PerUserRateLimitPerMinute` (set to `0` to disable a dimension).
+  - **Memory bound.** The in-memory limiter caps tracked keys at 100k by
+    default; beyond that, requests for new keys fail open. Operators can
+    inspect `TokenBucketLimiter.BucketCount()` and alert before the cap
+    is reached. The cap exists because `login_hint` is attacker-supplied
+    and otherwise unbounded.
+  - **Multi-replica:** implement `zeroid.RateLimiter` against your shared
+    store (Redis, Memcached, hosted KV) and install via
+    `Server.SetBackchannelRateLimiters(perClient, perUser)`. The interface
+    has two methods (`Allow`, `Stop`); the fail-open contract is documented
+    on the interface.
+  - **Defense-in-depth:** run an edge limiter (CDN, nginx, Envoy) for
+    per-IP throttling regardless. The in-process limiter exists for
+    per-actor semantic enforcement that an edge limiter cannot express.
+  - **Detection:** rate-limit rejections emit a structured WARN log with
+    `event=bc_authorize_rate_limited`, `reason`, `client_id`, tenant IDs,
+    and a `login_hint_hash` (SHA-256 prefix, no PII). Backend failures
+    (which trigger fail-open) emit `event=bc_authorize_rate_limiter_backend_error`.
+    Both events are designed for log-aggregation alerting.

config.go

@@ -60,6 +60,17 @@ type BackchannelConfig struct {
 	// register endpoints like https://localhost:9000/. Production deployments
 	// MUST keep this false (see GHSA-599q-j34m-33vc).
 	AllowPrivateNotificationEndpoints bool `koanf:"allow_private_notification_endpoints"`
+
+	// PerClientRateLimitPerMinute caps /oauth2/bc-authorize per
+	// (client_id, account_id, project_id). nil uses the service default
+	// (10/min); 0 explicitly disables. Pointer because 0 is meaningful.
+	PerClientRateLimitPerMinute *int `koanf:"per_client_rate_limit_per_minute"`
+
+	// PerUserRateLimitPerMinute caps /oauth2/bc-authorize per
+	// (login_hint, account_id, project_id). nil uses the service default
+	// (5/min); 0 explicitly disables. Prevents one user being spammed
+	// across multiple clients in the same tenant.
+	PerUserRateLimitPerMinute *int `koanf:"per_user_rate_limit_per_minute"`
 }
 
 // AttestationConfig governs the attestation verification subsystem. The

hooks.go

@@ -104,3 +104,21 @@ type BackchannelNotification struct {
 // debuggability but does not block request creation — the user may approve
 // through another channel.
 type BackchannelNotifier func(ctx context.Context, n BackchannelNotification) error
+
+// RateLimiter gates an /oauth2/bc-authorize request keyed by an opaque
+// string. ZeroID ships with an in-memory implementation; multi-replica
+// deployments should plug in a shared-store backend (Redis, Memcached,
+// hosted KV) via Server.SetBackchannelRateLimiters to avoid the per-replica
+// bypass where each instance enforces independently.
+//
+// Implementations must be safe for concurrent use. Result shape:
+//   - (true, 0, nil)            request permitted
+//   - (false, retryAfter, nil)  request rejected; retryAfter is surfaced
+//     via the Retry-After header, rounded up to whole seconds (RFC 7231 §7.1.3)
+//   - (_, _, err)               backend failed; ZeroID fails open and logs
+//     a WARN event so the operator notices the degraded posture
+type RateLimiter interface {
+	Allow(ctx context.Context, key string) (allowed bool, retryAfter time.Duration, err error)
+	// Stop releases resources held by the limiter. Idempotent.
+	Stop()
+}

internal/handler/oauth.go

@@ -3,7 +3,9 @@ package handler
 import (
 	"context"
 	"errors"
+	"math"
 	"net/http"
+	"strconv"
 
 	"github.com/danielgtaylor/huma/v2"
 	"github.com/rs/zerolog/log"
@@ -97,6 +99,21 @@ func extractOAuthError(err error) (code, description string, status int) {
 	return "server_error", "an unexpected error occurred", http.StatusInternalServerError
 }
 
+// retryAfterHeader formats an *OAuthError's RetryAfter as integer
+// delta-seconds (RFC 7231 §7.1.3). Returns "" when the error carries no
+// hint. Rounds up so the advertised wait is never shorter than the refill.
+func retryAfterHeader(err error) string {
+	var oauthErr *service.OAuthError
+	if !errors.As(err, &oauthErr) || oauthErr.RetryAfter <= 0 {
+		return ""
+	}
+	secs := int(math.Ceil(oauthErr.RetryAfter.Seconds()))
+	if secs < 1 {
+		secs = 1
+	}
+	return strconv.Itoa(secs)
+}
+
 type IntrospectInput struct {
 	Body struct {
 		Token string `json:"token" required:"true" minLength:"1" doc:"JWT to introspect"`
@@ -341,9 +358,12 @@ type BcAuthorizeInput struct {
 }
 
 // BcAuthorizeOutput mirrors the success response in CIBA Core §7.3.
+// RetryAfter is set on rate-limit rejections; Huma renders it as the
+// Retry-After header.
 type BcAuthorizeOutput struct {
-	Status int
-	Body   any // service.CreateAuthRequestOutput on success; oauthErrorBody on error
+	Status     int
+	RetryAfter string `header:"Retry-After"`
+	Body       any    // service.CreateAuthRequestOutput on success; oauthErrorBody on error
 }
 
 func (a *API) bcAuthorizeOp(ctx context.Context, input *BcAuthorizeInput) (*BcAuthorizeOutput, error) {
@@ -367,9 +387,18 @@ func (a *API) bcAuthorizeOp(ctx context.Context, input *BcAuthorizeInput) (*BcAu
 		ClientNotificationToken: input.Body.ClientNotificationToken,
 	})
 	if err != nil {
-		log.Error().Err(err).Str("client_id", input.Body.ClientID).Msg("bc-authorize failed")
 		code, desc, status := extractOAuthError(err)
-		return &BcAuthorizeOutput{Status: status, Body: oauthErrorBody{Error: code, ErrorDescription: desc}}, nil
+		// 429s are an expected signal already logged at Warn in the service;
+		// skip the Error-level handler line so error-rate alerts aren't
+		// noisy under sustained rate-limited traffic.
+		if status != http.StatusTooManyRequests {
+			log.Error().Err(err).Str("client_id", input.Body.ClientID).Msg("bc-authorize failed")
+		}
+		return &BcAuthorizeOutput{
+			Status:     status,
+			RetryAfter: retryAfterHeader(err),
+			Body:       oauthErrorBody{Error: code, ErrorDescription: desc},
+		}, nil
 	}
 	return &BcAuthorizeOutput{Status: http.StatusOK, Body: out}, nil
 }

internal/service/backchannel.go

@@ -4,7 +4,9 @@ import (
 	"bytes"
 	"context"
 	"crypto/rand"
+	"crypto/sha256"
 	"encoding/base64"
+	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -58,6 +60,12 @@ type BackchannelService struct {
 	// with a 10-second timeout enforced by pingClient.Timeout).
 	pingClient        *http.Client
 	pingDispatchAsync bool // overridable for tests
+
+	// Rate limiters for /oauth2/bc-authorize. Nil disables the dimension.
+	// Held as the interface so a shared-store backend can be plugged in via
+	// SetRateLimiters.
+	perClientLimiter RateLimiter
+	perUserLimiter   RateLimiter
 }
 
 // BackchannelNotifierFunc is the internal alias for the public
@@ -108,6 +116,15 @@ type BackchannelServiceConfig struct {
 	// setter; both should be set from the same source in the deployer's
 	// server construction code.
 	AllowPrivateNotificationEndpoints bool
+
+	// PerClientRateLimit caps /oauth2/bc-authorize requests per minute keyed
+	// on (client_id, account_id, project_id). 0 disables; default 10.
+	PerClientRateLimit int
+	// PerUserRateLimit caps /oauth2/bc-authorize requests per minute keyed
+	// on (login_hint, account_id, project_id). 0 disables; default 5.
+	// Prevents one user being spammed across multiple clients in the same
+	// tenant.
+	PerUserRateLimit int
 }
 
 // DefaultBackchannelConfig returns sensible defaults for production deployments.
@@ -121,6 +138,8 @@ func DefaultBackchannelConfig() BackchannelServiceConfig {
 		PingTimeout:            10 * time.Second,
 		PingMaxRetries:         3,
 		PingBaseDelay:          500 * time.Millisecond,
+		PerClientRateLimit:     10,
+		PerUserRateLimit:       5,
 	}
 }
 
@@ -163,7 +182,18 @@ func NewBackchannelService(
 		svcCancel:           svcCancel,
 		pingClient:          &http.Client{Timeout: cfg.PingTimeout},
 		pingDispatchAsync:   true,
+		perClientLimiter:    newPerMinuteLimiter(cfg.PerClientRateLimit),
+		perUserLimiter:      newPerMinuteLimiter(cfg.PerUserRateLimit),
+	}
+}
+
+// newPerMinuteLimiter builds the default in-memory limiter: capacity=n
+// (burst), refill=n/60 tokens/sec. Returns nil when n <= 0.
+func newPerMinuteLimiter(n int) RateLimiter {
+	if n <= 0 {
+		return nil
 	}
+	return NewTokenBucketLimiter(float64(n), float64(n)/60.0)
 }
 
 // Stop cancels the service's lifecycle context, signalling in-flight detached
@@ -179,6 +209,52 @@ func (s *BackchannelService) Stop() {
 		s.svcCancel = nil
 		s.svcCtx = nil
 	}
+	if s.perClientLimiter != nil {
+		s.perClientLimiter.Stop()
+	}
+	if s.perUserLimiter != nil {
+		s.perUserLimiter.Stop()
+	}
+}
+
+// SetRateLimits swaps in fresh in-memory limiters at the supplied
+// requests-per-minute; 0 disables a dimension. Atomic with the cfg update
+// so concurrent callers can't see torn state. For non-default backends,
+// use SetRateLimiters.
+func (s *BackchannelService) SetRateLimits(perClientRPM, perUserRPM int) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.cfg.PerClientRateLimit = perClientRPM
+	s.cfg.PerUserRateLimit = perUserRPM
+	s.replaceLimitersLocked(
+		newPerMinuteLimiter(perClientRPM),
+		newPerMinuteLimiter(perUserRPM),
+	)
+}
+
+// SetRateLimiters installs RateLimiter implementations for the per-client
+// and per-user dimensions. Nil disables a dimension. Previously-installed
+// limiters are stopped — except when the new instance equals the existing
+// one (calling Stop() on the default in-memory impl kills its reaper
+// goroutine, which cannot be restarted).
+func (s *BackchannelService) SetRateLimiters(perClient, perUser RateLimiter) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.replaceLimitersLocked(perClient, perUser)
+}
+
+// replaceLimitersLocked swaps limiters in atomically. Caller holds s.mu.
+// Skips Stop() when the new instance equals the existing one to avoid
+// rendering the supplied limiter non-functional.
+func (s *BackchannelService) replaceLimitersLocked(perClient, perUser RateLimiter) {
+	if s.perClientLimiter != nil && s.perClientLimiter != perClient {
+		s.perClientLimiter.Stop()
+	}
+	if s.perUserLimiter != nil && s.perUserLimiter != perUser {
+		s.perUserLimiter.Stop()
+	}
+	s.perClientLimiter = perClient
+	s.perUserLimiter = perUser
 }
 
 // SetNotifier wires the deployer's BackchannelNotifier. Safe to call any time
@@ -268,6 +344,15 @@ func (s *BackchannelService) CreateAuthRequest(ctx context.Context, in CreateAut
 		return nil, oauthBadRequestCause("invalid_client", fmt.Sprintf("unknown client %s", in.ClientID), err)
 	}
 
+	// Rate-limit BEFORE notifier dispatch and BEFORE persistence: rejecting
+	// after the notifier would still spam the user; rejecting after
+	// persistence would still flood backchannel_auth_requests. Tenant is
+	// part of the key because clients in ZeroID are global — keying on
+	// client_id alone would be a cross-tenant key.
+	if err := s.checkRateLimit(ctx, in); err != nil {
+		return nil, err
+	}
+
 	// Determine notification mode. CIBA Core §10 makes the delivery mode a
 	// property of the client registration, not the per-request — so we read
 	// the row off the client and let the per-request client_notification_token
@@ -364,6 +449,83 @@ func (s *BackchannelService) CreateAuthRequest(ctx context.Context, in CreateAut
 	}, nil
 }
 
+// checkRateLimit applies the per-client and per-user limiters. On reject,
+// returns 429 + error="slow_down" + Retry-After (CIBA Core §11 polling-side
+// semantics). On a limiter backend error the call fails open — a
+// malfunctioning limiter must not DoS the auth service it is protecting —
+// and a distinct WARN event records the degraded posture.
+//
+// Bucket keys are the raw client_id and login_hint — tenant fields are NOT
+// in the key because they are caller-supplied JSON-body strings unbound to
+// the client; including them would let an attacker bypass the cap by
+// varying tenant per request. Tenant remains in the WARN event for
+// forensics.
+func (s *BackchannelService) checkRateLimit(ctx context.Context, in CreateAuthRequestInput) error {
+	s.mu.RLock()
+	perClient := s.perClientLimiter
+	perUser := s.perUserLimiter
+	s.mu.RUnlock()
+
+	if perClient != nil {
+		ok, retryAfter, err := perClient.Allow(ctx, in.ClientID)
+		if err != nil {
+			logRateLimitBackendError("per_client", in, err)
+		} else if !ok {
+			logRateLimitRejection("per_client", in, retryAfter)
+			return oauthTooManyRequests(
+				"slow_down",
+				"too many bc-authorize requests for this client; retry after Retry-After seconds",
+				retryAfter,
+			)
+		}
+	}
+	if perUser != nil {
+		ok, retryAfter, err := perUser.Allow(ctx, in.LoginHint)
+		if err != nil {
+			logRateLimitBackendError("per_user", in, err)
+		} else if !ok {
+			logRateLimitRejection("per_user", in, retryAfter)
+			return oauthTooManyRequests(
+				"slow_down",
+				"too many bc-authorize requests for this user; retry after Retry-After seconds",
+				retryAfter,
+			)
+		}
+	}
+	return nil
+}
+
+func logRateLimitRejection(reason string, in CreateAuthRequestInput, retryAfter time.Duration) {
+	log.Warn().
+		Str("event", "bc_authorize_rate_limited").
+		Str("reason", reason).
+		Str("client_id", in.ClientID).
+		Str("account_id", in.AccountID).
+		Str("project_id", in.ProjectID).
+		Str("login_hint_hash", hashedLoginHint(in.LoginHint)).
+		Dur("retry_after", retryAfter).
+		Msg("bc-authorize rate limit exceeded")
+}
+
+func logRateLimitBackendError(reason string, in CreateAuthRequestInput, err error) {
+	log.Warn().
+		Err(err).
+		Str("event", "bc_authorize_rate_limiter_backend_error").
+		Str("reason", reason).
+		Str("client_id", in.ClientID).
+		Str("account_id", in.AccountID).
+		Str("project_id", in.ProjectID).
+		Str("login_hint_hash", hashedLoginHint(in.LoginHint)).
+		Msg("bc-authorize rate limiter backend error; failing open")
+}
+
+// hashedLoginHint returns a 16-hex-char SHA-256 prefix — enough to cluster
+// events by victim without leaking the raw login_hint (often PII) to logs.
+func hashedLoginHint(hint string) string {
+	sum := sha256.Sum256([]byte(hint))
+	return hex.EncodeToString(sum[:])[:16]
+}
+
 // ApproveInput resolves a pending request positively.
 type ApproveInput struct {
 	AuthReqID    string