diff --git a/.github/audit/canaries/pocket-presence-challenge-unchecked.patch b/.github/audit/canaries/pocket-presence-challenge-unchecked.patch index c0698ef21..80a57c6b5 100644 --- a/.github/audit/canaries/pocket-presence-challenge-unchecked.patch +++ b/.github/audit/canaries/pocket-presence-challenge-unchecked.patch @@ -4,18 +4,10 @@ Expect: lib/src/remote/client/pocket-client.ts Source: reverses part of #1223 (Pocket signed the challenge the Relay's `begin` answered instead of deriving its own) diff --git a/lib/src/remote/client/pocket-client.test.ts b/lib/src/remote/client/pocket-client.test.ts -index bb9c28ef3..d560251d5 100644 +index b7a91c6..25d98e3 100644 --- a/lib/src/remote/client/pocket-client.test.ts +++ b/lib/src/remote/client/pocket-client.test.ts -@@ -31,7 +31,6 @@ import { - generateNoiseKeyPair, - hashPasskeyPublicKey, - parsePairingInvitationUrl, -- presenceChallenge, - pushEndpointFingerprint, - randomBase64Url, - toBase64Url, -@@ -49,7 +48,6 @@ import { +@@ -51,7 +51,6 @@ import { BurrowIdentityMismatchError, PAIRING_DENIAL_MESSAGES, PASSKEY_UNAVAILABLE_MESSAGE, @@ -23,24 +15,7 @@ index bb9c28ef3..d560251d5 100644 PocketClient, SessionExpiredError, SessionSupersededError, -@@ -73,15 +71,7 @@ import { - } from '../direct/test-fake-peer'; - import type { DirectPeerLike } from '../direct/direct-peer'; - import type { RemoteTimer } from '../ws'; --import { -- createTestAuthenticator, -- openConnectionSession, -- readOutcome, -- sendE2eFrame, -- settle, -- testRoutingId, -- type TestAuthenticator, --} from '../test-e2e-client'; -+import { createTestAuthenticator, settle, type TestAuthenticator } from '../test-e2e-client'; - import { PasskeyAlreadyRegisteredError, type WebAuthnClient } from './webauthn'; - import { - ACCOUNT_ID, -@@ -2003,99 +1993,6 @@ describe('the presence proof', () => { +@@ -2005,12 +2004,6 @@ describe('the presence proof', () => { return { client, prompts, finished }; } @@ -50,78 +25,30 @@ index bb9c28ef3..d560251d5 100644 - challenge: await presenceChallenge({ ...binding, handshakeHash: randomBase64Url(32) }, answer.relayNonce), - }); - + /** + * A paired phone, past the window its pairing opened, and another connection + * on that phone's own paired static which the phone never opened: the binding +@@ -2078,38 +2071,6 @@ describe('the presence proof', () => { + return { harness, binding, nonce, present }; + } + - it('refuses a challenge bound to another connection before any prompt, so that one gains no presence', async () => { -- let burrowClock = Date.now(); -- const harness = await makeE2eHarness({ burrowNow: () => burrowClock }); -- const { burrowId, relay, noiseStatic, authenticator } = harness; -- await harness.pairAndApprove(await harness.mintInvitation()); -- // Past the window the pairing opened, so the connection needs a proof. -- burrowClock += PRESENCE_WINDOW_MAX_MS + 1; -- -- // Another connection on this phone's own paired static, which this phone never opened. -- const record = harness.knownBurrows.records.get(burrowId)!; -- const clientStatic = { -- privateKey: record.clientStaticKeyPair.privateKey, -- publicKey: fromBase64Url(record.clientStaticKeyPair.publicKeyRaw), -- }; -- const open = (clientId: string, connectionId: string) => -- openConnectionSession({ -- socket: relay.burrowSocket, -- burrowId, -- clientId, -- connectionId, -- clientStatic, -- burrowStaticPublicKey: noiseStatic.publicKey, -- }); -- const other = { clientId: testRoutingId(), connectionId: testRoutingId() }; -- const otherSession = await open(other.clientId, other.connectionId); -- const otherBinding: PresenceBinding = { -- kind: 'connection', -- burrowId, -- connectionId: other.connectionId, -- burrowChallenge: otherSession.burrowChallenge, -- handshakeHash: toBase64Url(otherSession.session.handshakeHash), -- passkeyCredentialId: authenticator.credentialId, -- }; -- const otherNonce = randomBase64Url(32); +- const { harness, binding, nonce, present } = await anotherConnectionOnThePairedStatic(); - const phone = await behindRelay(harness, async (answer) => ({ - ...answer, -- challenge: await presenceChallenge(otherBinding, otherNonce), +- challenge: await presenceChallenge(binding, nonce), - })); - -- await expect(phone.client.connect(burrowId)).rejects.toThrow(PRESENCE_CHALLENGE_MISMATCH_MESSAGE); +- await expect(phone.client.connect(harness.burrowId)).rejects.toThrow(PRESENCE_CHALLENGE_MISMATCH_MESSAGE); - expect(phone.prompts).toEqual([]); - expect(phone.finished).toEqual([]); - -- // The other connection presents whatever reached `finish`: with no tap, the -- // Burrow admits it to nothing and opens no window for the static. -- sendE2eFrame(relay.burrowSocket, { -- clientId: other.clientId, -- burrowId, -- kind: 'connection', -- id: other.connectionId, -- step: 'transport', -- ct: toBase64Url( -- otherSession.session.sendControl({ -- presence: { -- binding: otherBinding, -- relayNonce: otherNonce, -- accountId: ACCOUNT_ID, -- passkeyCredentialId: authenticator.credentialId, -- passkeyPublicKey: authenticator.publicKey, -- assertion: phone.finished[0], -- }, -- }), -- ), -- }); -- expect( -- await readOutcome(relay.burrowSocket, otherSession.session, 'connection', other.connectionId), -- ).toMatchObject({ ok: false }); -- expect((await open(testRoutingId(), testRoutingId())).offer).toBe('none'); +- // The other connection presents whatever reached `finish`: with no tap, it gains nothing. +- await present(phone.finished[0]); - - // An honest `begin` still connects, with the one prompt it needs. - const honest = await behindRelay(harness, async (answer) => answer); -- expect(await honest.client.connect(burrowId)).toEqual({ ok: true, burrowLabel: BURROW_LABEL }); +- expect(await honest.client.connect(harness.burrowId)).toEqual({ ok: true, burrowLabel: BURROW_LABEL }); - expect(honest.prompts).toHaveLength(1); - }); - @@ -141,7 +68,7 @@ index bb9c28ef3..d560251d5 100644 const harness = await makeE2eHarness(); const phone = await behindRelay( diff --git a/lib/src/remote/client/pocket-client.ts b/lib/src/remote/client/pocket-client.ts -index 79f5c541b..e65c42a3d 100644 +index 33381cf..352a714 100644 --- a/lib/src/remote/client/pocket-client.ts +++ b/lib/src/remote/client/pocket-client.ts @@ -38,7 +38,6 @@ import { @@ -152,7 +79,7 @@ index 79f5c541b..e65c42a3d 100644 pushEndpointFingerprint, pushSubscriptionDeletePath, randomBase64Url, -@@ -354,14 +353,6 @@ export const CONNECTION_DENIAL_MESSAGES: Record = +@@ -356,14 +355,6 @@ export const CONNECTION_DENIAL_MESSAGES: Record = 'burrow-error': 'The computer could not finish the connection.', }; @@ -167,7 +94,7 @@ index 79f5c541b..e65c42a3d 100644 /** * What a ceremony that simply never answered reports. * -@@ -1160,12 +1151,6 @@ export class PocketClient { +@@ -1170,12 +1161,6 @@ export class PocketClient { * authenticates no other pairing or connection; the Relay's `finish` proves * nothing to the Burrow, which recomputes the challenge and verifies the same * assertion itself. @@ -180,7 +107,7 @@ index 79f5c541b..e65c42a3d 100644 */ async #provePresence(binding: PresenceBinding): Promise { const auth = this.#auth(); -@@ -1174,13 +1159,7 @@ export class PocketClient { +@@ -1184,13 +1169,7 @@ export class PocketClient { { binding }, auth, ); diff --git a/.github/audit/canaries/pocket-signin-challenge-underived.patch b/.github/audit/canaries/pocket-signin-challenge-underived.patch new file mode 100644 index 000000000..cf26f3f5e --- /dev/null +++ b/.github/audit/canaries/pocket-signin-challenge-underived.patch @@ -0,0 +1,99 @@ +Domain: remote +Rule: S271 — `signin` must sign `signinChallenge` of the Relay's challenge, never its bytes +Expect: lib/src/remote/client/pocket-client.ts +Source: reverses part of #1243 (Pocket signed the challenge the Relay's sign-in `begin` answered as given) + +diff --git a/lib/src/remote/client/pocket-client.test.ts b/lib/src/remote/client/pocket-client.test.ts +index b7a91c6..5a2ab8e 100644 +--- a/lib/src/remote/client/pocket-client.test.ts ++++ b/lib/src/remote/client/pocket-client.test.ts +@@ -34,12 +34,10 @@ import { + presenceChallenge, + pushEndpointFingerprint, + randomBase64Url, +- signinChallenge, + toBase64Url, + type PasskeyAssertion, + type PresenceBinding, + type ReauthBeginResponse, +- type SigninFinishRequest, + type TerminalDataEvent, + } from 'remote-lib-common'; + +@@ -2125,44 +2123,6 @@ describe('the presence proof', () => { + await pairing; + expect(phone.prompts.map((prompt) => prompt.allowCredentials)).toEqual([[harness.authenticator.credentialId]]); + }); +- +- it('signs only a sign-in challenge it derived, so a sign-in tap is never presence for another connection', async () => { +- const { harness, binding, nonce, present } = await anotherConnectionOnThePairedStatic(); +- const relayChosen = await presenceChallenge(binding, nonce); +- +- // A Relay that answers sign-in's `begin` with that connection's presence challenge. +- const prompts: string[] = []; +- const finishes: SigninFinishRequest[] = []; +- const fetch = (async (url: string, init?: RequestInit) => { +- const path = new URL(url, 'http://test').pathname; +- if (path === '/api/signin/begin') { +- return { ok: true, status: 200, json: async () => ({ challenge: relayChosen, rpId: RP_ID }) } as Response; +- } +- if (path === '/api/signin/finish') finishes.push(JSON.parse(init!.body as string) as SigninFinishRequest); +- return harness.fetch(url, init); +- }) as typeof globalThis.fetch; +- const phone = new PocketClient({ +- wsBase: 'ws://test', +- fetch, +- webauthn: { +- ...fakeWebAuthn, +- getAssertion: (challenge) => { +- prompts.push(challenge); +- return harness.authenticator.assert(challenge, ORIGIN); +- }, +- }, +- createWebSocket: () => harness.relay.openClientSocket(), +- knownBurrows: memoryKnownBurrows(), +- pendingDeletions: memoryPendingDeletions(), +- storage: memoryStorage(), +- }); +- await phone.signin(); +- expect(prompts).toEqual([await signinChallenge(relayChosen)]); +- expect(finishes.map((finish) => finish.challenge)).toEqual([relayChosen]); +- +- // That connection presents the sign-in assertion as its presence proof. +- await present(finishes[0]!.assertion); +- }); + }); + + /** +diff --git a/lib/src/remote/client/pocket-client.ts b/lib/src/remote/client/pocket-client.ts +index 33381cf..3a96c14 100644 +--- a/lib/src/remote/client/pocket-client.ts ++++ b/lib/src/remote/client/pocket-client.ts +@@ -43,7 +43,6 @@ import { + pushSubscriptionDeletePath, + randomBase64Url, + samplePairingCode, +- signinChallenge, + toBase64Url, + type ConnectionDenialCode, + type ConnectionRequestV1, +@@ -606,17 +605,10 @@ export class PocketClient { + } + } + +- /** +- * Sign in with a discoverable passkey; the session is kept across launches. +- * +- * **The authenticator signs `signinChallenge(begin.challenge)`, never +- * `begin`'s bytes**, so a Relay cannot pass off a presence challenge for a +- * connection of its own as a sign-in and present the tap to a Burrow. +- */ ++ /** Sign in with a discoverable passkey; the session is kept across launches. */ + async signin(): Promise { + const begin = await this.#api(API_ROUTES.signinBegin, {}); +- const challenge = await signinChallenge(begin.challenge); +- const assertion = await this.#webauthn.getAssertion(challenge, begin.rpId); ++ const assertion = await this.#webauthn.getAssertion(begin.challenge, begin.rpId); + const request: SigninFinishRequest = { challenge: begin.challenge, assertion }; + const finish = await this.#api(API_ROUTES.signinFinish, request); + this.#setSession({ diff --git a/docs/specs/relay.md b/docs/specs/relay.md index 322132833..b4519d1cd 100644 --- a/docs/specs/relay.md +++ b/docs/specs/relay.md @@ -81,7 +81,7 @@ Source of truth: `relay/src/state.ts`, including `MAX_PUSH_SUBSCRIPTIONS_TOTAL`; No WebAuthn library (rationale): registration requests `attestation: 'none'` and reads the SPKI key from `response.getPublicKey()`. **Assertions go through `verifyPasskeyAssertion` in `remote-lib-common`, the Burrow's own verifier**, so Relay and Burrow cannot disagree on a valid assertion. - **Registration** (`checkRegistration`, shared with the Hosted Relay) **must redeem the challenge before the origin check**, so a wrong-origin registration still burns it, and requires a key that imports as ECDSA P-256. A credential id already stored is a 409, so re-registration cannot displace a stored key. -- **Must verify sign-in against the stored passkey under the Relay's UV policy**, consuming the challenge first; re-auth is `docs/specs/remote-security-model.md` -> "Presence proofs". An unknown credential is 404 on both. Both Relays refuse with the error constants in `remote-lib-common/src/remote/wire.ts`. +- **Must verify sign-in against the stored passkey under the Relay's UV policy**, over `signinChallenge` of the `begin` challenge `finish` names (`docs/specs/remote-security-model.md` -> "Presence proofs"), consuming that challenge first; re-auth is `docs/specs/remote-security-model.md` -> "Presence proofs". An unknown credential is 404 on both. Both Relays refuse with the error constants in `remote-lib-common/src/remote/wire.ts`. - **Setup and sign-in each get their own `ChallengeIssuer`**, so a challenge minted for one cannot redeem in the other; each is capped (`MAX_PENDING_CHALLENGES`) as well as swept (rationale). Re-auth uses `PresenceNonceStore`. - **The browser's `clientDataJSON.challenge` is canonicalized by decoded bytes before lookup**, so a padded serialization redeems without weakening single use. diff --git a/docs/specs/remote-security-model.md b/docs/specs/remote-security-model.md index 956eacd22..1333fb122 100644 --- a/docs/specs/remote-security-model.md +++ b/docs/specs/remote-security-model.md @@ -71,9 +71,10 @@ Source of truth: `remote-lib-common/src/security/acl.ts` (the schema and `Burrow - **`POST /api/reauth/begin` takes a required, kind-tagged binding** and answers the derived challenge over a one-use Relay nonce ([relay.md](./relay.md) owns both routes). **No binding, or no nonce to `finish`, is a 400** (rationale). `finish` consumes the nonce, recomputes the challenge, verifies the assertion against the **stored** key for that exact credential, and **extends nothing** — not the session's life, not the relay socket. - **`PresenceProofV1` travels only inside the first Client→Burrow transport payload**, carrying the binding, the Relay nonce, `accountId` (as the Relay answered sign-in), the passkey credential id, its canonical SPKI public key, and the assertion. The Burrow recomputes the challenge with the same builder, requires **every binding field to equal what it built from its own state**, verifies RP ID, origin, presence/verification policy, and signature against the *presented* key, and hashes that key for the ACL. **A Relay success flag is never evidence**, and **the verifier never throws**, including missing WebCrypto or rejected digest operations — a rejection is an ordinary denial (rationale). - **Every proof is fresh and single-use**: any restart — dropped transport, consumed challenge, failed handshake, later attempt — needs a new handshake, Burrow challenge, Relay nonce, and authenticator operation; one prompt per pairing, one per connection outside a [presence window](#presence-window), two on a self-hosted first run (rationale). A window ride replays no proof; it carries none. +- **A sign-in assertion is never a presence assertion.** Pocket signs `signinChallenge(challenge)`, base64url `SHA-256(lengthPrefixedConcat(domain, challenge's bytes))` under `dormouse/signin/v1`, never `POST /api/signin/begin`'s challenge as given, and `finish` names that challenge beside the assertion; both Relays verify against the derivation (`docs/specs/security-remote.md` -> "Trust boundary"). - **The Relay session is authentication-plane only.** Its bearer token ([relay.md](./relay.md)) is never reusable proof of presence for a Burrow, and **has no app-session signing key beside it** (rationale). -Source of truth: `presenceChallenge` / `isPresenceBinding` in `remote-lib-common/src/security/presence.ts`, `verifyPresenceProof` in `remote-lib-common/src/security/e2e-ceremony.ts`, `#provePresence` in `lib/src/remote/client/pocket-client.ts`, `relay/src/app.ts`. +Source of truth: `presenceChallenge` / `signinChallenge` / `isPresenceBinding` in `remote-lib-common/src/security/presence.ts`, `verifyPresenceProof` in `remote-lib-common/src/security/e2e-ceremony.ts`, `#provePresence` in `lib/src/remote/client/pocket-client.ts`, `relay/src/app.ts`. ## Presence window diff --git a/docs/specs/security-remote.md b/docs/specs/security-remote.md index 7fcb99bce..e117dbc2d 100644 --- a/docs/specs/security-remote.md +++ b/docs/specs/security-remote.md @@ -35,7 +35,7 @@ The setup-password and `burrowToken` rows describe the self-host Relay's passkey Only one local confirmation of the phone's two digits mints an ACL record. The webview relays the immutable ceremony id and typed digits; it cannot read the expected code, choose the record, or fabricate a pending request, so a compromised webview gets one 1/100 guess per ceremony (rationale). Removal: `docs/specs/remote-security-model.md` -> "Revocation propagation". - **FAIL IF** `S177` the Burrow stops being the final authority: before any session is established, `BurrowRuntime` in `lib/src/remote/burrow/burrow-runtime.ts` must consume its own challenge, then either verify the presence proof with `verifyPresenceProof` against a binding built from its own `burrowId`, connection id, message-2 payload, and handshake hash, or redeem the presence window its message 2 offered, opened only from such a verified proof for the same identities and IK-authenticated Client static (`docs/specs/remote-security-model.md` -> "Presence window"), and require one active `BurrowAclRecord` holding the passkey credential, that key's hash, and the IK-authenticated Client static — with no Relay-supplied claim standing in for any of them, and no presence window held or named outside the Burrow: not by the Relay, Hosted, or the one-time runtime and phone. The record's account is a label the Relay asserted at sign-in: a mismatch denies, and a match authorizes nothing. -- **FAIL IF** `S271` Pocket has its authenticator sign a presence challenge it did not derive: `#provePresence` in `lib/src/remote/client/pocket-client.ts` must recompute `presenceChallenge` from its own binding and the Relay's nonce, refuse before any prompt when `begin` answers another, and offer only the binding's credential in `allowCredentials` (rationale). Pinned by `lib/src/remote/client/pocket-client.test.ts`. +- **FAIL IF** `S271` Pocket has its authenticator sign a challenge it did not derive: `#provePresence` in `lib/src/remote/client/pocket-client.ts` must recompute `presenceChallenge` from its own binding and the Relay's nonce, refuse before any prompt when `begin` answers another, and offer only the binding's credential in `allowCredentials`; `signin` must sign `signinChallenge` of the Relay's challenge, never its bytes; and any other `getAssertion` call in Pocket must sign a challenge it derived under a domain of its own (rationale). Pinned by `lib/src/remote/client/pocket-client.test.ts`. - **FAIL IF** `S178` local confirmation stops being the only thing that **mints** an ACL record: `BurrowAcl.approve` must have no caller but `BurrowRuntime.#approvePairing`, the comparison must be constant-time and happen **exactly once** per ceremony, and it must match the displayed request's immutable `pairingId`, never a mutable `clientId` alone; and the record it mints authorizes nothing until its durable save succeeds, nor anything after a save that failed. `scripts/e2e-lint.mjs` holds the callers textually. - **FAIL IF** `S179` the expected two-digit code, or an invitation's private key, ever leaves the Burrow process: `PairingQueueItem` in `lib/src/host/remote/service-protocol.ts` carries `{ kind, clientId, pairingId, label, requestedAt }` and nothing else (rationale). An answer is routed by its `kind` — a one-time request's only to the runtime that asked, by the random ticket its modal displayed — and **a missing `kind` is a pairing** (`approvalKind`), so an answer that names none can never reach a one-time request. - **FAIL IF** `S180` any rule in `docs/specs/remote-security-model.md` -> "Burrow bounds" stops being enforced by the Burrow itself, on its own clock, with no help from the relay: no `client-gone` it must receive, no Relay gate, and no deadline that waits for a frame. The pending-pairing cap binds **both** `BurrowRuntime`'s client map in `lib/src/remote/burrow/burrow-runtime.ts` and the service's mirrored queue, oldest evicted first, and outstanding invitations are capped at `MAX_TOKENS_PER_BURROW`. A handshake that fails to decrypt, or an `init` the bucket refuses, allocates no entry, and a frame the Burrow refuses performs no operation and allocates nothing. Pinned by `lib/src/remote/burrow/burrow-bounds.test.ts`, the service queue's cap by `lib/src/host/remote/service.test.ts`, and `relay/test/malicious-relay.test.mjs` (rationale). diff --git a/docs/specs/security-remote.rationale.md b/docs/specs/security-remote.rationale.md index 328e03e5a..04905e2fd 100644 --- a/docs/specs/security-remote.rationale.md +++ b/docs/specs/security-remote.rationale.md @@ -4,7 +4,7 @@ **Why a Relay compromise buys no authorization.** A forged account, a forged presence stamp, and an injected ceremony frame all arrive in front of a Burrow that decrypts the request itself, recomputes the WebAuthn challenge from its *own* transcript, and checks its own ACL under the `ConnectionPolicy` recorded at enrollment. It cannot make the Burrow trust a Client the user never approved. On an established session every frame is authenticated under a `CipherState` from a handshake the Relay does not hold a key for, and the first invalid ciphertext destroys the session rather than resynchronizing. Web Push is no exception for confidentiality — the Burrow seals every notification to the recipient's own static and the Relay forwards ciphertext it holds no key for — and is one for freshness, which is accepted residual rather than a gap the seal closes. -**Why Pocket recomputes the presence challenge too.** The Burrow checks a proof against the connection that presents it, so a phone that signed whatever challenge `begin` answered could have its tap counted for another connection on the same Client static rather than its own. Recomputing before the prompt keeps the Relay to minting a nonce, and naming only the paired credential keeps it from steering the prompt to another passkey on the account. +**Why Pocket recomputes the presence challenge too.** The Burrow checks a proof against the connection that presents it, so a phone that signed whatever challenge `begin` answered could have its tap counted for another connection on the same Client static rather than its own. Recomputing before the prompt keeps the Relay to minting a nonce, and naming only the paired credential keeps it from steering the prompt to another passkey on the account. Sign-in is the same hole from the other side: it asserts the same passkey under the same RP ID, and a verifier reads only the challenge `clientDataJSON` names, so a sign-in that signed `begin`'s bytes as given would let a Relay holding the static answer `begin` with a presence challenge for its own connection and, with one ordinary sign-in it can force with a 401, present the tap to the Burrow as presence. Hashing the sign-in challenge under its own domain makes the two kinds of assertion disjoint. **Why the burrow-token edge exists.** A `burrowToken` mints setup tokens and a setup token is the only thing that registers an owner passkey, so account takeover is transitive rather than direct. That is deliberate: the QR *is* the credential, so whatever can mint one can be set up by one. It still buys no Burrow access for a structural reason — pairing runs Noise IK against an invitation keypair the Burrow generated locally and never sent anywhere, so the Burrow has no invitation to match a stolen setup token against. diff --git a/hosted/server/relay-api.ts b/hosted/server/relay-api.ts index 4d4483e07..a68a75d36 100644 --- a/hosted/server/relay-api.ts +++ b/hosted/server/relay-api.ts @@ -339,8 +339,8 @@ export function relayApiRoutes(app: Hono<{ Bindings: RelayEnv }>) { app.post(API_ROUTES.signinFinish, (c) => database(c, async (db) => { - const body = await readJson<{ assertion?: unknown }>(c); - const verdict = await verifySigninAssertion(body?.assertion, { + const body = await readJson(c); + const verdict = await verifySigninAssertion(body, { findPasskey: async (credentialId) => { const { rows: [row], diff --git a/hosted/server/tests/relay-room.test.ts b/hosted/server/tests/relay-room.test.ts index 2103c3981..813123216 100644 --- a/hosted/server/tests/relay-room.test.ts +++ b/hosted/server/tests/relay-room.test.ts @@ -27,6 +27,7 @@ import { REMOTE_METHODS, SESSION_END_V1, generateNoiseKeyPair, + signinChallenge, utf8Encode, } from "remote-lib-common"; import { @@ -205,8 +206,8 @@ async function signedIn(burrowToken: string) { }); expect(finished.status).toBe(200); const challenge = (await call(API_ROUTES.signinBegin, { body: {} })).json.challenge; - const assertion = await authenticator.assert({ challenge, origin }); - const signed = await call(API_ROUTES.signinFinish, { body: { assertion } }); + const assertion = await authenticator.assert({ challenge: await signinChallenge(challenge), origin }); + const signed = await call(API_ROUTES.signinFinish, { body: { challenge, assertion } }); expect(signed.status).toBe(200); return { authenticator, sessionToken: signed.json.sessionToken as string }; } diff --git a/hosted/server/tests/relay.test.ts b/hosted/server/tests/relay.test.ts index f26bd4613..cedf9afcb 100644 --- a/hosted/server/tests/relay.test.ts +++ b/hosted/server/tests/relay.test.ts @@ -23,6 +23,7 @@ import { pushDeliveryKey, pushSubscriptionDeletePath, sealPush, + signinChallenge, utf8Encode, isEnrollUserCode, isManagedVoiceToken, @@ -226,8 +227,9 @@ async function fixture({ bindings = {} }: { bindings?: Record f.call("POST", API_ROUTES.signinFinish, { body: { assertion } })), + [0, 1].map(() => f.call("POST", API_ROUTES.signinFinish, { body: { challenge, assertion } })), ); expect(signins.map((result) => result.status).sort()).toEqual([200, 400]); + + // And only over the derived challenge: an assertion over `begin`'s bytes as + // given, the shape of a presence assertion over a Relay-chosen value, is refused. + const raw = (await f.call("POST", API_ROUTES.signinBegin)).json!.challenge as string; + const unhashed = await phone.assert({ challenge: raw, origin }); + expect( + (await f.call("POST", API_ROUTES.signinFinish, { body: { challenge: raw, assertion: unhashed } })).json, + ).toEqual({ error: "unrecognized or expired challenge" }); }); test("a refused finish puts the token back on its own expiry; a removed Burrow's tokens die with it", async ({ @@ -582,8 +592,8 @@ test("a setup challenge redeems only with its own Burrow's tokens", async ({ onT json: { error: "unrecognized or expired challenge" }, }); // Nor does a sign-in challenge register. - const signinChallenge = (await f.call("POST", API_ROUTES.signinBegin)).json!.challenge; - expect((await f.finish(phone, token, signinChallenge)).json).toEqual({ + const signinBegun = (await f.call("POST", API_ROUTES.signinBegin)).json!.challenge; + expect((await f.finish(phone, token, signinBegun)).json).toEqual({ error: "unrecognized or expired challenge", }); // Both codes survive the refusals, and the challenge is still its own Burrow's. @@ -591,8 +601,11 @@ test("a setup challenge redeems only with its own Burrow's tokens", async ({ onT const otherBegin = await f.call("POST", API_ROUTES.setupBegin, { body: { setupToken: other } }); expect(otherBegin.status).toBe(200); // A setup challenge never signs in, even for a registered credential. - const assertion = await phone.assert({ challenge: otherBegin.json!.challenge, origin }); - expect((await f.call("POST", API_ROUTES.signinFinish, { body: { assertion } })).json).toEqual({ + const setupChallenge = otherBegin.json!.challenge as string; + const assertion = await phone.assert({ challenge: await signinChallenge(setupChallenge), origin }); + expect( + (await f.call("POST", API_ROUTES.signinFinish, { body: { challenge: setupChallenge, assertion } })).json, + ).toEqual({ error: "unrecognized or expired challenge", }); }); diff --git a/lib/src/remote/client/pocket-client.test.ts b/lib/src/remote/client/pocket-client.test.ts index f4caf0200..aed99f3d6 100644 --- a/lib/src/remote/client/pocket-client.test.ts +++ b/lib/src/remote/client/pocket-client.test.ts @@ -34,10 +34,12 @@ import { presenceChallenge, pushEndpointFingerprint, randomBase64Url, + signinChallenge, toBase64Url, type PasskeyAssertion, type PresenceBinding, type ReauthBeginResponse, + type SigninFinishRequest, type TerminalDataEvent, } from 'remote-lib-common'; @@ -2054,15 +2056,20 @@ describe('the presence proof', () => { challenge: await presenceChallenge({ ...binding, handshakeHash: randomBase64Url(32) }, answer.relayNonce), }); - it('refuses a challenge bound to another connection before any prompt, so that one gains no presence', async () => { + /** + * A paired phone, past the window its pairing opened, and another connection + * on that phone's own paired static which the phone never opened: the binding + * and nonce a proof for it would carry, and `present`, which hands the Burrow + * an assertion as that connection's proof and expects it admitted to nothing, + * with no window opened for the static. + */ + async function anotherConnectionOnThePairedStatic() { let burrowClock = Date.now(); const harness = await makeE2eHarness({ burrowNow: () => burrowClock }); const { burrowId, relay, noiseStatic, authenticator } = harness; await harness.pairAndApprove(await harness.mintInvitation()); - // Past the window the pairing opened, so the connection needs a proof. burrowClock += PRESENCE_WINDOW_MAX_MS + 1; - // Another connection on this phone's own paired static, which this phone never opened. const record = harness.knownBurrows.records.get(burrowId)!; const clientStatic = { privateKey: record.clientStaticKeyPair.privateKey, @@ -2079,7 +2086,7 @@ describe('the presence proof', () => { }); const other = { clientId: testRoutingId(), connectionId: testRoutingId() }; const otherSession = await open(other.clientId, other.connectionId); - const otherBinding: PresenceBinding = { + const binding: PresenceBinding = { kind: 'connection', burrowId, connectionId: other.connectionId, @@ -2087,45 +2094,52 @@ describe('the presence proof', () => { handshakeHash: toBase64Url(otherSession.session.handshakeHash), passkeyCredentialId: authenticator.credentialId, }; - const otherNonce = randomBase64Url(32); + const nonce = randomBase64Url(32); + const present = async (assertion: PasskeyAssertion | undefined) => { + sendE2eFrame(relay.burrowSocket, { + clientId: other.clientId, + burrowId, + kind: 'connection', + id: other.connectionId, + step: 'transport', + ct: toBase64Url( + otherSession.session.sendControl({ + presence: { + binding, + relayNonce: nonce, + accountId: ACCOUNT_ID, + passkeyCredentialId: authenticator.credentialId, + passkeyPublicKey: authenticator.publicKey, + assertion, + }, + }), + ), + }); + expect( + await readOutcome(relay.burrowSocket, otherSession.session, 'connection', other.connectionId), + ).toMatchObject({ ok: false }); + expect((await open(testRoutingId(), testRoutingId())).offer).toBe('none'); + }; + return { harness, binding, nonce, present }; + } + + it('refuses a challenge bound to another connection before any prompt, so that one gains no presence', async () => { + const { harness, binding, nonce, present } = await anotherConnectionOnThePairedStatic(); const phone = await behindRelay(harness, async (answer) => ({ ...answer, - challenge: await presenceChallenge(otherBinding, otherNonce), + challenge: await presenceChallenge(binding, nonce), })); - await expect(phone.client.connect(burrowId)).rejects.toThrow(PRESENCE_CHALLENGE_MISMATCH_MESSAGE); + await expect(phone.client.connect(harness.burrowId)).rejects.toThrow(PRESENCE_CHALLENGE_MISMATCH_MESSAGE); expect(phone.prompts).toEqual([]); expect(phone.finished).toEqual([]); - // The other connection presents whatever reached `finish`: with no tap, the - // Burrow admits it to nothing and opens no window for the static. - sendE2eFrame(relay.burrowSocket, { - clientId: other.clientId, - burrowId, - kind: 'connection', - id: other.connectionId, - step: 'transport', - ct: toBase64Url( - otherSession.session.sendControl({ - presence: { - binding: otherBinding, - relayNonce: otherNonce, - accountId: ACCOUNT_ID, - passkeyCredentialId: authenticator.credentialId, - passkeyPublicKey: authenticator.publicKey, - assertion: phone.finished[0], - }, - }), - ), - }); - expect( - await readOutcome(relay.burrowSocket, otherSession.session, 'connection', other.connectionId), - ).toMatchObject({ ok: false }); - expect((await open(testRoutingId(), testRoutingId())).offer).toBe('none'); + // The other connection presents whatever reached `finish`: with no tap, it gains nothing. + await present(phone.finished[0]); // An honest `begin` still connects, with the one prompt it needs. const honest = await behindRelay(harness, async (answer) => answer); - expect(await honest.client.connect(burrowId)).toEqual({ ok: true, burrowLabel: BURROW_LABEL }); + expect(await honest.client.connect(harness.burrowId)).toEqual({ ok: true, burrowLabel: BURROW_LABEL }); expect(honest.prompts).toHaveLength(1); }); @@ -2156,6 +2170,44 @@ describe('the presence proof', () => { await pairing; expect(phone.prompts.map((prompt) => prompt.allowCredentials)).toEqual([[harness.authenticator.credentialId]]); }); + + it('signs only a sign-in challenge it derived, so a sign-in tap is never presence for another connection', async () => { + const { harness, binding, nonce, present } = await anotherConnectionOnThePairedStatic(); + const relayChosen = await presenceChallenge(binding, nonce); + + // A Relay that answers sign-in's `begin` with that connection's presence challenge. + const prompts: string[] = []; + const finishes: SigninFinishRequest[] = []; + const fetch = (async (url: string, init?: RequestInit) => { + const path = new URL(url, 'http://test').pathname; + if (path === '/api/signin/begin') { + return { ok: true, status: 200, json: async () => ({ challenge: relayChosen, rpId: RP_ID }) } as Response; + } + if (path === '/api/signin/finish') finishes.push(JSON.parse(init!.body as string) as SigninFinishRequest); + return harness.fetch(url, init); + }) as typeof globalThis.fetch; + const phone = new PocketClient({ + wsBase: 'ws://test', + fetch, + webauthn: { + ...fakeWebAuthn, + getAssertion: (challenge) => { + prompts.push(challenge); + return harness.authenticator.assert(challenge, ORIGIN); + }, + }, + createWebSocket: () => harness.relay.openClientSocket(), + knownBurrows: memoryKnownBurrows(), + pendingDeletions: memoryPendingDeletions(), + storage: memoryStorage(), + }); + await phone.signin(); + expect(prompts).toEqual([await signinChallenge(relayChosen)]); + expect(finishes.map((finish) => finish.challenge)).toEqual([relayChosen]); + + // That connection presents the sign-in assertion as its presence proof. + await present(finishes[0]!.assertion); + }); }); /** diff --git a/lib/src/remote/client/pocket-client.ts b/lib/src/remote/client/pocket-client.ts index 46be491a6..bc8c2e6fe 100644 --- a/lib/src/remote/client/pocket-client.ts +++ b/lib/src/remote/client/pocket-client.ts @@ -43,6 +43,7 @@ import { pushSubscriptionDeletePath, randomBase64Url, samplePairingCode, + signinChallenge, toBase64Url, type ConnectionDenialCode, type ConnectionRequestV1, @@ -69,6 +70,7 @@ import { type SetupBeginResponse, type SetupFinishResponse, type SigninBeginResponse, + type SigninFinishRequest, type SigninFinishResponse, type TerminalAttachResult, } from 'remote-lib-common'; @@ -605,11 +607,19 @@ export class PocketClient { } } - /** Sign in with a discoverable passkey; the session is kept across launches. */ + /** + * Sign in with a discoverable passkey; the session is kept across launches. + * + * **The authenticator signs `signinChallenge(begin.challenge)`, never + * `begin`'s bytes**, so a Relay cannot pass off a presence challenge for a + * connection of its own as a sign-in and present the tap to a Burrow. + */ async signin(): Promise { const begin = await this.#api(API_ROUTES.signinBegin, {}); - const assertion = await this.#webauthn.getAssertion(begin.challenge, begin.rpId); - const finish = await this.#api(API_ROUTES.signinFinish, { assertion }); + const challenge = await signinChallenge(begin.challenge); + const assertion = await this.#webauthn.getAssertion(challenge, begin.rpId); + const request: SigninFinishRequest = { challenge: begin.challenge, assertion }; + const finish = await this.#api(API_ROUTES.signinFinish, request); this.#setSession({ token: finish.sessionToken, accountId: finish.accountId, diff --git a/relay/src/app.ts b/relay/src/app.ts index 3cc50e1b9..709060144 100644 --- a/relay/src/app.ts +++ b/relay/src/app.ts @@ -727,7 +727,7 @@ export function createApp(config: AppConfig): CreatedApp { // The pipeline both Relays run (`verifySigninAssertion`), under the same // Relay-wide UV policy re-auth enforces, so sign-in is not a softer path // than a presence proof when UV is required. - const verdict = await verifySigninAssertion(body?.assertion, { + const verdict = await verifySigninAssertion(body, { findPasskey: (credentialId) => accounts.findPasskey(credentialId), consumeChallenge: (challenge) => signinChallenges.consume(challenge), policy: { origin, rpId, requireUserVerification: config.requireUserVerification }, diff --git a/relay/test/helpers.mjs b/relay/test/helpers.mjs index 37e0b15f4..e8a3fbe5d 100644 --- a/relay/test/helpers.mjs +++ b/relay/test/helpers.mjs @@ -10,7 +10,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { serve } from '@hono/node-server'; -import { API_ROUTES, WS_ROUTES, WS_TOKEN_PARAM } from 'remote-lib-common'; +import { API_ROUTES, WS_ROUTES, WS_TOKEN_PARAM, signinChallenge } from 'remote-lib-common'; import { createApp } from '../dist/app.js'; import { @@ -151,13 +151,16 @@ export async function register(app, authenticator, options = {}) { }); } -/** begin → assert → finish sign-in for `authenticator`; returns the finish Response. */ +/** + * begin → assert → finish sign-in for `authenticator`, signing the derived + * `signinChallenge` as Pocket does; returns the finish Response. + */ export async function signin(app, authenticator, { origin = ORIGIN, rpId = RP_ID, tamper } = {}) { const begin = await post(app, API_ROUTES.signinBegin, {}); const { challenge } = await begin.json(); - const assertion = await authenticator.assert({ challenge, origin, rpId, tamper }); - const res = await post(app, API_ROUTES.signinFinish, { assertion }); - return { res, assertion }; + const assertion = await authenticator.assert({ challenge: await signinChallenge(challenge), origin, rpId, tamper }); + const res = await post(app, API_ROUTES.signinFinish, { challenge, assertion }); + return { res, assertion, challenge }; } // --- Slice 2: live Relay + WebSocket relay scaffolding -------------------- diff --git a/relay/test/signin.test.mjs b/relay/test/signin.test.mjs index 036f0c762..790169223 100644 --- a/relay/test/signin.test.mjs +++ b/relay/test/signin.test.mjs @@ -8,7 +8,7 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { Hono } from 'hono'; -import { API_ROUTES } from 'remote-lib-common'; +import { API_ROUTES, signinChallenge } from 'remote-lib-common'; import { SimAuthenticator } from '../../remote-lib-common/test/harness/actors.mjs'; import { MAX_PENDING_CHALLENGES } from '../dist/app.js'; @@ -58,11 +58,11 @@ test('sign-in rejects a replayed challenge/assertion', async () => { const authenticator = await newAuthenticator(); assert.equal((await register(app, authenticator)).status, 200); - const { res, assertion } = await signin(app, authenticator); + const { res, assertion, challenge } = await signin(app, authenticator); assert.equal(res.status, 200); // Same assertion again — its challenge was consumed on the first finish. - const replay = await post(app, API_ROUTES.signinFinish, { assertion }); + const replay = await post(app, API_ROUTES.signinFinish, { challenge, assertion }); assert.equal(replay.status, 400); assert.match((await replay.json()).error, /challenge/); }); @@ -74,16 +74,32 @@ test('sign-in accepts a padded base64url clientData challenge', async () => { const begin = await post(app, API_ROUTES.signinBegin, {}); const { challenge } = await begin.json(); + const signed = await signinChallenge(challenge); const assertion = await authenticator.assert({ - challenge, + challenge: signed, origin: ORIGIN, rpId: RP_ID, - tamper: { challenge: padBase64Url(challenge) }, + tamper: { challenge: padBase64Url(signed) }, }); - const res = await post(app, API_ROUTES.signinFinish, { assertion }); + const res = await post(app, API_ROUTES.signinFinish, { challenge, assertion }); assert.equal(res.status, 200); }); +test('sign-in refuses an assertion over begin’s bytes as given, so none is ever a presence assertion', async () => { + const { app } = await freshApp(); + const authenticator = await newAuthenticator(); + assert.equal((await register(app, authenticator)).status, 200); + + const begin = await post(app, API_ROUTES.signinBegin, {}); + const { challenge } = await begin.json(); + const raw = await authenticator.assert({ challenge, origin: ORIGIN, rpId: RP_ID }); + for (const body of [{ assertion: raw }, { challenge, assertion: raw }]) { + const res = await post(app, API_ROUTES.signinFinish, body); + assert.equal(res.status, 400); + assert.match((await res.json()).error, /unrecognized or expired challenge/); + } +}); + test('sign-in rejects a tampered signature', async () => { const { app } = await freshApp(); const authenticator = await newAuthenticator(); @@ -189,11 +205,11 @@ test('the sign-in challenge store is capped, not merely swept', async () => { // Evicted by the flood: a challenge the store no longer holds cannot redeem. const stale = await authenticator.assert({ - challenge: first.challenge, + challenge: await signinChallenge(first.challenge), origin: ORIGIN, rpId: RP_ID, }); - const res = await post(app, API_ROUTES.signinFinish, { assertion: stale }); + const res = await post(app, API_ROUTES.signinFinish, { challenge: first.challenge, assertion: stale }); assert.equal(res.status, 400); assert.match((await res.json()).error, /unrecognized or expired challenge/); diff --git a/remote-lib-common/src/remote/relay-common.ts b/remote-lib-common/src/remote/relay-common.ts index b0d2f3311..0b87d5318 100644 --- a/remote-lib-common/src/remote/relay-common.ts +++ b/remote-lib-common/src/remote/relay-common.ts @@ -23,6 +23,7 @@ import { type ConnectionPolicy, type PasskeyAssertion, } from '../security/passkey.js'; +import { signinChallenge } from '../security/presence.js'; import { boundedPushText } from '../security/push.js'; import { MAX_SEALED_PUSH_LENGTH, isSealedPushV1 } from '../security/push-seal.js'; import { getWebCrypto } from '../security/webcrypto.js'; @@ -361,13 +362,18 @@ export function assertionChallenge(assertion: { readonly clientDataJSON?: unknow /** * `POST /api/signin/finish`'s verifier, in order: the assertion's shape (400), * its credential's stored passkey through `findPasskey` (404), the challenge - * its `clientDataJSON` names (400), that challenge spent through - * `consumeChallenge` BEFORE verifying, so a captured assertion never replays - * even if verification succeeds (400), and `verifyPasskeyAssertion` against - * the stored key under `policy`, the same UV policy re-auth enforces (401). + * its `clientDataJSON` names (400), the `begin` challenge the request names + * spent through `consumeChallenge` BEFORE verifying, so a captured assertion + * never replays even if verification succeeds (400), the signed challenge + * being {@link signinChallenge} of that one and nothing else (400), and + * `verifyPasskeyAssertion` against the stored key under `policy`, the same UV + * policy re-auth enforces (401). + * + * The signed value is derived, never `begin`'s bytes, so no sign-in assertion + * is ever a presence assertion (`signinChallenge`). */ export async function verifySigninAssertion( - assertion: unknown, + request: unknown, { findPasskey, consumeChallenge, @@ -378,18 +384,24 @@ export async function verifySigninAssertion { - const candidate = assertion as Partial | null | undefined; + const body = (request ?? {}) as { readonly challenge?: unknown; readonly assertion?: unknown }; + const candidate = body.assertion as Partial | null | undefined; if (!candidate || typeof candidate !== 'object' || typeof candidate.credentialId !== 'string') { return refuse(400, MALFORMED_ASSERTION_ERROR); } const passkey = await findPasskey(candidate.credentialId); if (!passkey) return refuse(404, UNKNOWN_CREDENTIAL_ERROR); - const challenge = assertionChallenge(candidate); - if (challenge === null) return refuse(400, MALFORMED_CLIENT_DATA_ERROR); - if (!(await consumeChallenge(challenge))) return refuse(400, UNKNOWN_CHALLENGE_ERROR); + const signed = assertionChallenge(candidate); + if (signed === null) return refuse(400, MALFORMED_CLIENT_DATA_ERROR); + const issued = normalizeChallenge(body.challenge); + const expected = issued === null ? null : await signinChallenge(issued).catch(() => null); + if (issued === null || expected === null || !(await consumeChallenge(issued))) { + return refuse(400, UNKNOWN_CHALLENGE_ERROR); + } + if (signed !== expected) return refuse(400, UNKNOWN_CHALLENGE_ERROR); const result = await verifyPasskeyAssertion(candidate as PasskeyAssertion, passkey.publicKey, { ...policy, - challenge, + challenge: expected, }); if (!result.ok) return refuse(401, assertionRejectedError(result.reason)); return { ok: true, passkey }; diff --git a/remote-lib-common/src/remote/wire.ts b/remote-lib-common/src/remote/wire.ts index d90a6d6da..6e5fd9a1f 100644 --- a/remote-lib-common/src/remote/wire.ts +++ b/remote-lib-common/src/remote/wire.ts @@ -251,12 +251,18 @@ export interface SetupFinishResponse { } export interface SigninBeginResponse { - /** Base64url challenge for `navigator.credentials.get()`. */ + /** + * Base64url single-use challenge. The authenticator signs + * `signinChallenge(challenge)`, never these bytes. + */ challenge: string; rpId: string; } export interface SigninFinishRequest { + /** `begin`'s challenge, as answered. */ + challenge: string; + /** Signed over `signinChallenge(challenge)`. */ assertion: PasskeyAssertion; } export interface SigninFinishResponse { diff --git a/remote-lib-common/src/security/presence.ts b/remote-lib-common/src/security/presence.ts index 959556552..92384ee7b 100644 --- a/remote-lib-common/src/security/presence.ts +++ b/remote-lib-common/src/security/presence.ts @@ -22,6 +22,9 @@ import { getWebCrypto, type WebCryptoLike } from './webcrypto.js'; /** One domain per signed statement, like every other in this package. */ export const PRESENCE_DOMAIN = 'dormouse/presence/v1'; +/** The domain {@link signinChallenge} hashes under: never a presence statement. */ +export const SIGNIN_DOMAIN = 'dormouse/signin/v1'; + /** * The longest any single binding field may be. A type check alone bounds * nothing — a megabyte string is a `string` — and every real field here is a @@ -140,3 +143,31 @@ function bindingFields(binding: PresenceBinding): Uint8Array[] { utf8Encode(binding.passkeyCredentialId), ]; } + +/** + * The WebAuthn challenge a sign-in assertion signs: + * `SHA-256(lengthPrefixedConcat(SIGNIN_DOMAIN, relayChallenge))`, base64url, + * over the bytes `POST /api/signin/begin`'s challenge encodes. + * + * **Sign-in never signs a value the Relay chose.** Both kinds of assertion come + * from the same passkey under the same RP ID, and the verifier reads only the + * challenge `clientDataJSON` names, so a sign-in that signed `begin`'s bytes as + * given would let a Relay answer `begin` with a {@link presenceChallenge} for a + * connection of its own and present the user's sign-in tap to a Burrow as + * presence. Hashed under its own domain, a sign-in challenge equals a presence + * challenge only on a SHA-256 collision. + * + * **Throws on a challenge that is not base64url or is unbounded**; the Relay + * treats a throw as a malformed `finish`. + */ +export async function signinChallenge( + relayChallenge: string, + crypto: WebCryptoLike = getWebCrypto(), +): Promise { + if (!bounded(relayChallenge)) throw new Error('sign-in challenge is missing or over the field limit'); + const digest = await crypto.subtle.digest( + 'SHA-256', + lengthPrefixedConcat([utf8Encode(SIGNIN_DOMAIN), fromBase64Url(relayChallenge)]), + ); + return toBase64Url(new Uint8Array(digest)); +} diff --git a/remote-lib-common/test/presence.test.mjs b/remote-lib-common/test/presence.test.mjs index d1e1f9683..0c702a621 100644 --- a/remote-lib-common/test/presence.test.mjs +++ b/remote-lib-common/test/presence.test.mjs @@ -7,7 +7,13 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { createHash } from 'node:crypto'; -import { PRESENCE_DOMAIN, isPresenceBinding, presenceChallenge } from '../dist/index.js'; +import { + PRESENCE_DOMAIN, + SIGNIN_DOMAIN, + isPresenceBinding, + presenceChallenge, + signinChallenge, +} from '../dist/index.js'; const HANDSHAKE_HASH = Buffer.alloc(32, 0x11).toString('base64url'); const BURROW_CHALLENGE = Buffer.alloc(32, 0x22).toString('base64url'); @@ -162,3 +168,18 @@ test('a field that is not base64url throws rather than hashing garbage', async ( await assert.rejects(presenceChallenge({ ...PAIRING, handshakeHash: 'not base64url!' }, RELAY_NONCE)); await assert.rejects(presenceChallenge(PAIRING, 'not base64url!')); }); + +test('the sign-in challenge is the documented bytes, under its own domain', async () => { + const expected = expectedChallenge([utf8(SIGNIN_DOMAIN), b64u(RELAY_NONCE)]); + assert.equal(await signinChallenge(RELAY_NONCE), expected); + assert.notEqual(SIGNIN_DOMAIN, PRESENCE_DOMAIN); + // Never the Relay's bytes as given, and never a presence challenge over them. + assert.notEqual(expected, RELAY_NONCE); + assert.notEqual(expected, await presenceChallenge(PAIRING, RELAY_NONCE)); +}); + +test('the sign-in challenge refuses what it cannot bound or decode', async () => { + await assert.rejects(signinChallenge(undefined)); + await assert.rejects(signinChallenge('A'.repeat(1028))); + await assert.rejects(signinChallenge('not base64url!')); +}); diff --git a/remote-lib-common/test/relay-common.test.mjs b/remote-lib-common/test/relay-common.test.mjs index 6d7160b95..4740f69e6 100644 --- a/remote-lib-common/test/relay-common.test.mjs +++ b/remote-lib-common/test/relay-common.test.mjs @@ -20,6 +20,7 @@ import { normalizeChallenge, pocketContentSecurityPolicy, reducePasskeyLabel, + signinChallenge, toBase64Url, utf8Encode, } from '../dist/index.js'; @@ -184,7 +185,9 @@ test('checkRegistration runs its checks in order, redeeming the challenge before test('verifySigninAssertion spends the challenge before verifying, and verifies under the policy', async () => { const authenticator = await SimAuthenticator.create({ rpId: RP_ID, userVerification: false }); const challenge = randomSecret(); - const assertion = await authenticator.assert({ challenge, origin: ORIGIN }); + const signed = await signinChallenge(challenge); + const assertion = await authenticator.assert({ challenge: signed, origin: ORIGIN }); + const request = { challenge, assertion }; const passkey = { publicKey: authenticator.publicKey }; const steps = (live, policy = {}) => { const log = []; @@ -196,32 +199,67 @@ test('verifySigninAssertion spends the challenge before verifying, and verifies }; }; const ok = steps(true); - assert.deepEqual(await verifySigninAssertion(assertion, ok), { ok: true, passkey }); + assert.deepEqual(await verifySigninAssertion(request, ok), { ok: true, passkey }); assert.deepEqual(ok.log, [`find ${authenticator.credentialId}`, `consume ${challenge}`]); const refused = (status, error) => ({ ok: false, status, error }); assert.deepEqual(await verifySigninAssertion(undefined, steps(true)), refused(400, 'malformed assertion')); assert.deepEqual( - await verifySigninAssertion({ ...assertion, credentialId: 'other' }, steps(true)), + await verifySigninAssertion({ challenge, assertion: { ...assertion, credentialId: 'other' } }, steps(true)), refused(404, 'unknown credential'), ); assert.deepEqual( - await verifySigninAssertion({ ...assertion, clientDataJSON: '***' }, steps(true)), + await verifySigninAssertion({ challenge, assertion: { ...assertion, clientDataJSON: '***' } }, steps(true)), refused(400, 'malformed clientDataJSON'), ); - assert.deepEqual(await verifySigninAssertion(assertion, steps(false)), refused(400, 'unrecognized or expired challenge')); + assert.deepEqual(await verifySigninAssertion(request, steps(false)), refused(400, 'unrecognized or expired challenge')); + // A finish that names no `begin` challenge spends nothing. + for (const missing of [undefined, 'not base64url!', 'A'.repeat(1028)]) { + const none = steps(true); + assert.deepEqual( + await verifySigninAssertion({ challenge: missing, assertion }, none), + refused(400, 'unrecognized or expired challenge'), + ); + assert.deepEqual(none.log, [`find ${authenticator.credentialId}`]); + } // A forged signature is refused only after its challenge was spent: it never replays. const forged = await authenticator.assert({ - challenge, + challenge: signed, origin: ORIGIN, tamper: { signWith: await SimAuthenticator.foreignSigningKey() }, }); const spent = steps(true); - assert.deepEqual(await verifySigninAssertion(forged, spent), refused(401, 'assertion rejected: signature-invalid')); + assert.deepEqual( + await verifySigninAssertion({ challenge, assertion: forged }, spent), + refused(401, 'assertion rejected: signature-invalid'), + ); assert.equal(spent.log.at(-1), `consume ${challenge}`); // The UV policy reaches the verifier. assert.deepEqual( - await verifySigninAssertion(assertion, steps(true, { requireUserVerification: true })), + await verifySigninAssertion(request, steps(true, { requireUserVerification: true })), refused(401, 'assertion rejected: user-verification-missing'), ); }); + +test('verifySigninAssertion accepts only an assertion over the derived sign-in challenge', async () => { + const authenticator = await SimAuthenticator.create({ rpId: RP_ID, userVerification: false }); + const challenge = randomSecret(); + const passkey = { publicKey: authenticator.publicKey }; + const steps = () => ({ + findPasskey: async () => passkey, + consumeChallenge: () => true, + policy: { origin: ORIGIN, rpId: RP_ID }, + }); + const refusal = { ok: false, status: 400, error: 'unrecognized or expired challenge' }; + // Signed over `begin`'s own bytes: what a presence assertion over a + // Relay-chosen value would look like. + const raw = await authenticator.assert({ challenge, origin: ORIGIN }); + assert.deepEqual(await verifySigninAssertion({ challenge, assertion: raw }, steps()), refusal); + // Signed over another challenge's derivation. + const other = await authenticator.assert({ challenge: await signinChallenge(randomSecret()), origin: ORIGIN }); + assert.deepEqual(await verifySigninAssertion({ challenge, assertion: other }, steps()), refusal); + // A padded serialization of the derived challenge still signs in. + const signed = await signinChallenge(challenge); + const padded = await authenticator.assert({ challenge: signed, origin: ORIGIN, tamper: { challenge: `${signed}=` } }); + assert.deepEqual(await verifySigninAssertion({ challenge, assertion: padded }, steps()), { ok: true, passkey }); +}); diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index 0870e0730..0fa7257e3 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -28,7 +28,7 @@ "docs/specs/security-ci.md": 3400, "docs/specs/security-hosted.md": 3050, "docs/specs/security-local.md": 5900, - "docs/specs/security-remote.md": 7150, + "docs/specs/security-remote.md": 7200, "docs/specs/security-supply-chain.md": 1650, "docs/specs/security.md": 2750, "docs/specs/shortcuts.md": 1150,