F6: a relay that answers and will not prove consistency is a fork #177

2026-09-23T15:56:05Z

paulhitt commented

2026-09-23 15:56:05 +00:00

(Migrated from gitlab.com)

Three commits from the claim-register audit.

F6: a relay that answers and will not prove consistency is a fork

SPEC §19.4a, on gossip: "A refusal and a bad proof are treated identically." The reference engine throws KeyLogForkError. iOS sets forkDetected. This client caught every exception and returned, commented "A failed fetch is NOT a fork", so a hostile operator could equivocate and then answer 503 on the consistency route, and Android alone reported nothing. The check was optional from the attacker's side.

My audit register recorded this as a disagreement between two defensible positions and said §19.4 did not settle it. That was wrong, and the correction is why this is a fix rather than a question: 19.4a settles it in one sentence, and this was the one implementation of three that disagreed with its own specification. I talked myself out of fixing it by writing that the spec was silent without going back to read it.

What makes failing closed safe here is the other catch: no answer at all is not evidence, an answer that is not a proof is. The scheduled sweep keeps the more forgiving behaviour deliberately, because it asks "does this relay still keep a log" rather than "prove these two heads are one history".

Three tests had to change and all three had the same shape: a name and a comment describing an unreachable relay, over a fixture feeding a 503. A 503 is a relay answering. Treating an answer we did not like as no answer is the hole itself, and it hid behind the wording because the tests read as though they covered the network being down. They now cover both, separately.

Caught by a test, and worth recording: the first version of this fix caught RelayException.Transport. RelayClient never throws it — only RelayAccounts and MessengerApi construct that type — so a dead network arrives here as OkHttp's raw IOException and would have fallen straight through to the fork branch, turning every flaky moment into a security alarm. That is the exact cost the finding warned about, introduced by the change meant to close it. The shut-down-server test failed and said so.

Also: the filtered run was green. The full suite found a fourth test in another file the filter had never loaded.

The call id a handoff re-offers under

A call moved between devices has to leave the other person one entry in their log. The peer files by call id, so the whole promise is that the device taking the call re-offers under the original id. Android does this correctly and nothing checked it.

The iOS side gained the same guard, and breaking the line there deliberately showed why it is worth having: all eleven existing handoff tests still passed, because they encode a signal with a fixed id and check it survives the round trip. Kotlin's handoff tests have the same shape. The mistake is invisible from the side that makes it — the device handing over sees a correct log, the caller keeps talking, and only the peer gets two rows.

Four checks, in the shape PollSharesGoToOneMemberTest already uses here, including one that placing an ordinary call does mint an id, so "takeHandoff mints nothing" is a difference between two paths rather than a fact about a file that never mints anything.

Removed from this MR: the poll tally vectors

They were here and are not any more. CI clones flamenet-e2e at the commit .e2e-ref pins and exports a path per vector file, so a fixture that only exists on an unmerged branch cannot be found: the job failed with three AssertionError at PollTallyVectorTest.kt:40, which is "vectors not found".

That is not a defect in the test, it is this repo's documented cross-repo process working. It passed locally because a developer machine has a sibling flamenet-e2e checkout and the runner does not, which is exactly the gap the pin-and-clone exists to close.

So they move to the follow-up that bumps .e2e-ref and adds FLAMENET_POLL_VECTORS to the export list and its existence check, once flamenet-e2e!58 has merged. Splitting them lets F6 land on its own rather than waiting on another repository's review.

client 475, engine 722, app 652, no failures. Result files were checked for freshness rather than trusting "BUILD SUCCESSFUL", since gradle reporting up-to-date while running nothing is a trap this project has hit.

🤖 Generated with Claude Code

Three commits from the claim-register audit. ## F6: a relay that answers and will not prove consistency is a fork **SPEC §19.4a, on gossip: "A refusal and a bad proof are treated identically."** The reference engine throws `KeyLogForkError`. iOS sets `forkDetected`. This client caught every exception and returned, commented "A failed fetch is NOT a fork", so a hostile operator could equivocate and then answer 503 on the consistency route, and Android alone reported nothing. The check was optional from the attacker's side. **My audit register recorded this as a disagreement between two defensible positions and said §19.4 did not settle it. That was wrong**, and the correction is why this is a fix rather than a question: 19.4a settles it in one sentence, and this was the one implementation of three that disagreed with its own specification. I talked myself out of fixing it by writing that the spec was silent without going back to read it. What makes failing closed safe here is the other catch: **no answer at all is not evidence, an answer that is not a proof is.** The scheduled sweep keeps the more forgiving behaviour deliberately, because it asks "does this relay still keep a log" rather than "prove these two heads are one history". Three tests had to change and all three had the same shape: a name and a comment describing an *unreachable relay*, over a fixture feeding a **503**. A 503 is a relay answering. Treating an answer we did not like as no answer is the hole itself, and it hid behind the wording because the tests read as though they covered the network being down. They now cover both, separately. > Caught by a test, and worth recording: the first version of this fix caught `RelayException.Transport`. `RelayClient` never throws it — only `RelayAccounts` and `MessengerApi` construct that type — so a dead network arrives here as OkHttp's raw `IOException` and would have fallen straight through to the fork branch, turning every flaky moment into a security alarm. That is the exact cost the finding warned about, introduced by the change meant to close it. The shut-down-server test failed and said so. > > Also: the filtered run was green. The **full** suite found a fourth test in another file the filter had never loaded. ## The call id a handoff re-offers under A call moved between devices has to leave the other person one entry in their log. The peer files by call id, so the whole promise is that the device taking the call re-offers under the *original* id. Android does this correctly and nothing checked it. The iOS side gained the same guard, and breaking the line there deliberately showed why it is worth having: all eleven existing handoff tests still passed, because they encode a signal with a fixed id and check it survives the round trip. Kotlin's handoff tests have the same shape. The mistake is invisible from the side that makes it — the device handing over sees a correct log, the caller keeps talking, and only the peer gets two rows. Four checks, in the shape `PollSharesGoToOneMemberTest` already uses here, including one that placing an ordinary call **does** mint an id, so "takeHandoff mints nothing" is a difference between two paths rather than a fact about a file that never mints anything. ## Removed from this MR: the poll tally vectors They were here and are not any more. CI clones flamenet-e2e **at the commit `.e2e-ref` pins** and exports a path per vector file, so a fixture that only exists on an unmerged branch cannot be found: the job failed with three `AssertionError at PollTallyVectorTest.kt:40`, which is "vectors not found". That is not a defect in the test, it is this repo's documented cross-repo process working. It passed locally because a developer machine has a sibling `flamenet-e2e` checkout and the runner does not, which is exactly the gap the pin-and-clone exists to close. So they move to the follow-up that bumps `.e2e-ref` and adds `FLAMENET_POLL_VECTORS` to the export list and its existence check, once flamenet-e2e!58 has merged. Splitting them lets F6 land on its own rather than waiting on another repository's review. --- **client 475, engine 722, app 652, no failures.** Result files were checked for freshness rather than trusting "BUILD SUCCESSFUL", since gradle reporting up-to-date while running nothing is a trap this project has hit. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

paulhitt commented

2026-09-23 16:10:42 +00:00

(Migrated from gitlab.com)

changed the description

paulhitt commented

2026-09-24 18:02:57 +00:00

(Migrated from gitlab.com)

mentioned in commit c0327c8fc0

mentioned in commit c0327c8fc0c07afbd6c32d8c0115cd12cc071e60

paulhitt (Migrated from gitlab.com) merged commit c0327c8fc0 into main

2026-09-24 18:02:57 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: extant2000/flamenet-messenger-android#177