fix(stores)!: close the 2026-10-21 security audit findings of the stores

CHANGELOG.md

@@ -579,6 +579,29 @@ legacy packages (`authentication/`, `authorization/`, the in-tree
   loopback-issuer setting to both configurations, sets `AdvertiseJWKSURI`
   and gives the server its OpenID Connect scopes followed by the
   deployment's own.
+- `CheckDeployment(ctx)` and `ErrDeploymentRequirement` on
+  `oauth2/store/sql`, `oidc/client/store/sql`, `saml/sp/store/sql` and
+  `oauth2/store/redis`: the deployment requirements of each package
+  documentation that the server reports — on MySQL, InnoDB tables, binary
+  key columns and a strict `sql_mode` (oauth2); on Redis, version 6.0 or
+  later, `maxmemory-policy noeviction` and no cluster mode (through `INFO`,
+  which needs `+info` in a restricted ACL). Every requirement not met is
+  reported; a server error (NOPERM) is returned as such. Call it at boot
+  when the schema is applied by your own migration tool.
+- `PurgeExpired(ctx, now)` on `oauth2/store/sql` and `saml/sp/store/sql`:
+  deletes the rows expired before `now` in batches of 1000, each a READ
+  COMMITTED transaction (PostgreSQL, MySQL), oauth2 families last, never a
+  family a concurrent revocation extends. Add the `expires_at` indexes the
+  package documentation lists.
+- `oidcclient.ValidateStoreKey`, the check every `TransactionStore.Consume`
+  runs on its keys before any I/O (64 lowercase hex digits).
+- `saml/sp/store/redis.WithClock`: the clock the store measures its key
+  lifetimes on (give it `samlsp.Config.Now` when that one is injected).
+- `storetest.RunConformance` (oauth2) compares the full token records of
+  every lookup and rechecks, 1.5 s later, a consumed code, a rotated refresh
+  token and revoked families (`TokenRecordsRoundTrip`, `RetainedOverTime`);
+  the OIDC suite checks that `Consume` refuses malformed keys
+  (`ConsumeRefusesInvalidKeys`).
 
 ### Changed
 
@@ -1148,6 +1171,26 @@ legacy packages (`authentication/`, `authorization/`, the in-tree
   and validates its configured redirect URI with `ValidateRedirectURI` at
   startup, and `e2e/oidc` runs the relying party on `127.0.0.1` and the
   provider on `localhost`.
+- **The SQL stores' `Migrate` checks the deployment (breaking on
+  non-compliant MySQL deployments)**: it still only creates missing tables,
+  then runs `CheckDeployment` and fails with `ErrDeploymentRequirement` on
+  a MyISAM or missing table, on `VARCHAR` key columns of the oauth2 store
+  and on a non-strict `sql_mode` for the oauth2 store. The MySQL DDL names
+  `ENGINE=InnoDB DEFAULT CHARSET=utf8mb4`. On MySQL and MariaDB, `Migrate`
+  creates the tables and checks them under a named lock per store module
+  (`GET_LOCK`, released before it returns, waited for as long as its context
+  allows), like the advisory lock it takes on PostgreSQL: concurrent
+  `CREATE TABLE IF NOT EXISTS` of one table could fail with a metadata-lock
+  deadlock. See [MIGRATION.md](MIGRATION.md).
+- **`oauth2/store/redis.New` refuses a `*redis.ClusterClient` or a
+  `*redis.Ring`** (breaking): the store's multi-key scripts cannot work
+  with their sharding; Cluster clients used to fail at runtime (CROSSSLOT)
+  and Ring clients intermittently. See [MIGRATION.md](MIGRATION.md).
+- `oidc/client/store/sql.PurgeExpired` runs its DELETE in a READ COMMITTED
+  transaction on PostgreSQL and MySQL: at MySQL's default isolation an
+  unindexed purge locked the gaps it scanned, blocking the logins starting
+  meanwhile. A MySQL server logging statements (`binlog_format=STATEMENT`)
+  now refuses it: use ROW or MIXED.
 
 ### Fixed
 
@@ -1436,6 +1479,15 @@ legacy packages (`authentication/`, `authorization/`, the in-tree
   `unsupported_response_type`; RFC 6749 §4.1.1 makes the parameter
   REQUIRED, and §4.1.2.1 answers a missing one with `invalid_request` — an
   empty occurrence included (§3.1).
+- `oidcclient.TransactionStore.Consume` validates its keys in the memory,
+  SQL and Redis stores, as the interface documents: on MySQL a malformed
+  state hash — upper-case, space-padded — matched the canonical one under
+  the column's collation.
+- Test gaps of the stores: the real-backend oauth2 conformance runs warm
+  eight connections before each case, so that the concurrency cases race
+  for real (a check-then-act code consumption passed on PostgreSQL), and
+  each locking layer of the SQL token rotation — family lock, token lock,
+  single-row consumption — has a test of its own.
 
 ### Removed
 
@@ -2295,3 +2347,15 @@ legacy packages (`authentication/`, `authorization/`, the in-tree
   encodings, so a non-canonical encoding (y ≥ p, whose decoding RFC 8032
   §5.1.3 makes fail) was used as a key. PS* signatures are now verified with
   a hash-size salt and such keys are ignored, as in the `jwt` module.
+- **SQL stores on MySQL use InnoDB**: the MySQL DDL of the three SQL
+  stores named no storage engine, so on a server whose default engine is
+  MyISAM `Migrate` created tables without transactions or row locks. The
+  oauth2 store then lost the atomicity of its writes (`oauth2.Storage` I3):
+  a rotation cut short left the presented refresh token consumed without
+  its replacement, and a consumption acknowledged before a crash could be
+  lost. The DDL now names InnoDB, and `Migrate` refuses other engines.
+- **The SAML Redis store's replay marks no longer depend on Redis's
+  clock**: it set key expiries as absolute instants computed on the SP's
+  clock, so with Redis ahead of the SP by more than a mark's retention the
+  mark was deleted as written and the same assertion accepted again (SAML
+  2.0 Profiles §4.1.4.5). Key lifetimes are now sent as durations.

LIMITATIONS.md

@@ -54,12 +54,18 @@ future refactor phases.
   their lifetime short. See
   [docs/security-considerations.md](docs/security-considerations.md#offline-jwt-access-tokens).
 - **Redis Cluster** — not supported by `oauth2/store/redis`: its Lua
-  scripts touch keys of several hash slots. Use a standalone server or a
-  Sentinel primary.
-- **SQL expired-row cleanup** — `oauth2/store/sql` ships no expiry job:
-  rows — consumed and abandoned authorization codes included — stay until
-  an operator deletes those past `expires_at` (see the package
-  documentation for a safe order).
+  scripts touch keys of several hash slots (a token, its family, its
+  replacement), and lookups by token hash cannot place them in one slot.
+  Use a standalone server or a Sentinel primary: `New` refuses a Cluster or
+  Ring client, and `Store.CheckDeployment` a server in cluster mode; a
+  proxy sharding keys behind one endpoint is not detected and fails
+  closed at runtime.
+- **SQL expired-row cleanup is scheduled by you** — `oauth2/store/sql`
+  runs no job of its own: rows — consumed and abandoned authorization codes
+  included — stay until `Store.PurgeExpired` deletes those past
+  `expires_at`. Schedule it, and add the `expires_at` indexes the package
+  documentation lists: `Schema` creates none (MySQL has no `CREATE INDEX IF
+  NOT EXISTS`, and building one at boot would block a populated table).
 - **In-memory store growth** — `oauth2/storage/memory` never purges
   anything: consumed and expired codes, expired tokens and revoked
   families stay for the process lifetime. It is meant for development and
@@ -76,9 +82,20 @@ future refactor phases.
 - **SQL schema upgrades are manual** — `Store.Migrate` creates missing
   tables and never alters an existing one: the columns a release adds (#80's
   `consumed`, #83's `issuer` and `extension_data`, the codes'
-  `redirect_uri_omitted`, the refresh tokens' `family_issued_at`) and the
-  binary key columns of MySQL are added or converted with the statements
-  MIGRATION.md lists, through your migration tool.
+  `redirect_uri_omitted`, the refresh tokens' `family_issued_at`), the
+  binary key columns and the InnoDB engine of MySQL are added or converted
+  with the statements MIGRATION.md lists, through your migration tool.
+  `Migrate` — and `Store.CheckDeployment`, for a schema applied by your own
+  tool — refuses a MySQL database still missing the engine or the binary
+  keys; a missing column fails the statements that use it, closed.
+- **Deployment checks see what the server reports** — `CheckDeployment`
+  verifies on MySQL the engine, the key columns and the `sql_mode` of one
+  pooled connection (set the strict mode server-wide or in the DSN), and on
+  Redis what `INFO` reports. It checks nothing on PostgreSQL:
+  `synchronous_commit=off` can lose an acknowledged revocation in a crash,
+  but it can be set per role, session or transaction, so one connection's
+  value proves nothing — keep it on. Replicas, proxies and eviction set
+  after boot are not seen.
 - **A leaked code can be burned** — codes are consumed before they are
   validated, so a party holding a code (but not its client's credentials or
   PKCE verifier) can make the legitimate exchange fail with
@@ -349,8 +366,8 @@ PKCE. Not supported, and refused where a provider could select them:
   login starts, since a full store refuses every new login
   (`memory.ErrFull`) until transactions expire or are consumed.
   `oidc/client/store/sql` ships `PurgeExpired`, a single
-  unbatched `DELETE` the operator schedules (index `expires_at`, see its
-  package documentation). CI (`make test-backends`) runs the conformance
+  unbatched `DELETE` at READ COMMITTED the operator schedules (index
+  `expires_at`, see its package documentation). CI (`make test-backends`) runs the conformance
   suite and the replica races of the SQL and Redis stores on PostgreSQL
   16.15, MySQL 8.0.46 (end of life since April 2026) and Redis 7.4.11
   (`internal/interop/oidcstack`, `TestRealBackend*`), and the concurrent
@@ -569,8 +586,8 @@ PKCE. Not supported, and refused where a provider could select them:
 - **Stores** — `saml/sp/storage/memory` (development and tests) sweeps its
   expired records every 64 writes on its own clock (`memory.WithClock`) and
   is otherwise unbounded: it holds the login rate times `TransactionTTL`.
-  `saml/sp/store/sql` ships no expiry job (its package documentation gives
-  the cleanup statements). CI runs the stores on
+  `saml/sp/store/sql` deletes expired rows only when its `PurgeExpired`
+  runs: schedule it. CI runs the stores on
   SQLite and miniredis, and (`make test-backends`) their races,
   conformance suite and payload round trip on PostgreSQL 16.15, MySQL
   8.0.46 (end of life since April 2026) and Redis 7.4.11

MIGRATION.md

@@ -3225,3 +3225,132 @@ To migrate:
 2. A provider whose PS* ID Tokens now fail with `ErrIDTokenSignature` must
    sign with a salt as long as the hash (RFC 7518 §3.5), or the client use
    another algorithm it offers (`Config.Algorithms`).
+
+### SQL stores: MySQL tables are InnoDB, checked at boot
+
+`Schema(sqlstore.MySQL)` of `oauth2/store/sql`, `oidc/client/store/sql` and
+`saml/sp/store/sql` named no storage engine: on a server whose default
+engine is MyISAM (`default_storage_engine`), `Migrate` created tables
+without transactions or row locks. The oauth2 store then lost the atomic
+writes its contract promises (I3): a rotation cut short — a context
+cancelled, a lost connection — left the presented refresh token consumed
+without its replacement. MyISAM is not crash safe either: a consumption
+acknowledged before a crash could be lost. The DDL now names
+`ENGINE=InnoDB DEFAULT CHARSET=utf8mb4`, and every SQL store gains
+`CheckDeployment`, which `Migrate` runs once the tables exist.
+
+|                                                     | Before                               | After                                                                 |
+| --------------------------------------------------- | ------------------------------------ | --------------------------------------------------------------------- |
+| MySQL `CREATE TABLE` of the three SQL stores        | the server's default engine and charset | `ENGINE=InnoDB DEFAULT CHARSET=utf8mb4`                            |
+| `Migrate` on MySQL with a MyISAM (or missing) table | succeeds                             | fails with `ErrDeploymentRequirement`, naming the table               |
+| `oauth2/store/sql` `Migrate` with legacy `VARCHAR` key columns | succeeds                  | fails with `ErrDeploymentRequirement`, naming each column             |
+| `oauth2/store/sql` `Migrate` on a non-strict `sql_mode` | succeeds (over-long values truncated) | fails with `ErrDeploymentRequirement`                              |
+| `Store.CheckDeployment(ctx)`                        | —                                    | new, on the three SQL stores (no-op on PostgreSQL and SQLite)         |
+| Concurrent `Migrate` on MySQL                       | concurrent `CREATE TABLE IF NOT EXISTS` (could fail with a metadata-lock deadlock, 1213) | one after the other, under the named lock `hyperscale-stack/security/<module>.Migrate` (`GET_LOCK`) |
+| PostgreSQL and SQLite                               | —                                    | unchanged                                                             |
+
+`Migrate` still never alters a table. To migrate (MySQL and MariaDB only):
+
+1. **Find the tables to convert**:
+
+   ```sql
+   SELECT TABLE_NAME, ENGINE, TABLE_COLLATION FROM information_schema.TABLES
+   WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME IN (
+     'oauth2_auth_codes', 'oauth2_access_tokens', 'oauth2_refresh_tokens', 'oauth2_token_families',
+     'oidc_client_transactions', 'samlsp_transactions', 'samlsp_used_assertions');
+   ```
+
+2. **Convert every table that is not InnoDB** — keep the lines of the
+   tables the query listed:
+
+   ```sql
+   ALTER TABLE oauth2_auth_codes        ENGINE=InnoDB;
+   ALTER TABLE oauth2_access_tokens     ENGINE=InnoDB;
+   ALTER TABLE oauth2_refresh_tokens    ENGINE=InnoDB;
+   ALTER TABLE oauth2_token_families    ENGINE=InnoDB;
+   ALTER TABLE oidc_client_transactions ENGINE=InnoDB;
+   ALTER TABLE samlsp_transactions      ENGINE=InnoDB;
+   ALTER TABLE samlsp_used_assertions   ENGINE=InnoDB;
+   ```
+
+3. **Optionally, convert the oauth2 tables to utf8mb4** when the query
+   listed another character set: subjects, scopes and extension data are
+   UTF-8. The OIDC and SAML tables hold ASCII only. `CONVERT TO` turns a
+   `TEXT` column into `MEDIUMTEXT`, which changes nothing for the store;
+   the binary key columns are left as they are.
+
+   ```sql
+   ALTER TABLE oauth2_auth_codes     CONVERT TO CHARACTER SET utf8mb4;
+   ALTER TABLE oauth2_access_tokens  CONVERT TO CHARACTER SET utf8mb4;
+   ALTER TABLE oauth2_refresh_tokens CONVERT TO CHARACTER SET utf8mb4;
+   ALTER TABLE oauth2_token_families CONVERT TO CHARACTER SET utf8mb4;
+   ```
+
+4. **Plan the window.** Each `ALTER` copies its table under a metadata
+   lock: run it at a quiet time, or with an online schema-change tool.
+5. **Make `sql_mode` strict** for the oauth2 store's connections —
+   `STRICT_TRANS_TABLES` or `STRICT_ALL_TABLES`, the default of MySQL 8 and
+   MariaDB — in the server's configuration, or in the DSN with
+   `?sql_mode='TRADITIONAL'` (go-sql-driver/mysql): strict, and it keeps
+   `NO_ENGINE_SUBSTITUTION`; a DSN mode replaces the whole session mode. A
+   database still on `VARCHAR` key columns first needs the upgrade of the
+   section "`oauth2/store/sql`: MySQL compares keys byte for byte".
+6. **Check at boot.** `Migrate` now fails, with an error matching the
+   store's `ErrDeploymentRequirement`, until the deployment meets the
+   requirements; it lists every one it finds unmet. When you apply
+   `Schema` with your own migration tool, call `store.CheckDeployment(ctx)`
+   at boot, before serving traffic.
+7. **Concurrent boots.** On MySQL and MariaDB, `Migrate` holds the named
+   lock its package documentation names (`GET_LOCK`, server-wide) while it
+   creates the tables and checks them, and waits for it as long as its
+   context allows: pass a deadline. A migration tool applying `Schema`
+   while instances run `Migrate` should take the same lock. A holder whose
+   client vanished without closing its connection keeps the lock until the
+   server notices (`wait_timeout`, 8 hours by default): find it with
+   `SELECT IS_USED_LOCK('hyperscale-stack/security/oauth2/store/sql.Migrate')`
+   and end it with `KILL <id>`. On a Galera or Percona XtraDB cluster named
+   locks are local to a node (some versions refuse `GET_LOCK`): instances
+   migrating through different nodes are not serialized — run `Migrate`
+   through one node, or apply `Schema` with your own tool.
+
+### `oauth2/store/redis`: `New` refuses Cluster and Ring clients
+
+The store's scripts touch several keys at once — a token, its family, its
+replacement. A Redis Cluster puts them in different hash slots, and a
+client-side `redis.Ring` on different shards: a Cluster client failed every
+script with CROSSSLOT, a Ring client found a token only when it shared its
+family's shard. `New` now refuses both, with an error matching
+`redisstore.ErrDeploymentRequirement`, and `Store.CheckDeployment` checks
+the server (Redis 6.0 or later, `maxmemory-policy noeviction`, no cluster
+mode).
+
+|                                      | Before                         | After                                             |
+| ------------------------------------ | ------------------------------ | ------------------------------------------------- |
+| `New(*redis.ClusterClient)`          | accepted, scripts fail at runtime | error matching `ErrDeploymentRequirement`      |
+| `New(*redis.Ring)`                   | accepted, lookups fail at random | error matching `ErrDeploymentRequirement`       |
+| `New(*redis.Client)` (standalone, Sentinel failover client) | accepted | unchanged                                |
+| `Store.CheckDeployment(ctx)`         | —                              | new: checks the server through `INFO`            |
+
+To migrate:
+
+1. Run the store on a standalone Redis or the primary of a Sentinel
+   deployment (`redis.NewClient`, `redis.NewFailoverClient`).
+2. Call `store.CheckDeployment(ctx)` at boot. A restricted ACL user needs
+   `+info` for it (`INFO` is in the `@dangerous` category); without it the
+   call returns the server's NOPERM error — verify the requirements by hand
+   and skip the call. A `maxmemory-policy` other than `noeviction` is
+   refused: a "revoked" family key evicted within its retention lets the
+   next issuance revive the family.
+
+### `oidc/client`: `TransactionStore.Consume` validates its keys
+
+`Consume` of the memory, SQL and Redis stores now refuses, before any I/O, a
+state hash or binding hash that is not 64 lowercase hex digits, with an
+error matching `oidcclient.ErrInvalidTransaction`, as the
+`TransactionStore` contract states. `Client.Complete` always passes
+canonical keys: nothing changes for it.
+
+To migrate a `TransactionStore` of your own: call
+`oidcclient.ValidateStoreKey` on both keys first in `Consume`, and run
+`storetest.RunConformance`, which now checks it
+(`ConsumeRefusesInvalidKeys`).

docs/architecture.md

@@ -332,8 +332,10 @@ StartLogin / Begin                          HandleCallback / Complete
   completes a login; altering a transaction fails it, whoever runs it.
 - **`TransactionStore`** — `Save` never overwrites, `Consume` removes a
   transaction atomically in a pinned decision order (unknown, expired,
-  binding mismatch — which keeps it). In-memory, SQL and Redis
-  implementations pass `oidc/client/storetest`.
+  binding mismatch — which keeps it). Both refuse malformed input before
+  any I/O: `Save` what `Transaction.Validate` refuses, `Consume` keys
+  `ValidateStoreKey` refuses. In-memory, SQL and Redis implementations pass
+  `oidc/client/storetest`.
 - **ID Token validation** — OpenID Connect Core §3.1.3.7 on top of go-jose:
   asymmetric algorithms only, keys from the issuer's `jwks_uri` through a
   bounded, rate-limited cache, and every claim check including nonce,