Sealed-sender leak paths, the contact graph that outlived its removal, and two timing fixes #222

Merged
paulhitt merged 5 commits from audit/sealed-sender-leak-paths into main 2026-09-24 18:02:51 +00:00
paulhitt commented 2026-09-23 15:55:14 +00:00 (Migrated from gitlab.com)

Four commits from the claim-register audit. Every new test was made to fail against the code before the fix; where that revealed the fix was wrong, the commit says so.

Two sealed-sender leak paths that only a comment guarded

AUDIT-SCOPE §4 names three paths by which a sealed submission might still leak its sender: rate-limit state, attachment rows, error timing. Nothing tested the first two.

  • Rate-limit state. A spy limiter records every key the route asks about, and the sealed send is driven with the sender's bearer credentials attached, which is what a client reusing one HTTP session actually sends. The metered set must be exactly recipient and address. The identified send is the control: it does meter send:from:.
  • Attachment rows. attachments opens with "Deliberately no uploader column", and /metadata calls that removal the load-bearing one. A comment is not a guard. The test reads the migrated schema, with a control for PRAGMA table_info returning no rows on a missing table rather than failing.

The first draft of the rate-limit test failed, and wrongly: it asserted no metered key contains the sender's account id, the sender was account 1, and the test address is 203.0.113.9. A substring match nearly produced a finding against working code.

F7: the contact graph survived the removal meant to end it

This is the one that mattered. /metadata makes two promises that met in one table and were both false.

"The relay keeps no record of adding a friend": conversation_state held accepted_at, a Date() written on every add. The migration that dropped buddies.added_at, blocks.created_at and group_members.created_at in one sweep missed this column because the table is named for conversation state.

"Take my contact list off the server": forgetRoster deleted buddies rows and nothing else, then answered "Forgotten." while a row naming both accounts stayed behind for every contact.

And markAccepted sat outside the rosterOnServer switch, so a relay that had deliberately stopped keeping a contact graph kept one anyway under another table's name.

accepted_at was never read as a date — every use was a nil check — so it becomes the boolean it already was. forgetRoster now clears every row naming the account in both directions. One deliberate limit: a row carrying a read position is kept with its acceptance cleared, because deleting it would lose state nobody asked to lose. /metadata now says so.

Also adds PostgresMigrationTests, the first test in this repo that migrates on the dialect production runs. Verified on Postgres 16 and 17, since docker-compose.prod.yml pins 17.

Its first version opened with DROP SCHEMA public CASCADE against whatever URL it was given, and the port it was aimed at turned out to be a live Supabase database on this machine. It survived only because the password happened not to match. It now creates its own database and drops only that.

F8 and F10

F8, and the first fix was wrong. SPEC §14.5 requires the sealed rejection to use "the same generic error and timing". Measured: 151µs for an account with no delivery key against 182µs for one with a key and a wrong digest, a 30.7µs separation at 27× the noise floor, distributions barely overlapping. The route needs no credentials, so anyone can ask.

The obvious repair — make both digest paths decode, hash and compare unconditionally — moved it to 27.1µs. Almost nothing, because the cost was never the hash. It is the lookup. Had it shipped on the strength of the reasoning, this would say "fixed" about an oracle that was still there.

The refusal is now held to a 5ms floor. Re-measured: separation −1.3µs against a noise floor of 26.5µs, so the difference between the arms is twenty times smaller than the variance inside one arm. Only the refusal pays it.

F10. AddEmailVerification grandfathers existing accounts with UPDATE users SET email_verified_at = NOW(). SQLite has no NOW(), and the try? swallows the parse error, so on SQLite it reported success and grandfathered nobody. Harmless until an operator sets FN_EMAIL_VERIFICATION_REQUIRED, at which point every pre-existing account is refused a token and told to check an email that was never sent. CURRENT_TIMESTAMP works on both.

This is the second known trap on that switch; the Android one is already documented in 162ae32.

The metadata page stops contradicting itself

"This one is urgent" was described as "Never written down." Every ring_events row is written inside if urgent, so a row existing for a device is the flag, kept 7 days. The page already disclosed ring attempts seven rows further down; only one of the two sentences was true.

"Take my contact list off the server" gains its limit. "Per-conversation state" now admits a row names both accounts.

The roadmap says what the audit could not close

Nothing countersigns the key log, and your address depends on one server staying up. Neither is fixable in an afternoon and both were being carried in a scratchpad; a gap somebody could find in the source belongs on the page that promises to list gaps.


749 tests in 76 suites, no failures.

Note: this branch does not bump .e2e-ref. flamenet-e2e!58 has to merge first, then one small MR here bumps the pin and regenerates /spec and /audit, per .e2e-ref.md.

🤖 Generated with Claude Code

Four commits from the claim-register audit. Every new test was made to fail against the code before the fix; where that revealed the fix was wrong, the commit says so. ## Two sealed-sender leak paths that only a comment guarded AUDIT-SCOPE §4 names three paths by which a sealed submission might still leak its sender: rate-limit state, attachment rows, error timing. Nothing tested the first two. - **Rate-limit state.** A spy limiter records every key the route asks about, and the sealed send is driven *with* the sender's bearer credentials attached, which is what a client reusing one HTTP session actually sends. The metered set must be exactly recipient and address. The identified send is the control: it does meter `send:from:`. - **Attachment rows.** `attachments` opens with "Deliberately no uploader column", and `/metadata` calls that removal the load-bearing one. A comment is not a guard. The test reads the migrated schema, with a control for `PRAGMA table_info` returning no rows on a missing table rather than failing. The first draft of the rate-limit test failed, and wrongly: it asserted no metered key contains the sender's account id, the sender was account 1, and the test address is 203.0.113.9. A substring match nearly produced a finding against working code. ## F7: the contact graph survived the removal meant to end it **This is the one that mattered.** `/metadata` makes two promises that met in one table and were both false. "The relay keeps no record of adding a friend": `conversation_state` held `accepted_at`, a `Date()` written on every add. The migration that dropped `buddies.added_at`, `blocks.created_at` and `group_members.created_at` in one sweep missed this column because the table is named for conversation state. "Take my contact list off the server": `forgetRoster` deleted `buddies` rows and nothing else, then answered "Forgotten." while a row naming both accounts stayed behind for every contact. And `markAccepted` sat *outside* the `rosterOnServer` switch, so a relay that had deliberately stopped keeping a contact graph kept one anyway under another table's name. `accepted_at` was never read as a date — every use was a nil check — so it becomes the boolean it already was. `forgetRoster` now clears every row naming the account in both directions. **One deliberate limit:** a row carrying a read position is kept with its acceptance cleared, because deleting it would lose state nobody asked to lose. `/metadata` now says so. Also adds `PostgresMigrationTests`, the first test in this repo that migrates on the dialect production runs. Verified on Postgres 16 **and 17**, since `docker-compose.prod.yml` pins 17. > Its first version opened with `DROP SCHEMA public CASCADE` against whatever URL it was given, and the port it was aimed at turned out to be a live Supabase database on this machine. It survived only because the password happened not to match. It now creates its own database and drops only that. ## F8 and F10 **F8, and the first fix was wrong.** SPEC §14.5 requires the sealed rejection to use "the same generic error *and timing*". Measured: 151µs for an account with no delivery key against 182µs for one with a key and a wrong digest, a 30.7µs separation at 27× the noise floor, distributions barely overlapping. The route needs no credentials, so anyone can ask. The obvious repair — make both digest paths decode, hash and compare unconditionally — moved it to 27.1µs. Almost nothing, because the cost was never the hash. It is the lookup. Had it shipped on the strength of the reasoning, this would say "fixed" about an oracle that was still there. The refusal is now held to a 5ms floor. Re-measured: separation −1.3µs against a noise floor of 26.5µs, so the difference between the arms is twenty times *smaller* than the variance inside one arm. Only the refusal pays it. **F10.** `AddEmailVerification` grandfathers existing accounts with `UPDATE users SET email_verified_at = NOW()`. SQLite has no `NOW()`, and the `try?` swallows the parse error, so on SQLite it reported success and grandfathered nobody. Harmless until an operator sets `FN_EMAIL_VERIFICATION_REQUIRED`, at which point every pre-existing account is refused a token and told to check an email that was never sent. `CURRENT_TIMESTAMP` works on both. This is the second known trap on that switch; the Android one is already documented in `162ae32`. ## The metadata page stops contradicting itself "This one is urgent" was described as "Never written down." Every `ring_events` row is written inside `if urgent`, so a row existing for a device **is** the flag, kept 7 days. The page already disclosed ring attempts seven rows further down; only one of the two sentences was true. "Take my contact list off the server" gains its limit. "Per-conversation state" now admits a row names both accounts. ## The roadmap says what the audit could not close Nothing countersigns the key log, and your address depends on one server staying up. Neither is fixable in an afternoon and both were being carried in a scratchpad; a gap somebody could find in the source belongs on the page that promises to list gaps. --- **749 tests in 76 suites, no failures.** Note: this branch does **not** bump `.e2e-ref`. flamenet-e2e!58 has to merge first, then one small MR here bumps the pin and regenerates `/spec` and `/audit`, per `.e2e-ref.md`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
paulhitt commented 2026-09-24 18:02:50 +00:00 (Migrated from gitlab.com)

mentioned in commit e88fbfb97a

mentioned in commit e88fbfb97a4413ecb4455a2b0d0ab09f28353f0c
paulhitt (Migrated from gitlab.com) merged commit e88fbfb97a into main 2026-09-24 18:02:51 +00:00
Sign in to join this conversation.