diff --git a/CLAUDE.md b/CLAUDE.md index bd37f4b0..b7f3e75d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -132,14 +132,19 @@ All cuioss repositories have branch protection on `main`. Direct pushes to `main 2. Commit changes: `git add && git commit -m ""` 3. Push the branch: `git push -u origin ` 4. Create a PR: `gh pr create --repo cuioss/API-Sheriff --head --base main --title "" --body "<body>"` -5. Wait for CI + Gemini review (waits until checks complete): `gh pr checks --watch` -6. **Handle Gemini review comments** — fetch with `gh api repos/cuioss/API-Sheriff/pulls/<pr-number>/comments` and for each: - - If clearly valid and fixable: fix it, commit, push, then reply explaining the fix and resolve the comment +5. Wait for CI and the three automated reviewers — CodeRabbit, Sourcery and PR-Agent (waits until checks complete): `gh pr checks --watch` +6. **Handle the automated review comments from all three reviewers** — CodeRabbit, Sourcery and PR-Agent each comment independently, so treat the union of their comments as the work list. Fetch with `gh api repos/cuioss/API-Sheriff/pulls/<pr-number>/comments` and for each comment, whichever bot authored it: + - If clearly valid and fixable: fix it, commit, push, then reply explaining the fix and resolve the comment — a bot-fix commit is a commit, so the **Pre-Commit Process** section above applies unchanged - If disagree or out of scope: reply explaining why, then resolve the comment - If uncertain (not 100% confident): **ask the user** before acting - Every comment MUST get a reply (reason for fix or reason for not fixing) and MUST be resolved + - **Re-review after pushing fixes is not uniform**: CodeRabbit and Sourcery re-review automatically on push; PR-Agent deliberately does **not** (`.github/workflows/pr-agent.yml` triggers only on `opened`/`reopened`/`ready_for_review` plus on-demand `issue_comment` commands). Re-request a PR-Agent pass explicitly by posting a `/review` comment on the PR after the fix push. 7. Do **NOT** enable auto-merge unless explicitly instructed. Wait for user approval. -8. Return to main: `git checkout main && git pull` +8. **After the merge lands, verify the post-merge state**: + - Re-check the **PR-attached** post-merge run. `.github/workflows/benchmark.yml` is `pull_request: types: [closed]` gated on `merged == true`, so its run stays attached to the PR and remains visible through the CI abstraction after the merge. + - Also check the **main-branch** post-merge run, which is *not* attached to the PR. `maven.yml` triggers on `push: branches: [main]`, and its `deploy-snapshot` job is skipped on pull requests but runs on that push — so a snapshot-deployment failure appears only in the Maven Build run for the merge commit on `main`, never on the PR. Look it up by the merge commit rather than by PR number. + - Assert zero unresolved review threads: `python3 .plan/execute-script.py plan-marshall:tools-integration-ci:ci pr comments --pr-number <n> --unresolved-only`. Route any straggler into a follow-up issue or plan rather than leaving it unresolved. +9. Return to main: `git checkout main && git pull` ## Temporary Files diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBinding.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBinding.java index cfbad951..f647bf33 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBinding.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBinding.java @@ -18,6 +18,7 @@ import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; import java.time.Instant; import java.util.Base64; import java.util.List; @@ -63,8 +64,9 @@ * construction no previous key, so no cookie can carry a retired key id and this path never fires. * <p> * <strong>Session identity.</strong> Per the seam's single identity model, the reconstructed - * {@link SessionRecord#sessionId()} is a keyed digest over the sealed payload's login instant and - * {@code sub}. It is stable for the life of the session — so the refresh coordinator's single-flight + * {@link SessionRecord#sessionId()} is a keyed digest over the sealed payload's login instant, + * {@code sub} and the per-session nonce minted once at login. It is stable for the life of the + * session — so the refresh coordinator's single-flight * coalescing behaves exactly as in server mode — and is <strong>never emitted to the browser</strong>: * only the sealed value crosses. Single-flight is necessarily <em>per instance</em> here; a * cross-instance duplicate refresh cannot be prevented without shared state and is this variant's @@ -80,6 +82,14 @@ public final class CookieSessionBinding implements SessionBinding { private static final String DIGEST_ALGORITHM = "SHA-256"; private static final int IDENTITY_BYTES = 16; + /** + * The per-session nonce width, matched to {@code SessionRecord.newSessionId()}'s 32 bytes so the + * cookie-mode nonce carries the same entropy as the server-mode session id it stands in for. + */ + private static final int SESSION_NONCE_BYTES = 32; + + private static final SecureRandom SECURE_RANDOM = new SecureRandom(); + /** The bounded, non-sensitive disposition recorded when a retired-key session is rolled over. */ private static final String RESEAL_DISPOSITION = "previous-key rollover"; @@ -110,8 +120,10 @@ public CookieSessionBinding(SealedSessionCookieCodec codec, byte[] identitySalt) public BoundSession bind(SessionRecord session, Instant now) { Objects.requireNonNull(session, "session"); Objects.requireNonNull(now, "now"); - // A fresh login anchors the absolute lifetime at this instant. - return seal(payloadOf(session, now), now); + // A fresh login anchors the absolute lifetime at this instant and mints the session's one + // nonce. Minting here — once, on the login path only — is what makes two logins by the same + // subject within a single clock second resolve to distinct identities. + return seal(payloadOf(session, now, newSessionNonce()), now); } @Override @@ -131,6 +143,11 @@ public Optional<SessionRecord> resolve(@Nullable String cookieHeader, Instant no * the new sealed value. The original login instant is re-derived from the record's absolute * deadline rather than taken from {@code now}, so an endlessly-refreshed session still dies at * its original absolute deadline: a refresh rotates the tokens, never the session's lifetime. + * <p> + * The session nonce is re-sealed <strong>verbatim</strong> and never re-minted, for the same + * reason the login instant is re-derived rather than refreshed: both are identity inputs, so + * minting a new nonce here would change the derived {@link SessionRecord#sessionId()} mid-session + * and break the refresh coordinator's single-flight coalescing. * * @param rotated the session carrying the rotated token material * @param now the reference instant, used only for the cookie's remaining {@code Max-Age} @@ -141,7 +158,13 @@ public BoundSession persist(SessionRecord rotated, Instant now) { Objects.requireNonNull(rotated, "rotated"); Objects.requireNonNull(now, "now"); Instant loginInstant = rotated.expiresAt().minus(codec.sessionTtl()); - return seal(payloadOf(rotated, loginInstant), now); + // Fail loud rather than mint a replacement: a cookie-mode record always carries the nonce + // sealed at login, so an absent one means a caller reconstructed the record and dropped it — + // silently re-minting would change the session's identity mid-flight. + String sessionNonce = rotated.sessionNonce().orElseThrow(() -> new IllegalStateException( + "cookie-mode session record carries no sessionNonce — cannot re-seal without changing " + + "the derived session identity")); + return seal(payloadOf(rotated, loginInstant, sessionNonce), now); } @Override @@ -217,14 +240,26 @@ private SessionRecord markForReseal(SealedSessionCookieCodec.Unsealed unsealed) return toSessionRecord(unsealed.payload()); } - private static SealedSessionPayload payloadOf(SessionRecord session, Instant loginInstant) { + private static SealedSessionPayload payloadOf(SessionRecord session, Instant loginInstant, + String sessionNonce) { return new SealedSessionPayload(session.accessToken(), session.refreshToken(), session.idToken(), - session.sub(), session.sid(), session.acr(), session.authTime(), loginInstant); + session.sub(), session.sid(), session.acr(), session.authTime(), loginInstant, sessionNonce); + } + + /** + * Mints one session nonce: {@link #SESSION_NONCE_BYTES} secure-random bytes, base64url-encoded so + * it survives the payload's base64 field encoding unchanged. + */ + private static String newSessionNonce() { + byte[] bytes = new byte[SESSION_NONCE_BYTES]; + SECURE_RANDOM.nextBytes(bytes); + return Base64.getUrlEncoder().withoutPadding().encodeToString(bytes); } private SessionRecord toSessionRecord(SealedSessionPayload payload) { return SessionRecord.builder() .sessionId(derivedIdentity(payload)) + .sessionNonce(Optional.of(payload.sessionNonce())) .accessToken(payload.accessToken()) .refreshToken(payload.refreshToken()) .idToken(payload.idToken()) @@ -238,9 +273,15 @@ private SessionRecord toSessionRecord(SealedSessionPayload payload) { /** * Derives this binding's stable per-session identity: a salted digest over the sealed payload's - * login instant and {@code sub}. Both inputs are fixed for the life of the session, so the - * identity is stable across re-seals; the salt keeps it un-recomputable outside this gateway; - * and it is never emitted to the browser. + * login instant, {@code sub} and per-session nonce. All three inputs are fixed for the life of + * the session, so the identity is stable across re-seals; the salt keeps it un-recomputable + * outside this gateway; and it is never emitted to the browser. + * <p> + * The nonce is what makes the identity <em>unique</em> rather than merely stable: login instant + * and {@code sub} alone collide for two logins by the same subject inside one clock second, which + * would cross-wire the refresh coordinator's single-flight keying between two distinct sessions. + * The identity remains a digest — the raw nonce is never used as, or substituted for, the + * session id. */ private String derivedIdentity(SealedSessionPayload payload) { MessageDigest digest; @@ -253,6 +294,7 @@ private String derivedIdentity(SealedSessionPayload payload) { digest.update(identitySalt); digest.update(Long.toString(payload.loginInstant().getEpochSecond()).getBytes(StandardCharsets.UTF_8)); digest.update(payload.sub().getBytes(StandardCharsets.UTF_8)); + digest.update(payload.sessionNonce().getBytes(StandardCharsets.UTF_8)); byte[] identity = new byte[IDENTITY_BYTES]; System.arraycopy(digest.digest(), 0, identity, 0, IDENTITY_BYTES); return Base64.getUrlEncoder().withoutPadding().encodeToString(identity); diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodec.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodec.java index 40f4d604..37291c06 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodec.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodec.java @@ -94,8 +94,19 @@ public final class SealedSessionCookieCodec { private static final CuiLogger LOGGER = new CuiLogger(SealedSessionCookieCodec.class); - /** The current sealed-cookie format version, bound into the GCM associated data. */ - public static final byte FORMAT_VERSION = 1; + /** + * The current sealed-cookie format version, bound into the GCM associated data. + * <p> + * Version {@code 2} carries the nine-field payload that added the per-session nonce keying the + * derived session identity. The bump is a clean break with no migration path: {@link #unseal} + * reads the leading version byte and rejects anything other than {@code FORMAT_VERSION} at an + * explicit gate, before a {@link Cipher} is even constructed — so a version-1 cookie is refused + * outright rather than being mis-parsed against the nine-field shape, and decryption is never + * attempted for it. The version byte is additionally bound into the GCM associated data, so it + * cannot be forged onto a value sealed under a different version either. Pre-existing cookie + * sessions are therefore refused fail-closed and the browser simply re-logs in. + */ + public static final byte FORMAT_VERSION = 2; private static final String TRANSFORMATION = "AES/GCM/NoPadding"; private static final int NONCE_BYTES = 12; diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayload.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayload.java index b52e0459..99a72c85 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayload.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayload.java @@ -51,25 +51,42 @@ * @param acr the authentication context class, empty when absent * @param authTime the IdP authentication instant, empty when absent * @param loginInstant the absolute login instant anchoring the server-enforced session TTL + * @param sessionNonce the per-session random nonce minted once at login, folded into the derived + * session identity so two logins by the same subject within one clock second + * cannot collide. Re-sealed verbatim — never re-minted — so the identity is + * stable for the life of the session * @author API Sheriff Team * @since 1.0 */ public record SealedSessionPayload(String accessToken, Optional<String> refreshToken, String idToken, -String sub, Optional<String> sid, Optional<String> acr, Optional<Instant> authTime, Instant loginInstant) { +String sub, Optional<String> sid, Optional<String> acr, Optional<Instant> authTime, Instant loginInstant, +String sessionNonce) { private static final String REDACTED = "***REDACTED***"; private static final char FIELD_SEPARATOR = '\n'; - private static final int FIELD_COUNT = 8; + private static final int FIELD_COUNT = 9; /** * Canonical constructor rejecting absent mandatory components and normalizing absent optionals * to {@link Optional#empty()}. + * <p> + * {@code sessionNonce} is additionally rejected when blank: it keys the derived session + * identity, so an empty value would silently degrade that identity back to the colliding + * pre-nonce shape instead of failing. The nonce value itself never reaches the exception + * message. + * + * @throws NullPointerException when a mandatory component is {@code null} + * @throws IllegalArgumentException when {@code sessionNonce} is blank */ public SealedSessionPayload { Objects.requireNonNull(accessToken, "accessToken"); Objects.requireNonNull(idToken, "idToken"); Objects.requireNonNull(sub, "sub"); Objects.requireNonNull(loginInstant, "loginInstant"); + Objects.requireNonNull(sessionNonce, "sessionNonce"); + if (sessionNonce.isBlank()) { + throw new IllegalArgumentException("sessionNonce must not be blank"); + } refreshToken = Objects.requireNonNullElse(refreshToken, Optional.empty()); sid = Objects.requireNonNullElse(sid, Optional.empty()); acr = Objects.requireNonNullElse(acr, Optional.empty()); @@ -91,6 +108,7 @@ public byte[] encode() { appendField(encoded, acr.orElse("")); appendField(encoded, authTime.map(instant -> Long.toString(instant.getEpochSecond())).orElse("")); appendField(encoded, Long.toString(loginInstant.getEpochSecond())); + appendField(encoded, sessionNonce); return encoded.toString().getBytes(StandardCharsets.UTF_8); } @@ -98,9 +116,15 @@ public byte[] encode() { * Reads a payload back from the wire form. The input MUST already have been authenticated by the * codec's GCM tag — this method is not a parser for untrusted input. * + * The field-count guard is strict: only the current nine-field shape is accepted. There is no + * legacy eight-field (pre-nonce) acceptance path — a clean break, so a payload predating the + * session nonce is rejected outright rather than admitted with a synthesized nonce that would + * silently change the derived session identity. + * * @param encoded the UTF-8 bytes produced by {@link #encode()} - * @return the decoded payload; empty when the bytes do not carry the expected field shape - * (a defensive guard against a key that authenticates a foreign format) + * @return the decoded payload; empty when the bytes do not carry the expected field shape or + * carry a blank session nonce (a defensive guard against a key that authenticates a + * foreign format) */ public static Optional<SealedSessionPayload> decode(byte[] encoded) { Objects.requireNonNull(encoded, "encoded"); @@ -117,10 +141,12 @@ public static Optional<SealedSessionPayload> decode(byte[] encoded) { optionalField(fields[4]), optionalField(fields[5]), optionalField(fields[6]).map(seconds -> Instant.ofEpochSecond(Long.parseLong(seconds))), - Instant.ofEpochSecond(Long.parseLong(decodeField(fields[7]))))); + Instant.ofEpochSecond(Long.parseLong(decodeField(fields[7]))), + decodeField(fields[8]))); } catch (IllegalArgumentException | DateTimeException _) { - // Base64 decode failure, epoch-second parse failure, or an epoch second that parses as a - // long but lies outside Instant's supported range, on bytes that authenticated: a + // Base64 decode failure, epoch-second parse failure, an epoch second that parses as a + // long but lies outside Instant's supported range, or a blank session nonce rejected by + // the canonical constructor, on bytes that authenticated: a // foreign payload format under the same key. DateTimeException is NOT an // IllegalArgumentException, so it must be caught explicitly or it escapes unseal() as an // unhandled request error. Report "no session" rather than propagating. @@ -143,7 +169,8 @@ public boolean isExpired(Duration ttl, Instant now) { } /** - * Overridden to redact every credential — the three tokens. The default record + * Overridden to redact every credential — the three tokens — plus the session nonce, which keys + * the derived session identity and so is treated as secret material. The default record * {@code toString()} would otherwise print the raw token material into any log line, exception * message, or debugger view. * @@ -151,9 +178,9 @@ public boolean isExpired(Duration ttl, Instant now) { */ @Override public String toString() { - return "SealedSessionPayload[accessToken=%s, refreshToken=%s, idToken=%s, sub=%s, sid=%s, acr=%s, authTime=%s, loginInstant=%s]" + return "SealedSessionPayload[accessToken=%s, refreshToken=%s, idToken=%s, sub=%s, sid=%s, acr=%s, authTime=%s, loginInstant=%s, sessionNonce=%s]" .formatted(REDACTED, refreshToken.isPresent() ? "Optional[" + REDACTED + "]" : "Optional.empty", - REDACTED, sub, sid, acr, authTime, loginInstant); + REDACTED, sub, sid, acr, authTime, loginInstant, REDACTED); } private static void appendField(StringBuilder target, String value) { diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinator.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinator.java index 2b0ee01b..64ea6567 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinator.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinator.java @@ -185,6 +185,17 @@ private boolean nearExpiry(SessionRecord session, Instant now) { return !now.isBefore(expiry.minus(leeway)); } + /** + * Rebuilds the session with the rotated token material, carrying every non-token component over + * from {@code previous} unchanged. + * <p> + * Because this reconstructs the record component-by-component, any component NOT copied here is + * silently dropped from the rotated session. That is load-bearing for + * {@link SessionRecord#sessionNonce()}: dropping it would make the cookie-mode binding re-seal + * without a nonce — changing the derived session identity mid-session and breaking the + * single-flight coalescing this coordinator depends on. Add a copy line here for every component + * added to {@link SessionRecord}. + */ private static SessionRecord rotate(SessionRecord previous, RotationResult rotation) { String rotatedIdToken = rotation.idToken(); return SessionRecord.builder() @@ -197,6 +208,7 @@ private static SessionRecord rotate(SessionRecord previous, RotationResult rotat .expiresAt(previous.expiresAt()) .acr(previous.acr()) .authTime(previous.authTime()) + .sessionNonce(previous.sessionNonce()) .build(); } diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/BffRuntime.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/BffRuntime.java index 5a74494b..371495db 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/BffRuntime.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/BffRuntime.java @@ -35,12 +35,13 @@ import org.jspecify.annotations.Nullable; /** - * The assembled server-mode BFF runtime (D16 edge wiring) — the single collaborator the gateway edge + * The assembled BFF runtime (D16 edge wiring) — the single collaborator the gateway edge * consults to serve the {@code require: session} stage-4 runtime and to dispatch a matched reserved * OIDC path to its handler. * <p> * The runtime is built once at boot by {@code BffRuntimeProducer} <strong>only when a global - * {@code oidc} block with {@code session.mode=server} is configured</strong>; a gateway without such + * {@code oidc} block with a {@code redirect_uri} and a recognised {@code session.mode} — + * {@code server} or {@code cookie} — is configured</strong>; a gateway without such * a block gets the {@linkplain #inert() inert} instance, which reports {@link #isActive()} * {@code false} and dispatches nothing (the bearer-only proxy path is unchanged, and the * {@code ReservedPathRegistry} carries no reserved paths to dispatch anyway). @@ -102,8 +103,9 @@ private BffRuntime(boolean active, @Nullable SessionAuthenticationStage sessionS } /** - * Assembles an active server-mode runtime with every reserved-endpoint handler and the session - * stage-4 runtime wired. + * Assembles an active runtime with every reserved-endpoint handler and the session + * stage-4 runtime wired. The same wiring serves both session modes — {@code server} and + * {@code cookie} — which differ only in the {@code SessionBinding} the producer supplies. * * @param sessionStage the {@code require: session} stage-4 runtime * @param csrfDefence the fixed CSRF defence for unsafe-method session requests @@ -132,7 +134,7 @@ public BffRuntime(SessionAuthenticationStage sessionStage, CsrfDefence csrfDefen } /** - * @return the inert runtime — a gateway serving no server-mode BFF variant. It reports + * @return the inert runtime — a gateway serving no BFF variant in either session mode. It reports * {@link #isActive()} {@code false}, exposes no session stage, and dispatches nothing. */ public static BffRuntime inert() { @@ -142,7 +144,7 @@ public static BffRuntime inert() { /** * @return the RFC 9470 step-up coordinator (D7) that handles an upstream * {@code insufficient_user_authentication} challenge for a {@code require: session} route - * @throws IllegalStateException when this runtime is inert (no server-mode BFF variant) + * @throws IllegalStateException when this runtime is inert (no BFF variant is configured) */ public StepUpCoordinator stepUpCoordinator() { if (stepUpCoordinator == null) { @@ -152,8 +154,10 @@ public StepUpCoordinator stepUpCoordinator() { } /** - * @return {@code true} when the gateway serves a server-mode BFF variant (an {@code oidc} block - * with {@code session.mode=server} was configured); {@code false} for a bearer-only gateway + * @return {@code true} when the gateway serves a BFF variant in either session mode (an + * {@code oidc} block with a {@code redirect_uri} and a recognised {@code session.mode} — + * {@code server} or {@code cookie} — was configured); {@code false} for a bearer-only + * gateway */ public boolean isActive() { return active; @@ -162,7 +166,7 @@ public boolean isActive() { /** * @return the {@code require: session} stage-4 runtime the edge wires into the session-aware * {@code AuthenticationStage} - * @throws IllegalStateException when this runtime is inert (no server-mode BFF variant) + * @throws IllegalStateException when this runtime is inert (no BFF variant is configured) */ public SessionAuthenticationStage sessionStage() { if (sessionStage == null) { @@ -174,7 +178,7 @@ public SessionAuthenticationStage sessionStage() { /** * @return the fixed CSRF defence the edge enforces on unsafe-method {@code require: session} * requests - * @throws IllegalStateException when this runtime is inert (no server-mode BFF variant) + * @throws IllegalStateException when this runtime is inert (no BFF variant is configured) */ public CsrfDefence csrfDefence() { if (csrfDefence == null) { diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionRecord.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionRecord.java index 1a22c4ca..dc87e5d7 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionRecord.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionRecord.java @@ -44,12 +44,21 @@ * is itself a bearer credential;</li> * <li><strong>cookie mode</strong>: the minted id is <em>discarded</em> — it is not among the * fields sealed into the cookie. The binding replaces it with an identity <em>derived</em> - * from the sealed payload (a salted digest over the login instant and {@code sub}), so it is + * from the sealed payload (a salted digest over the login instant, {@code sub} and the + * per-session {@link #sessionNonce()}), so it is * stable across re-seals, un-recomputable outside this gateway, and never emitted to the * browser.</li> * </ul> * It is redacted from {@link #toString()} either way. * <p> + * <strong>The session nonce is cookie-mode-only.</strong> {@link #sessionNonce()} is + * {@link Optional#empty()} for every server-mode record — that mode's identity is the minted opaque + * id, which is already unique, so it needs no additional entropy. Only the cookie-mode binding + * populates it, minting it once at login and re-sealing it verbatim on every refresh; without it, + * two logins by the same subject inside one clock second would derive the same identity and + * cross-wire the refresh coordinator's single-flight keying. It is redacted from + * {@link #toString()} because it keys the derived identity. + * <p> * {@link #sid()} and {@link #sub()} back a server-mode store's secondary index for O(1) * back-channel logout destruction. * @@ -62,12 +71,15 @@ * @param expiresAt the absolute session expiry (from login), independent of activity * @param acr the authentication context class, empty when absent * @param authTime the IdP authentication instant, empty when absent + * @param sessionNonce the per-session nonce keying the cookie-mode derived identity; always empty in + * server mode (see the mode split above), and never blank when present * @author API Sheriff Team * @since 1.0 */ @Builder public record SessionRecord(String sessionId, String accessToken, Optional<String> refreshToken, String idToken, -String sub, Optional<String> sid, Instant expiresAt, Optional<String> acr, Optional<Instant> authTime) { +String sub, Optional<String> sid, Instant expiresAt, Optional<String> acr, Optional<Instant> authTime, +Optional<String> sessionNonce) { private static final String REDACTED = "***REDACTED***"; private static final SecureRandom SECURE_RANDOM = new SecureRandom(); @@ -76,6 +88,15 @@ public record SessionRecord(String sessionId, String accessToken, Optional<Strin /** * Canonical constructor rejecting absent mandatory components and normalizing absent * optionals to {@link Optional#empty()}. + * <p> + * {@code sessionNonce} stays optional — an <em>absent</em> nonce is valid and is the normal + * server-mode shape. Only a <em>present but blank</em> value is rejected: it keys the + * cookie-mode derived identity, so an empty string would silently degrade that identity back to + * the colliding pre-nonce shape instead of failing. The nonce value itself never reaches the + * exception message. + * + * @throws NullPointerException when a mandatory component is {@code null} + * @throws IllegalArgumentException when {@code sessionNonce} is present but blank */ public SessionRecord { Objects.requireNonNull(sessionId, "sessionId"); @@ -87,6 +108,10 @@ public record SessionRecord(String sessionId, String accessToken, Optional<Strin sid = Objects.requireNonNullElse(sid, Optional.empty()); acr = Objects.requireNonNullElse(acr, Optional.empty()); authTime = Objects.requireNonNullElse(authTime, Optional.empty()); + sessionNonce = Objects.requireNonNullElse(sessionNonce, Optional.empty()); + if (sessionNonce.filter(String::isBlank).isPresent()) { + throw new IllegalArgumentException("sessionNonce must not be blank when present"); + } } /** @@ -116,7 +141,8 @@ public boolean isExpired(Instant now) { } /** - * Overridden to redact every credential — the session id and all three tokens. The default + * Overridden to redact every credential — the session id, all three tokens, and the session + * nonce that keys the cookie-mode derived identity. The default * record {@code toString()} would otherwise print the bearer session id and the raw token * material into any log line, exception message, or debugger view. * @@ -124,8 +150,9 @@ public boolean isExpired(Instant now) { */ @Override public String toString() { - return "SessionRecord[sessionId=%s, accessToken=%s, refreshToken=%s, idToken=%s, sub=%s, sid=%s, expiresAt=%s, acr=%s, authTime=%s]" + return "SessionRecord[sessionId=%s, accessToken=%s, refreshToken=%s, idToken=%s, sub=%s, sid=%s, expiresAt=%s, acr=%s, authTime=%s, sessionNonce=%s]" .formatted(REDACTED, REDACTED, refreshToken.isPresent() ? "Optional[" + REDACTED + "]" : "Optional.empty", - REDACTED, sub, sid, expiresAt, acr, authTime); + REDACTED, sub, sid, expiresAt, acr, authTime, + sessionNonce.isPresent() ? "Optional[" + REDACTED + "]" : "Optional.empty"); } } diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducer.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducer.java index 63dfc047..731fa81c 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducer.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducer.java @@ -80,7 +80,7 @@ import jakarta.inject.Singleton; /** - * CDI producer of the server-mode {@link BffRuntime} — the D16 edge wiring that makes the D1–D12 BFF + * CDI producer of the {@link BffRuntime} — the D16 edge wiring that makes the D1–D12 BFF * components reachable at the live gateway edge. * <p> * The producer builds the active runtime <strong>only when a global {@code oidc} block with a @@ -101,9 +101,9 @@ * {@code StepUpHandler#initiate} for RFC 9470 re-drive — so the engine is reached at runtime. * <p> * <strong>Lazy discovery.</strong> The OIDC provider metadata is resolved through a memoized supplier - * on first engine use, not at boot: a server-mode gateway therefore boots (and is unit-testable) - * without a live IdP, and the discovery-dependent {@code end_session_endpoint} the logout leg needs is - * materialized only when the first logout arrives. + * on first engine use, not at boot: a BFF gateway in either session mode therefore boots (and is + * unit-testable) without a live IdP, and the discovery-dependent {@code end_session_endpoint} the + * logout leg needs is materialized only when the first logout arrives. * * @author API Sheriff Team * @since 1.0 @@ -127,7 +127,8 @@ public class BffRuntimeProducer { /** * @param gatewayConfig the bound global gateway document carrying the {@code oidc} block * @param tokenValidator a lazy handle to the gateway's shared offline validator, resolved only on - * the active server-mode path (a bearer-only gateway never triggers it) + * the active BFF path in either session mode (a bearer-only gateway never + * triggers it) */ public BffRuntimeProducer(GatewayConfig gatewayConfig, @GatewayValidator Instance<TokenValidator> tokenValidator) { diff --git a/api-sheriff/src/main/resources/application.properties b/api-sheriff/src/main/resources/application.properties index 6390b6a7..e7020767 100644 --- a/api-sheriff/src/main/resources/application.properties +++ b/api-sheriff/src/main/resources/application.properties @@ -46,7 +46,7 @@ quarkus.tls.cipher-suites=TLS_AES_256_GCM_SHA384,TLS_CHACHA20_POLY1305_SHA256,TL quarkus.tls.alpn=true # Native Image Configuration -# The three BFF records below each hold a `static final SecureRandom`. GraalVM initializes static +# The four BFF classes below each hold a `static final SecureRandom`. GraalVM initializes static # state at BUILD time by default, which would bake a seeded SecureRandom into the image heap # (a build failure — "instance of Random/SplittableRandom in the image heap" — AND a real security # defect: predictable randomness from a cached seed). Force runtime initialization for each so the @@ -60,7 +60,10 @@ quarkus.tls.alpn=true # package-prefix --initialize-at-run-time=de.cuioss.sheriff.token.client.flow covers the whole flow # package (GraalVM treats a bare package name as a prefix over that package and its classes), so # every current and future sibling flow class is forced to runtime initialization in one entry. -quarkus.native.additional-build-args=--enable-url-protocols=https,--enable-http,--enable-https,-O2,--initialize-at-run-time=de.cuioss.sheriff.gateway.bff.pending.PendingAuthorizationRecord,--initialize-at-run-time=de.cuioss.sheriff.gateway.bff.session.SessionRecord,--initialize-at-run-time=de.cuioss.sheriff.gateway.bff.logout.RpInitiatedLogout,--initialize-at-run-time=de.cuioss.sheriff.token.client.flow +# CookieSessionBinding joins the list for the same reason: the cookie-mode per-session nonce is +# minted from a `static final SecureRandom` on the login path, so its class initializer would +# otherwise bake a seeded instance into the image heap. +quarkus.native.additional-build-args=--enable-url-protocols=https,--enable-http,--enable-https,-O2,--initialize-at-run-time=de.cuioss.sheriff.gateway.bff.pending.PendingAuthorizationRecord,--initialize-at-run-time=de.cuioss.sheriff.gateway.bff.session.SessionRecord,--initialize-at-run-time=de.cuioss.sheriff.gateway.bff.logout.RpInitiatedLogout,--initialize-at-run-time=de.cuioss.sheriff.gateway.bff.cookie.CookieSessionBinding,--initialize-at-run-time=de.cuioss.sheriff.token.client.flow quarkus.native.resources.includes=**/*.p12,**/*.crt,**/*.key,schema/*.json quarkus.native.monitoring=jfr # Java 25 Mandrel builder image for JEP 491 (virtual threads without pinning) — see ADR-0001 diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieKeyMaterialTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieKeyMaterialTest.java index 0bf04bb6..7c8fbd68 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieKeyMaterialTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieKeyMaterialTest.java @@ -57,7 +57,7 @@ private static String base64Key(byte fill) { private static SealedSessionPayload payload() { return new SealedSessionPayload("access", Optional.empty(), "id-token", "user-sub-1", - Optional.empty(), Optional.empty(), Optional.empty(), LOGIN); + Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, "session-nonce"); } /** Reads the key-id byte a sealed value is stamped with (value layout: version, key-id, …). */ diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBindingTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBindingTest.java index 503df1c1..44e1f403 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBindingTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBindingTest.java @@ -18,6 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.time.Duration; @@ -40,6 +41,8 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; /** * Tests for {@link CookieSessionBinding} — the stateless seam implementation. Covers the @@ -48,7 +51,9 @@ * than the reset lifetime on a re-seal, the {@code previous_key} rotation (a retired-key cookie * resolves and its next write is sealed under the current key, while withdrawing the retired key * makes it no session), the stable-but-never-emitted session identity, and the - * {@code UNSUPPORTED} IdP-driven destruction capability, and the once-per-process rotation record. + * {@code UNSUPPORTED} IdP-driven destruction capability, the once-per-process rotation record, and + * the {@link SessionRecord} session-nonce contract (absent is the valid server-mode shape; a + * present-but-blank value is refused because it would degrade the derived identity). */ @EnableTestLogger class CookieSessionBindingTest { @@ -105,6 +110,37 @@ private static SessionRecord session(String accessToken, Instant expiresAt) { .build(); } + /** + * Rebuilds {@code original} with a rotated access token, carrying every other component — + * notably the session nonce — over verbatim, exactly as the refresh coordinator's + * {@code rotate()} does. A re-seal test must carry the nonce forward this way: {@code persist} + * refuses a record without one rather than re-minting it. + */ + private static SessionRecord withRotatedAccessToken(SessionRecord original, String rotatedAccessToken) { + return SessionRecord.builder() + .sessionId(original.sessionId()) + .accessToken(rotatedAccessToken) + .refreshToken(original.refreshToken()) + .idToken(original.idToken()) + .sub(original.sub()) + .sid(original.sid()) + .expiresAt(original.expiresAt()) + .acr(original.acr()) + .authTime(original.authTime()) + .sessionNonce(original.sessionNonce()) + .build(); + } + + /** + * A record that is legal to re-seal: bound then resolved, so it carries the session nonce minted + * at login. + */ + private SessionRecord resealable(String rotatedAccessToken) { + BoundSession bound = binding.bind(session(ACCESS_TOKEN, LOGIN.plus(TTL)), LOGIN); + SessionRecord resolved = binding.resolve(cookieHeaderOf(bound), LOGIN).orElseThrow(); + return withRotatedAccessToken(resolved, rotatedAccessToken); + } + private static String cookieHeaderOf(BoundSession bound) { String setCookie = bound.setCookieHeaders().getFirst(); return setCookie.substring(0, setCookie.indexOf(';')); @@ -193,6 +229,7 @@ void shouldResealRotatedMaterial() { .sub(resolved.sub()) .sid(resolved.sid()) .expiresAt(resolved.expiresAt()) + .sessionNonce(resolved.sessionNonce()) .build(); BoundSession reBound = binding.persist(rotated, LOGIN.plusSeconds(60)); @@ -206,7 +243,7 @@ void shouldResealRotatedMaterial() { @DisplayName("Should carry the remaining, not the reset, lifetime in Max-Age on a re-seal") void shouldNotExtendTheSessionOnReseal() { Instant halfway = LOGIN.plus(TTL.dividedBy(2)); - SessionRecord rotated = session("rotated-access-token", LOGIN.plus(TTL)); + SessionRecord rotated = resealable("rotated-access-token"); BoundSession reBound = binding.persist(rotated, halfway); @@ -219,7 +256,7 @@ void shouldNotExtendTheSessionOnReseal() { @DisplayName("Should keep the absolute deadline anchored across a re-seal") void shouldKeepDeadlineAnchoredAcrossReseal() { Instant halfway = LOGIN.plus(TTL.dividedBy(2)); - BoundSession reBound = binding.persist(session("rotated-access-token", LOGIN.plus(TTL)), halfway); + BoundSession reBound = binding.persist(resealable("rotated-access-token"), halfway); String cookieHeader = cookieHeaderOf(reBound); assertTrue(binding.resolve(cookieHeader, LOGIN.plus(TTL).minusSeconds(1)).isPresent()); @@ -318,12 +355,37 @@ void shouldDeriveStableIdentity() { BoundSession bound = binding.bind(session(ACCESS_TOKEN, LOGIN.plus(TTL)), LOGIN); SessionRecord first = binding.resolve(cookieHeaderOf(bound), LOGIN).orElseThrow(); - BoundSession reBound = binding.persist(session("rotated-access-token", LOGIN.plus(TTL)), - LOGIN.plusSeconds(60)); + SessionRecord rotated = withRotatedAccessToken(first, "rotated-access-token"); + BoundSession reBound = binding.persist(rotated, LOGIN.plusSeconds(60)); SessionRecord second = binding.resolve(cookieHeaderOf(reBound), LOGIN.plusSeconds(60)).orElseThrow(); assertEquals(first.sessionId(), second.sessionId(), - "the identity is derived from the login instant and sub, both fixed for the session's life"); + "the identity is derived from the login instant, sub and session nonce — all three fixed " + + "for the session's life, so a re-seal reproduces it byte-identically"); + } + + @Test + @DisplayName("Should derive distinct identities for two logins by the same subject in one clock second") + void shouldSeparateIdentitiesForSameSubjectWithinOneSecond() { + // Same subject, same login instant — before the per-session nonce these two collided, + // cross-wiring the refresh coordinator's single-flight keying between distinct sessions. + String first = binding.resolve(cookieHeaderOf(binding.bind(session(ACCESS_TOKEN, LOGIN.plus(TTL)), LOGIN)), + LOGIN).orElseThrow().sessionId(); + String second = binding.resolve(cookieHeaderOf(binding.bind(session(ACCESS_TOKEN, LOGIN.plus(TTL)), LOGIN)), + LOGIN).orElseThrow().sessionId(); + + assertNotEquals(first, second, + "two logins by the same sub inside one clock second must not share a session identity"); + } + + @Test + @DisplayName("Should refuse to re-seal a record carrying no session nonce rather than re-mint one") + void shouldRefuseReSealWithoutNonce() { + // A record reconstructed without the nonce would otherwise silently acquire a new identity. + SessionRecord nonceless = session(ACCESS_TOKEN, LOGIN.plus(TTL)); + + assertThrows(IllegalStateException.class, () -> binding.persist(nonceless, LOGIN), + "re-minting here would change the derived identity mid-session"); } @Test @@ -352,6 +414,65 @@ void shouldSeparateIdentitiesBySubject() { } } + @Nested + @DisplayName("Session-record nonce contract") + class SessionRecordNonceContract { + + private static SessionRecord.SessionRecordBuilder baseRecord() { + return SessionRecord.builder() + .sessionId("ignored-on-bind") + .accessToken(ACCESS_TOKEN) + .idToken(ID_TOKEN) + .sub(SUB) + .expiresAt(LOGIN.plus(TTL)); + } + + @ParameterizedTest + @ValueSource(strings = {"", " ", "\t", "\n", " "}) + @DisplayName("Should reject a present-but-blank session nonce") + void shouldRejectPresentButBlankNonce(String blank) { + SessionRecord.SessionRecordBuilder builder = baseRecord().sessionNonce(Optional.of(blank)); + + assertThrows(IllegalArgumentException.class, builder::build, + "a blank nonce would silently degrade the derived identity to the colliding pre-nonce shape"); + } + + @Test + @DisplayName("Should keep the rejected nonce out of the exception message") + void shouldNotLeakNonceIntoRejectionMessage() { + SessionRecord.SessionRecordBuilder builder = baseRecord().sessionNonce(Optional.of(" ")); + + IllegalArgumentException rejection = assertThrows(IllegalArgumentException.class, builder::build); + + assertEquals("sessionNonce must not be blank when present", rejection.getMessage()); + } + + @Test + @DisplayName("Should accept an absent session nonce — that is the server-mode shape") + void shouldAcceptAbsentNonce() { + SessionRecord serverMode = baseRecord().build(); + + assertTrue(serverMode.sessionNonce().isEmpty(), + "server-mode records carry no nonce and must stay constructible"); + } + + @Test + @DisplayName("Should normalize a null session nonce to empty rather than reject it") + void shouldNormalizeNullNonceToEmpty() { + SessionRecord serverMode = baseRecord().sessionNonce(null).build(); + + assertTrue(serverMode.sessionNonce().isEmpty()); + } + + @Test + @DisplayName("Should accept a non-blank session nonce") + void shouldAcceptNonBlankNonce() { + SessionRecord cookieMode = baseRecord().sessionNonce(Optional.of("session-nonce-material")).build(); + + assertEquals(Optional.of("session-nonce-material"), cookieMode.sessionNonce()); + } + } + @Nested @DisplayName("Destruction") class Destruction { diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodecTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodecTest.java index a34149d4..c810bc92 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodecTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodecTest.java @@ -65,6 +65,7 @@ class SealedSessionCookieCodecTest { private static final String REFRESH_TOKEN = "raw-refresh-token-SECRET-material"; private static final String ID_TOKEN = "raw-id-token-SECRET-material"; private static final String SUB = "user-sub-1"; + private static final String SESSION_NONCE = "session-nonce-SECRET-material"; private SecretKey key; private SealedSessionCookieCodec codec; @@ -84,7 +85,7 @@ private static SecretKey aesKey(byte fill) { private static SealedSessionPayload payload() { return new SealedSessionPayload(ACCESS_TOKEN, Optional.of(REFRESH_TOKEN), ID_TOKEN, SUB, Optional.of("idp-sid-9"), Optional.of("urn:acr:silver"), - Optional.of(Instant.parse("2026-07-27T09:59:00Z")), LOGIN); + Optional.of(Instant.parse("2026-07-27T09:59:00Z")), LOGIN, SESSION_NONCE); } private static String flipByteAt(String sealedValue, int index) { @@ -130,6 +131,13 @@ void shouldCarryVersionAndKeyIdHeader() throws Exception { assertEquals(SealedSessionCookieCodec.FORMAT_VERSION, raw[0]); assertEquals(KEY_ID, raw[1]); } + + @Test + @DisplayName("Should stamp format version 2 — the nine-field payload shape") + void shouldStampFormatVersionTwo() { + assertEquals(SealedSessionCookieCodec.FORMAT_VERSION, (byte) 2, + "the per-session nonce raised the payload to nine fields, which is a wire-format break"); + } } @Nested @@ -184,6 +192,18 @@ void shouldRejectUnknownVersion() throws Exception { "an unknown format version is refused before any decrypt attempt"); } + @Test + @DisplayName("Should reject a cookie stamped with the retired version 1 format") + void shouldRejectLegacyFormatVersionOne() throws Exception { + String sealed = codec.seal(payload()); + byte[] raw = Base64.getUrlDecoder().decode(sealed); + raw[0] = 1; + + assertTrue(codec.unseal(Base64.getUrlEncoder().withoutPadding().encodeToString(raw)).isEmpty(), + "the version bump to 2 is a clean break: a v1 cookie is refused outright rather than " + + "parsed against the nine-field payload shape"); + } + @Test @DisplayName("Should reject an unknown key id without trying the key it does hold") void shouldRejectUnknownKeyId() throws Exception { @@ -290,7 +310,7 @@ class SizeBudget { void shouldRefuseOversizedPayload() { String huge = "x".repeat(BUDGET * 2); SealedSessionPayload oversized = new SealedSessionPayload(huge, Optional.empty(), ID_TOKEN, SUB, - Optional.empty(), Optional.empty(), Optional.empty(), LOGIN); + Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, SESSION_NONCE); CookieSizeBudgetExceededException thrown = assertThrows(CookieSizeBudgetExceededException.class, () -> codec.seal(oversized)); @@ -396,7 +416,7 @@ void shouldNotLeakTokensIntoTheHeader() throws Exception { void shouldNotLeakKeyMaterialIntoTheOverBudgetMessage() { String huge = "x".repeat(BUDGET * 2); SealedSessionPayload oversized = new SealedSessionPayload(huge, Optional.empty(), ID_TOKEN, SUB, - Optional.empty(), Optional.empty(), Optional.empty(), LOGIN); + Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, SESSION_NONCE); CookieSizeBudgetExceededException thrown = assertThrows(CookieSizeBudgetExceededException.class, () -> codec.seal(oversized)); diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java index 194b3211..ce100079 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java @@ -31,6 +31,8 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; /** * Tests for {@link SealedSessionPayload} — the plaintext the cookie-mode codec seals. Covers the @@ -46,16 +48,17 @@ class SealedSessionPayloadTest { private static final String REFRESH_TOKEN = "raw-refresh-token-SECRET-material"; private static final String ID_TOKEN = "raw-id-token-SECRET-material"; private static final String SUB = "user-sub-1"; + private static final String SESSION_NONCE = "session-nonce-SECRET-material"; private static SealedSessionPayload full() { return new SealedSessionPayload(ACCESS_TOKEN, Optional.of(REFRESH_TOKEN), ID_TOKEN, SUB, Optional.of("idp-sid-9"), Optional.of("urn:acr:silver"), - Optional.of(Instant.parse("2026-07-27T09:59:00Z")), LOGIN); + Optional.of(Instant.parse("2026-07-27T09:59:00Z")), LOGIN, SESSION_NONCE); } private static SealedSessionPayload minimal() { return new SealedSessionPayload(ACCESS_TOKEN, Optional.empty(), ID_TOKEN, SUB, - Optional.empty(), Optional.empty(), Optional.empty(), LOGIN); + Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, SESSION_NONCE); } /** @@ -95,13 +98,16 @@ void shouldRoundTripAbsentOptionals() { assertTrue(decoded.sid().isEmpty()); assertTrue(decoded.acr().isEmpty()); assertTrue(decoded.authTime().isEmpty()); + assertEquals(SESSION_NONCE, decoded.sessionNonce(), + "the session nonce is mandatory and survives the round trip verbatim"); } @Test @DisplayName("Should round-trip values containing the field separator and other structural characters") void shouldRoundTripSeparatorBearingValues() { SealedSessionPayload original = new SealedSessionPayload("token\nwith\nnewlines", Optional.of("a=b;c"), - ID_TOKEN, "sub\nwith\nnewline", Optional.of("sid\n1"), Optional.empty(), Optional.empty(), LOGIN); + ID_TOKEN, "sub\nwith\nnewline", Optional.of("sid\n1"), Optional.empty(), Optional.empty(), LOGIN, + "nonce\nwith\nnewline"); assertEquals(Optional.of(original), SealedSessionPayload.decode(original.encode()), "each field is base64-encoded, so no value can collide with the separator"); @@ -117,12 +123,25 @@ void shouldDecodeNothingFromForeignShape() { @Test @DisplayName("Should decode nothing when a field is not valid base64") void shouldDecodeNothingFromMalformedFields() { - byte[] malformed = "~~~\n~~~\n~~~\n~~~\n\n\n\n~~~".getBytes(StandardCharsets.UTF_8); + // Nine fields, so the field-count guard passes and the base64 failure is genuinely what + // this test exercises rather than the shape check short-circuiting ahead of it. + byte[] malformed = "~~~\n~~~\n~~~\n~~~\n\n\n\n~~~\n~~~".getBytes(StandardCharsets.UTF_8); assertTrue(SealedSessionPayload.decode(malformed).isEmpty(), "a base64 failure on authenticated-but-foreign bytes is no session, never an exception"); } + @Test + @DisplayName("Should decode nothing from a legacy eight-field payload — no backward-compatible path") + void shouldDecodeNothingFromLegacyEightFieldPayload() { + byte[] legacy = wireForm(ACCESS_TOKEN, "", ID_TOKEN, SUB, "", "", "", + Long.toString(LOGIN.getEpochSecond())); + + assertTrue(SealedSessionPayload.decode(legacy).isEmpty(), + "the pre-nonce eight-field shape is rejected outright: admitting it would require " + + "synthesizing a nonce and would silently change the derived session identity"); + } + @Test @DisplayName("Should decode nothing when an epoch second parses as a long but exceeds Instant's range") void shouldDecodeNothingFromOutOfRangeEpochSecond() { @@ -131,13 +150,16 @@ void shouldDecodeNothingFromOutOfRangeEpochSecond() { String login = Long.toString(LOGIN.getEpochSecond()); assertTrue(SealedSessionPayload - .decode(wireForm(ACCESS_TOKEN, "", ID_TOKEN, SUB, "", "", "", beyondInstantMax)).isEmpty(), + .decode(wireForm(ACCESS_TOKEN, "", ID_TOKEN, SUB, "", "", "", beyondInstantMax, + SESSION_NONCE)).isEmpty(), "DateTimeException is not an IllegalArgumentException, so an out-of-range login instant " + "must still decode to no session rather than escaping unseal()"); assertTrue(SealedSessionPayload - .decode(wireForm(ACCESS_TOKEN, "", ID_TOKEN, SUB, "", "", "", beyondInstantMin)).isEmpty()); + .decode(wireForm(ACCESS_TOKEN, "", ID_TOKEN, SUB, "", "", "", beyondInstantMin, SESSION_NONCE)) + .isEmpty()); assertTrue(SealedSessionPayload - .decode(wireForm(ACCESS_TOKEN, "", ID_TOKEN, SUB, "", "", beyondInstantMax, login)).isEmpty(), + .decode(wireForm(ACCESS_TOKEN, "", ID_TOKEN, SUB, "", "", beyondInstantMax, login, + SESSION_NONCE)).isEmpty(), "the optional authTime field carries the same overflow risk as the login instant"); } } @@ -163,7 +185,7 @@ void shouldBeExpiredAtAndAfterDeadline() { @DisplayName("Should anchor the deadline on the login instant, so it cannot drift") void shouldAnchorDeadlineOnLoginInstant() { SealedSessionPayload resealed = new SealedSessionPayload("rotated-access", Optional.of("rotated-refresh"), - ID_TOKEN, SUB, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN); + ID_TOKEN, SUB, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, SESSION_NONCE); assertTrue(resealed.isExpired(TTL, LOGIN.plus(TTL)), "re-sealing rotated material does not move the absolute deadline"); @@ -178,20 +200,53 @@ class Contract { @DisplayName("Should reject an absent mandatory component") void shouldRejectAbsentMandatoryComponents() { assertThrows(NullPointerException.class, () -> new SealedSessionPayload(null, Optional.empty(), ID_TOKEN, - SUB, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN)); + SUB, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, SESSION_NONCE)); + assertThrows(NullPointerException.class, () -> new SealedSessionPayload(ACCESS_TOKEN, Optional.empty(), + null, SUB, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, SESSION_NONCE)); assertThrows(NullPointerException.class, () -> new SealedSessionPayload(ACCESS_TOKEN, Optional.empty(), - null, SUB, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN)); + ID_TOKEN, null, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, SESSION_NONCE)); assertThrows(NullPointerException.class, () -> new SealedSessionPayload(ACCESS_TOKEN, Optional.empty(), - ID_TOKEN, null, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN)); + ID_TOKEN, SUB, Optional.empty(), Optional.empty(), Optional.empty(), null, SESSION_NONCE)); assertThrows(NullPointerException.class, () -> new SealedSessionPayload(ACCESS_TOKEN, Optional.empty(), - ID_TOKEN, SUB, Optional.empty(), Optional.empty(), Optional.empty(), null)); + ID_TOKEN, SUB, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, null), + "the session nonce is mandatory — it keys the derived session identity"); + } + + @ParameterizedTest + @ValueSource(strings = {"", " ", "\t", "\n", " "}) + @DisplayName("Should reject a blank session nonce") + void shouldRejectBlankSessionNonce(String blank) { + assertThrows(IllegalArgumentException.class, () -> new SealedSessionPayload(ACCESS_TOKEN, + Optional.empty(), ID_TOKEN, SUB, Optional.empty(), Optional.empty(), Optional.empty(), + LOGIN, blank), + "a blank nonce would silently degrade the derived identity to the colliding pre-nonce shape"); + } + + @Test + @DisplayName("Should keep the rejected nonce out of the exception message") + void shouldNotLeakNonceIntoRejectionMessage() { + IllegalArgumentException rejection = assertThrows(IllegalArgumentException.class, + () -> new SealedSessionPayload(ACCESS_TOKEN, Optional.empty(), ID_TOKEN, SUB, Optional.empty(), + Optional.empty(), Optional.empty(), LOGIN, " ")); + + assertEquals("sessionNonce must not be blank", rejection.getMessage()); + } + + @Test + @DisplayName("Should decode nothing from authenticated bytes carrying a blank session nonce") + void shouldDecodeNothingFromBlankNonce() { + byte[] blankNonce = wireForm(ACCESS_TOKEN, "", ID_TOKEN, SUB, "", "", "", + Long.toString(LOGIN.getEpochSecond()), ""); + + assertTrue(SealedSessionPayload.decode(blankNonce).isEmpty(), + "the constructor rejection surfaces as no session, never as an escaping exception"); } @Test @DisplayName("Should normalize a null optional to empty") void shouldNormalizeNullOptionals() { SealedSessionPayload payload = new SealedSessionPayload(ACCESS_TOKEN, null, ID_TOKEN, SUB, - null, null, null, LOGIN); + null, null, null, LOGIN, SESSION_NONCE); assertTrue(payload.refreshToken().isEmpty()); assertTrue(payload.sid().isEmpty()); @@ -212,6 +267,8 @@ void shouldRedactCredentials() { assertFalse(rendered.contains(ACCESS_TOKEN), rendered); assertFalse(rendered.contains(REFRESH_TOKEN), rendered); assertFalse(rendered.contains(ID_TOKEN), rendered); + assertFalse(rendered.contains(SESSION_NONCE), + "the session nonce keys the derived identity and must not reach a log line"); assertTrue(rendered.contains("***REDACTED***"), rendered); } diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinatorTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinatorTest.java index 63d76e69..4d9c6af8 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinatorTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinatorTest.java @@ -367,6 +367,31 @@ void shouldServeTheRotatedMaterialFromTheReSealedCookie() { "the re-seal preserves the original absolute deadline — a refresh never extends the session"); } + @Test + @DisplayName("Should propagate the session nonce from the previous record onto the rotated one") + void shouldPropagateSessionNonceAcrossRotate() { + // rotate() rebuilds the record component-by-component, so an uncopied component is + // silently dropped. Dropping the nonce would make persist() refuse the re-seal outright — + // this asserts the copy directly rather than inferring it from identity stability. + assertTrue(cookieSession.sessionNonce().isPresent(), + "a resolved cookie-mode record always carries the nonce sealed at login"); + TokenRefreshCoordinator coordinator = cookieCoordinator(rt -> rotation()); + + RefreshOutcome outcome = coordinator.refresh(cookieSession, sealedCookieHeader, NOW); + + assertEquals(cookieSession.sessionNonce(), outcome.session().orElseThrow().sessionNonce(), + "the rotated record carries the previous record's nonce verbatim — never a fresh one"); + } + + @Test + @DisplayName("Should leave a server-mode record's session nonce empty") + void shouldLeaveServerModeNonceEmpty() { + SessionRecord serverModeRecord = session(CURRENT_REFRESH); + + assertTrue(serverModeRecord.sessionNonce().isEmpty(), + "the nonce is cookie-mode-only; server mode's minted opaque id is already unique"); + } + @Test @DisplayName("Should keep the derived identity stable across the re-seal, so single-flight keys the same") void shouldKeepTheSingleFlightKeyStable() { diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/session/InMemorySessionStoreTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/session/InMemorySessionStoreTest.java index bb0a27c4..e832603f 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/session/InMemorySessionStoreTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/session/InMemorySessionStoreTest.java @@ -238,18 +238,19 @@ void shouldRedactCredentialsInToString() { @Test @DisplayName("Should normalize absent optionals and reject null mandatory components") void shouldNormalizeAndReject() { - SessionRecord sparse = new SessionRecord("s", "at", null, "it", "sub", null, FUTURE, null, null); + SessionRecord sparse = new SessionRecord("s", "at", null, "it", "sub", null, FUTURE, null, null, null); assertTrue(sparse.refreshToken().isEmpty()); assertTrue(sparse.sid().isEmpty()); assertTrue(sparse.acr().isEmpty()); assertTrue(sparse.authTime().isEmpty()); + assertTrue(sparse.sessionNonce().isEmpty(), "an absent session nonce normalizes to empty"); assertThrows(NullPointerException.class, () -> new SessionRecord(null, "at", Optional.empty(), "it", "sub", Optional.empty(), FUTURE, - Optional.empty(), Optional.empty())); + Optional.empty(), Optional.empty(), Optional.empty())); assertThrows(NullPointerException.class, () -> new SessionRecord("s", "at", Optional.empty(), "it", null, Optional.empty(), FUTURE, - Optional.empty(), Optional.empty())); + Optional.empty(), Optional.empty(), Optional.empty())); } } } diff --git a/doc/README.adoc b/doc/README.adoc index e7796e1e..84db177a 100644 --- a/doc/README.adoc +++ b/doc/README.adoc @@ -105,6 +105,11 @@ configuration. Each has its own design section: | Task-oriented companions to the Variant 2 design section above: link:user/bff-session.adoc[operator setup] (configure and run the server-session BFF) and link:development/bff-session.adoc[contributor guide] (build, test, and extend it). + +| Variant 3 -- three-layer guides +| Task-oriented companions to the Variant 3 design section above: + link:user/bff-cookie.adoc[operator setup] (configure and run the cookie-based BFF) and + link:development/bff-cookie.adoc[contributor guide] (build, test, and extend it). |=== == Implementation Plans @@ -170,6 +175,26 @@ implementing it. of alias-resolvability violations. *(Status: accepted.)* +| link:adr/0010-YAML_expansion_limits_enforced_in_a_compose-only_pre-pass_before_bind.adoc[ADR-0010] +| YAML alias-expansion and nesting-bomb caps are enforced in a dedicated SnakeYAML compose-only + pre-pass before Jackson binds a config document, because Jackson's `YAMLParser` consumes the + event stream directly and never runs the Composer that counts collection aliases. + *(Status: proposed.)* + +| link:adr/0011-gatewayyaml_exposes_JWKS_trust_and_egress_as_neutral_names_bound_by_the_deployment.adoc[ADR-0011] +| `gateway.yaml` is API Sheriff's own configuration language, so the JWKS issuer surface exposes + egress and TLS trust as framework-neutral names (`allowed_egress_hosts`, `tls_profile`) that carry + no trust material; exactly one mapping component binds a logical trust-profile name to concrete + runtime trust anchors, and config stays neutral while startup diagnostics name the concrete + runtime key. *(Status: accepted.)* + +| link:adr/0012-Comparative_cross-gateway_benchmarking_runs_on-demand_and_stays_segregated_from_the_CI_baseline.adoc[ADR-0012] +| Cross-gateway comparison benchmarking is a target-neutral, on-demand-only lane whose results stay + strictly segregated from the authoritative CI baseline and its published pages; one target + parameter switches the whole matrix between gateways, and fairness parity (identical upstreams, + auth realm, TLS material and resource limits) is a precondition of any comparison. + *(Status: proposed.)* + | link:adr/0013-anchor-type-access-axes.adoc[ADR-0013] | Anchor type/access axes: two required, orthogonal classification axes on every anchor -- `type` (`proxy`/`bff`/`asset`) and `access` (`public`/`authenticated`) -- validated @@ -194,6 +219,24 @@ implementing it. | gRPC gateway rejections are emitted as trailers-only gRPC responses (a gRPC client cannot consume `application/problem+json`), with a canonical status mapping and h2-negotiation-failure to `UNAVAILABLE`. *(Status: accepted.)* + +| link:adr/0017-Accept-time_SNI_split_at_a_dedicated_front_listener_passthrough_relays_opaquely_at_L4_everything_else_terminates_internally.adoc[ADR-0017] +| A dedicated raw-TCP front listener owns the public TLS port, reassembles the ClientHello, and by + SNI either opaquely L4-relays a passthrough match to its resolved backend or hands the + still-encrypted connection to an internal terminating HTTPS listener that solely owns TLS + termination and mTLS. *(Status: proposed.)* + +| link:adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc[ADR-0018] +| Session mode selects only which `SessionBinding` is assembled; reserved paths, CSRF and session + identity are fixed, mode-neutral models behind that one seam -- including the cookie-mode + two-mode key material and the per-session identity nonce whose format bump forces a re-login. + *(Status: proposed.)* + +| link:adr/0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc[ADR-0019] +| Two deliberate inbound-validation relaxations the BFF requires: the url-parameter value pipeline + is not applied to matched reserved paths (they terminate at the gateway and their handlers + validate their own parameters), and the cookie-header value-length raise is necessarily + gateway-wide because inbound validation precedes route selection. *(Status: proposed.)* |=== == Reading Order diff --git a/doc/adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc b/doc/adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc new file mode 100644 index 00000000..771a7d1d --- /dev/null +++ b/doc/adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc @@ -0,0 +1,221 @@ += ADR-0018: BFF session mode is one SessionBinding seam behind a fixed reserved-path and CSRF model +:toc: left +:toclevels: 2 +:sectnums: + +// adr-metadata +// Progressive-disclosure metadata block (see manage-adr SKILL.md → "ADR Template +// Structure"). Read by `manage-adr.py scan` so a caller can assess an ADR's +// relevance without reading the full file. List fields are comma-separated. +// summary: Session mode selects only which SessionBinding is assembled; reserved paths, CSRF and session identity are fixed, mode-neutral models behind that one seam +// tags: bff, session-mode, csrf, reserved-paths, cookie-session, key-material, session-identity +// affects: api-sheriff +// supersedes: +// end-adr-metadata + +== Status + +Proposed + +== Context + +A Backend-For-Frontend gateway that acts as an OIDC confidential client has to answer four +structural questions at once, and each of them admits a tempting per-deployment or per-route answer +that fragments the security model. + +*Gateway-owned paths versus proxied namespaces.* The BFF must serve its own OIDC endpoints — the +auth-code callback, login initiation, logout and its return leg, the back-channel logout receiver, +and a curated user-info view. Those paths live on the same host as proxied traffic. If they are +resolved by the ordinary route table, a proxy prefix can swallow them: a route matching a broad +prefix silently captures the callback, and a login that appears to be misconfigured is in fact +being forwarded upstream. The failure is silent and configuration-dependent, which is the worst +combination for a security-critical path. + +*CSRF as a knob versus a fixed property.* A cookie-authenticated session is inherently exposed to +cross-site request forgery. Making the defence configurable creates a deployment in which it is +off, and the class of gateway most likely to turn it off is the one that least understands the +exposure. + +*Two session modes with one feature surface.* Token custody has two legitimate shapes: hold tokens +server-side and give the browser an opaque id, or seal the tokens into the cookie itself and hold +nothing. These differ in state, in failure modes, and in which capabilities are achievable — but +they do *not* differ in what the login flow, the refresh path, the CSRF defence, or the reserved +endpoints have to do. Implementing them as two parallel stacks would double the surface on which a +security defect can appear, and guarantee the two drift. + +*Key material for the stateless mode.* Sealing requires a key. Demanding one unconditionally makes +the simplest deployment impossible without key management; generating one silently makes a +multi-replica deployment fail in a way that looks like an unrelated session bug. + +*Session identity under a derived-identity scheme.* Any session needs a stable identity to key +per-session coordination — most importantly single-flight coalescing of token refresh, so +concurrent requests for one session do not each drive a refresh. A stateless mode has no store to +mint an id in, so identity must be *derived* from the sealed material. A derived identity creates +a constraint that a minted one does not: the derivation inputs must be simultaneously stable across +re-seals and unique across sessions. Choosing inputs that are merely stable produces collisions +between distinct sessions, which cross-wires precisely the coordination the identity exists to key. + +Decisions (i) through (iv) below originate in the earlier BFF implementation work and are recorded +here *retroactively* — they were made and implemented before this record was written, and this ADR +captures them rather than proposing them. Decision (v) is recorded as it was made. + +== Decision + +*Session mode selects only which `SessionBinding` is assembled. Everything else — reserved-path +resolution, CSRF, and the session-identity contract — is a fixed, mode-neutral model behind that one +seam.* + +=== (i) Reserved paths resolve before the proxy route table + +The gateway carves out its own exact paths on the OIDC host and resolves them *ahead of* the proxy +route table, via a `ReservedPathRegistry` the edge consults first. A matched reserved path is +terminated at the gateway and dispatched to its handler; it can never be captured by a proxy prefix, +whatever the route configuration says. + +The set is closed and each entry is configuration-driven, so operators can move a reserved path out +of a namespace they need — but they cannot make a reserved path proxyable, and no route ordering +can shadow one. + +=== (ii) The CSRF model is fixed, not a knob + +Origin verification on unsafe methods, plus a `SameSite=Lax` cookie, applies to every +`require: session` route as a fixed property. There is no switch to disable it. The only +configuration is the *allowlist* of trusted origins a cross-origin SPA is served from — that is, the +operator declares who may call, never whether the check happens. + +=== (iii) Server and cookie mode sit behind one `SessionBinding` seam + +The whole BFF foundation binds to a `SessionBinding` abstraction, never to a store. The configured +mode selects only which implementation is assembled: a store-backed binding, or a stateless binding +over the sealed-cookie codec. The login flow, CSRF defence, refresh and step-up coordinators, the +reserved endpoints, and the session-authentication runtime are written against the seam and are +unaware of which binding is in force. + +Where the modes genuinely differ, they differ by *declared capability*, not by mode name. A binding +reports whether it supports IdP-driven destruction; the back-channel logout endpoint is gated on +that signal and answers a deliberate `404` when unsupported, rather than claiming a destruction that +did not happen or letting the reserved path fall through to the proxy route table. + +=== (iv) Cookie-mode key material has two modes, and the key is optional + +Cookie mode accepts an explicitly supplied sealing key or, when none is configured, generates one at +startup. Boot does *not* require a configured key — the relaxed rule is deliberate, so the simplest +deployment needs no key management at all. + +The trade-off is made explicit rather than hidden: a generated key cannot be shared with another +replica and does not survive a restart, so sessions are dropped on restart and cannot be served by a +peer. A decrypt-only previous key supports rotation and requires a current key to roll onto; +declaring it alone is rejected at boot, because a rotation key with nothing to roll onto is +meaningless. + +=== (v) Cookie-mode session identity is a keyed digest including a per-session nonce + +The derived cookie-mode identity is a salted digest over the session's fixed material *plus a +per-session random nonce* minted once at login. The nonce is sealed with the session, re-sealed +verbatim on every refresh, and never re-minted; the identity remains a digest, and the raw nonce is +never used as or substituted for the session id. The identity is never emitted to the browser. + +This satisfies both halves of the derived-identity constraint: stable, because every input is fixed +for the session's life, so a re-seal reproduces the identity exactly; and unique, because the nonce +separates two sessions that would otherwise share every other input — notably two logins by the same +subject within one clock granule. + +Adding the nonce changed the sealed payload's field set and the session record's shape. Because the +sealed-cookie format version is bound into the cookie's authenticated data, the version was bumped +and there is deliberately no acceptance path for the previous format: admitting an older cookie +would require synthesizing the missing nonce, which would silently change the derived identity — +exactly the defect the nonce exists to prevent. + +== Consequences + +=== Positive + +* A security-critical path cannot be shadowed by configuration: reserved-path precedence is + structural, not an ordering convention. +* There is no deployment of this gateway with CSRF defence disabled, because there is no way to + express one. +* A capability added to the BFF is written once, against the seam, and is available in both session + modes without a second implementation — so the two modes cannot drift apart in behaviour. +* Mode-specific limitations surface as an explicit capability signal with a defined response, rather + than as a silently missing behaviour. +* The stateless mode is usable with no key management at all, while the multi-replica case remains + fully supported by supplying a key. +* Single-flight refresh coalescing keys correctly in both modes, including for concurrent logins by + one subject. + +=== Negative + +* Reserved paths permanently occupy their configured paths on the OIDC host; an operator who wants + those namespaces proxied must relocate the reserved paths instead. +* A truly cross-site SPA cannot be supported, because the fixed cookie policy will not attach the + session cookie cross-site — such an SPA must be hosted under the gateway's site. +* Any change to the sealed payload's field set forces a format-version bump, which invalidates every + existing cookie-mode session and forces re-login on deploy. There is no rolling migration. +* The session record carries a component that is meaningful in only one mode, and it must be + explicitly propagated wherever the record is rebuilt field-by-field. + +=== Risks + +* *Silent identity drift.* Any code path that reconstructs the session record component-by-component + and omits the nonce would change the derived identity mid-session, breaking refresh coalescing. + The re-seal path fails loud rather than re-minting a nonce, which converts this from a silent + corruption into an immediate error. +* *Operator misreading of the key default.* Omitting the key is easy and works perfectly on one + replica, so a deployment can scale out and only then discover its sessions are not portable. The + trade-off is therefore documented at the point of configuration rather than only in reference + material. +* *Cookie size ceiling.* Sealing tokens into the cookie makes session size a function of token size, + which is bounded by what browsers accept. The gateway refuses to seal an over-budget session + rather than emitting a cookie the browser would silently drop. + +== Alternatives Considered + +*Resolve reserved paths through the ordinary route table.* This would need no separate registry and +no precedence rule, letting one mechanism resolve every path. Rejected because it makes the +correctness of a security-critical endpoint depend on route ordering and prefix breadth: a +legitimate broad proxy prefix silently captures the callback, and the resulting failure is +configuration-dependent and hard to attribute. Precedence must be structural to be trustworthy. + +*Make CSRF defence configurable, with a secure default.* This would accommodate deployments that +believe they do not need it, while defaulting to protection. Rejected because the option's existence +is the vulnerability: a defence that can be switched off will be switched off in the deployment that +understands the exposure least, and a secure default provides no protection against a deliberate +override. + +*Implement the two session modes as independent stacks.* Each mode could then be optimised for its +own shape without abstraction overhead. Rejected because it doubles the surface where a +security-relevant defect can occur and guarantees divergence: a fix or hardening applied to one +stack does not reach the other, and the two slowly acquire different behaviour for what operators +reasonably read as one feature. + +*Require a sealing key unconditionally in cookie mode.* This would remove the weaker generated-key +mode and the possibility of an operator being surprised by non-portable sessions. Rejected because +it imposes key management on every deployment including single-node and development ones, which +pushes operators toward checked-in or trivially-derived keys — a worse security outcome than an +explicitly documented, deliberately-scoped generated key. + +*Derive the cookie-mode identity from the session's fixed material alone, without a nonce.* This +would need no extra sealed field and no format bump. Rejected because stability and uniqueness are +independent requirements and this satisfies only the first: two sessions sharing the same subject +and login granule derive the same identity, which cross-wires the per-session coordination the +identity exists to key. The defect is silent, load-dependent, and appears only under concurrency. + +*Accept the previous sealed-cookie format alongside the new one.* This would avoid forcing every +user to log in again on upgrade. Rejected because accepting an older cookie means supplying the +absent identity input, and any value chosen changes the derived identity for that session — +reintroducing exactly the collision class the nonce removes. A clean break with a forced re-login is +the honest cost. + +The alternatives fall into one class: each trades a structural guarantee for local convenience — +configurability, less abstraction, an easier upgrade — and each pays for it with a failure mode that +is silent and surfaces only in the deployments least equipped to diagnose it. + +== References + +* link:0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc[ADR-0019 -- Reserved-path and cookie-header relaxations] -- the inbound-validation relaxations this architecture requires. +* link:../variants/02-bff-session.adoc[Variant 2 -- BFF, Session-based] -- the server-side-custody realisation. +* link:../variants/03-bff-cookie.adoc[Variant 3 -- BFF, Cookie-based] -- the stateless realisation. +* link:../architecture.adoc[Architecture] -- the seam statement in context. +* link:../configuration.adoc#_oidc[Configuration Reference -- `oidc`] -- the authoritative key contract, including the relaxed cookie-mode key rule. +* link:../user/bff-cookie.adoc[Operator setup -- cookie-based BFF] -- the key-material trade-off and upgrade consequence as operators meet them. +* link:../development/bff-cookie.adoc[Contributor guide -- cookie-based BFF] -- the wire format and identity constraints as implementers meet them. diff --git a/doc/adr/0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc b/doc/adr/0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc new file mode 100644 index 00000000..44825d38 --- /dev/null +++ b/doc/adr/0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc @@ -0,0 +1,183 @@ += ADR-0019: Reserved BFF paths bypass the url-parameter value pipeline and the cookie-header length relaxation is gateway-wide +:toc: left +:toclevels: 2 +:sectnums: + +// adr-metadata +// Progressive-disclosure metadata block (see manage-adr SKILL.md → "ADR Template +// Structure"). Read by `manage-adr.py scan` so a caller can assess an ADR's +// relevance without reading the full file. List fields are comma-separated. +// summary: Two deliberate inbound-validation relaxations the BFF requires — the url-parameter value pipeline is not applied to matched reserved paths, and the cookie-header value-length raise is necessarily gateway-wide +// tags: security, bff, reserved-paths, input-validation, cookie-header, relaxation, blast-radius +// affects: api-sheriff +// supersedes: +// end-adr-metadata + +== Status + +Proposed + +== Context + +The gateway applies an inbound security-validation pipeline to every request before any routing +decision: path canonicalisation, url-parameter validation, and header validation, each fail-closed +and each refusing to echo an offending value. The pipeline is tuned for its primary job — defending +an *upstream* against traffic the gateway forwards. + +The BFF introduces two request shapes that pipeline does not fit, and both create the same kind of +choice point: relax the default policy for a narrow case, or leave a legitimate request permanently +rejected at the edge. + +*Gateway-terminated requests are not forwarded traffic.* The url-parameter value pipeline is an +anti-injection defence for parameters the gateway passes to an upstream — its threat model is a +value that means one thing to the gateway and another to the backend. A reserved BFF path is not +forwarded anywhere: it terminates at the gateway and is consumed by a handler the gateway itself +owns. Worse, some of its parameters legitimately carry structure the pipeline is designed to reject. +A same-origin post-login return target is a path, so it contains separators the anti-injection +policy treats as suspicious in a value destined for an upstream. Applying an upstream-oriented +policy to a request that has no upstream rejects correct traffic while defending nothing. + +*Validation runs before route selection.* The cookie-mode BFF's sealed session cookie is designed to +a multi-kilobyte budget, well past the default header-value length cap, so an authenticated request +in that mode fails the header check every single time. The obvious containment — raise the cap only +for the routes that need it — is not available at this position in the pipeline: the stage runs +*before* any route or anchor is resolved, so at the moment the header must be validated there is no +per-anchor context in existence to scope the relaxation to. This is a structural property of where +inbound validation has to sit (it must precede routing so routing decides on canonicalised, +validated input), not an implementation shortcut that could be tightened later. + +Both cases pose the same question: when a defence does not fit a legitimate request class, is the +correct answer to weaken the defence globally, to carve out the narrowest possible exception, or to +reject the request class? Answering it once, explicitly, and bounding what each relaxation gives up +is what this record exists to do. + +== Decision + +*Both relaxations are granted, each scoped as narrowly as its position in the pipeline permits, and +each leaves every other validator untouched.* + +=== (a) The url-parameter value pipeline is not applied to matched reserved paths + +The inbound stage takes a reserved-path predicate over host and canonical path. When a request +matches a reserved gateway path, the url-parameter *value* pipeline is not applied to it. + +The justification is that a matched reserved path is *terminated at the gateway and never proxied*, +so the threat the value pipeline addresses — a parameter reinterpreted by an upstream — does not +exist for it. Validation of those parameters is not skipped; it *moves to the reserved handler*, +which is the component that actually knows each parameter's grammar. The handler validates a return +target as a same-origin path, a state parameter as an opaque correlator, and so on — checks that are +strictly more precise than a generic value policy, because they are type-aware. + +What still constrains a reserved path is everything else: path canonicalisation still runs and still +yields the single canonical path every later stage consumes, the parameter- and header-*count* caps +still apply, and every other header validator still applies. The relaxation is confined to +url-parameter *values*, on *matched reserved paths only*. A gateway with no BFF configured matches +no reserved path and is byte-for-byte unaffected. + +=== (b) The cookie-header value-length relaxation is gateway-wide + +The stage accepts an optional cookie-header policy whose *only* difference from the default is a +raised maximum header-value length, and applies it to the `Cookie` and `Set-Cookie` header values +only. It is supplied only by a cookie-mode BFF gateway; a bearer-only or server-mode gateway +supplies nothing and is byte-for-byte unaffected. + +The relaxation is gateway-wide rather than per-anchor because, as established above, no anchor is +resolved at this pipeline position. The configuration key that supplies the budget sits on the +global session block for exactly that reason, and its placement must not be read as per-anchor +enforcement — it is global because the enforcement point is global. + +The blast radius is bounded on three independent axes: + +* *By header.* Only `Cookie` and `Set-Cookie` values are affected. Every other header keeps the + default cap. +* *By validator.* Only the length cap changes. The null-byte, control-character, and + injection-pattern validators apply to the cookie header unchanged, so the relaxation admits a + *longer* value, never a *differently-shaped* one. +* *By deployment.* Only a cookie-mode gateway supplies the policy at all. + +The residual exposure is therefore precisely: on a cookie-mode gateway, a client may send a longer +`Cookie` header value than the default policy would accept, subject to every other content check and +to the gateway's own transport limits. A value that is not a resolvable sealed session is refused by +the session binding fail-closed as "no session". + +== Consequences + +=== Positive + +* Legitimate BFF traffic is not rejected at the edge: a login return leg carrying a path-shaped + target, and an authenticated cookie-mode request carrying a multi-kilobyte sealed cookie, both + pass validation. +* Reserved-path parameters end up *better* validated, not less: a type-aware handler check is + stricter than a generic value policy. +* Each relaxation is inert on deployments that do not use the feature — a bearer-only gateway's + inbound validation is unchanged in both respects. +* The residual exposure of each relaxation is explicitly enumerable rather than emergent, which is + what makes it reviewable. + +=== Negative + +* Reserved-path parameter validation is now the reserved handlers' responsibility, so a new reserved + handler that neglects to validate its own parameters has no generic backstop behind it. +* The cookie-header length relaxation cannot be scoped to the routes that need it, so on a + cookie-mode gateway it applies to every request including ones that will never carry a session + cookie. +* Two documented exceptions now exist to what was a uniform inbound policy, which is a cost in + comprehensibility for anyone reasoning about the pipeline. + +=== Risks + +* *A reserved handler forgetting its own validation.* The relaxation moves a check rather than + removing it, so the guarantee holds only while every reserved handler upholds its half. Adding a + reserved endpoint therefore carries an obligation to validate its parameters explicitly. +* *Relaxation creep.* Each carve-out makes the next easier to justify. The bounding described above + — one header pair, one validator, one deployment mode — is the limit, not a starting point; a + future request to relax a *different* validator for the same convenience is a separate decision + requiring its own record. +* *Misreading the configuration key's placement.* The budget key sitting on the global session block + can be misread as per-anchor enforcement; it is global because the enforcement point precedes + routing. + +== Alternatives Considered + +*Apply the url-parameter value pipeline to reserved paths unchanged.* This would keep one uniform +inbound policy with no exceptions to reason about. Rejected because it rejects correct traffic while +protecting nothing: a legitimately path-shaped return target fails a policy whose threat model +(reinterpretation by an upstream) does not apply to a request that is never forwarded. The result +would be a BFF whose login return leg cannot function. + +*Loosen the url-parameter value policy globally so reserved paths pass.* This would need no +predicate and no per-request branch. Rejected because it weakens the defence exactly where it is +load-bearing — on parameters actually forwarded to an upstream — in order to accommodate requests +that are not forwarded at all. It inverts the scoping: the narrow case should be the exception, not +the new global baseline. + +*Raise the header-value cap globally for all headers.* This is the simplest change and needs no +optional policy. Rejected because it discards the cap's protection on every other header to solve a +problem confined to one. The relaxation must name the header it applies to, or it is not a +relaxation but a removal. + +*Make the cookie-header cap per-anchor.* This is the natural containment and would confine the +raised cap to routes that need it. Rejected because it is not achievable at this pipeline position: +inbound validation must precede route selection so routing decides on canonicalised, validated +input, and therefore no anchor exists to scope against at the moment of the check. Deferring the +header check until after routing would mean routing on unvalidated input — a strictly worse exposure +than the raised cap it would contain. + +*Reject the cookie-mode variant instead of relaxing anything.* This would preserve a uniform policy +absolutely. Rejected because the stateless variant answers a real deployment need, and the +relaxation it requires is bounded and reviewable — the trade is a longer permitted cookie value +against a gateway that scales out without shared session state. + +The alternatives fall into two classes, and both are rejected for the same reason: one class widens +a relaxation past the case that motivates it, and the other refuses a bounded relaxation at the cost +of a legitimate capability. The chosen position is the narrowest scoping each relaxation's +enforcement point actually permits — and where that scoping is coarser than one would like, the +limit is structural and is stated as such rather than left to be discovered. + +== References + +* link:0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc[ADR-0018 -- BFF session mode and the reserved-path model] -- the architecture these relaxations serve. +* link:../architecture.adoc[Architecture] -- the inbound pipeline's position ahead of routing. +* link:../configuration.adoc#_oidc[Configuration Reference -- `oidc`] -- the session block carrying the cookie-size budget. +* link:../variants/03-bff-cookie.adoc[Variant 3 -- BFF, Cookie-based] -- the variant whose sealed cookie motivates the length relaxation. +* link:../development/bff-cookie.adoc[Contributor guide -- cookie-based BFF] -- the sealed-cookie format the budget bounds. diff --git a/doc/architecture.adoc b/doc/architecture.adoc index 2b55b351..666697c2 100644 --- a/doc/architecture.adoc +++ b/doc/architecture.adoc @@ -172,40 +172,57 @@ the engine wires it internally. The engine's API surface is specified in ==== [[_bff_session_runtime]] -==== Server-session BFF runtime (edge wiring) +==== BFF runtime (edge wiring) -The *server-session* variant realizes the BFF side of that seam as four structural pieces, all +The BFF realizes its side of that seam as four structural pieces, all framework-agnostic (raw request pieces in, a rendered response out -- no JAX-RS/Vert.x coupling) -so each is unit-testable without a container: - -* *Session-resolution seam.* The opaque session-id cookie is the only browser-held state. A - `SessionCookieCodec` reads the id out of the request `Cookie` header and a `SessionStore` - (the in-memory `InMemorySessionStore`, bounded by `oidc.session.max_sessions`) resolves it to - the server-held `SessionRecord` carrying the tokens. A `require: session` route is served by a +so each is unit-testable without a container. The decisions behind this shape -- the reserved-path +model, the fixed CSRF model, the one `SessionBinding` seam the two session modes sit behind, the +cookie-mode key material, and the session-identity contract -- are recorded in +link:adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc[ADR-0018]; +the two inbound-validation relaxations the BFF requires are recorded in +link:adr/0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc[ADR-0019]. + +* *Session-resolution seam.* Every collaborator binds to `SessionBinding`, never to a store; the + configured `session.mode` selects only which implementation is assembled (ADR-0018). In + *server* mode a `SessionCookieCodec` reads an opaque session id out of the request `Cookie` + header and a `SessionStore` (the in-memory `InMemorySessionStore`, bounded by + `oidc.session.max_sessions`) resolves it to the server-held `SessionRecord` carrying the tokens; + in *cookie* mode the sealed cookie itself carries that state and is unsealed per request, with no + *session* store at all -- the in-memory `PendingAuthorizationStore` that holds the login leg + between redirect and callback is assembled in *both* modes + (link:variants/03-bff-cookie.adoc#_stateless_after_login_not_during_it[stateless after login, not + during it]). Either way a `require: session` route is served by a `SessionAuthenticationStage` injected into the session-aware authentication stage -- so the - route is *mediated* (the stored access token injected upstream) rather than rejected. + route is *mediated* (the resolved access token injected upstream) rather than rejected. * *Reserved-path registry + live-edge dispatch.* The `ReservedPathRegistry` classifies each of the up-to-six gateway-owned OIDC paths (callback, logout, logout-return, back-channel logout, user-info, login-initiation) by *exact* host+path match. The gateway edge consults it *before* the proxy route table, so a matched reserved path is dispatched to its handler and a proxy route - such as `path_prefix: /auth` can never swallow `/auth/callback`. The - link:configuration.adoc#_oidc[`oidc`] block is the single source of the registered paths. + such as `path_prefix: /auth` can never swallow `/auth/callback` (ADR-0018). The + link:configuration.adoc#_oidc[`oidc`] block is the single source of the registered paths. Because + a matched reserved path terminates at the gateway rather than being proxied, the inbound + url-parameter *value* pipeline is deliberately not applied to it -- its parameters are validated + by the reserved handler itself, which knows their grammar (ADR-0019). * *Engine boundary.* The gateway re-implements no OAuth leg. Every reserved handler (login initiation, callback code-exchange, RP-initiated + back-channel logout, transparent refresh, RFC 9470 step-up) drives the `token-sheriff-client` engine across a small set of seams and adds only the browser-facing concern on top (binding/session cookies, CSRF, curated user-info view, - RFC 9457 rendering). The BFF never holds token material outside the `SessionStore`. + RFC 9457 rendering). The BFF never holds token material outside the binding in force -- the + `SessionStore` in server mode, the sealed cookie in cookie mode. * *`BffRuntime` / `BffRuntimeProducer` CDI wiring.* `BffRuntime` is the single collaborator the edge consults: it carries the `SessionAuthenticationStage` + fixed `CsrfDefence` and a `dispatch(...)` fan-out that routes each matched reserved kind to its wired handler and normalizes the outcomes into one response the edge renders verbatim. `BffRuntimeProducer` builds - it once at boot *only when a global `oidc` block with `session.mode=server` and a `redirect_uri` - is configured*; otherwise it produces the *inert* runtime, so a bearer-only gateway (or a - cookie-mode BFF) is unchanged and never touches the confidential-client engine. Provider-metadata - discovery is resolved lazily on first engine use, so a server-mode gateway boots without a live IdP. + it once at boot *only when a global `oidc` block with a `redirect_uri` and a recognised + `session.mode` -- `server` or `cookie` -- is configured*; otherwise it produces the *inert* + runtime, so a bearer-only gateway is unchanged and never touches the confidential-client engine. + Both modes drive the same wiring and both reach the engine; the mode selects only which + `SessionBinding` is assembled. Provider-metadata + discovery is resolved lazily on first engine use, so a BFF gateway boots without a live IdP. === Forward Policy (Zero-Trust) diff --git a/doc/development/README.adoc b/doc/development/README.adoc index 00f13d12..0a7a8a4e 100644 --- a/doc/development/README.adoc +++ b/doc/development/README.adoc @@ -59,6 +59,13 @@ This tree is seeded here and grows as contributor-facing material lands. how to build and test it, the engine-boundary rule (drive the `token-sheriff-client` engine, never re-implement an OAuth leg), the integration-test and benchmark entry points, and how to add a new reserved endpoint. + +| link:bff-cookie.adoc[Cookie-Based BFF -- Contributor Guide] +| Where the cookie-mode code lives (the `bff.cookie` types), the `SessionBinding` seam that keeps + every reserved endpoint mode-neutral, the sealed-cookie wire format and the rule that a format + change requires a `FORMAT_VERSION` bump, the derived session identity with its + stability-and-uniqueness constraints, the engine-boundary rule, and the unit/integration test + entry points. |=== == Scope of This Layer diff --git a/doc/development/bff-cookie.adoc b/doc/development/bff-cookie.adoc new file mode 100644 index 00000000..3d017e3c --- /dev/null +++ b/doc/development/bff-cookie.adoc @@ -0,0 +1,165 @@ += Cookie-Based BFF -- Contributor Guide +:toc: +:toclevels: 3 +:sectnums: + +A contributor-facing guide to the *cookie-based BFF* implementation (Variant 3): where its code +lives, how the sealed cookie is put together, and the rules that keep it correct. It describes how to +*work in* the code; the design rationale is in +link:../variants/03-bff-cookie.adoc[Variant 3 -- BFF, Cookie-based] and the architectural decision in +link:../adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc[ADR-0018]. + +== The engine boundary rule (read first) + +*The gateway re-implements no OAuth leg.* The confidential-client *engine* (`token-sheriff-client`) +owns retrieval and flow -- auth-code + PKCE, `state`/`nonce`/exact `redirect_uri`, mix-up defence, +refresh-with-rotation, RP-initiated logout, RFC 9470 step-up. The *BFF* (this code) owns only the +browser-facing part: the binding/session cookies, CSRF, request proxying, the curated user-info view, +logout receivers, and RFC 9457 rendering. + +This rule is identical in both BFF variants -- cookie mode reaches the engine exactly as server mode +does. When you add or change a reserved handler, drive an engine seam; never hand-roll an OAuth +request, a token exchange, or a JWKS fetch inside the gateway. + +== Where the code lives + +The cookie-specific code is in the `api-sheriff` module under +`de.cuioss.sheriff.gateway.bff.cookie`: + +[cols="1,2"] +|=== +| Type | Responsibility + +| `SealedSessionPayload` +| The plaintext that gets sealed: the mediated tokens, the identity/session claims, the login instant + anchoring the TTL, and the per-session identity nonce. Owns the inner wire format (`encode()` / + `decode(...)`) and redacts every credential from `toString()`. + +| `SealedSessionCookieCodec` +| The AES-256-GCM seal/unseal, the outer value layout, the `FORMAT_VERSION` stamp, deterministic + key selection by key id, the `Set-Cookie` hardening, and the seal-time size budget. + +| `CookieSessionBinding` +| The `SessionBinding` implementation: `bind` (seal on login, mint the nonce), `resolve` (unseal, + enforce the absolute TTL server-side), `persist` (re-seal a refresh), and the derived session + identity. Reports `IdpDestruction.UNSUPPORTED`. + +| `CookieKeyMaterial` +| Resolves the key material -- passed `encryption_key` versus generate-on-startup -- plus the + decrypt-only `previous_key` and the derived per-gateway identity salt. +|=== + +Everything else is shared with the server-session variant; see +link:bff-session.adoc#_where_the_code_lives[the server-session guide] for the `bff.*` package map. + +== The SessionBinding seam + +`SessionBinding` is the seam the whole BFF foundation binds to -- never a store directly. The +configured `session.mode` selects only *which implementation* `BffRuntimeProducer` assembles: +`ServerSessionBinding` over the in-memory store, or `CookieSessionBinding` over the sealing codec. + +Every reserved endpoint, the CSRF defence, the refresh and step-up coordinators, and the +`require: session` stage-4 runtime are consequently *mode-neutral*: they are written against the +seam and are unaware of which binding is in force. That is why adding a cookie-mode capability +almost never means touching a reserved endpoint. + +The one place the modes genuinely differ is capability: `idpDestruction()` reports `UNSUPPORTED` +for the cookie binding, and the back-channel logout endpoint is *gated on that signal* rather than +on the mode name -- it answers a deliberate `404` instead of claiming a destruction that never +happened. + +== The sealed-cookie wire format + +The cookie value is a base64url-encoded *outer envelope*: + +[source] +---- +version(1B) | key-id(1B) | GCM nonce(12B) | ciphertext | GCM tag(16B) +---- + +The `version` and `key-id` bytes are bound into the GCM *associated data*, so tampering with either +fails the tag check rather than silently selecting a different parse. Key selection is deterministic +by the stamped key id -- never a try-every-key decrypt. + +The *inner* plaintext (what `SealedSessionPayload.encode()` produces) is nine newline-separated +fields in record-declaration order, each URL-safe base64 of its UTF-8 bytes so no value can collide +with the separator; an empty field stands for an absent optional. It is read only *after* the GCM +tag has authenticated the bytes -- `decode(...)` is never a parser for untrusted input. + +[IMPORTANT] +==== +*Any change to the inner field set requires a `FORMAT_VERSION` bump.* The field-count guard is +strict and there is no legacy-shape acceptance path: a cookie in the previous format must fail to +unseal, not be admitted with a synthesized value. Bumping the version is what makes that failure +clean, since the version byte is authenticated. A bump forces every existing cookie-mode session to +re-login -- call that out in the operator documentation when you make one. +==== + +== The derived session identity + +`SessionRecord.sessionId()` is the one identity model the seam defines, and it is the key the refresh +coordinator uses for single-flight coalescing. Cookie mode *discards* the id minted at login and +derives its own instead: a salted digest over the login instant, `sub`, and the per-session nonce. + +Two constraints hold it together: + +* *Stable across re-seals.* All three digest inputs are fixed for the life of the session, so a + refresh reproduces the identity byte-identically. `persist` re-seals the nonce *verbatim* and + fails loud rather than re-minting; `TokenRefreshCoordinator.rotate()` must copy the nonce onto the + rotated record. `rotate()` rebuilds the record component-by-component, so *any component you add to + `SessionRecord` needs a copy line there* or it is silently dropped. +* *Unique.* The nonce is what makes two logins by the same subject inside one clock second resolve to + distinct identities; without it they collided and cross-wired the single-flight keying. + +The identity is never emitted to the browser -- only the sealed value crosses -- and the raw nonce is +never used as, or substituted for, the session id. Single-flight coalescing is necessarily *per +instance* here; a cross-instance duplicate refresh is this variant's documented, accepted trade-off. + +== Build and test + +The canonical build commands are the ones in the repository `CLAUDE.md` (invoked through the build +executor, never a raw `mvn`). Both the quality gate and full verify must pass with zero findings +before a commit. + +Unit-test entry points: + +* `SealedSessionPayloadTest` -- the inner wire format: round trips, absent optionals, + separator-bearing values, the strict field-count guard, and credential redaction. +* `SealedSessionCookieCodecTest` -- seal/unseal, the version and key-id header, fail-closed rejection + of a tampered nonce/ciphertext/tag, unknown version or key id, `previous_key` rotation, and the + size budget. +* `CookieSessionBindingTest` -- bind/resolve/persist, the server-side TTL enforcement, rotation + re-seal, and the identity properties (stability across a re-seal, uniqueness within a clock + second). +* `TokenRefreshCoordinatorTest` -- its `CookieMode` group covers the stateless refresh: re-seal into + a new `Set-Cookie` and nonce propagation across `rotate()`. + +Integration tests live in the `integration-tests` module and run against the native image on the +Docker compose stack under the integration-tests Maven profile: + +* `BffCookieSessionIT` -- the full login -> mediated call -> logout round trip through a real IdP. +* `BffCookieStatelessnessIT` -- the two-instance topology (ports `10445` and `10446`) sharing only + the sealing key. It logs in against the *first* instance only, then exercises the second, which + is what makes "the peer held no record of this session" the evidence rather than an assumption. + That asymmetry is deliberate: it also documents the login-leg affinity boundary. + +Both IT suites are behaviour-level and are *not* bound to the inner wire form, so a format change +should leave them untouched and green. + +== How to extend + +* *A new sealed field* -- add the record component, append it in `encode()` in declaration order, + read it back in `decode(...)`, raise the field count, and *bump `FORMAT_VERSION`*. If it + participates in identity, mind the stability constraint above. +* *A new session-runtime capability* -- write it against the `SessionBinding` seam so it stays + mode-neutral; if it genuinely cannot work statelessly, gate it on a capability signal (as + back-channel logout does) rather than on the mode name. +* *Anything touching an OAuth leg* -- drive an engine seam, per the boundary rule. + +== See also + +* link:../variants/03-bff-cookie.adoc[Variant 3 -- BFF, Cookie-based] -- the design. +* link:../adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc[ADR-0018 -- BFF architecture] -- the decision record. +* link:../adr/0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc[ADR-0019 -- BFF security relaxations] -- the accepted trade-offs. +* link:../user/bff-cookie.adoc[Operator setup -- cookie-based BFF] -- how it is configured and run. +* link:bff-session.adoc[Contributor guide -- server-session BFF] -- the shared package map. diff --git a/doc/user/README.adoc b/doc/user/README.adoc index a900340d..2e59f351 100644 --- a/doc/user/README.adoc +++ b/doc/user/README.adoc @@ -44,6 +44,13 @@ layer. confidential client, protecting a route with `require: session`, the gateway-owned reserved paths, serving a curated user-info view, CSRF for a cross-origin SPA, and operating the in-memory session store. Links to the Configuration Reference for the `oidc` key contract. + +| link:bff-cookie.adoc[Cookie-Based BFF -- Operator Setup] +| A task-oriented guide for standing up the cookie-based BFF (Variant 3): registering the OIDC + confidential client, selecting `session.mode: cookie`, the two key-material modes and their + trade-off, decrypt-only key rotation, the cookie size budget, what IdP-driven destruction cannot + do in this variant, the login-leg session affinity, and the forced re-login on a sealed-cookie + format bump. Links to the Configuration Reference for the `oidc` key contract. |=== == Scope of This Layer diff --git a/doc/user/bff-cookie.adoc b/doc/user/bff-cookie.adoc new file mode 100644 index 00000000..ef88cba7 --- /dev/null +++ b/doc/user/bff-cookie.adoc @@ -0,0 +1,170 @@ += Cookie-Based BFF -- Operator Setup +:toc: +:toclevels: 3 +:sectnums: + +A task-oriented guide for operators standing up the *cookie-based BFF* (Variant 3): API Sheriff as +an OIDC confidential client that keeps no server-side session store and hands the browser a sealed +cookie that *is* the session. It tells you *how* to configure and run the variant; the field-by-field +contract lives in the link:../configuration.adoc#_oidc[Configuration Reference (`oidc`)] and the +design rationale in link:../variants/03-bff-cookie.adoc[Variant 3 -- BFF, Cookie-based]. + +== When to choose this variant + +Choose the cookie-based BFF when you need a *stateless* gateway: multiple replicas with no shared +session store and no sticky routing for ordinary traffic. Tokens still never reach the browser in +readable form -- they are sealed with AES-256-GCM and the cookie is `HttpOnly`. + +Choose the link:../variants/02-bff-session.adoc[server-session variant] instead when server-side-only +token custody is a hard requirement (the token must never leave the gateway, not even encrypted), or +when you need IdP-driven back-channel logout. + +== Prerequisites + +* An OIDC provider (IdP) where you can register a *confidential client* and obtain a + `client_id` / `client_secret`. +* A registered *redirect URI* pointing at the gateway's own callback path (`oidc.redirect_uri`), + matched exactly at the IdP. +* The client secret and any session keys available as environment variables (never written inline -- + secrets in `oidc` must be `${ENV_VAR}` references). + +== Configure the confidential client + +Add an `oidc` block to `gateway.yaml` and set at least one route's effective auth to +`require: session`. A minimal cookie-based configuration: + +[source,yaml] +---- +# gateway.yaml +oidc: + issuer: https://idp.example.com/realms/main + client_id: api-sheriff + client_secret: ${SHERIFF_CLIENT_SECRET} # must be an ${ENV_VAR} reference + redirect_uri: https://gw.example.com/auth/callback # registered exactly at the IdP + scopes: [openid, profile, orders.read] # include every scope a route requires + session: + mode: cookie # sealed cookie, no store (this variant) + cookie_name: __Host-sheriff + encryption_key: ${SHERIFF_SESSION_KEY} # optional -- see "Key material" below + previous_key: ${SHERIFF_SESSION_KEY_PREVIOUS} # optional; decrypt-only (rotation) + max_cookie_size: 4096 # seal-time budget; see "Cookie size budget" + ttl_seconds: 3600 # absolute lifetime from login + csrf: + trusted_origins: [https://app.example.com] + refresh: { enabled: true, leeway_seconds: 30, on_failure: reauthenticate } +---- + +The BFF runtime is built from exactly two inputs: a recognised `session.mode` (`server` or `cookie`) +*and* a configured `redirect_uri`. Route auth is not part of that decision -- `require: session` +routes are what *use* the runtime once it exists, not what turns it on. With no recognised +`session.mode` the runtime stays inert, so a bearer-only deployment is unaffected. + +== Key material + +The sealing key has *two modes*, selected by whether you declare `session.encryption_key`. + +[cols="1,2,2"] +|=== +| Mode | How to select | Trade-off + +| Passed key +| Declare `session.encryption_key` as an `${ENV_VAR}` reference holding an AES-256 key. +| Sessions survive a gateway restart and are shareable across replicas. Requires you to generate, + distribute and safeguard the key. + +| Generate on startup +| Omit `session.encryption_key` entirely. +| No operator input at all -- the gateway mints a key at boot. But *sessions are dropped on every + restart* (users re-authenticate) and the key *cannot be shared* with another replica, so each + replica can only unseal its own cookies. +|=== + +Use generate-on-startup for a single-replica deployment or a development environment. Use a passed +key for anything multi-replica or restart-sensitive. The gateway raises a startup diagnostic naming +the active key mode and rotation state -- it never logs key bytes. + +=== Rotating the key + +`session.previous_key` is *decrypt-only*: a cookie still sealed under it keeps resolving, and the +session's next write re-seals it under the current key. Sealing is always done with +`encryption_key`, never with `previous_key`. + +`previous_key` *requires* an `encryption_key` -- declaring it alone is rejected at boot, because a +decrypt-only rotation key with no current key to roll onto is nonsensical. Rotation therefore +composes only with the passed-key mode; generate-on-startup has no previous key by construction. + +Withdraw `previous_key` once the rotation window has passed; cookies still carrying the retired key +id are then refused as "no session" and those users log in again. + +== Cookie size budget + +Because the session travels in the cookie, its sealed value has a size budget -- +`session.max_cookie_size`, defaulting to 4096 bytes. Browsers are only required to accept 4 KB per +cookie, so a larger value risks being *silently dropped* by the browser, leaving a session that can +never be resolved. + +The gateway therefore refuses to seal an over-budget session rather than truncating it, and logs a +warning naming the offending length. If you hit the budget, *reduce the token set* -- trim `scopes` +so the IdP issues a smaller access token, and cut `user_info.allowed_claims` -- or *run the +link:../variants/02-bff-session.adoc[server-session variant]*, where token size is irrelevant +because nothing crosses to the browser. + +== What is unavailable in this variant + +A stateless gateway holds no session index and cannot reach another browser's cookie, so +IdP-driven destruction is *unsupported*: + +* *Back-channel logout answers `404`.* The reserved path stays wired and deliberately returns `404` + rather than falling through to the proxy route table -- it never claims a destruction that did not + happen. +* *No destroy-by-`sid` / destroy-by-`sub`.* Nothing can be revoked centrally mid-session. + +RP-initiated logout (the user logging themselves out) works normally: the browser's cookie is +cleared, and an expired cookie the browser keeps anyway is still refused server-side by the absolute +TTL check. Where instant central revocation is a hard requirement, use the server-session variant. +Mitigate in this variant with a short `ttl_seconds` and short access-token lifetimes. + +== Session affinity for the login leg + +The variant is stateless once a session is *established*, but not during the login exchange. The +pending authorization held between the login redirect and the OIDC callback is per-instance and +never shared, so a login *begun* on one replica must *complete* on that replica. + +*Configure session affinity for the login leg only* -- the redirect through to the callback. Once +the session cookie is issued, requests may be balanced freely across replicas with no affinity. + +== Upgrading: forced re-login on a format bump + +The sealed-cookie format carries a version that is bound into the cookie's authenticated data. When +that version is bumped, cookies in the previous format fail to unseal rather than being mis-parsed, +and there is deliberately no backward-compatible acceptance path. + +*Plan for it*: on an upgrade that bumps the format version, all existing cookie-mode sessions are +invalidated at deploy time and users must log in again. There is no rolling migration -- schedule +the deploy accordingly. The most recent bump added the per-session identity nonce. + +== Operate + +* *Absolute TTL.* `ttl_seconds` is counted from login and is never extended by a refresh; the + deadline is recomputed from the original login on every re-seal, so an endlessly-refreshed session + still dies on time. +* *Refresh re-seals the cookie.* A transparent token refresh emits a new `Set-Cookie` -- that is the + whole of "persisting" in this variant. +* *IdP reachability.* Provider metadata is discovered lazily on first use, so the gateway boots + without the IdP -- but logins fail until the issuer is reachable. + +== Private-CA / self-signed IdP + +If your IdP presents a certificate from a *private CA* (or a self-signed cert), add that CA to the +*JVM default truststore* -- the confidential-client engine uses it for outbound OIDC calls +(discovery, token, refresh, logout), and the per-issuer `tls_profile` that validates JWKS does not +cover those calls. See link:bff-session.adoc#_private_ca_self_signed_idp[the server-session guide] +for the exact system properties; the requirement is identical in both variants. + +== See also + +* link:../configuration.adoc#_oidc[Configuration Reference -- `oidc`] -- the authoritative key contract. +* link:../variants/03-bff-cookie.adoc[Variant 3 -- BFF, Cookie-based] -- design and flow rationale. +* link:../adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc[ADR-0018 -- BFF architecture] -- why the variant is shaped this way. +* link:../adr/0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc[ADR-0019 -- BFF security relaxations] -- the accepted trade-offs. +* link:../development/bff-cookie.adoc[Contributor guide -- cookie-based BFF] -- building and extending it. diff --git a/doc/variants/03-bff-cookie.adoc b/doc/variants/03-bff-cookie.adoc index 9c3df561..1b807e9d 100644 --- a/doc/variants/03-bff-cookie.adoc +++ b/doc/variants/03-bff-cookie.adoc @@ -67,15 +67,20 @@ and the gateway holds no per-user state. `__Host-` prefix. | Key management -| A single server-side symmetric key, provided by environment indirection - (`${SHERIFF_SESSION_KEY}`), shared across all gateway instances. Key rotation is supported by +| A single server-side symmetric key. `session.encryption_key` is *optional*: declared as an + environment indirection (`${SHERIFF_SESSION_KEY}`) it is shared across all gateway instances and + survives a restart; omitted, the gateway mints a key at boot, so sessions are dropped on every + restart and the key cannot be shared with another replica. Key rotation is supported by the optional, decrypt-only `session.previous_key`: during the grace window cookies sealed with the old key are still accepted and re-sealed with the new key on the next write. | Size -| The cookie must stay within browser limits (~4 KB). If the token set exceeds the budget, - the refresh and/or ID token may be split into their own cookies; oversized IdPs are a - deployment consideration. +| The cookie must stay within browser limits (~4 KB), enforced as a seal-time budget: an + over-budget seal *fails* rather than being silently truncated. Splitting the token set across + several cookies is a deliberate non-goal, so the two available remedies are to reduce the token + set (fewer scopes, a leaner IdP token shape) or to run the + link:02-bff-session.adoc[session-based variant]. Oversized IdP tokens are a deployment + consideration. |=== == Statelessness @@ -84,8 +89,10 @@ Because the encrypted cookie carries all session state: * No session store, no Redis, no database -- the manifest's self-contained requirement holds fully. -* Any gateway instance can serve any request as long as it has the encryption key -- no sticky - sessions, no coordination. +* Any gateway instance can serve any request *of an established session* as long as it has the + encryption key -- no sticky sessions, no coordination. The login leg is the one exception: it + holds per-instance pending state and requires affinity until the callback lands (see + <<_stateless_after_login_not_during_it,Stateless after login, not during it>>). * Horizontal scaling is linear; a node can be added or removed with no session migration. This is the property that distinguishes Variant A from Variant B and the reason it is the @@ -151,7 +158,7 @@ oidc: session: mode: cookie # encrypted cookie, no store (this variant) cookie_name: __Host-sheriff - encryption_key: ${SHERIFF_SESSION_KEY} # AES-256-GCM key + encryption_key: ${SHERIFF_SESSION_KEY} # AES-256-GCM key; optional -- omit to generate on startup previous_key: ${SHERIFF_SESSION_KEY_PREVIOUS} # optional; decrypt-only (key rotation) ttl_seconds: 3600 refresh: { enabled: true, leeway_seconds: 30, on_failure: reauthenticate } @@ -174,7 +181,14 @@ endpoint: ---- The only material difference from the link:02-bff-session.adoc[session-based] configuration is -`session.mode: cookie` and the required `session.encryption_key`. +`session.mode: cookie`. + +`session.encryption_key` is *optional*. Declare it when sessions must be shared across replicas +and survive a restart -- that is the only thing it buys. Omitting it selects the +generate-on-startup key mode: the gateway mints a key at boot, so sessions are dropped on every +restart and the key cannot be shared with another replica. `previous_key` is optional and +decrypt-only, and requires an `encryption_key` to roll onto. See +link:../configuration.adoc[the configuration reference] for the authoritative validation rules. == Properties @@ -184,3 +198,36 @@ The only material difference from the link:02-bff-session.adoc[session-based] co defence and short token lifetimes). * *Self-contained* -- instances share only the encryption key. * *Full OIDC RP* -- confidential-client login, transparent refresh, RFC 9470 step-up. + +[#_stateless_after_login_not_during_it] +=== Stateless after login, not during it + +The variant is stateless once a session is *established*, not for the whole login exchange. The +pending authorization -- the state the gateway holds between issuing the login redirect and +receiving the OIDC callback -- lives in an in-memory `PendingAuthorizationStore`, which is +per-instance and never shared. A login *begun* on one instance must therefore *complete* on that +same instance; a callback landing on a different replica finds no pending authorization and the +login fails. + +An *established* session is fully portable: any replica holding the sealing key can unseal the +cookie and serve the request. That is exactly the boundary the cookie-statelessness integration +test documents -- it logs in only against the first instance, then exercises the second instance +with the resulting cookie. + +*Deployment consequence*: session affinity is required for the *login leg only* (the redirect +through to the callback). Once the session cookie is issued, requests may be balanced freely +across replicas with no affinity at all. + +=== Sealed-cookie format version + +The sealed-cookie format version is bound into the cookie's authenticated data, so a cookie +carrying a retired version fails to unseal rather than being mis-parsed. The version was bumped +when the per-session identity nonce was added to the sealed payload. + +*Upgrade consequence*: existing cookie-mode sessions are invalidated on deploy and users must log +in again. There is deliberately no backward-compatible acceptance path for the previous format -- +accepting it would mean synthesizing the missing nonce, which would silently change the derived +session identity. See +link:../adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc[ADR-0018] +for the architectural +decision behind the identity model. diff --git a/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffCookieSessionIT.java b/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffCookieSessionIT.java index eb5c53ce..99903372 100644 --- a/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffCookieSessionIT.java +++ b/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffCookieSessionIT.java @@ -131,8 +131,8 @@ void sealedCookieValueCarriesNoReadableToken() { "the sealed cookie must carry no readable segment of the mediated access token"); } byte[] raw = Base64.getUrlDecoder().decode(sealed); - assertEquals((byte) 1, raw[0], - "the cookie value must be the version-1 sealed layout, not an opaque server-side handle"); + assertEquals((byte) 2, raw[0], + "the cookie value must be the version-2 sealed layout, not an opaque server-side handle"); } @Test @@ -202,15 +202,23 @@ void tamperedCookieIsUnauthenticatedNeverServerError() { } /** - * Flips the final character of the sealed value, which sits inside the authentication tag. + * Flips a bit inside the trailing 16-byte GCM authentication tag. + * <p> + * The mutation is applied to the <em>decoded</em> bytes rather than to the final base64url + * character, because editing that character is not reliably a mutation at all: when the sealed + * length is not a multiple of three, the final character carries 2 or 4 unused padding bits that + * {@link Base64#getUrlDecoder()} discards without validating. Flipping only those bits — which is + * what {@code 'A' -> 'B'} does — re-decodes to the identical ciphertext and tag, so the peer + * unseals it successfully and answers 200. Mutating a decoded tag byte is unambiguous. * * @param sealed the sealed cookie value * @return the tampered value, still in the base64url alphabet */ private static String tamper(String sealed) { assertNotNull(sealed, "the login must establish a sealed session cookie"); - char last = sealed.charAt(sealed.length() - 1); - return sealed.substring(0, sealed.length() - 1) + (last == 'A' ? 'B' : 'A'); + byte[] raw = Base64.getUrlDecoder().decode(sealed); + raw[raw.length - 1] ^= 0x01; + return Base64.getUrlEncoder().withoutPadding().encodeToString(raw); } /**