From 5d21106cc35bf42021e477159da1ebebc02c3853 Mon Sep 17 00:00:00 2001 From: "Claude Opus 5.5" Date: Sat, 17 Oct 2026 20:35:41 +0000 Subject: [PATCH 1/2] feat(oidc): SessionKey::derive from a generated secret SessionKey::from_bytes takes 32 raw bytes, but a deployment's secret store usually holds a generated string (ADR-style "generated secrets"), so every host hashes it into a key on its own. derive does it once: HKDF-SHA256 over the secret, with a context that says what the key is for, so one secret used for two purposes gives two unrelated keys. A secret shorter than 32 bytes is refused with OidcConfigError::ShortSessionSecret, which names the length, never the bytes. A known-answer test pins the derivation: changing it would sign every session of every deployment out. Co-Authored-By: Claude Opus 5.5 --- cli-framework-oidc/README.md | 5 ++ cli-framework-oidc/src/browser/session_key.rs | 74 +++++++++++++++++++ cli-framework-oidc/src/lib.rs | 2 + 3 files changed, 81 insertions(+) diff --git a/cli-framework-oidc/README.md b/cli-framework-oidc/README.md index d6c33b7..6f19124 100644 --- a/cli-framework-oidc/README.md +++ b/cli-framework-oidc/README.md @@ -271,6 +271,11 @@ match sessions.resolve(request.headers()).await { cross-site`, or a POST with neither header is refused with 403. It ends the realm session with the refresh token, clears the cookie, and redirects to the realm's end-session endpoint. +- **Session key**: `SessionKey::from_bytes` takes 32 raw bytes. When the + secret store holds a generated string instead, use + `SessionKey::derive(secret, b" session v1")` (HKDF-SHA256; at least + 32 bytes of secret). The same secret and context give the same key, so + changing either signs every session out. - **Cookie size**: `HostSessions::new` refuses a configuration whose expected tokens would not fit `max_cookie_bytes` (4096), and sign-in refuses real tokens that don't. The sealed size is logged as `cookie_bytes`. diff --git a/cli-framework-oidc/src/browser/session_key.rs b/cli-framework-oidc/src/browser/session_key.rs index 5e30e46..d3d6f8b 100644 --- a/cli-framework-oidc/src/browser/session_key.rs +++ b/cli-framework-oidc/src/browser/session_key.rs @@ -1,3 +1,6 @@ +use crate::OidcConfigError; +use hkdf::Hkdf; +use sha2::Sha256; use std::sync::Arc; use zeroize::Zeroize; @@ -17,11 +20,82 @@ impl Drop for KeyBytes { pub struct SessionKey(Arc); impl SessionKey { + /// The shortest secret [`derive`](Self::derive) accepts, in bytes. + pub const MIN_SECRET_LEN: usize = 32; + pub fn from_bytes(bytes: [u8; 32]) -> Self { Self(Arc::new(KeyBytes(bytes))) } + /// A key derived with HKDF-SHA256 from a generated secret, for a + /// deployment whose secret store holds a random string rather than 32 raw + /// bytes. `context` says what the key is for (e.g. `b"acme host session + /// v1"`): the same secret with another context gives an unrelated key. + /// + /// The secret is used as given, so trim a file's trailing newline first. + /// Refuses a secret shorter than [`MIN_SECRET_LEN`](Self::MIN_SECRET_LEN) + /// bytes; the error names its length, never its bytes. The same secret + /// and context always give the same key, so changing either signs every + /// session out. + pub fn derive(secret: &[u8], context: &[u8]) -> Result { + if secret.len() < Self::MIN_SECRET_LEN { + return Err(OidcConfigError::ShortSessionSecret(secret.len())); + } + let mut key = [0u8; 32]; + Hkdf::::new(None, secret) + .expand(context, &mut key) + .expect("HKDF expand: 32 bytes always fits"); + let derived = Self::from_bytes(key); + key.zeroize(); + Ok(derived) + } + pub(crate) fn as_bytes(&self) -> &[u8; 32] { &self.0 .0 } } + +#[cfg(test)] +mod tests { + use super::*; + + const SECRET: &[u8] = b"0123456789abcdef0123456789abcdef"; + + #[test] + fn derive_gives_hkdf_sha256_of_the_secret_for_the_context() { + // HKDF-SHA256, no salt, info = the context, 32 bytes (RFC 5869), + // computed independently. Pinned: a change here would sign every + // session of every deployment out. + let key = SessionKey::derive(SECRET, b"test host session v1").unwrap(); + let hex: String = key.as_bytes().iter().map(|b| format!("{b:02x}")).collect(); + assert_eq!( + hex, + "d0e55e2ac06e1ea80c1c087f7b54dae770444e3bf4a682e6e1ee68e8b5d44508" + ); + } + + #[test] + fn derive_depends_on_both_the_secret_and_the_context() { + let key = |secret: &[u8], context: &[u8]| { + *SessionKey::derive(secret, context).unwrap().as_bytes() + }; + assert_eq!(key(SECRET, b"a"), key(SECRET, b"a")); + assert_ne!(key(SECRET, b"a"), key(SECRET, b"b")); + assert_ne!( + key(SECRET, b"a"), + key(b"0123456789abcdef0123456789abcdeF", b"a") + ); + assert_ne!(&key(SECRET, b"a")[..], SECRET); + } + + #[test] + fn derive_refuses_a_short_secret_without_naming_it() { + let short = b"short-secret-canary-0123456789a"; + assert_eq!(short.len(), SessionKey::MIN_SECRET_LEN - 1); + let error = SessionKey::derive(short, b"a").err().unwrap().to_string(); + assert!(error.contains("31 bytes"), "{error}"); + assert!(!error.contains("canary"), "{error}"); + assert!(SessionKey::derive(b"", b"a").is_err()); + assert!(SessionKey::derive(SECRET, b"a").is_ok()); + } +} diff --git a/cli-framework-oidc/src/lib.rs b/cli-framework-oidc/src/lib.rs index e2cd1f7..28da862 100644 --- a/cli-framework-oidc/src/lib.rs +++ b/cli-framework-oidc/src/lib.rs @@ -27,6 +27,8 @@ pub enum OidcConfigError { InvalidClaimPath(String), #[error("invalid static JWKS: {0}")] InvalidJwks(String), + #[error("session secret too short: {0} bytes (at least 32)")] + ShortSessionSecret(usize), } /// Validate that a JWKS URI is secure: must be https, or http to loopback only. From a56b6b8a6b7d08b0e714c942909cc29f00f3ce4a Mon Sep 17 00:00:00 2001 From: "Claude Opus 5.5" Date: Sat, 17 Oct 2026 20:38:15 +0000 Subject: [PATCH 2/2] docs(oidc): move the session key note next to the binding note Keeps it apart from the host-session list's other new bullet, so the two PRs merge in either order. Co-Authored-By: Claude Opus 5.5 --- cli-framework-oidc/README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cli-framework-oidc/README.md b/cli-framework-oidc/README.md index 6f19124..e11215b 100644 --- a/cli-framework-oidc/README.md +++ b/cli-framework-oidc/README.md @@ -264,6 +264,11 @@ match sessions.resolve(request.headers()).await { The cookie has no `Max-Age` unless `session_ttl` is set. - **Binding**: a cookie sealed for another deployment string, or with another key, ends as `BindingMismatch` or `Invalid`. +- **Session key**: `SessionKey::from_bytes` takes 32 raw bytes. When the + secret store holds a generated string instead, use + `SessionKey::derive(secret, b" session v1")` (HKDF-SHA256; at least + 32 bytes of secret). The same secret and context give the same key, so + changing either signs every session out. - **Refresh**: an access token within `refresh_skew` of expiry is refreshed. A refused refresh ends the session; an unreachable realm keeps the old token until it expires, then answers `Unavailable` without clearing the cookie. @@ -271,11 +276,6 @@ match sessions.resolve(request.headers()).await { cross-site`, or a POST with neither header is refused with 403. It ends the realm session with the refresh token, clears the cookie, and redirects to the realm's end-session endpoint. -- **Session key**: `SessionKey::from_bytes` takes 32 raw bytes. When the - secret store holds a generated string instead, use - `SessionKey::derive(secret, b" session v1")` (HKDF-SHA256; at least - 32 bytes of secret). The same secret and context give the same key, so - changing either signs every session out. - **Cookie size**: `HostSessions::new` refuses a configuration whose expected tokens would not fit `max_cookie_bytes` (4096), and sign-in refuses real tokens that don't. The sealed size is logged as `cookie_bytes`.