gsadmin/certctl
1
0
Fork 0
mirror of https://github.com/shankar0123/certctl.git synced 2026-06-13 15:48:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

harden(oidc): pre-login UA/IP binding (MED-16) — RFC 9700 §4.7.1

Browse Source 
Audit 2026-05-10 MED-16 closure.

WHAT.

Binds the OIDC pre-login row to the (clientIP, userAgent) tuple of
the /auth/oidc/login request, and enforces a constant-time compare
against the /auth/oidc/callback request at consume time. Defeats
replay of a stolen pre-login cookie by a different browser /
source — the secondary defense layer recommended by RFC 9700 §4.7.1
when the primary layer (HMAC integrity + Path=/ + SameSite=Lax on
the cookie) is bypassed via CSRF / XSS / TLS-termination leak.

WHY.

Pre-fix, the pre-login cookie's HMAC verified only that 'some'
caller of /auth/oidc/login was talking to /auth/oidc/callback; it
did not verify that the SAME browser / source was on both sides.
An attacker who exfiltrated the cookie value via any vector could
replay the bytes through their own user-agent and ride the victim's
authorization. RFC 9700 §4.7.1 calls out the gap explicitly and
recommends binding state to a user-agent fingerprint + source IP.

HOW.

Migration:
  migrations/000044_prelogin_uaip.up.sql
    ALTER TABLE oidc_pre_login_sessions
      ADD COLUMN IF NOT EXISTS client_ip   TEXT,
      ADD COLUMN IF NOT EXISTS user_agent  TEXT;
  Both nullable for in-flight rolling-deploy compat — the consume-
  side check only enforces when both row AND request carry non-empty
  values for the leg in question.

Domain:
  internal/repository/oidc.go (PreLoginSession) — adds ClientIP +
    UserAgent fields.

Repository:
  internal/repository/postgres/oidc_prelogin.go — Create persists
    via sql.NullString (empty → NULL); LookupAndConsume reads back.
    Re-uses package-local nullableString from discovery.go.

Service:
  internal/auth/oidc/service.go
    - PreLoginStore.CreatePreLogin signature takes (clientIP,
      userAgent) as positions 5–6.
    - PreLoginStore.LookupAndConsume returns (clientIP, userAgent)
      as positions 5–6.
    - HandleAuthRequest signature gains (clientIP, userAgent),
      threaded to the store.
    - HandleCallback adds Step 1.5 — UA / IP constant-time compare
      between stored row and incoming request. Per-leg toggles via
      preLoginRequireUA / preLoginRequireIP service fields. Empty
      values on either side pass through (rolling-deploy + headless-
      proxy compat).
    - New sentinels ErrPreLoginUAMismatch, ErrPreLoginIPMismatch.
    - SetPreLoginBindingRequirements(requireUA, requireIP) helper
      for main.go config wiring.

Adapter:
  internal/auth/oidc/prelogin.go — PreLoginAdapter passes the new
    fields through to the repo row.

Handler:
  internal/api/handler/auth_session_oidc.go
    - OIDCAuthHandshaker.HandleAuthRequest signature updated.
    - LoginInitiate captures clientIPFromRequest + r.UserAgent()
      and passes to the service.
    - classifyOIDCFailure adds errors.Is dispatch for the two new
      sentinels → prelogin_ua_mismatch / prelogin_ip_mismatch
      audit categories.

Config:
  internal/config/config.go
    + AuthConfig.OIDCPreLoginRequireUA (default true)
      env CERTCTL_OIDC_PRELOGIN_REQUIRE_UA
    + AuthConfig.OIDCPreLoginRequireIP (default true)
      env CERTCTL_OIDC_PRELOGIN_REQUIRE_IP
  cmd/server/main.go calls oidcService.SetPreLoginBindingRequirements
    from cfg.Auth.OIDCPreLoginRequire{UA,IP}.

Tests (internal/auth/oidc/service_test.go):
  - TestService_HandleCallback_MED16_UAMismatchRejected
  - TestService_HandleCallback_MED16_IPMismatchRejected
  - TestService_HandleCallback_MED16_BothMatch_Succeeds
  - TestService_HandleCallback_MED16_LegacyRowEmptyValues  (rolling-
    deploy compat — empty stored values pass through)
  - TestService_HandleCallback_MED16_RequireUAFalse_AllowsMismatch
    (operator escape-hatch — UA mismatch silently allowed)

Mechanical fan-out:
  - stubPreLogin / stubPreLoginRepo signatures updated.
  - All existing call sites in service_test.go (~40), prelogin_test.go,
    bench_test.go, logging_test.go, provider_enabled_test.go,
    integration_keycloak_test.go, integration_okta_smoke_test.go,
    auth_session_oidc_test.go updated to pass empty strings for the
    new params — pre-existing tests do not exercise UA/IP binding
    semantics.

VERIFY.

- go vet ./internal/auth/oidc/... ./internal/api/handler/...
  ./internal/config/...                                       PASS
- go test -short -count=1 -run MED16 ./internal/auth/oidc/... PASS (5/5)
- go test -short -count=1 ./internal/auth/oidc/...            PASS (4.6s)
- go test -short -count=1 ./internal/api/handler/...          PASS (4.3s)
- go test -short -count=1 ./internal/config/...               PASS

Refs: cowork/auth-bundles-audit-2026-05-10.md MED-16
      cowork/auth-bundles-fixes-2026-05-10/HANDOFF.md item 6
      RFC 9700 §4.7.1 — OAuth 2.0 Security Best Current Practice
This commit is contained in:
shankar0123
2026-05-10 23:18:23 +00:00
parent ecef8295bb
commit cb73547af5
18 changed files with 441 additions and 103 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/auth/oidc/service.go
+81 -5
View File
@@ -85,6 +85,17 @@ type Service struct {
// resolution + user upsert; on grantAdmin=true the user's resolved
// role IDs are extended with r-admin. See bootstrap_hook.go.
adminBootstrapHook AdminBootstrapHook
// Audit 2026-05-10 MED-16 — Per-leg toggles for the pre-login UA/IP
// binding check. Both default to true; operators on enterprise
// proxies (UA rewrite) or dual-stack v4/v6 (IP flip) flip the
// affected leg false via CERTCTL_OIDC_PRELOGIN_REQUIRE_UA /
// CERTCTL_OIDC_PRELOGIN_REQUIRE_IP. Even when both are false,
// the binding values are still persisted so audit forensics can
// detect mismatches retroactively — only the in-band reject is
// suppressed.
preLoginRequireUA bool
preLoginRequireIP bool
}
// providerEntry caches the go-oidc Provider + the OAuth2 config + the
@@ -126,16 +137,28 @@ type PreLoginStore interface {
// CreatePreLogin persists a row with the given identifiers.
// providerID is the configured op-... id; state, nonce, verifier
// are server-generated random strings the callback will validate.
// clientIP + userAgent (Audit 2026-05-10 MED-16) are the
// /auth/oidc/login request's source IP (post LOW-5 XFF gating) +
// User-Agent header, persisted into the row so HandleCallback can
// reject mismatches at consume time. Empty strings are tolerated
// (rolling-deploy compat + headless / proxy contexts) — the
// consume-side check only enforces when both sides carry non-empty
// values for the leg in question.
// Returns the opaque cookie value the handler sets, plus the
// session ID (used as the audit trail anchor).
CreatePreLogin(ctx context.Context, providerID, state, nonce, verifier string) (cookieValue, sessionID string, err error)
CreatePreLogin(ctx context.Context, providerID, state, nonce, verifier, clientIP, userAgent string) (cookieValue, sessionID string, err error)
// LookupAndConsume reads the pre-login row by cookie value AND
// deletes it atomically. Single-use: a second call with the same
// cookie value returns ErrPreLoginNotFound. Returns the stored
// state/nonce/verifier/providerID for the caller to validate
// against the callback parameters.
LookupAndConsume(ctx context.Context, cookieValue string) (providerID, state, nonce, verifier string, err error)
//
// Audit 2026-05-10 MED-16 — also returns the row's persisted
// clientIP + userAgent so HandleCallback can defeat replay of a
// stolen pre-login cookie by a different browser. Empty values are
// returned for rows persisted before migration 000044.
LookupAndConsume(ctx context.Context, cookieValue string) (providerID, state, nonce, verifier, clientIP, userAgent string, err error)
}
// SessionMinter wraps the post-login session creation. Phase 4's
@@ -193,6 +216,20 @@ var (
// RFC 9207 §2.3. HTTP 400.
ErrIssParamMismatch = errors.New("oidc: callback iss parameter does not match provider issuer URL")
// ErrPreLoginUAMismatch: pre-login row's User-Agent doesn't match
// the request hitting /auth/oidc/callback. Audit 2026-05-10 MED-16
// closure — RFC 9700 §4.7.1 binding-state recommendation. HTTP 400.
// Operators on enterprise proxies that rewrite UA may set
// CERTCTL_OIDC_PRELOGIN_REQUIRE_UA=false to disable.
ErrPreLoginUAMismatch = errors.New("oidc: pre-login row User-Agent does not match callback request")
// ErrPreLoginIPMismatch: pre-login row's client IP doesn't match
// the request hitting /auth/oidc/callback. Audit 2026-05-10
// MED-16. HTTP 400. Operators on dual-stack v4/v6 environments
// where source IP routinely flips may set
// CERTCTL_OIDC_PRELOGIN_REQUIRE_IP=false to disable.
ErrPreLoginIPMismatch = errors.New("oidc: pre-login row client IP does not match callback request")
// ErrAudienceMismatch: ID token `aud` doesn't include the
// configured client_id. HTTP 400.
ErrAudienceMismatch = errors.New("oidc: audience mismatch")
@@ -322,6 +359,11 @@ func NewService(
encryptionKey: encryptionKey,
cache: make(map[string]*providerEntry),
clockNow: time.Now,
// MED-16 defaults: both legs ON. cmd/server/main.go reads
// CERTCTL_OIDC_PRELOGIN_REQUIRE_UA / _IP and calls
// SetPreLoginBindingRequirements to override.
preLoginRequireUA: true,
preLoginRequireIP: true,
}
}
@@ -331,6 +373,16 @@ func (s *Service) SetClockForTest(now func() time.Time) {
s.clockNow = now
}
// SetPreLoginBindingRequirements wires the MED-16 UA/IP enforcement
// toggles. Both default to true; set false to log-only behaviour for
// a given leg (the binding is still persisted + audited; only the
// in-band reject is suppressed). Called by cmd/server/main.go from
// the config layer.
func (s *Service) SetPreLoginBindingRequirements(requireUA, requireIP bool) {
s.preLoginRequireUA = requireUA
s.preLoginRequireIP = requireIP
}
// =============================================================================
// HandleAuthRequest: kicks off the OIDC handshake.
//
@@ -346,7 +398,14 @@ func (s *Service) SetClockForTest(now func() time.Time) {
// HandleAuthRequest builds the IdP redirect URL + persists the
// pre-login session row holding state + nonce + PKCE verifier.
func (s *Service) HandleAuthRequest(ctx context.Context, providerID string) (authURL, cookieValue, preLoginID string, err error) {
//
// Audit 2026-05-10 MED-16 — clientIP + userAgent are persisted into
// the pre-login row so HandleCallback can reject a stolen cookie
// replayed by a different browser. Empty values are tolerated for
// headless / proxy callers; the consume-side check only enforces
// when both row and request carry non-empty values on the leg in
// question.
func (s *Service) HandleAuthRequest(ctx context.Context, providerID, clientIP, userAgent string) (authURL, cookieValue, preLoginID string, err error) {
entry, err := s.getOrLoad(ctx, providerID)
if err != nil {
return "", "", "", err
@@ -371,7 +430,7 @@ func (s *Service) HandleAuthRequest(ctx context.Context, providerID string) (aut
// (well within the RFC 7636 43-128 character bound).
verifier := oauth2.GenerateVerifier()
cookieValue, preLoginID, err = s.preLogin.CreatePreLogin(ctx, providerID, state, nonce, verifier)
cookieValue, preLoginID, err = s.preLogin.CreatePreLogin(ctx, providerID, state, nonce, verifier, clientIP, userAgent)
if err != nil {
return "", "", "", fmt.Errorf("oidc: pre-login store: %w", err)
}
@@ -428,11 +487,28 @@ func (s *Service) HandleCallback(
preLoginCookie, code, callbackState, callbackIss, ip, userAgent string,
) (*CallbackResult, error) {
// Step 1: consume the pre-login row (single-use).
providerID, storedState, storedNonce, verifier, err := s.preLogin.LookupAndConsume(ctx, preLoginCookie)
providerID, storedState, storedNonce, verifier, storedIP, storedUA, err := s.preLogin.LookupAndConsume(ctx, preLoginCookie)
if err != nil {
return nil, ErrPreLoginNotFound
}
// Step 1.5: Audit 2026-05-10 MED-16 — UA / IP binding compare.
// Enforced only when (a) the leg's toggle is on, (b) the row
// carries a non-empty stored value (legacy rows pre-migration
// 000044 have NULL → empty string), and (c) the incoming request
// carries a non-empty value too. Constant-time compares for both
// legs to avoid leaking UA/IP length differences via timing.
if s.preLoginRequireUA && storedUA != "" && userAgent != "" {
if subtle.ConstantTimeCompare([]byte(userAgent), []byte(storedUA)) != 1 {
return nil, ErrPreLoginUAMismatch
}
}
if s.preLoginRequireIP && storedIP != "" && ip != "" {
if subtle.ConstantTimeCompare([]byte(ip), []byte(storedIP)) != 1 {
return nil, ErrPreLoginIPMismatch
}
}
// Step 2: state constant-time compare.
if subtle.ConstantTimeCompare([]byte(callbackState), []byte(storedState)) != 1 {
return nil, ErrStateMismatch