fix(sqlstore)!: harden sealing keys, SQLite mode, resource server and client key wiring

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

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

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
@@ -77,6 +83,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 +117,60 @@ 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.
+- **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
 
 | | Durable | AtomicConsume | CrossInstanceConsistent | SerializableRedemption | EncryptedAtRest |
@@ -142,15 +206,49 @@ 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
   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})
+  ```
+- **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
 

sqlstore/client.go

@@ -5,8 +5,11 @@ import (
 	"database/sql"
 	"errors"
 	"fmt"
+	"iter"
 
 	fapi "github.com/idfoundry/fapigo"
+	"github.com/idfoundry/fapigo/fapihttp"
+	"github.com/idfoundry/fapigo/keys"
 	"github.com/idfoundry/fapigo/storage"
 )
 
@@ -31,17 +34,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 +72,86 @@ 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.
+// 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. 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{})
+}
+
+// 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 +190,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
+}