From 046e3d0a3a50c0ee910ee3a55a40100ea85ce063 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 8 Jun 2026 17:28:19 -0700 Subject: [PATCH 1/2] fix(auth): derive stable CSRF key from encryption key (closes #1120) Production built the auth service with no CSRFKey, so NewService minted a random per-process key. CSRF tokens are HMAC-SHA256(csrfKey, rawSessionToken) minted at login and validated on every write, so a token issued by one Lambda instance was rejected by another instance (and by the same instance after a cold-start). This surfaced as intermittent "CSRF validation failed" on plan create and admin user-group updates for both Standard and Admin users. Derive a stable CSRFKey from the deploy-provided credential encryption key via HKDF-SHA256 with a domain-separation label, so every instance and every cold-start derives the same key. The encryption key is already mandatory in production (the app refuses to start without it), so this fails closed without requiring a new secret. The key load is memoized via sync.Once, so seeding the CSRF key before the credential store adds no extra work. Add a cross-instance regression test: a token minted by instance A validates on instance B that shares the same master secret, and is rejected by an instance built from a different secret (the pre-fix random-key situation). DeriveCSRFKey also fails closed on an empty master secret. --- internal/auth/service_helpers.go | 35 ++++++++++ internal/auth/service_security_test.go | 97 ++++++++++++++++++++++++++ internal/server/app.go | 25 +++++-- 3 files changed, 153 insertions(+), 4 deletions(-) diff --git a/internal/auth/service_helpers.go b/internal/auth/service_helpers.go index 4dc2a99bf..331e2665b 100644 --- a/internal/auth/service_helpers.go +++ b/internal/auth/service_helpers.go @@ -7,9 +7,44 @@ import ( "crypto/sha256" "encoding/base64" "encoding/hex" + "fmt" + "io" "time" + + "golang.org/x/crypto/hkdf" ) +// csrfKeyHKDFInfo is the HKDF "info" (domain-separation) label used when +// deriving the CSRF key from a master secret. It binds the derived key to this +// specific purpose so the CSRF key can never collide with a key derived from +// the same master secret for any other use. +const csrfKeyHKDFInfo = "CUDly:csrf-key:v1" + +// DeriveCSRFKey derives a stable 32-byte CSRF key from a master secret using +// HKDF-SHA256 with a fixed domain-separation label. +// +// Production wiring passes the (already stable, deploy-provided) credential +// encryption key as the master secret, so every process and every Lambda +// cold-start derives the SAME CSRF key. This is what makes a CSRF token minted +// by one instance validate on another instance: ValidateCSRFToken recomputes +// HMAC-SHA256(csrfKey, rawSessionToken), and the csrfKey is now identical +// across the fleet instead of a per-process random value. +// +// HKDF domain separation (csrfKeyHKDFInfo) ensures the derived CSRF key is +// cryptographically independent of the master secret, so leaking one does not +// reveal the other. The master secret must be non-empty. +func DeriveCSRFKey(masterSecret []byte) ([]byte, error) { + if len(masterSecret) == 0 { + return nil, fmt.Errorf("auth: DeriveCSRFKey requires a non-empty master secret") + } + r := hkdf.New(sha256.New, masterSecret, nil /* salt */, []byte(csrfKeyHKDFInfo)) + key := make([]byte, 32) + if _, err := io.ReadFull(r, key); err != nil { + return nil, fmt.Errorf("auth: derive CSRF key: %w", err) + } + return key, nil +} + // hashSessionToken creates a SHA-256 hash of a session token for secure storage. // This prevents session hijacking if the database is compromised, as attackers // would only have hashes, not usable session tokens. diff --git a/internal/auth/service_security_test.go b/internal/auth/service_security_test.go index 8f41cb8e1..a807b0c93 100644 --- a/internal/auth/service_security_test.go +++ b/internal/auth/service_security_test.go @@ -270,3 +270,100 @@ func TestValidateUserAPIKey_LastUsedSingleflight(t *testing.T) { t.Fatal("background UpdateAPIKeyLastUsed did not complete within 5s") } } + +// --------------------------------------------------------------------------- +// Cross-instance CSRF: a token minted on one instance must validate on another +// --------------------------------------------------------------------------- + +// TestCSRFKey_CrossInstanceStable reproduces the production bug behind QA's +// "CSRF validation failed" reports on plan-save and user-save: production built +// the auth service with no CSRFKey, so NewService minted a RANDOM per-process +// key. A CSRF token issued at login by Lambda instance A was then rejected by +// instance B (and by A itself after a cold-start), because each instance had a +// different key. +// +// The fix derives a STABLE CSRFKey from the deploy-provided encryption key via +// DeriveCSRFKey, so every instance derives the same key. This test simulates +// two instances that share the same master secret and asserts a token minted by +// instance A validates on instance B. It also asserts that an instance built +// from a DIFFERENT master secret (the pre-fix per-process-random situation) +// rejects the token — i.e. the bug is real and the derivation is what fixes it. +func TestCSRFKey_CrossInstanceStable(t *testing.T) { + ctx := context.Background() + + // One stable master secret shared across the fleet (the deploy-provided + // credential encryption key in production). + masterSecret := []byte("shared-stable-master-secret-32by") + csrfKey, err := DeriveCSRFKey(masterSecret) + require.NoError(t, err) + require.Len(t, csrfKey, 32, "derived CSRF key must be 32 bytes") + + // Derivation is deterministic: two instances seeded with the same secret + // produce byte-identical keys. + csrfKey2, err := DeriveCSRFKey(masterSecret) + require.NoError(t, err) + assert.Equal(t, csrfKey, csrfKey2, "DeriveCSRFKey must be deterministic for a given secret") + + user := &User{ID: "user-x", Email: "x@example.com"} + + // Instance A mints a session + CSRF token (the login path). + storeA := new(MockStore) + t.Cleanup(func() { storeA.AssertExpectations(t) }) + instanceA := NewService(ServiceConfig{Store: storeA, CSRFKey: csrfKey}) + storeA.On("CreateSession", ctx, mock.AnythingOfType("*auth.Session")).Return(nil).Once() + + clientSession, err := instanceA.createSession(ctx, user, "ua", "1.2.3.4") + require.NoError(t, err) + require.NotEmpty(t, clientSession.CSRFToken) + + // Instance B is a separate service (separate Lambda instance) seeded with + // the SAME master secret via DeriveCSRFKey. + storeB := new(MockStore) + t.Cleanup(func() { storeB.AssertExpectations(t) }) + csrfKeyB, err := DeriveCSRFKey(masterSecret) + require.NoError(t, err) + instanceB := NewService(ServiceConfig{Store: storeB, CSRFKey: csrfKeyB}) + + // B looks up the session by its hashed token (the validation path). + hashedToken := hashSessionToken(clientSession.Token) + storeB.On("GetSession", ctx, hashedToken).Return(&Session{ + Token: hashedToken, + UserID: user.ID, + ExpiresAt: time.Now().Add(1 * time.Hour), + CreatedAt: time.Now(), + }, nil).Once() + + // THE INVARIANT: a token minted on instance A must validate on instance B. + // On the pre-fix code (random per-process key) this fails — exactly the QA + // symptom. + err = instanceB.ValidateCSRFToken(ctx, clientSession.Token, clientSession.CSRFToken) + require.NoError(t, err, "CSRF token minted on instance A must validate on instance B sharing the same master secret") + + // Conversely, an instance built from a DIFFERENT master secret (the broken + // per-process-random case) must reject A's token. + storeC := new(MockStore) + t.Cleanup(func() { storeC.AssertExpectations(t) }) + csrfKeyC, err := DeriveCSRFKey([]byte("a-totally-different-master-secret")) + require.NoError(t, err) + assert.NotEqual(t, csrfKey, csrfKeyC, "different master secrets must yield different CSRF keys") + instanceC := NewService(ServiceConfig{Store: storeC, CSRFKey: csrfKeyC}) + storeC.On("GetSession", ctx, hashedToken).Return(&Session{ + Token: hashedToken, + UserID: user.ID, + ExpiresAt: time.Now().Add(1 * time.Hour), + CreatedAt: time.Now(), + }, nil).Once() + + err = instanceC.ValidateCSRFToken(ctx, clientSession.Token, clientSession.CSRFToken) + require.Error(t, err, "a CSRF token must NOT validate under a different master secret") + assert.Contains(t, err.Error(), "invalid CSRF token") +} + +// TestDeriveCSRFKey_RejectsEmptySecret verifies DeriveCSRFKey fails closed on an +// empty master secret rather than silently deriving a key from nothing. +func TestDeriveCSRFKey_RejectsEmptySecret(t *testing.T) { + _, err := DeriveCSRFKey(nil) + require.Error(t, err) + _, err = DeriveCSRFKey([]byte{}) + require.Error(t, err) +} diff --git a/internal/server/app.go b/internal/server/app.go index e737a1efe..bace714a6 100644 --- a/internal/server/app.go +++ b/internal/server/app.go @@ -687,6 +687,25 @@ func (app *Application) reinitializeAfterConnect(ctx context.Context, dbConn *da return fmt.Errorf("failed to create PostgreSQL auth store") } + // Load the credential encryption key (deploy-provided, stable across + // instances and cold-starts) up front: it seeds the CSRF key below and is + // reused for the credential store further down. loadAndGuardEncryptionKey + // memoizes via sync.Once, so the later credential-store call is free. + encKey, encKeySource, err := loadAndGuardEncryptionKey(ctx, app.secretResolver) + if err != nil { + return err + } + + // Derive a STABLE CSRF key from the encryption key so every instance and + // every Lambda cold-start uses the same key (closes the cross-instance CSRF + // failure: a token minted on one instance must validate on another). Without + // an explicit CSRFKey, auth.NewService would mint a random per-process key, + // invalidating tokens across the fleet and on every cold-start. + csrfKey, err := auth.DeriveCSRFKey(encKey) + if err != nil { + return fmt.Errorf("failed to derive CSRF key: %w", err) + } + // Update auth service with PostgreSQL auth store app.Auth = auth.NewService(auth.ServiceConfig{ Store: authStore, @@ -694,6 +713,7 @@ func (app *Application) reinitializeAfterConnect(ctx context.Context, dbConn *da SessionDuration: 24 * time.Hour, DashboardURL: app.appConfig.DashboardURL, OnPasswordChange: buildAdminPasswordSyncCallback(authStore, app.secretResolver), + CSRFKey: csrfKey, }) if app.Auth == nil { return fmt.Errorf("failed to create auth service") @@ -721,10 +741,7 @@ func (app *Application) reinitializeAfterConnect(ctx context.Context, dbConn *da log.Println("Initialized PostgreSQL analytics store and snapshot collector") // Initialize credential store (AES-256-GCM encrypted credential blobs). - encKey, encKeySource, err := loadAndGuardEncryptionKey(ctx, app.secretResolver) - if err != nil { - return err - } + // encKey/encKeySource were loaded above (memoized) to seed the CSRF key. credStore := credentials.NewCredentialStore(dbConn.Pool(), encKey) app.encKeySource = encKeySource log.Println("Initialized encrypted credential store") From a7db5e15032d47385b0e8c9062bf9154354675c1 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 8 Jun 2026 18:07:19 -0700 Subject: [PATCH 2/2] refactor(server): extract CSRF key derivation to satisfy gocyclo Extract CSRF key derivation and auth.Service construction from reinitializeAfterConnect into a new (*Application).initAuthService helper. Reduces reinitializeAfterConnect cyclomatic complexity from 11 to 10 (project limit), with initAuthService at 3. No behavior, ordering, error handling, or fail-closed semantics changed. --- internal/server/app.go | 53 ++++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/internal/server/app.go b/internal/server/app.go index bace714a6..cd4fd8579 100644 --- a/internal/server/app.go +++ b/internal/server/app.go @@ -670,6 +670,34 @@ func loadAndGuardEncryptionKey(ctx context.Context, resolver secrets.Resolver) ( return encKey, source, nil } +// initAuthService derives a stable CSRF key from encKey via HKDF and +// constructs the auth.Service. Extracted from reinitializeAfterConnect to keep +// cyclomatic complexity within the project limit. Returns an error if CSRF key +// derivation fails or if NewService returns nil (fail-closed). +func (app *Application) initAuthService(authStore *auth.PostgresStore, encKey []byte) (*auth.Service, error) { + // Derive a STABLE CSRF key from the encryption key so every instance and + // every Lambda cold-start uses the same key (closes the cross-instance CSRF + // failure: a token minted on one instance must validate on another). Without + // an explicit CSRFKey, auth.NewService would mint a random per-process key, + // invalidating tokens across the fleet and on every cold-start. + csrfKey, err := auth.DeriveCSRFKey(encKey) + if err != nil { + return nil, fmt.Errorf("failed to derive CSRF key: %w", err) + } + svc := auth.NewService(auth.ServiceConfig{ + Store: authStore, + EmailSender: app.Email, + SessionDuration: 24 * time.Hour, + DashboardURL: app.appConfig.DashboardURL, + OnPasswordChange: buildAdminPasswordSyncCallback(authStore, app.secretResolver), + CSRFKey: csrfKey, + }) + if svc == nil { + return nil, fmt.Errorf("failed to create auth service") + } + return svc, nil +} + // reinitializeAfterConnect re-creates all stores and services that depend on the // database connection. This is called after the lazy DB connect succeeds. // Returns an error if any store or service initialization fails. @@ -696,28 +724,13 @@ func (app *Application) reinitializeAfterConnect(ctx context.Context, dbConn *da return err } - // Derive a STABLE CSRF key from the encryption key so every instance and - // every Lambda cold-start uses the same key (closes the cross-instance CSRF - // failure: a token minted on one instance must validate on another). Without - // an explicit CSRFKey, auth.NewService would mint a random per-process key, - // invalidating tokens across the fleet and on every cold-start. - csrfKey, err := auth.DeriveCSRFKey(encKey) + // Update auth service with PostgreSQL auth store. The CSRF key is derived + // from the encryption key so every instance uses the same stable key. + authSvc, err := app.initAuthService(authStore, encKey) if err != nil { - return fmt.Errorf("failed to derive CSRF key: %w", err) - } - - // Update auth service with PostgreSQL auth store - app.Auth = auth.NewService(auth.ServiceConfig{ - Store: authStore, - EmailSender: app.Email, - SessionDuration: 24 * time.Hour, - DashboardURL: app.appConfig.DashboardURL, - OnPasswordChange: buildAdminPasswordSyncCallback(authStore, app.secretResolver), - CSRFKey: csrfKey, - }) - if app.Auth == nil { - return fmt.Errorf("failed to create auth service") + return err } + app.Auth = authSvc // Initialize distributed rate limiter for Lambda (multi-instance) // For Fargate/containers, we already have in-memory rate limiter from startup