From e2428764fc35e903265911e742269bdbb438fa03 Mon Sep 17 00:00:00 2001 From: TheCryptoDonkey Date: Fri, 25 Sep 2026 01:01:07 +0100 Subject: [PATCH 1/2] feat: follow relay-update announcements with a jittered connect Heartwood now sends a kind 24135 relay-update (identical construction to a lock announcement, only the sealed t differs) when its relays change outside a restart. judge already refuses to prompt for it and withRelaysFrom already follows the relays; add the firmware's own vector and tests proving both hold, plus the duplicate/stale cases. Add RelayGate/RelayJitter so a relay Cambium has never spoken to before is not connected the instant a message teaches it about it -- a uniform 30 second to 10 minute jitter, drawn from SecureRandom by default and injectable for tests, so that relay cannot correlate its first connection to the moment the board's broadcast changed. A relay already known from pairing, enrolment or a prior session is unaffected. deliver's relay choice and sync's own return are untouched by the jitter, so answering a genuine lock prompt is never held up by it. --- AGENTS.md | 27 +++++- .../forgesworn/cambium/unlock/RelayGate.kt | 69 ++++++++++++++ .../cambium/unlock/UnlockCoordinator.kt | 26 +++++- .../cambium/unlock/LockMatcherTest.kt | 42 ++++++++- .../cambium/unlock/PhoneUnlockTest.kt | 27 ++++++ .../cambium/unlock/RelayGateTest.kt | 93 +++++++++++++++++++ .../cambium/unlock/RelayJitterTest.kt | 55 +++++++++++ .../resources/phone-unlock-v1-relays.json | 23 +++++ 8 files changed, 353 insertions(+), 9 deletions(-) create mode 100644 app/src/main/kotlin/dev/forgesworn/cambium/unlock/RelayGate.kt create mode 100644 app/src/test/kotlin/dev/forgesworn/cambium/unlock/RelayGateTest.kt create mode 100644 app/src/test/kotlin/dev/forgesworn/cambium/unlock/RelayJitterTest.kt create mode 100644 app/src/test/resources/phone-unlock-v1-relays.json diff --git a/AGENTS.md b/AGENTS.md index 2ad712f..9fcb0ba 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -552,17 +552,32 @@ Android apps Websites JVM-tested: `PhoneUnlock.kt` (K = HKDF(S), the per-boot hint, opening the sealed lock message with `ChaCha20.kt` + HMAC-SHA256, the delivery plaintext, and `judge`, the prompt rule) held to the firmware's own vectors (`src/test/resources/phone-unlock-v1.json`, copied from heartwood-esp32 - `common/tests/fixtures/`; regenerate only with a deliberate format bump there); + `common/tests/fixtures/`; regenerate only with a deliberate format bump there). Kind 24135 also + carries a relay-update variant -- identical construction to a lock announcement, only the sealed + `t` differs (`"relays"` instead of `"locked"`), sent whenever the board's own relay list changes + outside of a restart; `phone-unlock-v1-relays.json` (also copied from heartwood-esp32) is its + vector. `judge` returns `NOT_LOCKED` for it unconditionally (checked before staleness/replay/ + duplicate), so it never raises a prompt; `LockMatcher.withRelaysFrom` follows its relay list the + same as a lock announcement's, regardless of verdict. `Enrolment.kt` (the `heartwood-unlock:enrol?...` code Cambium shows, and the kind-24137 hand-off, whose content is the board's enrolment answer passed through); `LockMatcher.kt` (matching one announcement against every enrolled board, relay-list following, and `Reachability`, the - "gone quiet" rule). Android-side: `SlotSecretVault.kt` (per-board Keystore AES key, per-use - authentication by strong biometric or, on API 30+, the device credential (the owner's + "gone quiet" rule); `RelayGate.kt` (`RelayJitter`, a uniform 30 s-10 min draw from a + `SecureRandom` by default, and `RelayGate`, which withholds a relay a message just taught Cambium + about until that jitter elapses -- a relay Cambium already knows, from pairing/enrolment/a prior + session, is trusted immediately with no delay; both take their randomness/jitter as constructor + parameters so `RelayGateTest`/`RelayJitterTest` run entirely on `kotlinx.coroutines.test`'s + virtual time rather than real minutes). Android-side: `SlotSecretVault.kt` (per-board Keystore AES + key, per-use authentication by strong biometric or, on API 30+, the device credential (the owner's GrapheneOS phone has no biometrics, deliberately), invalidated on biometric enrolment change, StrongBox when present), `UnlockStore.kt` (enrolments and ping records in their own EncryptedSharedPreferences, `commit()` writes), `UnlockCoordinator.kt` (process-wide listener owner: current requests as a `StateFlow`, - sent deliveries, "still locked" detection from a same-boot repeat 25 s after answering), + sent deliveries, "still locked" detection from a same-boot repeat 25 s after answering; owns the + `RelayGate` -- a board's relays are trusted the first time its id is seen this process, a relay + a running board later teaches it about goes through `RelayGate.learn`, and `deliver`'s target + relays come straight from the lock message/enrolment, never gated, so answering a genuine prompt + is never held up by another relay's jitter), `UnlockNotifications.kt`, `UnlockActivity.kt` (always acts on the board's *current* request, not the notification's) and `UnlockEnrolActivity.kt` (enrolment key in memory only; `configChanges` so a rotation cannot lose it). @@ -570,7 +585,9 @@ Android apps Websites Metadata rules the code must keep (design section 6): the lock subscription has no filter but the kind; the relay client has no signer (no NIP-42 answer with a stable key); every delivery is from a fresh key; nothing pings the board because of a lock message (the gone-quiet alert uses - only the keep-alive's scheduled pings). + only the keep-alive's scheduled pings); a relay Cambium has never spoken to before does not see + its first connection from this phone land at the same moment the board's broadcast changed + (`RelayGate`'s jitter). - `signer/UnlockRelay.kt` -- the rust-nostr half of phone unlock: throwaway-key delivery events, opening the enrolment hand-off (NIP-44), and `RelayWatch`, one signer-less `Client` per purpose (the unfiltered 24135 listener; the enrolment screen's rendezvous subscription). Native calls diff --git a/app/src/main/kotlin/dev/forgesworn/cambium/unlock/RelayGate.kt b/app/src/main/kotlin/dev/forgesworn/cambium/unlock/RelayGate.kt new file mode 100644 index 0000000..7147e23 --- /dev/null +++ b/app/src/main/kotlin/dev/forgesworn/cambium/unlock/RelayGate.kt @@ -0,0 +1,69 @@ +package dev.forgesworn.cambium.unlock + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch +import java.security.SecureRandom +import java.time.Duration +import java.util.Random +import java.util.concurrent.ConcurrentHashMap + +/** + * How long Cambium waits before connecting to a relay a message just taught it about, so that + * relay -- one it has never spoken to before -- cannot tie this phone's first connection to the + * exact moment the board's broadcast reached it. A relay Cambium already knows (from pairing, + * enrolment, or a previous session) is connected immediately; see [RelayGate]. + */ +object RelayJitter { + val MIN: Duration = Duration.ofSeconds(30) + val MAX: Duration = Duration.ofMinutes(10) + + /** Uniform in [[min], [max]), drawn from [random] -- a [SecureRandom] by default. */ + fun next(random: Random = SecureRandom(), min: Duration = MIN, max: Duration = MAX): Duration { + require(!max.isNegative && max >= min) { "max must be at least min" } + val spanMillis = max.toMillis() - min.toMillis() + val offsetMillis = if (spanMillis <= 0L) 0L else (random.nextDouble() * spanMillis).toLong() + return min.plusMillis(offsetMillis) + } +} + +/** + * Which relays [UnlockCoordinator] may connect to right now. A relay already known -- from + * pairing, enrolment, or a previous session -- is ready immediately via [trust]; a relay a message + * just taught Cambium about is withheld until a random jitter elapses ([learn]), so it cannot + * correlate its first connection from this phone to the moment the board's broadcast changed. + * + * Pure Kotlin bar the coroutine delay itself, so tests can run [learn] against virtual time + * (`kotlinx.coroutines.test`) instead of real minutes, and supply their own [jitter]. + */ +class RelayGate(private val jitter: () -> Duration = { RelayJitter.next() }) { + private val readyRelays = ConcurrentHashMap.newKeySet() + private val pending = ConcurrentHashMap.newKeySet() + + /** The subset of [relays] safe to connect to right now. */ + fun ready(relays: Collection): List = relays.filter { it in readyRelays } + + /** Marks [relays] ready with no delay: already known, not something a message just taught us. */ + fun trust(relays: Collection) { + readyRelays += relays + } + + /** + * Schedules each of [relays] not already ready or already scheduled to become ready after a + * jitter, on [scope], calling [onReady] once each one does. Returns immediately -- the wait + * happens in the launched coroutines, never on the caller, so this never delays the caller's + * own work (in particular, an unlock delivery must stay fast; see [UnlockCoordinator.deliver]). + */ + fun learn(scope: CoroutineScope, relays: Collection, onReady: () -> Unit) { + for (relay in relays) { + if (relay in readyRelays || !pending.add(relay)) continue + val wait = jitter() + scope.launch { + delay(wait.toMillis()) + readyRelays += relay + pending -= relay + onReady() + } + } + } +} diff --git a/app/src/main/kotlin/dev/forgesworn/cambium/unlock/UnlockCoordinator.kt b/app/src/main/kotlin/dev/forgesworn/cambium/unlock/UnlockCoordinator.kt index 57df63c..3265649 100644 --- a/app/src/main/kotlin/dev/forgesworn/cambium/unlock/UnlockCoordinator.kt +++ b/app/src/main/kotlin/dev/forgesworn/cambium/unlock/UnlockCoordinator.kt @@ -53,6 +53,15 @@ object UnlockCoordinator { private var watchedRelays: List = emptyList() @Volatile private var boards: List = emptyList() + /** + * Gates which relays are actually connected to: a board's relays at the point it is first seen + * this process (pairing, enrolment, restart) are trusted immediately, but a relay a lock or + * relay-update message teaches Cambium about while a board is already known waits out a jitter + * first -- see [RelayGate] and [onAnnouncement]. + */ + private val relayGate = RelayGate() + private val knownBoardIds = ConcurrentHashMap.newKeySet() + /** * Starts, restarts or stops the listener so it watches exactly the enrolled boards' relays. * Runs to completion even if the caller is cancelled (an activity closed mid-start): a relay @@ -62,7 +71,14 @@ object UnlockCoordinator { mutex.withLock { val app = context.applicationContext boards = UnlockStore(app).enrolments() - val relays = LockMatcher.relayUnion(boards) + // A board seen for the first time this process (fresh pairing, enrolment, or the + // process's first sync after a restart) has its relays trusted at once: they came from + // pairing/enrolment, not from a stray relay message. A board already known keeps + // whatever relayGate already granted it -- a newly learned relay for it only becomes + // ready once onAnnouncement's jitter elapses. + val freshBoards = boards.filter { knownBoardIds.add(it.id) } + if (freshBoards.isNotEmpty()) relayGate.trust(freshBoards.flatMap { it.relays }) + val relays = relayGate.ready(LockMatcher.relayUnion(boards)) if (relays == watchedRelays && watch != null) return@withLock watch?.stop() watch = null @@ -90,6 +106,7 @@ object UnlockCoordinator { _requests.update { it - enrolmentId } sent.remove(enrolmentId) warnedStillLocked.remove(enrolmentId) + knownBoardIds.remove(enrolmentId) } private fun onAnnouncement(app: Context, raw: RawAnnouncement) { @@ -99,10 +116,15 @@ object UnlockCoordinator { val store = UnlockStore(app) // Any authentic message carries the board's relay list: follow it, whatever the verdict. + // The relay list itself is trusted and persisted at once; connecting to any brand-new + // relay in it is delayed by a jitter (relayGate.learn), so a relay Cambium has never spoken + // to before cannot correlate its first connection to the moment this message arrived. val followed = LockMatcher.withRelaysFrom(match.enrolment, match.context) if (followed !== match.enrolment) { + val newRelays = followed.relays.filterNot { it in match.enrolment.relays } store.modify(id) { it.copy(relays = followed.relays) } - scope.launch { sync(app) } + boards = boards.map { if (it.id == id) it.copy(relays = followed.relays) else it } + relayGate.learn(scope, newRelays) { scope.launch { sync(app) } } } when (match.verdict) { diff --git a/app/src/test/kotlin/dev/forgesworn/cambium/unlock/LockMatcherTest.kt b/app/src/test/kotlin/dev/forgesworn/cambium/unlock/LockMatcherTest.kt index 2857106..9b6e61a 100644 --- a/app/src/test/kotlin/dev/forgesworn/cambium/unlock/LockMatcherTest.kt +++ b/app/src/test/kotlin/dev/forgesworn/cambium/unlock/LockMatcherTest.kt @@ -28,9 +28,16 @@ class LockMatcherTest { last = last, ) - private fun announcement(forEnrolment: UnlockEnrolment, id: Long = forEnrolment.id, boot: Long = 5, createdAt: Long = now): RawAnnouncement { + private fun announcement( + forEnrolment: UnlockEnrolment, + id: Long = forEnrolment.id, + boot: Long = 5, + createdAt: Long = now, + t: String = "locked", + relays: List = listOf("wss://new.example"), + ): RawAnnouncement { val k = forEnrolment.phoneKeyHex.hexToBytesOrNull()!! - val context = LockContext(1, "locked", id, boot, "power-on", "home", "", "0.18.0-beta.17", listOf("wss://new.example")) + val context = LockContext(1, t, id, boot, "power-on", "home", "", "0.18.0-beta.17", relays) val content = PhoneUnlock.sealContext(k, author, Json.encodeToString(LockContext.serializer(), context), ByteArray(12)) return RawAnnouncement("e".repeat(64), authorHex, createdAt, content, PhoneUnlock.hint(k, author)) } @@ -69,6 +76,37 @@ class LockMatcherTest { assertEquals(Verdict.STALE, LockMatcher.match(announcement(desk, boot = 6, createdAt = now - 600), listOf(desk), now)?.verdict) } + @Test + fun `a relay-update message is matched and opened but never prompts`() { + val desk = enrolment(1, 1) + val match = LockMatcher.match(announcement(desk, t = "relays"), listOf(desk), now) + assertNotNull(match) + assertEquals("relays", match.context.t) + assertEquals(Verdict.NOT_LOCKED, match.verdict) + // withRelaysFrom follows it regardless of verdict, exactly like a lock announcement would. + val followed = LockMatcher.withRelaysFrom(match.enrolment, match.context) + assertEquals(listOf("wss://relay.example", "wss://new.example"), followed.relays) + } + + @Test + fun `a duplicate or stale relay-update never prompts either`() { + val desk = enrolment(1, 1, last = LastPrompt(5, authorHex)) + // Same boot and author as the recorded last prompt: a lock announcement here would be + // Verdict.DUPLICATE; a relay-update is NOT_LOCKED regardless. + assertEquals(Verdict.NOT_LOCKED, LockMatcher.match(announcement(desk, t = "relays"), listOf(desk), now)?.verdict) + // An older boot: a lock announcement here would be Verdict.REPLAY; still NOT_LOCKED. + assertEquals( + Verdict.NOT_LOCKED, + LockMatcher.match(announcement(desk, t = "relays", boot = 4), listOf(desk), now)?.verdict, + ) + // Outside the announce-age window: a lock announcement here would be Verdict.STALE; still + // NOT_LOCKED. + assertEquals( + Verdict.NOT_LOCKED, + LockMatcher.match(announcement(desk, t = "relays", boot = 6, createdAt = now - 600), listOf(desk), now)?.verdict, + ) + } + @Test fun `relay lists are merged, never shrunk`() { val desk = enrolment(1, 1) diff --git a/app/src/test/kotlin/dev/forgesworn/cambium/unlock/PhoneUnlockTest.kt b/app/src/test/kotlin/dev/forgesworn/cambium/unlock/PhoneUnlockTest.kt index d9c8ebc..26cdd9c 100644 --- a/app/src/test/kotlin/dev/forgesworn/cambium/unlock/PhoneUnlockTest.kt +++ b/app/src/test/kotlin/dev/forgesworn/cambium/unlock/PhoneUnlockTest.kt @@ -63,6 +63,33 @@ class PhoneUnlockTest { assertEquals(field("delivery"), PhoneUnlock.deliveryJson(context.id, s)) } + /** + * The firmware's relay-update vector (heartwood-esp32 + * `common/tests/fixtures/phone-unlock-v1-relays.json`): identical construction to a lock + * announcement, only `t` differs. A phone opens it exactly the same way and never prompts. + */ + @Test + fun `the firmware relay-update vector opens and is never a prompt`() { + val text = javaClass.classLoader!!.getResource("phone-unlock-v1-relays.json")!!.readText() + val fixture = Json.parseToJsonElement(text) as JsonObject + fun field(name: String) = fixture[name]!!.jsonPrimitive.content + val s = field("slot_secret").hexToBytesOrNull()!! + val k = PhoneUnlock.phoneKey(s) + assertEquals(field("phone_key"), k.toHex()) + val author = field("author").hexToBytesOrNull()!! + assertEquals(field("hint"), PhoneUnlock.hint(k, author)) + val context = Json.decodeFromJsonElement(LockContext.serializer(), fixture["context"]!!) + assertEquals(ctx(PhoneUnlock.TYPE_RELAYS), context) + assertEquals(PhoneUnlock.TYPE_RELAYS, context.t) + val sealed = PhoneUnlock.sealContext(k, author, json(context), field("nonce").hexToBytesOrNull()!!) + assertEquals(field("content"), sealed) + assertEquals(context, PhoneUnlock.openContext(k, author, field("content"))) + assertEquals( + Verdict.NOT_LOCKED, + PhoneUnlock.judge(context, author.toHex(), 1_800_000_000L, 1_800_000_000L, null), + ) + } + @Test fun `a phone recognises and opens its own announcement only`() { val k = PhoneUnlock.phoneKey(bytes(1)) diff --git a/app/src/test/kotlin/dev/forgesworn/cambium/unlock/RelayGateTest.kt b/app/src/test/kotlin/dev/forgesworn/cambium/unlock/RelayGateTest.kt new file mode 100644 index 0000000..a6987bf --- /dev/null +++ b/app/src/test/kotlin/dev/forgesworn/cambium/unlock/RelayGateTest.kt @@ -0,0 +1,93 @@ +package dev.forgesworn.cambium.unlock + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import java.time.Duration +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * [RelayGate] against virtual time (`kotlinx.coroutines.test`), so a 30 s-10 min jitter runs in + * milliseconds here rather than really waiting. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class RelayGateTest { + + @Test + fun `a trusted relay is ready immediately, with no jitter call at all`() { + var jitterCalls = 0 + val gate = RelayGate(jitter = { jitterCalls++; Duration.ofMinutes(5) }) + gate.trust(listOf("wss://known.example")) + assertEquals(listOf("wss://known.example"), gate.ready(listOf("wss://known.example", "wss://unknown.example"))) + assertEquals(0, jitterCalls) + } + + @Test + fun `a learned relay is withheld until its jitter elapses`() = runTest { + val gate = RelayGate(jitter = { Duration.ofMinutes(2) }) + var readyCalls = 0 + gate.learn(scope = this, relays = listOf("wss://new.example")) { readyCalls++ } + // Not ready yet: neither the relay nor the onReady callback have fired. + assertEquals(emptyList(), gate.ready(listOf("wss://new.example"))) + assertEquals(0, readyCalls) + + advanceTimeBy(Duration.ofMinutes(2).toMillis() - 1) + assertEquals(emptyList(), gate.ready(listOf("wss://new.example"))) + assertEquals(0, readyCalls) + + advanceUntilIdle() + assertEquals(listOf("wss://new.example"), gate.ready(listOf("wss://new.example"))) + assertEquals(1, readyCalls) + } + + @Test + fun `learning the same relay twice schedules only one jitter`() = runTest { + var jitterCalls = 0 + val gate = RelayGate(jitter = { jitterCalls++; Duration.ofSeconds(30) }) + var readyCalls = 0 + gate.learn(this, listOf("wss://new.example")) { readyCalls++ } + gate.learn(this, listOf("wss://new.example")) { readyCalls++ } + advanceUntilIdle() + assertEquals(1, jitterCalls, "already-pending relay must not schedule a second jitter") + assertEquals(1, readyCalls) + assertEquals(listOf("wss://new.example"), gate.ready(listOf("wss://new.example"))) + } + + @Test + fun `an already-trusted relay is not re-jittered when learned again`() = runTest { + var jitterCalls = 0 + val gate = RelayGate(jitter = { jitterCalls++; Duration.ofMinutes(1) }) + gate.trust(listOf("wss://known.example")) + var readyCalls = 0 + gate.learn(this, listOf("wss://known.example")) { readyCalls++ } + advanceUntilIdle() + assertEquals(0, jitterCalls) + assertEquals(0, readyCalls, "trust() already made it ready; learn() has nothing new to report") + assertEquals(listOf("wss://known.example"), gate.ready(listOf("wss://known.example"))) + } + + @Test + fun `learn returns immediately -- the wait happens in the launched coroutine, not the caller`() { + val dispatcher = StandardTestDispatcher() + val scope = TestScope(dispatcher) + val gate = RelayGate(jitter = { Duration.ofMinutes(10) }) + var readyCalls = 0 + // learn() itself must not suspend: this call returns without the dispatcher ever running. + gate.learn(scope, listOf("wss://new.example")) { readyCalls++ } + assertTrue(dispatcher.scheduler.currentTime == 0L) + assertEquals(0, readyCalls) + } + + @Test + fun `each learned relay draws its own jitter`() = runTest { + var draws = 0 + val gate = RelayGate(jitter = { draws++; Duration.ofSeconds(1) }) + gate.learn(this, listOf("wss://a.example", "wss://b.example")) {} + assertEquals(2, draws) + } +} diff --git a/app/src/test/kotlin/dev/forgesworn/cambium/unlock/RelayJitterTest.kt b/app/src/test/kotlin/dev/forgesworn/cambium/unlock/RelayJitterTest.kt new file mode 100644 index 0000000..29048d3 --- /dev/null +++ b/app/src/test/kotlin/dev/forgesworn/cambium/unlock/RelayJitterTest.kt @@ -0,0 +1,55 @@ +package dev.forgesworn.cambium.unlock + +import java.time.Duration +import java.util.Random +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertTrue + +class RelayJitterTest { + + private fun fixed(value: Double) = object : Random() { + override fun nextDouble(): Double = value + } + + @Test + fun `defaults span 30 seconds to 10 minutes`() { + assertEquals(Duration.ofSeconds(30), RelayJitter.MIN) + assertEquals(Duration.ofMinutes(10), RelayJitter.MAX) + } + + @Test + fun `the lowest draw returns the minimum, a high draw approaches the maximum`() { + assertEquals(Duration.ofSeconds(30), RelayJitter.next(fixed(0.0))) + val span = RelayJitter.MAX.toMillis() - RelayJitter.MIN.toMillis() + assertEquals(RelayJitter.MIN.plusMillis((span * 0.5).toLong()), RelayJitter.next(fixed(0.5))) + val nearMax = RelayJitter.next(fixed(0.999999)) + assertTrue(nearMax < RelayJitter.MAX, "expected $nearMax below the maximum") + assertTrue(nearMax > RelayJitter.MIN, "expected $nearMax above the minimum") + } + + @Test + fun `a real SecureRandom source always lands inside the bounds`() { + repeat(200) { + val wait = RelayJitter.next() + assertTrue(wait >= RelayJitter.MIN, "wait $wait below minimum") + assertTrue(wait < RelayJitter.MAX || wait == RelayJitter.MAX, "wait $wait above maximum") + } + } + + @Test + fun `custom bounds are honoured`() { + val min = Duration.ofSeconds(1) + val max = Duration.ofSeconds(2) + assertEquals(min, RelayJitter.next(fixed(0.0), min, max)) + assertEquals(max, RelayJitter.next(fixed(1.0), min, max)) + } + + @Test + fun `a maximum below the minimum is refused`() { + assertFailsWith { + RelayJitter.next(fixed(0.0), min = Duration.ofMinutes(1), max = Duration.ofSeconds(1)) + } + } +} diff --git a/app/src/test/resources/phone-unlock-v1-relays.json b/app/src/test/resources/phone-unlock-v1-relays.json new file mode 100644 index 0000000..94bd69c --- /dev/null +++ b/app/src/test/resources/phone-unlock-v1-relays.json @@ -0,0 +1,23 @@ +{ + "author": "0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20", + "content": "AcDBwsPExcbHyMnKyzFQxcXFQyzcXtoO8O9od0CIMkGCmHdrZ/R9qJBMXsGnmbhrTQhez1rRlZBiT7rDmChTqf1l5KWW4mGZtfYRpUTO5jqeoSh+GYytdJStg61ANn0BmGJhOQqMvTArYS3qfGXA5qnW9ruxRhbRVxc06JsMPYglF6arkBWvTqDcgxmMqKWeyZ6sk2pFApYnGClttMq5GkNsF708bFYmPbUZsRwvFVy1uaQFtmeANEjRrQzz+hcgULICgRonj8omWwqEeULp1g1CJhcKL4n30TpleM6GoGg=", + "context": { + "boot": 212, + "bssid": "aa:bb:cc:dd:ee:ff", + "fw": "0.18.0-beta.17", + "id": 7, + "relays": [ + "wss://relay.example", + "wss://two.example" + ], + "reset": "poweron", + "ssid": "devolo-753", + "t": "relays", + "v": 1 + }, + "description": "Phone unlock v1 relay-update vector: the v1 keys and context with t = relays. A phone opens it exactly like a lock announcement, follows its relays and never prompts. See common/src/phone_unlock.rs.", + "hint": "e692cafe2a6e4a8d", + "nonce": "c0c1c2c3c4c5c6c7c8c9cacb", + "phone_key": "8d2ad99453444b5a25f142b69b0e1c0df9e49b6dcf3d755e39a1112ad2c6bc12", + "slot_secret": "5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a" +} From b9ff9a10eaa6a63d8e26b011742e17855e267975 Mon Sep 17 00:00:00 2001 From: TheCryptoDonkey Date: Fri, 25 Sep 2026 01:04:32 +0100 Subject: [PATCH 2/2] fix: never jitter a genuine unlock delivery's own relays deliver() called sync() and then published to match.context.relays, but sync() now only connects relayGate-ready relays, and RelayWatch.publish falls back to whatever is already connected. If the board had moved to a new relay less than a jitter ago and then restarted locked on that same relay, a tap would send the 24136 to the board's stale relays -- exactly the case relay-update exists to avoid -- until the jitter elapsed (up to 10 minutes). deliver() now trusts a genuine lock prompt's own relays outright before calling sync(): the owner's tap already exposes the timing, so there is nothing left for the jitter to protect there. Guarded on t == "locked" even though judge() already guarantees a relay-update can never reach this method. sync() blocks on RelayWatch.locks's own connection wait, so by the time deliver() reaches publish() the newly trusted relay is normally already connected; publish()'s fallback to whatever succeeded is left as the fastest remaining option for a relay that failed to connect in that window, rather than blocking every delivery on a relay that may not answer at all. Also prune a forgotten board's relays from RelayGate's ready set, unless another remaining board still needs one of them. --- AGENTS.md | 20 ++++++++-- .../forgesworn/cambium/unlock/RelayGate.kt | 9 +++++ .../cambium/unlock/UnlockCoordinator.kt | 20 ++++++++++ .../cambium/unlock/RelayGateTest.kt | 37 +++++++++++++++++++ 4 files changed, 82 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9fcb0ba..3d2355a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -574,10 +574,22 @@ Android apps Websites `UnlockStore.kt` (enrolments and ping records in their own EncryptedSharedPreferences, `commit()` writes), `UnlockCoordinator.kt` (process-wide listener owner: current requests as a `StateFlow`, sent deliveries, "still locked" detection from a same-boot repeat 25 s after answering; owns the - `RelayGate` -- a board's relays are trusted the first time its id is seen this process, a relay - a running board later teaches it about goes through `RelayGate.learn`, and `deliver`'s target - relays come straight from the lock message/enrolment, never gated, so answering a genuine prompt - is never held up by another relay's jitter), + `RelayGate` -- a board's relays are trusted the first time its id is seen this process, a relay a + running board later teaches it about while passively listening goes through `RelayGate.learn`, + and `forget` prunes a forgotten board's relays out of the ready set, unless another remaining + board still needs one of them. `deliver` -- an owner-tapped, genuine unlock -- trusts its own lock + message's relays outright (`relayGate.trust`) before calling `sync`, rather than routing them + through `learn`'s jitter: the owner's tap already exposes the timing, so there is nothing left to + protect by delaying, and this is exactly the case the relay-update message exists for (the board + restarted locked on a relay it only announced less than a jitter ago; without this, the delivery + would go to the board's *old* relays, where it is no longer listening, until the jitter finally + elapsed). Guarded on `t == "locked"` even though `judge` already guarantees a relay-update can + never produce a `PROMPT`. `sync` itself blocks until `RelayWatch.locks` has tried to connect + (`waitForConnection`, ~10 s), so by the time `deliver` reaches `RelayWatch.publish` the newly + trusted relay is normally already in its `added` set; `publish`'s fallback to whatever is + already connected only matters if that connection attempt itself failed, and is left as-is + deliberately -- retrying or waiting longer there has no better chance of reaching an unreachable + relay and would only slow down every other delivery too), `UnlockNotifications.kt`, `UnlockActivity.kt` (always acts on the board's *current* request, not the notification's) and `UnlockEnrolActivity.kt` (enrolment key in memory only; `configChanges` so a rotation cannot lose it). diff --git a/app/src/main/kotlin/dev/forgesworn/cambium/unlock/RelayGate.kt b/app/src/main/kotlin/dev/forgesworn/cambium/unlock/RelayGate.kt index 7147e23..d70d33e 100644 --- a/app/src/main/kotlin/dev/forgesworn/cambium/unlock/RelayGate.kt +++ b/app/src/main/kotlin/dev/forgesworn/cambium/unlock/RelayGate.kt @@ -48,6 +48,15 @@ class RelayGate(private val jitter: () -> Duration = { RelayJitter.next() }) { readyRelays += relays } + /** + * Drops [relays] from the ready set -- a board being forgotten. Cheap best-effort cleanup, not + * required for correctness: a relay already scheduled in [learn] (`pending`) is left alone and + * simply becomes ready, harmlessly, whenever its jitter elapses. + */ + fun untrust(relays: Collection) { + readyRelays -= relays.toSet() + } + /** * Schedules each of [relays] not already ready or already scheduled to become ready after a * jitter, on [scope], calling [onReady] once each one does. Returns immediately -- the wait diff --git a/app/src/main/kotlin/dev/forgesworn/cambium/unlock/UnlockCoordinator.kt b/app/src/main/kotlin/dev/forgesworn/cambium/unlock/UnlockCoordinator.kt index 3265649..d887223 100644 --- a/app/src/main/kotlin/dev/forgesworn/cambium/unlock/UnlockCoordinator.kt +++ b/app/src/main/kotlin/dev/forgesworn/cambium/unlock/UnlockCoordinator.kt @@ -107,6 +107,14 @@ object UnlockCoordinator { sent.remove(enrolmentId) warnedStillLocked.remove(enrolmentId) knownBoardIds.remove(enrolmentId) + // The caller removes the enrolment from the store before calling this, but boards (this + // process's cache) has not been reloaded yet, so the forgotten board's relays are still + // here to prune -- unless another remaining board still needs one of them. + val forgotten = boards.firstOrNull { it.id == enrolmentId } + if (forgotten != null) { + val stillNeeded = boards.asSequence().filter { it.id != enrolmentId }.flatMap { it.relays }.toSet() + relayGate.untrust(forgotten.relays.filterNot { it in stillNeeded }) + } } private fun onAnnouncement(app: Context, raw: RawAnnouncement) { @@ -160,6 +168,15 @@ object UnlockCoordinator { * wipes [slotSecret] afterwards and has already checked [request] is still the board's * current one. The delivery goes only to the relays the board itself listed, not to every * relay this phone listens on. True once at least one relay accepted it. + * + * The board may have moved to a brand-new relay less than a jitter ago (that is exactly what + * the relay-update message is for): if we waited out [relayGate]'s jitter here too, a delivery + * could sit unconnected to the board's new relay for up to 10 minutes. The owner's tap already + * exposes the timing -- there is nothing left to protect by delaying it -- so a genuine lock + * prompt's own relays are trusted immediately, before [sync] rebuilds the watch and waits for + * the connection. [match]'s verdict is only ever `PROMPT` for `t == "locked"` (see + * [PhoneUnlock.judge]), so a relay-update can never reach this method, but the check is kept + * explicit rather than relied on implicitly. */ suspend fun deliver(context: Context, request: Request, slotSecret: ByteArray): Boolean { val app = context.applicationContext @@ -168,6 +185,9 @@ object UnlockCoordinator { match.announcement.authorHex, PhoneUnlock.deliveryJson(match.enrolment.id, slotSecret), ) + if (match.context.t == PhoneUnlock.TYPE_LOCKED) { + relayGate.trust(match.context.relays.map { it.trimEnd('/') }.filter(::isRelayUrl)) + } sync(app) val targets = match.context.relays.map { it.trimEnd('/') }.filter(::isRelayUrl) .ifEmpty { match.enrolment.relays } diff --git a/app/src/test/kotlin/dev/forgesworn/cambium/unlock/RelayGateTest.kt b/app/src/test/kotlin/dev/forgesworn/cambium/unlock/RelayGateTest.kt index a6987bf..c280a65 100644 --- a/app/src/test/kotlin/dev/forgesworn/cambium/unlock/RelayGateTest.kt +++ b/app/src/test/kotlin/dev/forgesworn/cambium/unlock/RelayGateTest.kt @@ -90,4 +90,41 @@ class RelayGateTest { gate.learn(this, listOf("wss://a.example", "wss://b.example")) {} assertEquals(2, draws) } + + /** + * The bug UnlockCoordinator.deliver guards against: a board moved to a brand-new relay less + * than a jitter ago (a relay-update) and then restarted locked, announcing on that same new + * relay. If a genuine lock delivery had to wait out the jitter too, it would go to the board's + * stale relay for up to 10 minutes -- exactly the case the relay-update exists to avoid. + * `trust()` is how `deliver` escapes the jitter: it must make an already-learn()-scheduled + * relay ready at once, with the pending jitter's own callback still safe to fire later. + */ + @Test + fun `a lock delivery trusting an already-jittered relay makes it ready immediately`() = runTest { + var jitterCalls = 0 + val gate = RelayGate(jitter = { jitterCalls++; Duration.ofMinutes(9) }) + var readyCalls = 0 + // A passive relay-update taught Cambium about this relay a moment ago; still on jitter. + gate.learn(this, listOf("wss://new.example")) { readyCalls++ } + assertEquals(emptyList(), gate.ready(listOf("wss://new.example"))) + + // The board then restarts locked and announces on that same relay; the owner taps unlock. + gate.trust(listOf("wss://new.example")) + assertEquals(listOf("wss://new.example"), gate.ready(listOf("wss://new.example")), "trust() must not wait for the jitter") + + // The original jitter is still scheduled; letting it elapse must not misbehave (it simply + // re-confirms readiness and fires its own onReady once, harmlessly). + advanceUntilIdle() + assertEquals(1, jitterCalls) + assertEquals(1, readyCalls) + assertEquals(listOf("wss://new.example"), gate.ready(listOf("wss://new.example"))) + } + + @Test + fun `untrust drops a relay from the ready set`() { + val gate = RelayGate(jitter = { Duration.ofMinutes(1) }) + gate.trust(listOf("wss://a.example", "wss://b.example")) + gate.untrust(listOf("wss://a.example")) + assertEquals(listOf("wss://b.example"), gate.ready(listOf("wss://a.example", "wss://b.example"))) + } }