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 diff --git a/sqlstore/README.md b/sqlstore/README.md index 8492d280..14261220 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 @@ -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 diff --git a/sqlstore/client.go b/sqlstore/client.go index c018e1d1..01b87b3d 100644 --- a/sqlstore/client.go +++ b/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 +} 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/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/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 55e03c13..b7011ace 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 @@ -83,6 +103,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 @@ -168,6 +194,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 @@ -328,6 +359,40 @@ // (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. +// +// 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 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) 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. 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) + } + } +} 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) {