Skip to content

Encrypt session tokens with a dedicated key - #3709

Merged
tefkah merged 3 commits into
mainfrom
kf/session-encryption-key
Oct 30, 2026
Merged

tefkah merged 3 commits into
mainfrom
kf/session-encryption-key

Conversation

@tefkah

@tefkah tefkah commented Oct 30, 2026

Copy link
Copy Markdown
Member

Merge only after kf-console #76 is deployed to production. (This change doesn't call the console and works without #76. The line is here because every PR in this batch carries it. Drop it if you prefer.)

PubPub derived its AES-256-GCM key from OIDC_CLIENT_SECRET. That key encrypts the OIDC state (which carries the PKCE verifier), the one-time custom-domain session-set token, and the kfRefreshToken stored on each express session. So rolling the client secret, which kf-console now supports with a grace period, would log everyone out. This PR gives that encryption its own key, KF_SESSION_ENCRYPTION_KEY.

What changed

  • server/kf/oidc.server.ts:
    • encryptPayload uses KF_SESSION_ENCRYPTION_KEY when it is set.
    • decryptPayload tries that key first, then falls back to the key derived from the client secret.
    • The new decryptPayloadWithKeyInfo also reports whether a value was read with the fallback.
    • Unset (or empty), behaviour is exactly as before.
    • A key that isn't 32 bytes throws a clear error.
  • server/kf/sessionCheck.ts: when the periodic liveness check reads a refresh token stored under the old key, it re-encrypts it under the new key, and the existing session save persists it. There is no bulk migration. State and session-set tokens live for minutes, so they need nothing: old ones decrypt through the fallback, and new ones use the new key.
  • server/envSchema.ts, infra/ENV.md: declare KF_SESSION_ENCRYPTION_KEY (optional; it must decode to 32 bytes; generate it with openssl rand -base64 32). Startup fails on a malformed value. I added the one row to ENV.md by hand: the file has drifted from the schema, and regenerating it rewrites about 60 unrelated rows.

When the fallback can go

KF Auth issues refresh tokens for 30 days (refreshTokenExpiresIn). PubPub writes the token once, at login, and introspection never rotates it. So 30 days after the key is live in production, every token written under the old key has expired: its session is torn down at the next check whether or not it can be decrypted. After that (allow a day's margin), remove legacyDerivedKey() and the fallback, which are marked TODO(session-key-fallback), and make the env var required. Only then can the client secret roll without logging anyone out.

Rollout

  1. Add KF_SESSION_ENCRYPTION_KEY (a different value per environment) to infra/.env.dev.enc (DuqDuq) and infra/.env.enc (prod), and optionally to infra/.env.local.enc.

  2. The first rolling update that sets the key briefly mixes replicas with and without it. A replica without the key can't read what one with the key wrote. Two things can go wrong in that window, which lasts minutes:

    • A login can fail with "Invalid or expired authentication state", and retrying works.
    • A session whose refresh token was re-encrypted on a new replica, and then checked again on an old one, is logged out.

    Both are rare.

How it was checked

  • New server/kf/__tests__/sessionEncryption.test.ts (11 tests):
    • a round trip with the new key, checked against the raw key;
    • a value encrypted under the derived key still decrypts and is flagged for re-encryption;
    • unset behaves as before (the output decrypts with sha256 of the client secret);
    • a value under a different key returns null;
    • tampered or malformed values return null;
    • a 32-byte check in the code and in the env schema;
    • a stored token is re-encrypted under the new key after a check, including after a transient introspection failure;
    • a token is left alone when it is already on the new key or the key is unset;
    • an undecryptable token tears the session down.
  • pnpm run lint (biome and tsc --noEmit) passes.
  • server/kf, server/login, server/org, server/user and server/authToken all pass locally (86 tests). The full suite with the Firebase emulator runs in CI. Locally, firebase-tools doesn't run on Node 26, and the Homebrew Node 24 install on this machine is broken.

Not changed: decodeIdTokenClaims

decodeIdTokenClaims reads the ID token without checking its signature. The token comes straight from the token endpoint, in the response to PubPub's own authenticated POST. In production that is HTTPS to the public KF Auth URL, because OIDC_ISSUER_INTERNAL_URL is only set locally. OIDC Core 3.1.3.7 allows TLS server validation in place of a signature check in that case.

Only sid is used from it: it targets the session.revoked webhook. Identity comes from userinfo over the same channel, so anyone able to forge the ID token could forge userinfo too, and verifying the ID token alone adds little.

The fix, if wanted: verify the token with jose (jwtVerify against createRemoteJWKSet(discovery.jwks_uri), checking iss, aud equal to the client id, and exp), and check that its sub equals the userinfo sub. This also guards against a misconfigured internal URL that uses plain HTTP outside Docker.

@tefkah
tefkah marked this pull request as ready for review October 30, 2026 18:06
@tefkah
tefkah merged commit 2e0b69b into main Oct 30, 2026
1 check passed
@tefkah
tefkah deleted the kf/session-encryption-key branch October 30, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant