diff --git a/04-ISSUES/337-the-stores-reader-can-read-the-controllers-signing-key/00-report.md b/04-ISSUES/337-the-stores-reader-can-read-the-controllers-signing-key/00-report.md new file mode 100644 index 00000000..9d32b362 --- /dev/null +++ b/04-ISSUES/337-the-stores-reader-can-read-the-controllers-signing-key/00-report.md @@ -0,0 +1,74 @@ +--- +status: located +opened: 2026-10-08 +located-in: [mesh-catalog modules/postgres (cmd/postgres-provider/client.go, the reader's grant)] +fixed-by: +amended-design: +replay-none: The hole is a grant on a store, not a behaviour of the core a lab replay can lay over a commit. The module's own tests hold it against a real throwaway server, and the one that uses only what the module had before the fix reads a fixture signing key on the commit before it and is refused on the fix. +--- + +# 337. The store's reader can read the controller's signing key + +**Severity: critical.** It touches the core's trust: the key every node-engine believes. + +## Symptom + +On 2026-10-08 the store seat's `query` verb (`mesh-store.query`), called through the mesh MCP server, +answered as the login `mesh_store_reader`. Asked about privileges only, never about a value, the +`identity` database answered: + +- the login is a member of `pg_read_all_data`; +- it may `SELECT` the column `private` of `signing_key`, the controller's Ed25519 private key, which signs + every declaration a node-engine verifies and then applies as root (the identity context's migration + `0001-signing-key.sql`); +- it may `SELECT` the column `private` of `authority`, the table meant for the mesh's certificate + authority (it holds no row today). + +The same login may read, in `inventory`, the sealed values and operator keys of `secret` and +`module_secret`, `operator_key`, `bus_membership.sealed`, `bus_user.password_hash` and +`enrolment_token.secret`; in `licences`, the sealed tokens of `licence_holder` and `refresh_grant`; and in +every consumer's database, whatever it holds. In the identity provider's database it may read 20 columns +whose names say they hold a secret, across 8 tables; in the forge's, 29 across 15. Those counts come from +the catalogue (`has_column_privilege` over column names), not from reading a row. + +## Why it is an issue + +[ADR 0159](../../02-DECISIONS/0159-a-tool-call-names-the-machine-and-a-holder-serves-its-seats-verbs.md) +gave the store seat a `query` verb: one read-only statement against one database. +[Issue 193](../193-the-store-seats-read-only-query-is-read-only-by-convention/00-report.md) made it read-only +by its login instead of by text around the statement, and chose `pg_read_all_data` for that login. The +role kept the verb from writing and let it read **everything**, keys included. Nothing in the design said +what the verb may read, so nothing refused it. + +[ADR 0004](../../02-DECISIONS/0004-a-node-and-how-it-joins.md) rests a node's trust in the mesh on that key: +"each declaration is verified by its signature, every time", so that a compromised broker cannot forge +one. Whoever holds the private half can sign a declaration every node-engine accepts. The key was one +`select` away from every caller allowed to call `mesh-store.query`, and that includes every agent +session on a machine running the mesh MCP server. + +## Impact + +- **Anyone who could call the verb could take the key.** Forging a declaration also needs a way to put it + on a node's subject on the bus, so the key alone is not a node; but it removes the one check + ADR 0004 put between the bus and root on every node. +- **Every consumer's secrets were readable the same way**: the forge's, the identity provider's, the + cloud's, the mail server's. A module's database was meant to be reachable by its own login alone + ([ADR 0048](../../02-DECISIONS/0048-a-provider-creates-the-credential-the-mesh-minted.md)). +- **Whether anyone did read the key is not known.** The diagnosis says what the store can tell and + what it cannot. + +## How it is checked + +- **In the module's tests, against a real throwaway server** (`MESH_POSTGRES_LIVE`): a reader set up as an + install before the fix left it (`pg_read_all_data` and a table granted by hand) asks for a fixture + `signing_key.private` through `ReadOnlyQuery`. On the commit before the fix it reads it. On the fix it + is refused, and the reader is no longer a member of `pg_read_all_data`. +- **The probe:** across every database the server holds, no column whose name says it holds a secret + (private, secret, seal, token, password, credential, salt, a key) may be read by the reader after a + reconcile. The same probe runs in the reader's own session before every statement, and a statement is + refused while it finds one. +- **Live, after the fix is taken:** `has_column_privilege('mesh_store_reader', 'signing_key', 'private', + 'SELECT')` in `identity` answers false, `pg_has_role('mesh_store_reader', 'pg_read_all_data', 'member')` + answers false, and `postgres_reader_grants` lists no database with an exposed column. + +The diagnosis is in [01-diagnosis.md](01-diagnosis.md). diff --git a/04-ISSUES/337-the-stores-reader-can-read-the-controllers-signing-key/01-diagnosis.md b/04-ISSUES/337-the-stores-reader-can-read-the-controllers-signing-key/01-diagnosis.md new file mode 100644 index 00000000..51330062 --- /dev/null +++ b/04-ISSUES/337-the-stores-reader-can-read-the-controllers-signing-key/01-diagnosis.md @@ -0,0 +1,128 @@ +# 337 — Diagnosis + +## 2026-10-08 + +**The records first.** `records_search` for `mesh_store_reader` found nothing. For `pg_read_all_data` it +found [issue 193](../193-the-store-seats-read-only-query-is-read-only-by-convention/00-report.md), whose fix +chose the role: "Run the verb as a role that can only read, granted `pg_read_all_data`". Issue 193 was +about writing. Nobody asked what the role lets the verb read. A search for "signing key" found +[ADR 0004](../../02-DECISIONS/0004-a-node-and-how-it-joins.md) and nothing on rotating it. + +**Where the grant is.** The `postgres` module's provider (mesh-catalog `modules/postgres`, the holder of +`mesh-store`) makes the reader in `ReaderStatements`, `cmd/postgres-provider/client.go`: +`GRANT pg_read_all_data TO mesh_store_reader`. It ran once per process, on the first query. Both +`postgres_query` and the seat's `query` verb call `ReadOnlyQuery`, which connects as the reader to +whichever database the caller names. The role gives `SELECT` on every table, view and sequence of every +database, and `USAGE` on every schema. So every column the store holds was one statement away. + +**Confirmed live, by privileges only.** Through `mesh-store.query` against `identity`: +`current_user` is `mesh_store_reader`, `pg_has_role('pg_read_all_data', 'member')` is true, and +`has_column_privilege` is true for `signing_key.private` and `authority.private`. `authority` holds no row +(`count(*)` is 0), so the mesh's certificate authority key was not exposed through this table. The +signing key table holds one active key. Against `inventory`, `licences` and two consumers' databases the +same check was run over column names, with the results the report gives. No statement selected a key or +a secret value. + +**Ruled out.** + +- *The verb is limited to some databases.* It is not. `ReadOnlyQuery` takes any database name, and the + role covers every database. +- *The seat's `invokes` grant limits who can call it.* It limits who can call it, not what the call can + read. The mesh MCP server's account on a machine may call it, and every agent session on that machine + goes through that account. +- *The controller's databases are protected by their own logins* ([ADR 0008](../../02-DECISIONS/0008-a-context-owns-its-store.md) + as the identity context states it: "reached with a credential no other context holds"). They are, for + logging in as their owners. A predefined role that reads all data goes around ownership. + +### Did anyone read the key? + +What the store can say, read without printing any value: + +- **No statement log.** `log_statement` is `none`, `log_min_duration_statement` is `-1`, and + `log_connections` is `off`. `pg_stat_statements` is not installed in `identity`. +- **The table's scan counts.** Since the server last started (2026-10-03, 22:53 UTC), `signing_key` was + read by sequential scan 7 times, the last at 01:02:40 UTC on 2026-10-08. `node_key` and `authority` + were scanned 7 times each, with exactly the same last time. I think those 7 are the nightly dump, which + reads every table of the database at once, because three tables with the same count and the same + second fit that and nothing else. The controller reads the key by index (1,725 index scans), so its own + reads are not among the 7. +- **What the counts cannot rule out.** A statement that reads the key with a condition on `id` would be an + index scan, indistinguishable from the controller's. Anything before the 2026-10-03 start is not in + these counts. Before issue 193's fix (2026-10-02) the verb ran as the server's administrator, which + could read the key as well. And the tool runner keeps no record of a call's arguments that this + diagnosis could search. + +**So whether the key was read is not known, and cannot be known from the store.** There is no sign of a +read beyond the nightly dump since 2026-10-03. Before that there is no record at all. + +### Should the signing key be rotated? + +**Yes, after the fix is live, and through a mechanism that does not exist yet.** + +- *Why rotate.* The key was readable by every caller of the verb since the verb existed. Its exposure + cannot be excluded, and a forged declaration is applied as root on every node that receives it. +- *Why after the fix.* A new key made before the fix is taken would be exactly as readable as the old one. +- *How a rotation is done in this mesh today.* It is not. The controller's `rotate` verb replaces a pair + credential or a module's own secret, not the signing key. The identity context keeps a `retired` + timestamp on `signing_key` and signs with whichever key is not retired, read per signature, so the + controller side could change keys with one retirement and one new key. But a node-engine holds one + signing key, the one its enrolment token carried, and refuses a declaration signed by any other. So + changing the key today means re-enrolling every node with a new token. ADR 0004 names the missing + piece: "Rotating the signing identity is a fleet-wide operation with an overlapping rollover". That + is a node-engine that accepts a successor key announced in a declaration signed by the current one, + and a controller that retires the old key only once every node has acknowledged the new one. That + needs a decision before anyone builds it. +- **Nothing was rotated in this diagnosis.** + +### The fix + +mesh-catalog pull request 143, in the `postgres` module: + +1. **The reader holds no role.** `pg_read_all_data` is no longer granted. Every role the reader is a + member of is revoked on the first query and on every reconcile pass, so an existing install loses it + without anybody acting. +2. **Deny by default, per column.** A consumer declares what the reader may read in its contribution to + `postgres-database`, under `readable`: `[schema.]table` or `[schema.]table(column, …)`. A database no + contribution names, the controller's own among them, gives the reader nothing. +3. **A secret is never granted.** A column or table whose name says it holds a secret is refused even + when declared, and the refusal is reported. +4. **Reconciled every minute, on every database.** Grants the reader holds that are not declared are + revoked, and declared ones that are missing are granted. `postgres_reader_grants` answers the last + pass, by name only. +5. **Fails closed where it is used.** Before a caller's statement runs, the reader's own session lists + the secret-named columns it can read. If there are any, for example from a grant to `PUBLIC` or one + made by hand, the statement is refused and the columns are named. A statement naming one of the + controller's known secret tables, or a name that reads as a key or a secret, is refused before it is + sent. That check is a second layer: a name built from pieces inside a function call gets past it, and + the privileges still refuse the read. + +**Why the declaration is in the contribution, and the controller learns nothing.** Contribution values +are passed through to the provider as written. The controller's module check does not read them, and the +existing `extensions` value already works this way. `readable` is a list, not an object, because the +controller tells a contribution's single shape from its several-instance shape by whether every value is +an object. The lenient read of an unknown manifest field +([ADR 0262](../../02-DECISIONS/0262-a-preference-has-a-default-in-its-manifest-and-the-operators-own-value-has-none.md)) +is not involved either: the key sits inside `contributes`, whose values are read as they are. A +catalogue test parses every manifest's `readable`, so a malformed one is found at review. + +**What it costs.** Until modules declare `readable`, the verb reads nothing in any database, and none +does yet. Diagnosis that read the controller's `inventory` through the verb loses that. The controller is +not a consumer of the provision, so it has no contribution in which to declare what of its own is +readable. Giving it one is a follow-up, and so is each module's own declaration. + +### Still exposed until the fix is taken + +The module's upgrade policy is `record`: a person takes each build after a backup. Until that build is +taken on the control node, the reader keeps `pg_read_all_data`, and the signing key and every secret +above remain one statement away. Merging the pull request does not close the hole. + +### Follow-ups + +- **A decision on rotating the signing key**, with the overlapping rollover ADR 0004 asks for. Then the + rotation itself. +- **What the controller lets the reader read** of `inventory`, `identity` and `licences`. None of + `identity`'s tables should be readable. +- **The consumers' declarations**, module by module, each reviewed for columns whose names hide a secret. + The name check catches what is named for what it is, and a view can rename a column. +- **The lesson for the knowledge base**: a role that makes a login read-only is not a role that makes + it harmless. `pg_read_all_data` is a superuser's reach without the writes.