Pin the relay's identity, not just its TLS (ios#19, Android half) #67

Merged
paulhitt merged 2 commits from feat/pin-the-relay-identity into main 2026-09-04 20:49:24 +00:00
paulhitt commented 2026-09-04 17:46:59 +00:00 (Migrated from gitlab.com)

This was not a missing feature on Android. It was a wrong conclusion.

Trust.verifyHeadSignature already pinned issuerPublicKey the first time a
signed log head arrived. When the key later changed, the head signature simply
stopped verifying — and the only conclusion available at that point was
KeyLogForkException. So a relay key rotation somebody performed on purpose
was reported to the user as the relay showing two different histories, with
forkDetected set.

Those need different responses. A fork means nothing the relay says can be
trusted and the client should stop. A rotation means somebody ran a command, or
somebody else is answering on that address, and only a person can tell which.
Reporting the second as the first is alarming and unactionable — and an alarm
people cannot act on is one they learn to dismiss, which costs the real fork
detection its credibility as well.

RelayIdentityPin now decides what the published key MEANS before it is used,
with the same four verdicts as iOS, and a change raises
RelayIdentityChangedException — deliberately NOT a KeyLogForkException, and
without setting forkDetected, because the log may be perfectly consistent.

A relay that publishes no key is a weaker deployment, not an attack, and an
unparseable key is broken rather than hostile. Both read as "not published"
rather than as a change: refusing to talk to an older self-hosted build would
be a worse failure than the one this prevents.

The warning names both possibilities and asserts neither, because nothing in
the client can tell a rotation from an impostor.

Two things found while writing the tests. RelayClient.host already existed, so
the one I added was a duplicate declaration — removed rather than renamed. And
the compiler folded changed !is KeyLogForkException to a constant when the
value was typed concretely, which is the compiler proving the separation
statically and also an assertion testing nothing; it is now held as the sealed
base, which is the check a caller's when actually performs.

9 unit tests. App builds, every JVM suite green.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com

This was not a missing feature on Android. It was a wrong conclusion. `Trust.verifyHeadSignature` already pinned `issuerPublicKey` the first time a signed log head arrived. When the key later changed, the head signature simply stopped verifying — and the only conclusion available at that point was `KeyLogForkException`. So a relay key rotation somebody performed on purpose was reported to the user as the relay showing two different histories, with `forkDetected` set. Those need different responses. A fork means nothing the relay says can be trusted and the client should stop. A rotation means somebody ran a command, or somebody else is answering on that address, and only a person can tell which. Reporting the second as the first is alarming and unactionable — and an alarm people cannot act on is one they learn to dismiss, which costs the real fork detection its credibility as well. `RelayIdentityPin` now decides what the published key MEANS before it is used, with the same four verdicts as iOS, and a change raises `RelayIdentityChangedException` — deliberately NOT a `KeyLogForkException`, and without setting `forkDetected`, because the log may be perfectly consistent. A relay that publishes no key is a weaker deployment, not an attack, and an unparseable key is broken rather than hostile. Both read as "not published" rather than as a change: refusing to talk to an older self-hosted build would be a worse failure than the one this prevents. The warning names both possibilities and asserts neither, because nothing in the client can tell a rotation from an impostor. Two things found while writing the tests. `RelayClient.host` already existed, so the one I added was a duplicate declaration — removed rather than renamed. And the compiler folded `changed !is KeyLogForkException` to a constant when the value was typed concretely, which is the compiler proving the separation statically and also an assertion testing nothing; it is now held as the sealed base, which is the check a caller's `when` actually performs. 9 unit tests. App builds, every JVM suite green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
paulhitt commented 2026-09-04 17:46:59 +00:00 (Migrated from gitlab.com)

assigned to @paulhitt

assigned to @paulhitt
paulhitt (Migrated from gitlab.com) changed target branch from feat/zero-key-material to main 2026-09-04 20:49:18 +00:00
paulhitt (Migrated from gitlab.com) merged commit 8466fc1b57 into main 2026-09-04 20:49:24 +00:00
paulhitt commented 2026-09-09 19:01:35 +00:00 (Migrated from gitlab.com)

changed the description

changed the description
Sign in to join this conversation.