From 963cfdc815f207a68dbb804c3b3757b68d404802 Mon Sep 17 00:00:00 2001 From: Oscar Sanderson Date: Mon, 2 Nov 2026 13:58:52 +0800 Subject: [PATCH 1/6] fix(sqlstore)!: refuse a sealing key that isn't random AESGCMSealer checked only a key's length, so an all-zero placeholder, a passphrase or a hex or base64 key passed undecoded sealed every record under a key anyone could guess: no confidentiality, and anyone who could write to the database could forge sealed records. FAPIgo's other sealers refuse such keys; AESGCMSealer now makes the same check (copied, since sqlstore uses FAPIgo's public API only), and RotatingSealer refuses two AESGCMSealers over the same key. BREAKING CHANGE: AESGCMSealer returns an error for a key that is one byte repeated or made entirely of printable text, which v0.3 accepted. Generate the key with crypto/rand and decode an encoded key before passing it. The README's Upgrading from v0.3 section shows how to keep reading the records sealed under the old key. Co-Authored-By: Claude Opus 5.5 --- sqlstore/README.md | 46 +++++++++++++++++++++++++++++++++ sqlstore/doc.go | 5 ++++ sqlstore/seal.go | 60 ++++++++++++++++++++++++++++++++++++++----- sqlstore/seal_test.go | 38 +++++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 6 deletions(-) diff --git a/sqlstore/README.md b/sqlstore/README.md index 8492d280..dd80daac 100644 --- a/sqlstore/README.md +++ b/sqlstore/README.md @@ -77,6 +77,10 @@ sealer, err := sqlstore.AESGCMSealer(key) // 32 random bytes, from a secrets man db, err := sqlstore.New(sqlDB, sqlstore.Postgres, sqlstore.Options{Sealer: sealer}) ``` +`AESGCMSealer` refuses a key that's certainly not random: one byte +repeated (an all-zero placeholder), or all printable text (a +passphrase, or a hex or base64 key not yet decoded: decode it first). + A sealed blob moved to another row, client, table or table prefix, altered, or opened under another key is refused (`ErrRecordFormat`, as an outage), as is one whose row's expiry (or an interaction's pushed @@ -107,6 +111,48 @@ and bind them too. Upgrade every instance sharing a database at once: a v0.2 instance can't open what an upgraded one seals, and rolling back to v0.2 leaves the records sealed since unreadable. +## Upgrading from v0.3 + +v0.4 refuses configurations v0.3 accepted: + +- **A sealing key that isn't random.** `AESGCMSealer` returns an error + for a key that's one byte repeated (all zeros, say) or all printable + text: a passphrase, or a hex or base64 key passed without decoding + it. Such a key gave no confidentiality, and let anyone who could + guess it and write to the database forge sealed records. Generate 32 + bytes with `crypto/rand`, and decode an encoded key before passing + it: + + ```go + key, err := hex.DecodeString(os.Getenv("FAPIGO_SEAL_KEY")) // `openssl rand -hex 32` + sealer, err := sqlstore.AESGCMSealer(key) + ``` + + The records sealed under the old key open only under it, and + `AESGCMSealer` no longer takes it. Either let them go (as with a lost + key, clients authorize again), or open them with a `Sealer` of your + own over the old key, in `RotatingSealer(newSealer, oldKeySealer)`, + until the longest-lived of them (a refresh token) expires. It reads + `AESGCMSealer`'s format, a 12-byte nonce then the ciphertext: + + ```go + type oldKeySealer struct{ aead cipher.AEAD } // cipher.NewGCM(aes.NewCipher(oldKey)) + + func (s oldKeySealer) Seal(plaintext, additionalData []byte) ([]byte, error) { + return nil, errors.New("the old key only opens") + } + + func (s oldKeySealer) Open(sealed, additionalData []byte) ([]byte, error) { + if len(sealed) < s.aead.NonceSize() { + return nil, errors.New("sealed data is too short") + } + n := s.aead.NonceSize() + return s.aead.Open(nil, sealed[:n], sealed[n:], additionalData) + } + ``` + + `RotatingSealer` also refuses two `AESGCMSealer`s over the same key. + ## What each database declares | | Durable | AtomicConsume | CrossInstanceConsistent | SerializableRedemption | EncryptedAtRest | diff --git a/sqlstore/doc.go b/sqlstore/doc.go index 55e03c13..3e403e14 100644 --- a/sqlstore/doc.go +++ b/sqlstore/doc.go @@ -168,6 +168,11 @@ // sealer, err := sqlstore.AESGCMSealer(key) // 32 bytes from crypto/rand, kept outside the database // db, err := sqlstore.New(sqlDB, sqlstore.Postgres, sqlstore.Options{Sealer: sealer}) // +// AESGCMSealer refuses a key that's certainly not random, as FAPIgo's +// other sealers do: one byte repeated (an all-zero placeholder), or all +// printable text (a passphrase, or a hex or base64 key not yet decoded: +// decode it first). +// // Sealed are the blobs that hold what a copy of the database mustn't // reveal: pushed requests and interactions' copies of them (the // authorization request, its claims and PKCE challenge), authorization diff --git a/sqlstore/seal.go b/sqlstore/seal.go index 305c1bbe..d609788e 100644 --- a/sqlstore/seal.go +++ b/sqlstore/seal.go @@ -1,10 +1,12 @@ package sqlstore import ( + "bytes" "context" "crypto/aes" "crypto/cipher" "crypto/rand" + "crypto/sha256" "database/sql" "encoding/binary" "encoding/json" @@ -30,6 +32,10 @@ type Sealer interface { // AESGCMSealer is a Sealer that seals with AES-256-GCM under key (32 // bytes, from a cryptographically secure source, kept outside the // database), with a random 96-bit nonce from crypto/rand for each seal. +// It refuses a key that's certainly not random, as FAPIgo's other +// sealers do: one byte repeated (all zeros, say: a placeholder), or all +// printable text (a passphrase, or a hex or base64 key not yet decoded: +// decode it first). // // Random nonces bound how much one key may seal: NIST SP 800-38D // (§8.3) allows at most 2^32 seals with random 96-bit nonces under one @@ -46,8 +52,8 @@ type Sealer interface { // unreadable; to rotate it, seal with the new key and open with both, // until the records sealed under the old one expire: RotatingSealer. func AESGCMSealer(key []byte) (Sealer, error) { - if len(key) != 32 { - return nil, fmt.Errorf("sqlstore: record sealer: key is %d bytes, want 32 (AES-256)", len(key)) + if err := checkSealKey(key); err != nil { + return nil, fmt.Errorf("sqlstore: record sealer: %w", err) } block, err := aes.NewCipher(key) if err != nil { @@ -57,7 +63,31 @@ func AESGCMSealer(key []byte) (Sealer, error) { if err != nil { return nil, fmt.Errorf("sqlstore: record sealer: %w", err) } - return gcmSealer{aead}, nil + return gcmSealer{aead, sha256.Sum256(key)}, nil +} + +// sealKeySize is AESGCMSealer's key length: AES-256. +const sealKeySize = 32 + +// checkSealKey refuses key unless it's sealKeySize bytes, not one byte +// repeated (all zeros, say: a placeholder, not a random key) and not +// made entirely of printable text (a passphrase, or a hex or base64 key +// not yet decoded). It's the check FAPIgo's own sealers make +// (internal/sealkeys), copied: sqlstore uses FAPIgo's public API only, +// so a release of it builds against any later FAPIgo. It only catches +// keys that are certainly not random: 32 random bytes are all printable +// with probability (98/256)^32, about 4e-14. +func checkSealKey(key []byte) error { + if len(key) != sealKeySize { + return fmt.Errorf("key is %d bytes, want %d (AES-256)", len(key), sealKeySize) + } + if bytes.Count(key, key[:1]) == sealKeySize { + return errors.New("key is one byte repeated, not a random key") + } + if !slices.ContainsFunc(key, func(c byte) bool { return (c < 0x20 || c > 0x7e) && c != '\t' && c != '\n' && c != '\r' }) { + return fmt.Errorf("key is all printable text (a passphrase, or a hex or base64 key not yet decoded), not %d random bytes: generate it with crypto/rand, and decode an encoded key before passing it", sealKeySize) + } + return nil } // RotatingSealer is a Sealer for rotating keys: it seals with current, @@ -69,12 +99,25 @@ func AESGCMSealer(key []byte) (Sealer, error) { // blobs' format is current's, unchanged, so the database needs nothing // to switch to it, and a blob sealed under no key it holds is refused // as before. Every open of an old record tries current first; with an -// AEAD such as AESGCMSealer, that failure is cheap. +// AEAD such as AESGCMSealer, that failure is cheap. It refuses two +// AESGCMSealers under the same key: a rotation that changes nothing. func RotatingSealer(current Sealer, previous ...Sealer) (Sealer, error) { if current == nil || slices.Contains(previous, nil) { return nil, errors.New("sqlstore: rotating sealer: nil Sealer") } - return rotatingSealer{append([]Sealer{current}, previous...)}, nil + sealers := append([]Sealer{current}, previous...) + seen := map[[sha256.Size]byte]int{} + for i, s := range sealers { + g, ok := s.(gcmSealer) + if !ok { + continue + } + if j, ok := seen[g.keyID]; ok { + return nil, fmt.Errorf("sqlstore: rotating sealer: sealer %d has the same key as sealer %d", i, j) + } + seen[g.keyID] = i + } + return rotatingSealer{sealers}, nil } type rotatingSealer struct{ sealers []Sealer } @@ -95,7 +138,12 @@ func (s rotatingSealer) Open(sealed, additionalData []byte) ([]byte, error) { return nil, errors.Join(errs...) } -type gcmSealer struct{ aead cipher.AEAD } +// gcmSealer is AESGCMSealer's Sealer. keyID, its key's SHA-256 digest, +// lets RotatingSealer refuse one key twice. +type gcmSealer struct { + aead cipher.AEAD + keyID [sha256.Size]byte +} func (s gcmSealer) Seal(plaintext, additionalData []byte) ([]byte, error) { nonce := make([]byte, s.aead.NonceSize()) diff --git a/sqlstore/seal_test.go b/sqlstore/seal_test.go index c97e86d9..fa35e49a 100644 --- a/sqlstore/seal_test.go +++ b/sqlstore/seal_test.go @@ -314,6 +314,44 @@ func TestAESGCMSealer(t *testing.T) { } } +// AESGCMSealer refuses a key that's certainly not random — a placeholder, +// a passphrase, or an encoded key not yet decoded — as FAPIgo's other +// sealers do, and RotatingSealer one key twice. +func TestAESGCMSealerRefusesWeakKeys(t *testing.T) { + printable := bytes.Repeat([]byte("ab"), 16) + printable[31] = '\n' // whitespace is text too + for name, c := range map[string]struct { + key []byte + want string + }{ + "all zero": {make([]byte, 32), "one byte repeated"}, + "all 0xff": {bytes.Repeat([]byte{0xff}, 32), "one byte repeated"}, + "passphrase": {[]byte("correct horse battery staple!!!!"), "printable text"}, + "hex": {[]byte("00112233445566778899aabbccddeeff"), "printable text"}, + "base64": {[]byte("q83vEjRWeJCrze8SNFZ4kKvN7xI0VniQ")[:32], "printable text"}, + "text": {printable, "printable text"}, + } { + _, err := sqlstore.AESGCMSealer(c.key) + if err == nil || !strings.Contains(err.Error(), c.want) || !strings.HasPrefix(err.Error(), "sqlstore: record sealer: ") { + t.Errorf("AESGCMSealer(%s key) = %v, want an error saying %q", name, err, c.want) + } + } + key := make([]byte, 32) + _, err := rand.Read(key) + must(t, err) + key[0] = 0x80 // never all printable + a, err := sqlstore.AESGCMSealer(key) + must(t, err) + b, err := sqlstore.AESGCMSealer(bytes.Clone(key)) + must(t, err) + if _, err := sqlstore.RotatingSealer(a, testSealer(t), b); err == nil || !strings.Contains(err.Error(), "same key") { + t.Errorf("RotatingSealer(one key twice) = %v, want an error", err) + } + if _, err := sqlstore.RotatingSealer(a, testSealer(t)); err != nil { + t.Errorf("RotatingSealer(two keys) = %v", err) + } +} + // A record that opens but isn't one of this package's — not JSON, // another version, an unknown value — is unreadable, as an outage. func TestMalformedRecordsAreUnreadable(t *testing.T) { From 49c8d3989573d1ad5206904dab5ef0b496080074 Mon Sep 17 00:00:00 2001 From: Oscar Sanderson Date: Mon, 2 Nov 2026 14:11:49 +0800 Subject: [PATCH 2/6] fix(sqlstore)!: refuse a SQLite database that isn't in WAL mode SQLite opened without the documented pragmas passed every check, then failed most concurrent requests with server_error: outside WAL mode, a read made while a write commits fails at once unless its connection has a busy timeout, and Options.SQLiteBusyTimeout only ever applied to sqlstore's own writes. Migrate and CheckSchema now read PRAGMA journal_mode and refuse anything but wal, quoting the DSN to open the file with, and the docs say which busy timeout covers reads. BREAKING CHANGE: on SQLite, Migrate and CheckSchema return an error unless the database is in WAL mode, which v0.3 accepted. Open the file with both pragmas in its DSN, busy_timeout first, as in file:fapigo.db?_pragma=busy_timeout(10000)&_pragma=journal_mode(WAL). Co-Authored-By: Claude Opus 5.5 --- sqlstore/README.md | 18 +++++++++++++++++ sqlstore/db.go | 10 ++++++++-- sqlstore/db_test.go | 48 +++++++++++++++++++++++++++++++++++++++++++++ sqlstore/doc.go | 6 ++++++ sqlstore/migrate.go | 40 +++++++++++++++++++++++++++++++++++-- 5 files changed, 118 insertions(+), 4 deletions(-) diff --git a/sqlstore/README.md b/sqlstore/README.md index dd80daac..7493f785 100644 --- a/sqlstore/README.md +++ b/sqlstore/README.md @@ -59,6 +59,12 @@ pure Go, no cgo) needs WAL mode and a busy timeout: sql.Open("sqlite", "file:fapigo.db?_pragma=busy_timeout(10000)&_pragma=journal_mode(WAL)") ``` +`Migrate` and `CheckSchema` refuse a SQLite database that isn't in WAL +mode: without it, a read made while a write commits fails with +`SQLITE_BUSY`, and under concurrent requests most fail as outages. +`Options.SQLiteBusyTimeout` covers sqlstore's writes; reads get the +DSN's `busy_timeout`, so give the DSN both. + On a new file, the first connection switches it to WAL mode as it opens, and a connection another process opens at that moment fails with `SQLITE_BUSY`. `Migrate` waits that out, so run it first in each @@ -152,6 +158,18 @@ v0.4 refuses configurations v0.3 accepted: ``` `RotatingSealer` also refuses two `AESGCMSealer`s over the same key. +- **A SQLite database not in WAL mode.** `Migrate` and `CheckSchema` + return an error unless `PRAGMA journal_mode` is `wal`. Without it, a + read made while a write commits failed with `SQLITE_BUSY`, answered + as an outage: under concurrent requests, most of them. Open the file + with both pragmas, `busy_timeout` first: + + ```go + sql.Open("sqlite", "file:fapigo.db?_pragma=busy_timeout(10000)&_pragma=journal_mode(WAL)") + ``` + + The first connection opened that way switches the file to WAL mode, + which lasts. ## What each database declares diff --git a/sqlstore/db.go b/sqlstore/db.go index 96925bc6..190440df 100644 --- a/sqlstore/db.go +++ b/sqlstore/db.go @@ -32,7 +32,8 @@ const ( // database/sql driver (modernc.org/sqlite, say, which needs no cgo). // Every write runs in a transaction started with BEGIN IMMEDIATE, // taking the database's write lock up front. Open it with a busy - // timeout and in WAL mode — see the package documentation. + // timeout and in WAL mode — see the package documentation; Migrate + // and CheckSchema refuse a database that isn't in WAL mode. SQLite ) @@ -58,7 +59,12 @@ type Options struct { // SQLiteBusyTimeout is how long a SQLite write waits for another // connection's write lock before failing. Zero means 10 seconds. - // Ignored for Postgres. + // Ignored for Postgres. It applies to this package's writes only, + // which set it on their connection: a read waits as long as the + // busy_timeout in the DSN says, which is why the DSN needs one too. + // In WAL mode, which Migrate and CheckSchema require, a read doesn't + // wait for a write at all; it waits only on the rare moments WAL + // mode can't serve it, such as a new file's switch to WAL. SQLiteBusyTimeout time.Duration // Sealer, if set, seals every record blob the stores write — the diff --git a/sqlstore/db_test.go b/sqlstore/db_test.go index b7802d98..d9461ccc 100644 --- a/sqlstore/db_test.go +++ b/sqlstore/db_test.go @@ -3,7 +3,10 @@ package sqlstore_test import ( "context" "crypto/sha256" + "database/sql" "errors" + "path/filepath" + "strings" "sync" "testing" "time" @@ -145,6 +148,51 @@ func TestCheckSchemaRefusals(t *testing.T) { }) } +// withSQLite runs fn on a DB over the SQLite file at path opened with +// DSN query query (with its "?", or empty), closing it after. +func withSQLite(t *testing.T, path, query string, fn func(d *sqlstore.DB) error) error { + t.Helper() + db, err := sql.Open("sqlite", "file:"+path+query) + if err != nil { + t.Fatal(err) + } + defer func() { _ = db.Close() }() + d, err := sqlstore.New(db, sqlstore.SQLite, sqlstore.Options{}) + if err != nil { + t.Fatal(err) + } + return fn(d) +} + +// Migrate and CheckSchema refuse a SQLite database that isn't in WAL +// mode, whose reads fail with SQLITE_BUSY while a write commits, with +// an error naming the DSN that fixes it; a file once switched to WAL +// mode stays in it, whatever DSN opens it next. +func TestSQLiteRequiresWAL(t *testing.T) { + ctx := context.Background() + path := filepath.Join(t.TempDir(), "store.db") + migrate := func(d *sqlstore.DB) error { return d.Migrate(ctx) } + checkSchema := func(d *sqlstore.DB) error { return d.CheckSchema(ctx) } + for _, query := range []string{"", "?_pragma=busy_timeout(10000)", "?_pragma=journal_mode(DELETE)", "?_pragma=journal_mode(TRUNCATE)"} { + wantNotWAL(t, "Migrate("+query+")", withSQLite(t, path, query, migrate)) + } + if err := withSQLite(t, path, "?_pragma=busy_timeout(10000)&_pragma=journal_mode(WAL)", migrate); err != nil { + t.Fatalf("Migrate in WAL mode: %v", err) + } + if err := withSQLite(t, path, "", checkSchema); err != nil { + t.Fatalf("CheckSchema of a WAL file opened without the pragma: %v", err) + } + wantNotWAL(t, "CheckSchema(after switching back to a rollback journal)", withSQLite(t, path, "?_pragma=journal_mode(DELETE)", checkSchema)) +} + +// wantNotWAL fails t unless err is checkJournalMode's refusal. +func wantNotWAL(t *testing.T, what string, err error) { + t.Helper() + if err == nil || !strings.Contains(err.Error(), "not WAL") || !strings.Contains(err.Error(), "_pragma=journal_mode(WAL)") { + t.Errorf("%s = %v, want a refusal naming journal_mode(WAL)", what, err) + } +} + // Two prefixes in one database are two separate sets of stores. func TestTablePrefixesKeepStoresApart(t *testing.T) { forEachDialect(t, func(t *testing.T, dialect sqlstore.Dialect) { diff --git a/sqlstore/doc.go b/sqlstore/doc.go index 3e403e14..5d4bbe89 100644 --- a/sqlstore/doc.go +++ b/sqlstore/doc.go @@ -83,6 +83,12 @@ // // sql.Open("sqlite", "file:fapigo.db?_pragma=busy_timeout(10000)&_pragma=journal_mode(WAL)") // +// Migrate and CheckSchema refuse a database that isn't in WAL mode: +// without it, a read made while a write commits fails with SQLITE_BUSY +// unless its connection has a busy timeout, and under concurrent +// requests most of them would fail as outages. Options.SQLiteBusyTimeout +// covers this package's writes, which set it on their connection; +// reads get the DSN's busy_timeout, so give the DSN both. // List busy_timeout first, so it applies to the pragmas after it. On a // new file, the first connection switches the file to WAL mode (which // lasts) as it opens, holding the write lock without waiting the busy diff --git a/sqlstore/migrate.go b/sqlstore/migrate.go index b1c43d09..e5690bdc 100644 --- a/sqlstore/migrate.go +++ b/sqlstore/migrate.go @@ -108,6 +108,10 @@ func (d *DB) statements(m migration) []string { // Nothing migrates implicitly — New doesn't: run it from a deployment // step, or at startup before serving, as a role that may create tables. // +// On SQLite, it refuses a database that isn't in WAL mode (PRAGMA +// journal_mode), as CheckSchema does: open it with journal_mode(WAL) +// in its DSN — see the package documentation's Setup section. +// // On SQLite, a connection to a new file opened with journal_mode(WAL) // in its DSN switches the file to WAL mode as it opens, taking the // write lock without waiting the busy timeout, so connections other @@ -141,8 +145,36 @@ func sqliteBusy(err error) bool { return strings.Contains(msg, "SQLITE_BUSY") || strings.Contains(msg, "database is locked") } +// sqliteDSNAdvice is what checkJournalMode's error tells an operator +// to open SQLite with. +const sqliteDSNAdvice = "open it with both pragmas in its DSN, busy_timeout first, such as file:fapigo.db?_pragma=busy_timeout(10000)&_pragma=journal_mode(WAL)" + +// checkJournalMode refuses a SQLite database that isn't in WAL mode. +// Without it, a read made while a write commits fails at once with +// SQLITE_BUSY — unless the connection has a busy timeout, which only +// the DSN can give every connection a read uses — and the server +// answers it as an outage: under concurrent requests, most of them. +// WAL mode is a property of the file, which every connection then +// reports. Postgres needs no check. +func (d *DB) checkJournalMode(ctx context.Context) error { + if d.dialect != SQLite { + return nil + } + var mode string + if err := d.db.QueryRowContext(ctx, "PRAGMA journal_mode").Scan(&mode); err != nil { + return fmt.Errorf("read the SQLite journal mode: %w", err) + } + if !strings.EqualFold(mode, "wal") { + return fmt.Errorf("the SQLite database's journal mode is %q, not WAL, so a read made while a write commits fails rather than waits: %s", mode, sqliteDSNAdvice) + } + return nil +} + // migrate is Migrate, once. func (d *DB) migrate(ctx context.Context) error { + if err := d.checkJournalMode(ctx); err != nil { + return fmt.Errorf("sqlstore: migrate: %w", err) + } for _, m := range migrations { if err := d.write(ctx, func(q querier) error { return d.apply(ctx, q, m) }); err != nil { return fmt.Errorf("sqlstore: migrate: %w", err) @@ -243,9 +275,13 @@ func checkApplied(applied map[int]bool, requireAll bool) error { // CheckSchema returns an error unless the database's schema is exactly // the one this package's stores need: every migration applied, and // none newer than SchemaVersion (a newer release's, whose changes this -// one doesn't know). Call it at startup, where Migrate is run -// separately, to fail fast rather than on the first request. +// one doesn't know). On SQLite it also requires WAL mode, as Migrate +// does. Call it at startup, where Migrate is run separately, to fail +// fast rather than on the first request. func (d *DB) CheckSchema(ctx context.Context) error { + if err := d.checkJournalMode(ctx); err != nil { + return fmt.Errorf("sqlstore: check schema: %w", err) + } applied, err := d.appliedVersions(ctx, d.db) if err != nil { return fmt.Errorf("sqlstore: check schema (has Migrate run?): %w", err) From 09895771113c9a5db855c0883afad8065e95e627 Mon Sep 17 00:00:00 2001 From: Oscar Sanderson Date: Mon, 2 Nov 2026 14:14:04 +0800 Subject: [PATCH 3/6] feat(sqlstore): add ResourceServer for a standalone resource server's stores A resource server running apart from the authorization server needs two DBs: the server's, for Revocation, Replay and AccessTokens, and one with a TablePrefix of its own, for Nonces. Both have every store method, so the shortest wiring, everything from the resource server's own DB, type-checked and passed production checks while its Revocation read a table the server never writes: a revoked grant's tokens kept working until they expired. ResourceServer(server, own) returns the four stores, each from the right DB, and refuses a nil DB or two DBs on one TablePrefix. The package documentation and README show it. Co-Authored-By: Claude Opus 5.5 --- sqlstore/README.md | 27 ++++++++-- sqlstore/doc.go | 20 +++++++ sqlstore/resource_server.go | 57 ++++++++++++++++++++ sqlstore/resource_server_test.go | 92 ++++++++++++++++++++++++++++++++ 4 files changed, 191 insertions(+), 5 deletions(-) create mode 100644 sqlstore/resource_server.go create mode 100644 sqlstore/resource_server_test.go diff --git a/sqlstore/README.md b/sqlstore/README.md index 7493f785..576b11f1 100644 --- a/sqlstore/README.md +++ b/sqlstore/README.md @@ -206,11 +206,28 @@ between instances: use SQLite there. whose digests each role namespaces), through a DB with the same `TablePrefix` and `Sealer`. Their DPoP nonce store is the exception: `Nonces()` from a DB with a `TablePrefix` of its own, never the - server's, whose nonces it would then accept. Give a resource server a - database role of its own: `SELECT` on the server's `access_tokens`, - `revocations` and `schema_migrations`, `SELECT`/`INSERT` on `replay`, - and its own prefix's tables; and let only the server `Sweep` the - tables they share. + server's, whose nonces it would then accept. A resource server in a + process of its own takes all four from + `sqlstore.ResourceServer(serverDB, rsDB)`, which reads each from the + right DB: one whose `Revocation()` came from its own prefix, as its + `Nonces()` must, never sees the server's revocations, and nothing + fails. + + ```go + stores, err := sqlstore.ResourceServer(serverDB, rsDB) // rsDB: TablePrefix "fapigo_rs_", say + deps := resource.Dependencies{ + AccessTokens: resource.OpaqueAccessTokens{Store: stores.AccessTokens, Issuer: issuer, Audience: audience}, // or JWTs + Replay: stores.Replay, + Revocation: stores.Revocation, + Nonces: stores.Nonces, + // … + } + ``` + + Give a resource server a database role of its own: `SELECT` on the + server's `access_tokens`, `revocations` and `schema_migrations`, + `SELECT`/`INSERT` on `replay`, and its own prefix's tables; and let + only the server `Sweep` the tables they share. - **Clients** are rows the operator manages: a `client_id` and a versioned JSON configuration, written with `Clients().Put` or `MarshalClientConfig`. See the package documentation's Clients diff --git a/sqlstore/doc.go b/sqlstore/doc.go index 5d4bbe89..edbf907e 100644 --- a/sqlstore/doc.go +++ b/sqlstore/doc.go @@ -63,6 +63,26 @@ // (serverresource.NewVerifier refuses the server's own db.Nonces(), but // can't tell two DBs with one TablePrefix apart.) // +// A resource server in a process of its own, which builds +// resource.Dependencies itself, needs both DBs, and the shortest path +// is the wrong one: the same rsDB for everything, whose Revocation reads +// a table the server never writes, so a grant the server revokes is +// never revoked there, with nothing failing. Take its stores from +// ResourceServer instead, which takes each from the right DB and +// refuses two on one TablePrefix: +// +// serverDB, err := sqlstore.New(sqlDB, sqlstore.Postgres, sqlstore.Options{Sealer: sealer}) // the server's tables +// rsDB, err := sqlstore.New(sqlDB, sqlstore.Postgres, sqlstore.Options{TablePrefix: "fapigo_rs_"}) +// err = rsDB.Migrate(ctx) +// stores, err := sqlstore.ResourceServer(serverDB, rsDB) +// deps := resource.Dependencies{ +// AccessTokens: resource.OpaqueAccessTokens{Store: stores.AccessTokens, Issuer: issuer, Audience: audience}, // or JWTAccessTokens +// Replay: stores.Replay, +// Revocation: stores.Revocation, +// Nonces: stores.Nonces, +// // ... +// } +// // A resource server needs far less of the database than the server // does, and is often run in more places, so give it a database role of // its own with just that, rather than the server's: SELECT on the diff --git a/sqlstore/resource_server.go b/sqlstore/resource_server.go new file mode 100644 index 00000000..0b0b3543 --- /dev/null +++ b/sqlstore/resource_server.go @@ -0,0 +1,57 @@ +package sqlstore + +import "errors" + +// ResourceServerStores are a resource server's stores, for a resource +// server that runs apart from the authorization server (one in the same +// process uses serverresource.NewVerifier, which takes the server's +// Dependencies): ResourceServer returns them. +type ResourceServerStores struct { + // Revocation is resource.Dependencies.Revocation: the server's + // revocations, which server.Server.RevokeGrant and a reused + // authorization code record. + Revocation *RevocationStore + + // Replay is resource.Dependencies.Replay: the server's replay + // table, in which each role keeps its digests in a namespace of its + // own. + Replay *ReplayStore + + // AccessTokens is resource.OpaqueAccessTokens' Store: the server's + // opaque access tokens. Unused with JWT access tokens. + AccessTokens *AccessTokenStore + + // Nonces is resource.Dependencies.Nonces: the resource server's own + // DPoP nonces, never the server's, whose nonces it would then accept + // (RFC 9449 keeps the server's, §8, apart from a resource server's, + // §9). + Nonces *NonceStore +} + +// ResourceServer returns a resource server's stores: Revocation, +// Replay and AccessTokens from server, a DB on the authorization +// server's tables (its TablePrefix and, for opaque access tokens, its +// Sealer), and Nonces from own, a DB with a TablePrefix of its own. +// +// Take them all from here rather than from either DB's methods: a +// resource server whose Revocation came from own, as its Nonces must, +// would read a table the server never writes, and keep accepting the +// tokens of a revoked grant until they expire, with nothing failing. +// It refuses a nil DB, and server and own on one TablePrefix (the same +// DB, or two over it), whose nonces would be the server's. Migrate +// (or CheckSchema) both, as for any DB; only the server runs Sweep on +// the tables they share, and the resource server on own. +func ResourceServer(server, own *DB) (ResourceServerStores, error) { + switch { + case server == nil || own == nil: + return ResourceServerStores{}, errors.New("sqlstore: resource server: nil DB") + case server == own || server.prefix == own.prefix: + return ResourceServerStores{}, errors.New("sqlstore: resource server: own is on the server's TablePrefix, " + server.prefix + ", so its nonces would be the server's: give it a TablePrefix of its own") + } + return ResourceServerStores{ + Revocation: server.Revocation(), + Replay: server.Replay(), + AccessTokens: server.AccessTokens(), + Nonces: own.Nonces(), + }, nil +} diff --git a/sqlstore/resource_server_test.go b/sqlstore/resource_server_test.go new file mode 100644 index 00000000..b0395acd --- /dev/null +++ b/sqlstore/resource_server_test.go @@ -0,0 +1,92 @@ +package sqlstore_test + +import ( + "context" + "crypto/sha256" + "strings" + "testing" + "time" + + "github.com/idfoundry/fapigo/sqlstore" + "github.com/idfoundry/fapigo/storage" +) + +// ResourceServer gives a standalone resource server the server's +// revocations, replay table and opaque access tokens, and nonces of its +// own: a grant the server revokes is revoked there, which it isn't +// through the resource server's own DB. +func TestResourceServerStores(t *testing.T) { + forEachDialect(t, func(t *testing.T, dialect sqlstore.Dialect) { + ctx := context.Background() + p := newPlace(t, dialect) + serverDB := p.migratedWith(sqlstore.Options{Sealer: testSealer(t)}) + ownPlace := newPlace(t, dialect) + ownPlace.path = p.path + ownDB := ownPlace.migrated() + + rs, err := sqlstore.ResourceServer(serverDB, ownDB) + must(t, err) + expires := time.Now().Add(time.Hour) + must(t, serverDB.Revocation().Revoke(ctx, "grant:g1", expires)) + wantRevoked(t, "the resource server's Revocation", rs.Revocation, true) + wantRevoked(t, "the resource server's own DB's Revocation", ownDB.Revocation(), false) + + hash := sha256.Sum256([]byte("opaque access token")) + must(t, serverDB.AccessTokens().CreateAccessToken(ctx, storage.NewAccessToken{TokenHash: hash, ClientID: "c", ExpiresAt: expires})) + if _, err := rs.AccessTokens.LookupAccessToken(ctx, storage.AccessTokenLookup{TokenHash: hash}); err != nil { + t.Errorf("the resource server's AccessTokens can't read the server's token: %v", err) + } + + must(t, rs.Nonces.Issue(ctx, storage.NonceIssuance{Nonce: "rs-nonce", ExpiresAt: expires})) + if _, err := serverDB.Nonces().Lookup(ctx, storage.NonceLookup{Nonce: "rs-nonce"}); err == nil { + t.Error("the server's Nonces finds the resource server's nonce") + } + if _, err := ownDB.Nonces().Lookup(ctx, storage.NonceLookup{Nonce: "rs-nonce"}); err != nil { + t.Errorf("the resource server's own DB doesn't hold its nonce: %v", err) + } + + used := storage.ReplayUse{Namespace: "resource:dpop", Digest: sha256.Sum256([]byte("jti-1")), ExpiresAt: expires} + must(t, rs.Replay.UseOnce(ctx, used)) + if err := serverDB.Replay().UseOnce(ctx, used); err == nil { + t.Error("the resource server's replay digest isn't in the server's replay table") + } + }) +} + +// wantRevoked fails t unless checker reports grant g1 revoked as want. +func wantRevoked(t *testing.T, what string, checker interface { + IsRevoked(context.Context, string) (bool, error) +}, want bool) { + t.Helper() + got, err := checker.IsRevoked(context.Background(), "grant:g1") + if err != nil || got != want { + t.Errorf("%s: IsRevoked(a grant the server revoked) = %v, %v, want %v", what, got, err, want) + } +} + +// ResourceServer refuses a nil DB, and nonces on the server's +// TablePrefix: the same DB, or another over the same prefix. +func TestResourceServerRefusals(t *testing.T) { + p := newPlace(t, sqlstore.SQLite) + serverDB, sqlDB := p.open() + samePrefix, err := sqlstore.New(sqlDB, sqlstore.SQLite, sqlstore.Options{TablePrefix: p.prefix}) + must(t, err) + defaultPrefix, err := sqlstore.New(sqlDB, sqlstore.SQLite, sqlstore.Options{}) + must(t, err) + defaultPrefix2, err := sqlstore.New(sqlDB, sqlstore.SQLite, sqlstore.Options{TablePrefix: "fapigo_"}) + must(t, err) + for name, c := range map[string]struct { + server, own *sqlstore.DB + want string + }{ + "nil server": {nil, serverDB, "nil DB"}, + "nil own": {serverDB, nil, "nil DB"}, + "one DB": {serverDB, serverDB, "TablePrefix of its own"}, + "same prefix": {serverDB, samePrefix, "TablePrefix of its own"}, + "default prefix": {defaultPrefix, defaultPrefix2, "TablePrefix of its own"}, + } { + if _, err := sqlstore.ResourceServer(c.server, c.own); err == nil || !strings.Contains(err.Error(), c.want) { + t.Errorf("ResourceServer(%s) = %v, want an error saying %q", name, err, c.want) + } + } +} From 2cc26dbb9725580c0577e0589ecc3f2f1fb8d417 Mon Sep 17 00:00:00 2001 From: Oscar Sanderson Date: Mon, 2 Nov 2026 14:19:13 +0800 Subject: [PATCH 4/6] feat(sqlstore): keep clients' keys in the clients table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The clients table had no place for a client's keys, so every client was registered twice: its configuration in the table, its keys in a keys.ClientKeySource built elsewhere, typically at startup, so a client added with Put couldn't authenticate until a restart, and the two registries could drift. The client configuration document gains optional jwks_uri and jwks members (ClientKeyRegistration; version 1 takes new optional members), written with ClientRepository.PutWithKeys or MarshalClientConfigWithKeys and read with ParseClientKeys. Both refuse a registration with both members (RFC 7591 §2), a jwks_uri that isn't https, and a jwks that isn't a JWK Set with a usable key, or that the document's strict decoding would refuse. DB.ClientKeys returns a keys.ClientJWKSSource whose lookup reads the client's row on every resolution, fetching a jwks_uri through the given fapihttp.Client, so it declares LiveFetchHardened exactly as that source does. A row whose key members are malformed is an outage (ErrClientConfig wrapping storage.ErrStoreUnavailable), as for its other members. Co-Authored-By: Claude Opus 5.5 --- sqlstore/README.md | 11 ++ sqlstore/client.go | 75 ++++++++++-- sqlstore/client_keys_test.go | 216 +++++++++++++++++++++++++++++++++++ sqlstore/clientconfig.go | 143 ++++++++++++++++++++++- sqlstore/doc.go | 17 +++ sqlstore/production_test.go | 33 +++++- 6 files changed, 480 insertions(+), 15 deletions(-) create mode 100644 sqlstore/client_keys_test.go diff --git a/sqlstore/README.md b/sqlstore/README.md index 576b11f1..1348e4aa 100644 --- a/sqlstore/README.md +++ b/sqlstore/README.md @@ -232,6 +232,17 @@ between instances: use SQLite there. versioned JSON configuration, written with `Clients().Put` or `MarshalClientConfig`. See the package documentation's Clients section for the document's members. +- **Client keys** live in the same document, as a `jwks_uri` or an + inline `jwks`, so the clients table is the one registry: write them + with `Clients().PutWithKeys` (or `MarshalClientConfigWithKeys`) and + serve them with `db.ClientKeys(fetcher)` as + `server.Dependencies.ClientKeys`. A client added or rekeyed at + runtime authenticates on its next request, without a restart. + + ```go + deps.ClientKeys, err = db.ClientKeys(fetcher) // a hardened fapihttp.Client; nil if no client has a jwks_uri + err = db.Clients().PutWithKeys(ctx, cfg, sqlstore.ClientKeyRegistration{JWKS: jwks}) + ``` ## Tests diff --git a/sqlstore/client.go b/sqlstore/client.go index c018e1d1..9354a685 100644 --- a/sqlstore/client.go +++ b/sqlstore/client.go @@ -7,6 +7,8 @@ import ( "fmt" fapi "github.com/idfoundry/fapigo" + "github.com/idfoundry/fapigo/fapihttp" + "github.com/idfoundry/fapigo/keys" "github.com/idfoundry/fapigo/storage" ) @@ -31,17 +33,33 @@ func (d *DB) Clients() *ClientRepository { return &ClientRepository{d} } // ErrClientConfig and storage.ErrStoreUnavailable, and the server // answers it with 500 server_error rather than invalid_client. func (r *ClientRepository) ResolveClient(ctx context.Context, id fapi.ClientID) (storage.RegisteredClient, error) { + config, err := r.config(ctx, "resolve client", id) + if err != nil { + return storage.RegisteredClient{}, err + } + return resolveRow(id, config) +} + +// config is the configuration document of the client registered as +// id, for op: an error wrapping ErrNotFound for an unknown id. +func (r *ClientRepository) config(ctx context.Context, op string, id fapi.ClientID) ([]byte, error) { if !validTextKey(string(id)) { - return storage.RegisteredClient{}, fmt.Errorf("sqlstore: resolve client: %w", notFound("client")) + return nil, fmt.Errorf("sqlstore: %s: %w", op, notFound("client")) } var config []byte err := r.d.queryRow(ctx, "SELECT config FROM {clients} WHERE client_id = ?", string(id)).Scan(&config) switch { case errors.Is(err, sql.ErrNoRows): - return storage.RegisteredClient{}, fmt.Errorf("sqlstore: resolve client %q: %w", id, notFound("client")) + return nil, fmt.Errorf("sqlstore: %s %q: %w", op, id, notFound("client")) case err != nil: - return storage.RegisteredClient{}, unavailable("resolve client", err) + return nil, unavailable(op, err) } + return config, nil +} + +// resolveRow is the client registered as id with configuration config, +// as ResolveClient builds it. +func resolveRow(id fapi.ClientID, config []byte) (storage.RegisteredClient, error) { cfg, err := ParseClientConfig(id, config) if err != nil { return storage.RegisteredClient{}, fmt.Errorf("sqlstore: resolve client %q: %w: %w", id, storage.ErrStoreUnavailable, err) @@ -53,17 +71,27 @@ func (r *ClientRepository) ResolveClient(ctx context.Context, id fapi.ClientID) return client, nil } -// Put registers cfg's client, replacing any client registered under its -// ID. It refuses a configuration storage.NewRegisteredClient refuses, -// or one MarshalClientConfig can't write. +// Put registers cfg's client with no keys in the clients table, +// replacing any client registered under its ID, keys included: use +// PutWithKeys for a client whose keys DB.ClientKeys serves. It refuses +// a configuration storage.NewRegisteredClient refuses, or one +// MarshalClientConfig can't write. func (r *ClientRepository) Put(ctx context.Context, cfg storage.RegisteredClientConfig) error { + return r.PutWithKeys(ctx, cfg, ClientKeyRegistration{}) +} + +// PutWithKeys is Put for a client whose keys the clients table holds +// too, for DB.ClientKeys: its jwks_uri, or its inline JWK Set. It +// refuses what MarshalClientConfigWithKeys refuses. The keys take +// effect on the next request, as the configuration does. +func (r *ClientRepository) PutWithKeys(ctx context.Context, cfg storage.RegisteredClientConfig, reg ClientKeyRegistration) error { if _, err := storage.NewRegisteredClient(cfg); err != nil { return fmt.Errorf("sqlstore: put client: %w", err) } if !validTextKey(string(cfg.ID)) { return errors.New("sqlstore: put client: client_id isn't valid UTF-8 without NUL") } - config, err := MarshalClientConfig(cfg) + config, err := MarshalClientConfigWithKeys(cfg, reg) if err != nil { return fmt.Errorf("sqlstore: put client: %w", err) } @@ -102,3 +130,36 @@ func (r *ClientRepository) Delete(ctx context.Context, id fapi.ClientID) (bool, // Capabilities implements storage.StoreAssurance — see the package // documentation. func (r *ClientRepository) Capabilities() storage.Capabilities { return r.d.capabilities() } + +// ClientKeys returns a keys.ClientJWKSSource serving each client's keys +// from its row of the clients table — its jwks_uri or jwks member +// (ClientKeyRegistration) — for server.Dependencies.ClientKeys (and +// ClientEncryptionKeys), so the clients table is the one registry of +// clients and their keys: a client PutWithKeys registers, or whose keys +// it changes, is served its keys on the next request, with no restart. +// The source reads the row on every resolution; it parses an inline +// JWK Set once per distinct set, and fetches a jwks_uri through +// fetcher, caching it as keys.ClientJWKSSource describes (opts tune +// that). fetcher may be nil when no client registers a jwks_uri; under +// server.AssuranceProduction, a fetcher must be +// fapihttp.Client.HardenedForProduction (see +// keys.ClientJWKSSource.Capabilities). An unknown client is an error +// wrapping ErrNotFound; a row whose keys don't parse, one wrapping +// ErrClientConfig and storage.ErrStoreUnavailable. +func (d *DB) ClientKeys(fetcher *fapihttp.Client, opts ...keys.ClientJWKSOption) (*keys.ClientJWKSSource, error) { + r := d.Clients() + return keys.NewClientJWKSSource(fetcher, r.lookupKeys, opts...) +} + +// lookupKeys is ClientKeys' keys.ClientJWKSLookup. +func (r *ClientRepository) lookupKeys(ctx context.Context, id fapi.ClientID) (fapi.URL, []byte, error) { + config, err := r.config(ctx, "client keys", id) + if err != nil { + return fapi.URL{}, nil, err + } + reg, err := ParseClientKeys(config) + if err != nil { + return fapi.URL{}, nil, fmt.Errorf("sqlstore: client keys %q: %w: %w", id, storage.ErrStoreUnavailable, err) + } + return reg.JWKSURI, reg.JWKS, nil +} diff --git a/sqlstore/client_keys_test.go b/sqlstore/client_keys_test.go new file mode 100644 index 00000000..13766869 --- /dev/null +++ b/sqlstore/client_keys_test.go @@ -0,0 +1,216 @@ +package sqlstore_test + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "reflect" + "strings" + "testing" + "time" + + fapi "github.com/idfoundry/fapigo" + "github.com/idfoundry/fapigo/fapihttp" + "github.com/idfoundry/fapigo/keys" + "github.com/idfoundry/fapigo/sqlstore" + "github.com/idfoundry/fapigo/storage" +) + +// clientJWKS is a JWK Set of one fresh P-256 public key, kid kid. +func clientJWKS(t *testing.T, kid string) json.RawMessage { + t.Helper() + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + pub, err := priv.PublicKey.Bytes() // 0x04 || X || Y + if err != nil { + t.Fatal(err) + } + x := base64.RawURLEncoding.EncodeToString(pub[1:33]) + y := base64.RawURLEncoding.EncodeToString(pub[33:]) + return json.RawMessage(fmt.Sprintf(`{"keys":[{"kty":"EC","crv":"P-256","x":%q,"y":%q,"kid":%q,"alg":"ES256","use":"sig"}]}`, x, y, kid)) +} + +// keyedClient is a client configuration NewRegisteredClient accepts, +// for client id. +func keyedClient(id fapi.ClientID) storage.RegisteredClientConfig { + return storage.RegisteredClientConfig{ID: id, RedirectURIs: []fapi.RegisteredRedirectURI{"https://rp.example/cb"}, ClientAssertionAlgorithm: fapi.ES256} +} + +// resolveKIDs is the kids ClientKeys' source resolves for client id's +// ES256 keys, or its error. +func resolveKIDs(source keys.ClientKeySource, id fapi.ClientID) ([]string, error) { + set, err := source.ResolveVerificationKeys(context.Background(), keys.ClientKeyRequest{ClientID: id, Algorithm: fapi.ES256}) + if err != nil { + return nil, err + } + var kids []string + for _, k := range set.Keys { + kids = append(kids, k.KeyID) + } + return kids, nil +} + +// ClientKeys keeps each client's keys to that client. +func TestClientKeysContract(t *testing.T) { + forEachDialect(t, func(t *testing.T, dialect sqlstore.Dialect) { + keys.TestClientKeySourceContract(t, func(t *testing.T, clients map[fapi.ClientID][]byte) keys.ClientKeySource { + d := freshDB(t, dialect) + for id, jwks := range clients { + must(t, d.Clients().PutWithKeys(context.Background(), keyedClient(id), sqlstore.ClientKeyRegistration{JWKS: jwks})) + } + source, err := d.ClientKeys(nil) + must(t, err) + return source + }) + }) +} + +// A client registered, rekeyed or removed in the clients table while +// the server runs is served its current keys on the next request: the +// table is the one registry of clients and their keys. +func TestClientKeysFollowTheTable(t *testing.T) { + forEachDialect(t, func(t *testing.T, dialect sqlstore.Dialect) { + ctx := context.Background() + d := freshDB(t, dialect) + source, err := d.ClientKeys(nil) + must(t, err) + repo := d.Clients() + if _, err := resolveKIDs(source, "c"); !errors.Is(err, sqlstore.ErrNotFound) { + t.Fatalf("keys of an unregistered client = %v, want ErrNotFound", err) + } + + must(t, repo.PutWithKeys(ctx, keyedClient("c"), sqlstore.ClientKeyRegistration{JWKS: clientJWKS(t, "k1")})) + wantKIDs(t, source, "c", "k1") + must(t, repo.PutWithKeys(ctx, keyedClient("c"), sqlstore.ClientKeyRegistration{JWKS: clientJWKS(t, "k2")})) + wantKIDs(t, source, "c", "k2") + + must(t, repo.Put(ctx, keyedClient("c"))) // replaces the keys too + wantKIDs(t, source, "c") + + _, err = repo.Delete(ctx, "c") + must(t, err) + if _, err := resolveKIDs(source, "c"); !errors.Is(err, sqlstore.ErrNotFound) { + t.Fatalf("keys of a deleted client = %v, want ErrNotFound", err) + } + }) +} + +// wantKIDs fails t unless source resolves exactly want for client id. +func wantKIDs(t *testing.T, source keys.ClientKeySource, id fapi.ClientID, want ...string) { + t.Helper() + got, err := resolveKIDs(source, id) + if err != nil || strings.Join(got, ",") != strings.Join(want, ",") { + t.Fatalf("keys of %s = %v, %v, want %v", id, got, err, want) + } +} + +// A client registered with a jwks_uri is served the JWK Set published +// there, fetched through ClientKeys' fetcher. +func TestClientKeysFetchJWKSURI(t *testing.T) { + jwks := clientJWKS(t, "published") + ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/jwk-set+json") + _, _ = w.Write(jwks) + })) + defer ts.Close() + fetcher, err := fapihttp.New(ts.Client(), fapihttp.Config{MaxResponseBytes: 1 << 16, RequestTimeout: 5 * time.Second, AllowLoopbackHTTP: true}) + must(t, err) + + forEachDialect(t, func(t *testing.T, dialect sqlstore.Dialect) { + d := freshDB(t, dialect) + must(t, d.Clients().PutWithKeys(context.Background(), keyedClient("c"), sqlstore.ClientKeyRegistration{JWKSURI: mustURL(t, ts.URL+"/jwks")})) + source, err := d.ClientKeys(fetcher) + must(t, err) + wantKIDs(t, source, "c", "published") + + // Without a fetcher, nothing is fetched. + noFetcher, err := d.ClientKeys(nil) + must(t, err) + if _, err := resolveKIDs(noFetcher, "c"); err == nil { + t.Fatal("keys of a jwks_uri client without a fetcher = nil error, want an error") + } + }) +} + +// A key registration that can't serve keys is refused when written, and +// a row an operator wrote with one is an outage, never another client's +// keys or none. +func TestClientKeyRegistrationRefusals(t *testing.T) { + jwks := clientJWKS(t, "k") + uri := mustURL(t, "https://rp.example/jwks") + for name, reg := range map[string]sqlstore.ClientKeyRegistration{ + "both": {JWKSURI: uri, JWKS: jwks}, + "not an object": {JWKS: json.RawMessage(`["keys"]`)}, + "not JSON": {JWKS: json.RawMessage(`{"keys":`)}, + "no usable key": {JWKS: json.RawMessage(`{"keys":[]}`)}, + "a private key": {JWKS: json.RawMessage(`{"keys":[{"kty":"oct","k":"c2VjcmV0"}]}`)}, + "duplicate member": {JWKS: json.RawMessage(strings.Replace(string(jwks), `"keys":`, `"keys":[],"keys":`, 1))}, + } { + _, err := sqlstore.MarshalClientConfigWithKeys(keyedClient("c"), reg) + if !errors.Is(err, sqlstore.ErrClientConfig) { + t.Errorf("MarshalClientConfigWithKeys(%s) = %v, want ErrClientConfig", name, err) + } + } + + forEachDialect(t, func(t *testing.T, dialect sqlstore.Dialect) { + ctx := context.Background() + p := newPlace(t, dialect) + d := p.migrated() + _, db := p.open() + if err := d.Clients().PutWithKeys(ctx, keyedClient("c"), sqlstore.ClientKeyRegistration{JWKSURI: uri, JWKS: jwks}); err == nil { + t.Error("PutWithKeys(jwks_uri and jwks) = nil error, want an error") + } + source, err := d.ClientKeys(nil) + must(t, err) + for id, config := range map[string]string{ + "both": `{"version":1,"redirect_uris":["https://rp.example/cb"],"jwks_uri":"https://rp.example/jwks","jwks":` + string(jwks) + `}`, + "http uri": `{"version":1,"redirect_uris":["https://rp.example/cb"],"jwks_uri":"http://rp.example/jwks"}`, + "jwks array": `{"version":1,"redirect_uris":["https://rp.example/cb"],"jwks":[]}`, + "jwks case": `{"version":1,"redirect_uris":["https://rp.example/cb"],"JWKS":` + string(jwks) + `}`, + "unparseable": `{"version":1,"redirect_uri":"https://rp.example/cb"}`, + } { + rawExec(t, p, db, "INSERT INTO "+p.prefix+"clients (client_id, config) VALUES (?, ?)", id, config) + _, err := resolveKIDs(source, fapi.ClientID(id)) + if !errors.Is(err, storage.ErrStoreUnavailable) || !errors.Is(err, sqlstore.ErrClientConfig) { + t.Errorf("keys of a client row %s = %v, want ErrStoreUnavailable and ErrClientConfig", id, err) + } + if _, err := d.Clients().ResolveClient(ctx, fapi.ClientID(id)); !errors.Is(err, sqlstore.ErrClientConfig) { + t.Errorf("ResolveClient(row %s) = %v, want ErrClientConfig", id, err) + } + } + }) +} + +// A key registration survives MarshalClientConfigWithKeys and +// ParseClientKeys, beside the configuration, which ParseClientConfig +// still reads; a document without one registers no keys. +func TestClientKeyRegistrationRoundTrips(t *testing.T) { + jwks := clientJWKS(t, "k") + for name, reg := range map[string]sqlstore.ClientKeyRegistration{ + "jwks": {JWKS: jwks}, + "jwks_uri": {JWKSURI: mustURL(t, "https://rp.example/jwks")}, + "neither": {}, + } { + doc, err := sqlstore.MarshalClientConfigWithKeys(fullClientConfig(t), reg) + must(t, err) + got, err := sqlstore.ParseClientKeys(doc) + must(t, err) + if got.JWKSURI != reg.JWKSURI || string(got.JWKS) != string(reg.JWKS) || got.IsZero() != (name == "neither") { + t.Errorf("%s: ParseClientKeys(MarshalClientConfigWithKeys) = %+v, want %+v", name, got, reg) + } + cfg, err := sqlstore.ParseClientConfig("client-full", doc) + must(t, err) + if !reflect.DeepEqual(cfg, fullClientConfig(t)) { + t.Errorf("%s: ParseClientConfig = %+v, want the configuration", name, cfg) + } + } +} diff --git a/sqlstore/clientconfig.go b/sqlstore/clientconfig.go index 339fa575..7d64ae28 100644 --- a/sqlstore/clientconfig.go +++ b/sqlstore/clientconfig.go @@ -2,12 +2,14 @@ package sqlstore import ( "bytes" + "context" "encoding/json" "errors" "fmt" "io" fapi "github.com/idfoundry/fapigo" + "github.com/idfoundry/fapigo/keys" "github.com/idfoundry/fapigo/storage" ) @@ -15,7 +17,10 @@ import ( // document MarshalClientConfig writes and ParseClientConfig reads: its // "version" member. A field storage.RegisteredClientConfig gains later // becomes a new optional member of this version; a change an older -// reader would misread gets a new version. +// reader would misread gets a new version. The jwks_uri and jwks +// members (ClientKeyRegistration) are such members, added in sqlstore +// v0.4: a v0.3 reader refuses a document that has them, as it refuses +// any member it doesn't know. const ClientConfigVersion = 1 // ErrClientConfig is what ParseClientConfig returns, wrapped, for a @@ -71,6 +76,58 @@ type clientConfigV1 struct { Display *clientDisplayV1 `json:"display,omitempty"` SubjectType string `json:"subject_type,omitempty"` Sector string `json:"sector,omitempty"` + + // The client's keys (ClientKeyRegistration), which DB.ClientKeys + // serves: not part of storage.RegisteredClientConfig. + JWKSURI string `json:"jwks_uri,omitempty"` + JWKS json.RawMessage `json:"jwks,omitempty"` +} + +// ClientKeyRegistration is where a client's keys are registered, in +// the clients table's configuration document beside its +// storage.RegisteredClientConfig: its jwks_uri, or its inline JWK Set +// (jwks), as in OAuth 2.0 Dynamic Client Registration (RFC 7591 §2), +// which forbids registering both. DB.ClientKeys serves them to the +// server. A client registered with neither has no keys: one that +// authenticates with a TLS client certificate, say, and signs nothing. +type ClientKeyRegistration struct { + // JWKSURI is the https URL the client publishes its JWK Set at, + // fetched (and cached) by DB.ClientKeys' fapihttp.Client. + JWKSURI fapi.URL + + // JWKS is the client's JWK Set document, a JSON object with a + // "keys" array of public keys. + JWKS json.RawMessage +} + +// IsZero reports whether reg registers no keys. +func (reg ClientKeyRegistration) IsZero() bool { + return reg.JWKSURI.IsZero() && len(reg.JWKS) == 0 +} + +// check refuses reg unless it registers at most one of its members, an +// https JWKSURI or a JWKS that's a JSON object. It doesn't parse the +// keys: checkJWKS does. +func (reg ClientKeyRegistration) check() error { + if !reg.JWKSURI.IsZero() && len(reg.JWKS) > 0 { + return errors.New("jwks_uri and jwks are both registered (RFC 7591 §2 allows one)") + } + if !reg.JWKSURI.IsZero() { + if _, err := fapi.ParseEndpointURL(reg.JWKSURI.String()); err != nil { + return fmt.Errorf("jwks_uri: %w", err) + } + } + if len(reg.JWKS) > 0 { + if trimmed := bytes.TrimLeft(reg.JWKS, " \t\r\n"); len(trimmed) == 0 || trimmed[0] != '{' || !json.Valid(reg.JWKS) { + return errors.New("jwks: not a JSON object") + } + // As the document holding it is read: a member twice, or text + // that doesn't decode losslessly, is refused there. + if err := checkStrictJSON(reg.JWKS, map[string]any(nil)); err != nil { + return fmt.Errorf("jwks: %w", err) + } + } + return nil } // clientDisplayV1 is storage.ClientDisplay in clientConfigV1. @@ -115,15 +172,32 @@ func urlString(u fapi.URL) string { } // MarshalClientConfig encodes cfg, less its ID, as the clients table's -// configuration document (version ClientConfigVersion). It refuses a -// configuration with AutomaticFederationRegistration set: that's -// federation.AutomaticClientRepository's to set, never a stored -// client's. +// configuration document (version ClientConfigVersion), registering no +// keys. It refuses a configuration with AutomaticFederationRegistration +// set: that's federation.AutomaticClientRepository's to set, never a +// stored client's. func MarshalClientConfig(cfg storage.RegisteredClientConfig) ([]byte, error) { + return MarshalClientConfigWithKeys(cfg, ClientKeyRegistration{}) +} + +// MarshalClientConfigWithKeys is MarshalClientConfig for a client whose +// keys the document registers too, as its jwks_uri or jwks member, +// for DB.ClientKeys. It refuses a registration with both, a jwks_uri +// that isn't an https URL, and a jwks that isn't a JWK Set with a key +// the server can use. The document keeps jwks compacted. +func MarshalClientConfigWithKeys(cfg storage.RegisteredClientConfig, reg ClientKeyRegistration) ([]byte, error) { if cfg.AutomaticFederationRegistration { return nil, fmt.Errorf("%w: AutomaticFederationRegistration is set only by federation.AutomaticClientRepository", ErrClientConfig) } + if err := reg.check(); err != nil { + return nil, fmt.Errorf("%w: %w", ErrClientConfig, err) + } + if err := checkJWKS(cfg.ID, reg.JWKS); err != nil { + return nil, fmt.Errorf("%w: %w", ErrClientConfig, err) + } doc := clientConfigV1{ + JWKSURI: urlString(reg.JWKSURI), + JWKS: reg.JWKS, Version: ClientConfigVersion, ApplicationType: wire(cfg.ApplicationType), ClientAuthMethod: wire(cfg.ClientAuthMethod), @@ -267,9 +341,68 @@ func ParseClientConfig(id fapi.ClientID, data []byte) (storage.RegisteredClientC if p.err != nil { return storage.RegisteredClientConfig{}, fmt.Errorf("%w: %w", ErrClientConfig, p.err) } + if _, err := docKeys(doc); err != nil { + return storage.RegisteredClientConfig{}, fmt.Errorf("%w: %w", ErrClientConfig, err) + } return cfg, nil } +// ParseClientKeys decodes data, a clients table configuration document, +// as ParseClientConfig does, and returns the keys it registers: its +// jwks_uri or jwks member, or neither. It refuses what ParseClientConfig +// refuses, a document registering both, a jwks_uri that isn't an https +// URL, and a jwks that isn't a JSON object, each with an error wrapping +// ErrClientConfig. It doesn't parse the keys themselves: +// DB.ClientKeys does, as MarshalClientConfigWithKeys and +// ClientRepository.PutWithKeys do before writing them. +func ParseClientKeys(data []byte) (ClientKeyRegistration, error) { + doc, err := decodeClientConfig(data) + if err != nil { + return ClientKeyRegistration{}, fmt.Errorf("%w: %w", ErrClientConfig, err) + } + reg, err := docKeys(doc) + if err != nil { + return ClientKeyRegistration{}, fmt.Errorf("%w: %w", ErrClientConfig, err) + } + return reg, nil +} + +// docKeys is the ClientKeyRegistration in doc, checked. +func docKeys(doc clientConfigV1) (ClientKeyRegistration, error) { + reg := ClientKeyRegistration{JWKS: doc.JWKS} + if doc.JWKSURI != "" { + u, err := fapi.ParseEndpointURL(doc.JWKSURI) + if err != nil { + return ClientKeyRegistration{}, fmt.Errorf("jwks_uri: %w", err) + } + reg.JWKSURI = u + } + if err := reg.check(); err != nil { + return ClientKeyRegistration{}, err + } + return reg, nil +} + +// checkJWKS refuses inline, client id's JWK Set, unless it parses as +// DB.ClientKeys parses it, with at least one key the server can use. +// Empty inline registers no JWK Set, which it accepts. +func checkJWKS(id fapi.ClientID, inline json.RawMessage) error { + if len(inline) == 0 { + return nil + } + lookup := func(context.Context, fapi.ClientID) (fapi.URL, []byte, error) { return fapi.URL{}, inline, nil } + source, err := keys.NewClientJWKSSource(nil, lookup) + if err != nil { + return err + } + // Any algorithm: resolving parses the whole set, and an empty match + // isn't an error. + if _, err := source.ResolveVerificationKeys(context.Background(), keys.ClientKeyRequest{ClientID: id, Algorithm: fapi.ES256}); err != nil { + return fmt.Errorf("jwks: %w", err) + } + return nil +} + // decodeClientConfig strictly decodes data as a version 1 document: one // JSON object, with no unknown, duplicate or case-variant member, that // decodes losslessly (checkStrictJSON). diff --git a/sqlstore/doc.go b/sqlstore/doc.go index edbf907e..107ed88b 100644 --- a/sqlstore/doc.go +++ b/sqlstore/doc.go @@ -359,6 +359,23 @@ // (ErrClientConfig, wrapping storage.ErrStoreUnavailable), not as an // unknown client. // +// The document holds the client's keys too, so the clients table is the +// one registry of clients and their keys: a jwks_uri member (an https +// URL) or an inline jwks member (a JWK Set), never both (RFC 7591 §2), +// which PutWithKeys and MarshalClientConfigWithKeys write +// (ClientKeyRegistration) and DB.ClientKeys serves as +// server.Dependencies.ClientKeys: +// +// clientKeys, err := db.ClientKeys(fetcher) // fapihttp.Client, hardened for production; nil if no client has a jwks_uri +// deps.ClientKeys = clientKeys +// err = db.Clients().PutWithKeys(ctx, cfg, sqlstore.ClientKeyRegistration{JWKS: jwks}) +// +// A client added or rekeyed at runtime authenticates with its new keys +// on its next request, with no restart. Put registers a client with no +// keys, replacing any (for a client that signs nothing, such as one +// authenticating with a TLS client certificate). These members are +// new in sqlstore v0.4: an earlier release refuses a row that has them. +// // # Sweeping // // The stores never delete an expired row themselves: the server refuses diff --git a/sqlstore/production_test.go b/sqlstore/production_test.go index b82416b4..45d2693d 100644 --- a/sqlstore/production_test.go +++ b/sqlstore/production_test.go @@ -5,6 +5,7 @@ import ( "crypto/ecdsa" "crypto/elliptic" "crypto/rand" + "net/http" "strings" "testing" "time" @@ -31,8 +32,9 @@ func (discardAudit) Record(context.Context, server.AuditEvent) error { return ni // package provides taken from d: CIBA on, with d's // BackchannelAuthenticationStore; opaque access tokens in its // AccessTokenStore; and pairwise subjects by default, mapped in its -// PairwiseSubjectStore. What it doesn't provide — signing keys and the -// client key source — are the library's own production-ready ones. +// PairwiseSubjectStore; and clients' keys from d's clients table +// (ClientKeys, with no fetcher). What it doesn't provide — signing +// keys — are the library's own production-ready ones. func productionServer(t *testing.T, d *sqlstore.DB, deployment server.Deployment) (server.Config, server.Dependencies) { t.Helper() url := func(raw string) fapi.URL { return mustURL(t, raw) } @@ -52,7 +54,7 @@ func productionServer(t *testing.T, d *sqlstore.DB, deployment server.Deployment if err != nil { t.Fatal(err) } - clientKeys, err := keys.NewStaticClientKeys(nil) + clientKeys, err := d.ClientKeys(nil) if err != nil { t.Fatal(err) } @@ -127,9 +129,34 @@ func TestProductionServerAcceptsStores(t *testing.T) { } } checkMemstoreRefused(t, d, deployment) + checkClientKeysFetcherHardened(t, d, cfg, deps) }) } +// checkClientKeysFetcherHardened checks that AssuranceProduction's gate +// accepts d's ClientKeys fetching jwks_uris through a hardened +// fapihttp.Client, and refuses one fetching through any other. +func checkClientKeysFetcherHardened(t *testing.T, d *sqlstore.DB, cfg server.Config, deps server.Dependencies) { + t.Helper() + cfg.Deployment = server.DeploymentSingleInstance + hardened, err := fapihttp.NewClient(fapihttp.RecommendedTransportConfig()) + must(t, err) + fetcher, err := fapihttp.New(hardened, fapihttp.RecommendedConfig()) + must(t, err) + deps.ClientKeys, err = d.ClientKeys(fetcher) + must(t, err) + if _, err := server.New(cfg, deps); err != nil { + t.Fatalf("server.New(production, ClientKeys with a hardened fetcher): %v", err) + } + plain, err := fapihttp.New(http.DefaultClient, fapihttp.RecommendedConfig()) + must(t, err) + deps.ClientKeys, err = d.ClientKeys(plain) + must(t, err) + if _, err := server.New(cfg, deps); err == nil || !strings.Contains(err.Error(), "LiveFetchHardened") { + t.Fatalf("server.New(production, ClientKeys with an unhardened fetcher) = %v, want it refused", err) + } +} + // checkMemstoreRefused checks that swapping any one of the stores for // memstore's, which declares nothing, fails AssuranceProduction's gate: // it checks each of them. From d4665ca622346811f1c81647747b7979024a1b0a Mon Sep 17 00:00:00 2001 From: Oscar Sanderson Date: Mon, 2 Nov 2026 14:19:54 +0800 Subject: [PATCH 5/6] feat(sqlstore): list clients, and document CheckClientRegistration ClientRepository.Put checks a client only as storage.NewRegisteredClient does, and the operator tooling writing the clients table often holds no *server.Server, so a client the server can't honour (an unregistered RAR type, AllowedResources without resource indicators, CIBA ping without a notifier) went unnoticed until its first request failed. The package documentation and README now say to run server.Server.CheckClientRegistration on every client at startup and after each Put, and ClientRepository.All lists the clients for that sweep, yielding a bad row's error in its place. Co-Authored-By: Claude Opus 5.5 --- sqlstore/README.md | 6 ++++ sqlstore/client.go | 62 ++++++++++++++++++++++++++++++++++- sqlstore/clientconfig_test.go | 32 ++++++++++++++++++ sqlstore/doc.go | 17 ++++++++++ 4 files changed, 116 insertions(+), 1 deletion(-) diff --git a/sqlstore/README.md b/sqlstore/README.md index 1348e4aa..14261220 100644 --- a/sqlstore/README.md +++ b/sqlstore/README.md @@ -243,6 +243,12 @@ between instances: use SQLite there. deps.ClientKeys, err = db.ClientKeys(fetcher) // a hardened fapihttp.Client; nil if no client has a jwks_uri err = db.Clients().PutWithKeys(ctx, cfg, sqlstore.ClientKeyRegistration{JWKS: jwks}) ``` +- **Check each client against the server**: `Put` checks only what + `storage.NewRegisteredClient` does. Run + `srv.CheckClientRegistration` on every client at startup and after + each `Put`, for what only the server knows (RAR types it doesn't + register, `AllowedResources` without resource indicators, CIBA ping + without a notifier); `Clients().All(ctx)` lists them. ## Tests diff --git a/sqlstore/client.go b/sqlstore/client.go index 9354a685..01b87b3d 100644 --- a/sqlstore/client.go +++ b/sqlstore/client.go @@ -5,6 +5,7 @@ import ( "database/sql" "errors" "fmt" + "iter" fapi "github.com/idfoundry/fapigo" "github.com/idfoundry/fapigo/fapihttp" @@ -71,11 +72,70 @@ func resolveRow(id fapi.ClientID, config []byte) (storage.RegisteredClient, erro return client, nil } +// All returns every registered client, in client_id order, as +// ResolveClient builds each: a row that doesn't make a valid client +// yields its error (wrapping ErrClientConfig) in place of the client, +// and the iteration goes on. A failure to read the table is yielded +// once, last. It reads the rows before yielding any, so the loop may +// use the database. Run server.Server.CheckClientRegistration over it +// at startup, to find a client the server can't honour before its +// first request does: +// +// for client, err := range db.Clients().All(ctx) { +// if err == nil { +// err = srv.CheckClientRegistration(client) +// } +// if err != nil { +// return err +// } +// } +func (r *ClientRepository) All(ctx context.Context) iter.Seq2[storage.RegisteredClient, error] { + return func(yield func(storage.RegisteredClient, error) bool) { + ids, configs, err := r.rows(ctx) + for i, id := range ids { + if !yield(resolveRow(id, configs[i])) { + return + } + } + if err != nil { + yield(storage.RegisteredClient{}, unavailable("list clients", err)) + } + } +} + +// rows reads every row of the clients table, in client_id order, and +// any error that stopped it. +func (r *ClientRepository) rows(ctx context.Context) ([]fapi.ClientID, [][]byte, error) { + rows, err := r.d.db.QueryContext(ctx, r.d.q("SELECT client_id, config FROM {clients} ORDER BY client_id")) // NOSONAR: go:S2077 — a constant query of this package's; q substitutes only the validated TablePrefix. + if err != nil { + return nil, nil, err + } + defer func() { _ = rows.Close() }() + var ( + ids []fapi.ClientID + configs [][]byte + ) + for rows.Next() { + var ( + id string + config []byte + ) + if err := rows.Scan(&id, &config); err != nil { + return ids, configs, err + } + ids = append(ids, fapi.ClientID(id)) + configs = append(configs, config) + } + return ids, configs, rows.Err() +} + // Put registers cfg's client with no keys in the clients table, // replacing any client registered under its ID, keys included: use // PutWithKeys for a client whose keys DB.ClientKeys serves. It refuses // a configuration storage.NewRegisteredClient refuses, or one -// MarshalClientConfig can't write. +// MarshalClientConfig can't write. Run +// server.Server.CheckClientRegistration on the client too, for what +// only the server can check. func (r *ClientRepository) Put(ctx context.Context, cfg storage.RegisteredClientConfig) error { return r.PutWithKeys(ctx, cfg, ClientKeyRegistration{}) } diff --git a/sqlstore/clientconfig_test.go b/sqlstore/clientconfig_test.go index 13626782..638f50a9 100644 --- a/sqlstore/clientconfig_test.go +++ b/sqlstore/clientconfig_test.go @@ -232,3 +232,35 @@ func TestDocumentedClientConfigParses(t *testing.T) { t.Fatal(err) } } + +// All yields every client in client_id order, a bad row's error in its +// place, and stops when the loop does. +func TestClientRepositoryAll(t *testing.T) { + forEachDialect(t, func(t *testing.T, dialect sqlstore.Dialect) { + ctx := context.Background() + p := newPlace(t, dialect) + repo := p.migrated().Clients() + _, db := p.open() + for _, id := range []fapi.ClientID{"b", "a"} { + must(t, repo.Put(ctx, keyedClient(id))) + } + rawExec(t, p, db, "INSERT INTO "+p.prefix+"clients (client_id, config) VALUES (?, ?)", "c", `{"version":1,"redirect_uri":"https://rp.example/cb"}`) + var got []string + for client, err := range repo.All(ctx) { + switch { + case err == nil: + got = append(got, string(client.ID())) + case errors.Is(err, sqlstore.ErrClientConfig) && strings.Contains(err.Error(), `"c"`): + got = append(got, "error") + default: + t.Fatalf("All: %v", err) + } + } + if strings.Join(got, ",") != "a,b,error" { + t.Errorf("All = %v, want a, b and c's error", got) + } + for range repo.All(ctx) { + break // stopping early doesn't panic + } + }) +} diff --git a/sqlstore/doc.go b/sqlstore/doc.go index 107ed88b..b7011ace 100644 --- a/sqlstore/doc.go +++ b/sqlstore/doc.go @@ -376,6 +376,23 @@ // authenticating with a TLS client certificate). These members are // new in sqlstore v0.4: an earlier release refuses a row that has them. // +// Put checks a client only as storage.NewRegisteredClient does. What +// only the server can check — Rich Authorization Request types its +// Config.RAR doesn't register, AllowedResources while it accepts no +// resource indicators, CIBA ping delivery without a notifier — is +// server.Server.CheckClientRegistration's: run it on each client after +// Put, and on every client at startup, since the operator's tooling +// writing the table may hold no *server.Server. All lists them: +// +// for client, err := range db.Clients().All(ctx) { +// if err == nil { +// err = srv.CheckClientRegistration(client) +// } +// if err != nil { +// return err +// } +// } +// // # Sweeping // // The stores never delete an expired row themselves: the server refuses From 1674aeac1ccc791afc1014619582bc856f2d7e93 Mon Sep 17 00:00:00 2001 From: Oscar Sanderson Date: Mon, 2 Nov 2026 14:20:16 +0800 Subject: [PATCH 6/6] docs: point sqlstore deployments at ResourceServer, ClientKeys and CheckClientRegistration GETTING_STARTED's durable-storage section now says where a sqlstore server's ClientKeys come from (db.ClientKeys) and that a standalone resource server takes its stores from sqlstore.ResourceServer, so its Revocation can't come from its own prefix. The production checklist's sqlstore section gains running CheckClientRegistration on every client at startup and after each Put. Co-Authored-By: Claude Opus 5.5 --- GETTING_STARTED.md | 7 +++++-- docs/PRODUCTION_CHECKLIST.md | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/GETTING_STARTED.md b/GETTING_STARTED.md index 8d7c8b58..fdb49799 100644 --- a/GETTING_STARTED.md +++ b/GETTING_STARTED.md @@ -596,7 +596,7 @@ deps := server.Dependencies{ AccessTokens: accessTokens, Backchannel: db.Backchannel(), // only for CIBA PairwiseSubjects: mapping, - ClientKeys: clientKeys, + ClientKeys: clientKeys, // db.ClientKeys(fetcher): each client's jwks or jwks_uri in its clients row Keys: keyManager, // keys.NewKeyManagerFromSigners, with keys.DeclareCustody Audit: auditSink, ClientCertificateTrust: server.NoPKIClientAuthentication{}, @@ -663,7 +663,10 @@ What to decide when you deploy it: `TablePrefix` and `Sealer` — `serverresource.NewVerifier` does that for you from the server's `Dependencies`. Its DPoP nonce store must not be the server's (RFC 9449 keeps the two roles' nonces apart): - give it `Nonces()` from a DB with a prefix of its own. Give it a + give it `Nonces()` from a DB with a prefix of its own. A resource + server in a process of its own takes all of these from + `sqlstore.ResourceServer(serverDB, rsDB)`, so its `Revocation` can't + come from its own prefix, where the server never writes. Give it a database role of its own, too: `SELECT` on the server's `access_tokens`, `revocations` and `schema_migrations` tables, `SELECT` and `INSERT` on `replay`, and its own prefix's tables — not diff --git a/docs/PRODUCTION_CHECKLIST.md b/docs/PRODUCTION_CHECKLIST.md index e166d4fb..25fe058a 100644 --- a/docs/PRODUCTION_CHECKLIST.md +++ b/docs/PRODUCTION_CHECKLIST.md @@ -171,6 +171,8 @@ For a deployment keeping its stores in the `RotatingSealer`, which opens with the old and new keys until the old one's records expire, well before a key's 2^32 seals (weeks, on a busy server). +- [ ] *(yours)* **`CheckClientRegistration` on every client** at + startup (`Clients().All`) and after each `Clients().Put`. - [ ] **`ReadUnsealed` only while migrating** an existing database to sealing: deploy with it, run `SealRecords`, then deploy without it. - [ ] *(yours)* **Write access to the database as tight as read