diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index dbd8388..96b678c 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -14,10 +14,10 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 25 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: persist-credentials: false - - uses: actions/setup-node@v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 22 cache: npm @@ -30,7 +30,7 @@ jobs: mkdir -p security-artifacts npm sbom --sbom-format cyclonedx > security-artifacts/sbom.cdx.json find packages/widget/dist packages/demo/dist -type f -print0 | sort -z | xargs -0 sha256sum > security-artifacts/SHA256SUMS - - uses: actions/upload-artifact@v4 + - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: bitlogin-security-artifacts path: security-artifacts/ @@ -40,7 +40,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: persist-credentials: false fetch-depth: 0 @@ -51,6 +51,6 @@ jobs: - name: Enforce local randomness rules (entropy audit §13) run: semgrep scan --config .semgrep.yml --error --timeout 0 packages - name: Scan repository history for secrets - uses: gitleaks/gitleaks-action@v2 + uses: gitleaks/gitleaks-action@ff98106e4c7b2bc287b24eaf42907196329070c7 # v2 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.semgrep.yml b/.semgrep.yml index 2c66c73..c96d992 100644 --- a/.semgrep.yml +++ b/.semgrep.yml @@ -36,7 +36,14 @@ rules: - pattern: $C.getRandomValues(...) ?? $FALLBACK - pattern: $C.getRandomValues(...) || $FALLBACK - pattern: | - try { ... $C.getRandomValues(...) ... } catch(...) { ... return $FALLBACK; ... } + try { + ... + $C.getRandomValues(...) + ... + } catch ($E) { + ... + return $FALLBACK; + } - id: bitlogin-no-time-or-id-seeded-secrets languages: [typescript, javascript] diff --git a/CHANGELOG.md b/CHANGELOG.md index dd6c65b..aecf43b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,18 @@ site with its own vendored copy (see "Updating your integration" in sitting on an older vendored copy is missing a fix or behavior change it would plausibly want. Newest first. +## 2026-08-04 (security-audit response, same day) + +- **External signers are now verified, not trusted.** NIP-07 and NIP-46 + `signEvent` results are checked for a valid signature, the session's + identity, and field-for-field equality with the requested event; NIP-46 + `auth_url` is surfaced only as a validated https URL with its hostname + shown. Relay queries enforce `filter.limit` locally, drop replayed + duplicates, cap buffering, and can no longer be stranded by a relay dying + mid-query. Passkey PRF fallback assertions are pinned to the + just-created credential. If your integration relied on an extension + returning altered events, that now errors — by design. + ## 2026-08-04 - **CSP fix every self-hosting site needs: add `'wasm-unsafe-eval'` to diff --git a/docs/TODO.md b/docs/TODO.md index 9edc063..ce26d19 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -14,6 +14,24 @@ broken since 2026-07-31), the service-worker `cache: "reload"` precache fix, `window.bitlogin.activeMethod()/activeSession()`, and welcome/sign-in UI polish. +## Security audit response (2026-08-04, same day) + +All confirmed findings fixed and regression-tested: BL-16 (auth_url now +https-only via `sanitizeAuthUrl`, hostname shown on the link, hostile-scheme +tests), BL-17 (post-creation PRF fallback pinned via `allowCredentials` to +the created credential), BL-18 (dead-socket CLOSE can no longer strand a +query promise), BL-19 (local `filter.limit` enforcement + duplicate-id drop ++ 1000-event ceiling per buffered query), BL-20 (.semgrep.yml try/catch +pattern was unparseable — fixed and verified firing on hostile code under +the pinned Semgrep 1.171.0). Lows: NIP-07 and NIP-46 now verify returned +signed events against the request (signature, identity, field equality); +`Nip46Client.close()` wipes the client and conversation keys; passkey +labels are distinguishable; postcss ≥8.5.25; Actions pinned to commit SHAs. + +Deferred from the audit, non-security interop items: NIP-46 protocol-level +logout on teardown (no stable method name across deployed signers) and the +recommended `switch_relays` request. + Owner action items: 1. **Live-fire the NIP-46 QR with Amber** on bitlogin.network (flow was diff --git a/package-lock.json b/package-lock.json index e7a3dc9..07a19e6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1542,9 +1542,9 @@ } }, "node_modules/postcss": { - "version": "8.5.20", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.20.tgz", - "integrity": "sha512-lW616l85ucIQL+FocMmL7pQFPqBmwejrCMg+iPxyImlrANNJG9NHq/RkyCZopDhd8C3LA03PHRJDjkbGu8vvug==", + "version": "8.5.25", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.25.tgz", + "integrity": "sha512-DTPx3RWSSnWyzLxQnlH0rJP+EW5ekl16ZU4/psbIhA0e53kJfdgaN5vKM+xP7yJtXVu+nfdVFmlgFDEKAe4Pyw==", "dev": true, "funding": [ { diff --git a/packages/core/src/nostr/nip46.test.ts b/packages/core/src/nostr/nip46.test.ts index 2dcb28d..a75b66f 100644 --- a/packages/core/src/nostr/nip46.test.ts +++ b/packages/core/src/nostr/nip46.test.ts @@ -9,7 +9,8 @@ import { Nip46Client, Nip46ErrorResponse, Nip46RequestTimeoutError, - parseBunkerUri + parseBunkerUri, + sanitizeAuthUrl } from "./nip46.js"; import { getConversationKey, nip44Decrypt, nip44Encrypt } from "../crypto/nip44.js"; import { generatePrivateKey, getPublicKeyHex } from "../crypto/secp256k1.js"; @@ -60,6 +61,32 @@ describe("buildNostrconnectUri", () => { }); }); +describe("sanitizeAuthUrl", () => { + it("passes https and loopback-http, normalized", () => { + expect(sanitizeAuthUrl("https://approve.example/req?id=1")).toBe("https://approve.example/req?id=1"); + expect(sanitizeAuthUrl(" https://approve.example ")).toBe("https://approve.example/"); + expect(sanitizeAuthUrl("http://localhost:8080/ok")).toBe("http://localhost:8080/ok"); + expect(sanitizeAuthUrl("http://127.0.0.1/ok")).toBe("http://127.0.0.1/ok"); + }); + + it("rejects every active or non-https scheme and garbage", () => { + // eslint-disable-next-line no-script-url -- hostile input under test + for (const hostile of [ + "javascript:alert(document.domain)", + "data:text/html,", + "blob:https://x.example/abc", + "vbscript:msgbox(1)", + "file:///etc/passwd", + "http://evil.example/not-loopback", + "nostr:nevent1...", + "//protocol-relative.example", + "not a url" + ]) { + expect(sanitizeAuthUrl(hostile)).toBeNull(); + } + }); +}); + /** * A minimal in-test remote signer: listens for kind-24133 requests addressed * to its pubkey, decrypts them per client, and answers per `handle`. @@ -233,6 +260,62 @@ describe("Nip46Client against a fake bunker", () => { expect(authUrls).toEqual(["https://approve.example/req"]); }); + it("drops a hostile-scheme auth_url without consuming the surfacing slot", async () => { + bunker = new FakeBunker(relay.url); + const inner = bunker.handle.bind(bunker); + bunker.handle = (request) => { + if (request.method === "get_public_key") { + return [ + // eslint-disable-next-line no-script-url -- hostile input under test + { id: request.id, result: "auth_url", error: "javascript:alert(document.domain)" }, + { id: request.id, result: "auth_url", error: "https://approve.example/real" }, + ...inner(request) + ]; + } + return inner(request); + }; + await bunker.start(); + + const authUrls: string[] = []; + client = new Nip46Client({ + clientSecretKey: generatePrivateKey(), + pointer: { signerPubkey: bunker.signerPubkey, relayUrls: [relay.url] }, + onAuthUrl: (url) => authUrls.push(url), + requestTimeoutMs: 5000 + }); + expect(await client.getUserPublicKey()).toBe(bunker.userPubkey); + // The javascript: URL never surfaced; the later https one did. + expect(authUrls).toEqual(["https://approve.example/real"]); + }); + + it("rejects a signed event whose fields differ from the request", async () => { + bunker = new FakeBunker(relay.url); + const inner = bunker.handle.bind(bunker); + bunker.handle = (request) => { + if (request.method === "sign_event") { + const unsigned = JSON.parse(request.params[0]!) as { + kind: number; + content: string; + tags: string[][]; + created_at: number; + }; + const signed = signNostrEvent( + { ...unsigned, pubkey: bunker.userPubkey, content: "tampered" }, + bunker.userKey + ); + return [{ id: request.id, result: JSON.stringify(signed) }]; + } + return inner(request); + }; + await bunker.start(); + const c = makeClient(); + await c.connect(); + await c.getUserPublicKey(); + await expect( + c.signEvent({ kind: 1, content: "original", tags: [], created_at: 1_700_000_000 }) + ).rejects.toThrow(/different event/); + }); + it("times out with a typed error when the signer never answers", async () => { bunker = new FakeBunker(relay.url); bunker.handle = () => []; // listens, never replies diff --git a/packages/core/src/nostr/nip46.ts b/packages/core/src/nostr/nip46.ts index 18cf631..d43a7b8 100644 --- a/packages/core/src/nostr/nip46.ts +++ b/packages/core/src/nostr/nip46.ts @@ -29,6 +29,7 @@ import { getConversationKey, nip44Encrypt, nip44Decrypt } from "../crypto/nip44. import { getPublicKeyHex } from "../crypto/secp256k1.js"; import { randomBytes } from "../crypto/random.js"; import { bytesToHex } from "../crypto/encoding.js"; +import { wipe } from "../crypto/memory.js"; const HEX64 = /^[0-9a-f]{64}$/u; @@ -56,6 +57,26 @@ export class Nip46ErrorResponse extends Error { override name = "Nip46ErrorResponse"; } +/** + * Validates a signer-supplied auth_url before it may be shown as a link. + * Only https (or http to loopback, for local development signers) survives; + * anything else -- javascript:, data:, blob:, custom schemes, unparseable + * strings -- returns null and is never surfaced. + */ +export function sanitizeAuthUrl(candidate: string): string | null { + let url: URL; + try { + url = new URL(candidate.trim()); + } catch { + return null; + } + const isLoopback = url.hostname === "localhost" || url.hostname === "127.0.0.1"; + if (url.protocol === "https:" || (url.protocol === "http:" && isLoopback)) { + return url.toString(); + } + return null; +} + export function parseBunkerUri(uri: string): BunkerPointer { let url: URL; try { @@ -197,10 +218,18 @@ export class Nip46Client { if (!request) return; // answered already by a faster relay, or stale if (payload.result === "auth_url") { // Interim, not an answer: the signer wants interactive approval at the - // URL (carried in `error`, per NIP-46). Surface once, keep waiting. - if (!request.authUrlSeen) { - request.authUrlSeen = true; - if (typeof payload.error === "string" && payload.error) this.onAuthUrl?.(payload.error); + // URL (carried in `error`, per NIP-46). The URL is REMOTE-SIGNER + // CONTROLLED input that UIs render as a clickable link, so only a + // validated https URL is ever surfaced (sanitizeAuthUrl) -- a hostile + // signer must not be able to hand the page a javascript:/data: href. + // An invalid URL is dropped without consuming the one surfacing slot, + // so a subsequent well-formed auth_url can still arrive. + if (!request.authUrlSeen && typeof payload.error === "string" && payload.error) { + const authUrl = sanitizeAuthUrl(payload.error); + if (authUrl) { + request.authUrlSeen = true; + this.onAuthUrl?.(authUrl); + } } return; } @@ -289,6 +318,17 @@ export class Nip46Client { if (this.userPubkeyKnown && event.pubkey !== this.userPubkeyKnown) { throw new Error("The signer returned an event signed by a different identity."); } + // Signature and identity are necessary but not sufficient: the signer + // could sign a DIFFERENT event than the one requested. Enforce field + // equality so callers get exactly what they asked to have signed. + if ( + event.kind !== unsigned.kind || + event.content !== unsigned.content || + event.created_at !== unsigned.created_at || + JSON.stringify(event.tags) !== JSON.stringify(unsigned.tags) + ) { + throw new Error("The signer returned a different event than the one requested."); + } return event; } @@ -318,6 +358,10 @@ export class Nip46Client { request.reject(new Error("The signer connection was closed.")); } this.pending.clear(); + // Shortest-practical-lifetime handling (§11.10): the ephemeral client + // key and the conversation key derived from it die with the session. + wipe(this.clientSecretKey); + wipe(this.conversationKey); } } diff --git a/packages/core/src/nostr/relay.ts b/packages/core/src/nostr/relay.ts index 49cf29a..83df45d 100644 --- a/packages/core/src/nostr/relay.ts +++ b/packages/core/src/nostr/relay.ts @@ -50,6 +50,12 @@ export interface RelayConnectionOptions { connectTimeoutMs?: number; } +/** Hard ceiling on events buffered per query, regardless of the requested + * limit -- the local backstop against an adversarial relay spraying events + * before EOSE. Far above any legitimate BitLogin query (capsules and lists + * are single-digit result sets). */ +const MAX_BUFFERED_EVENTS = 1000; + export class RelayConnection { readonly url: string; private ws: WebSocketLike | null = null; @@ -58,6 +64,8 @@ export class RelayConnection { string, { events: NostrEvent[]; + /** Ids already buffered, for replay dedup (buffered queries only). */ + seenIds: Set; filter: NostrFilter; onEose: () => void; /** Present on live subscriptions (subscribeLive): each verified, @@ -151,8 +159,18 @@ export class RelayConnection { verifyNostrEvent(event) && matchesNostrFilter(event, sub.filter) ) { - if (sub.onEvent) sub.onEvent(event); - else sub.events.push(event); + if (sub.onEvent) { + sub.onEvent(event); + } else { + // Relays are untrusted: enforce the requested limit and a global + // ceiling LOCALLY, and drop replayed duplicates, so an adversarial + // relay cannot amplify memory/CPU by spraying copies before EOSE. + if (sub.seenIds.has(event.id)) return; + const cap = Math.min(sub.filter.limit ?? MAX_BUFFERED_EVENTS, MAX_BUFFERED_EVENTS); + if (sub.events.length >= cap) return; + sub.seenIds.add(event.id); + sub.events.push(event); + } } return; } @@ -236,7 +254,7 @@ export class RelayConnection { ): Promise<{ close: () => void }> { await this.connect(); const subId = bytesToHex(randomBytes(8)); - this.subs.set(subId, { events: [], filter, onEose: () => {}, onEvent }); + this.subs.set(subId, { events: [], seenIds: new Set(), filter, onEose: () => {}, onEvent }); this.send(["REQ", subId, filter]); return { close: () => { @@ -266,7 +284,14 @@ export class RelayConnection { const settle = (fail?: Error) => { clearTimeout(timer); this.subs.delete(subId); - this.send(["CLOSE", subId]); + try { + this.send(["CLOSE", subId]); + } catch { + // The socket died mid-query. The relay-side subscription died with + // it; what matters is settling the promise below REGARDLESS -- an + // unreliable relay must not strand a query (and with it everything + // queued behind the worker's serial queue) until reload. + } if (fail) reject(fail); else resolve(events); }; @@ -286,7 +311,7 @@ export class RelayConnection { ), timeoutMs, ); - this.subs.set(subId, { events, filter, onEose: () => settle() }); + this.subs.set(subId, { events, seenIds: new Set(), filter, onEose: () => settle() }); this.send(["REQ", subId, filter]); }); } diff --git a/packages/core/src/nostr/relayHardening.test.ts b/packages/core/src/nostr/relayHardening.test.ts new file mode 100644 index 0000000..170f543 --- /dev/null +++ b/packages/core/src/nostr/relayHardening.test.ts @@ -0,0 +1,79 @@ +/** Adversarial-relay hardening (audit BL-18, BL-19): a malicious or flaky + * relay must neither strand a query promise nor amplify memory before EOSE. */ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { MockRelay } from "../test-support/mockRelay.js"; +import { RelayConnection } from "./relay.js"; +import { signNostrEvent } from "./event.js"; +import { generatePrivateKey, getPublicKeyHex } from "../crypto/secp256k1.js"; + +describe("relay hardening", () => { + let relay: MockRelay; + let conn: RelayConnection; + + beforeEach(async () => { + relay = await MockRelay.start(); + conn = new RelayConnection(relay.url); + }); + + afterEach(async () => { + conn.close(); + await relay.close(); + }); + + function signedNote(key: Uint8Array, content: string, created_at = 1_700_000_000) { + return signNostrEvent( + { pubkey: getPublicKeyHex(key), created_at, kind: 1, tags: [], content }, + key + ); + } + + it("BL-18: a relay that dies mid-query still settles the promise", async () => { + await conn.connect(); + // Subscription opens against a relay that never answers (no EOSE), and + // the relay then goes away entirely before the client's timeout. + relay.suppressEose = true; + const key = generatePrivateKey(); + const pending = conn.queryOnce({ kinds: [1], authors: [getPublicKeyHex(key)] }, 400); + await new Promise((resolve) => setTimeout(resolve, 50)); // let REQ hit the wire + await relay.close(); + // Before the fix, the timeout path threw sending CLOSE on the dead + // socket and the promise never settled -- wedging everything behind the + // worker's serial queue. Now it must reject, promptly. + await expect(pending).rejects.toThrow(/did not answer/); + }); + + it("BL-19: replayed duplicates are dropped and filter.limit is enforced locally", async () => { + const key = generatePrivateKey(); + const note = signedNote(key, "hello"); + // A hostile relay spraying 20 copies of the same valid, filter-matching + // event before EOSE. + relay.unsolicitedQueryEvents = Array.from({ length: 20 }, () => note); + const limited = await conn.queryOnce( + { kinds: [1], authors: [getPublicKeyHex(key)], limit: 1 }, + 3000 + ); + expect(limited).toHaveLength(1); + }); + + it("BL-19: duplicates collapse even without an explicit limit", async () => { + const key = generatePrivateKey(); + const noteA = signedNote(key, "a"); + const noteB = signedNote(key, "b", 1_700_000_001); + relay.unsolicitedQueryEvents = [noteA, noteA, noteB, noteA, noteB]; + const events = await conn.queryOnce({ kinds: [1], authors: [getPublicKeyHex(key)] }, 3000); + expect(events).toHaveLength(2); + expect(new Set(events.map((e) => e.id)).size).toBe(2); + }); + + it("BL-19: distinct events beyond the requested limit are not buffered", async () => { + const key = generatePrivateKey(); + relay.unsolicitedQueryEvents = Array.from({ length: 10 }, (_, i) => + signedNote(key, `n${i}`, 1_700_000_000 + i) + ); + const events = await conn.queryOnce( + { kinds: [1], authors: [getPublicKeyHex(key)], limit: 3 }, + 3000 + ); + expect(events).toHaveLength(3); + }); +}); diff --git a/packages/core/src/test-support/mockRelay.ts b/packages/core/src/test-support/mockRelay.ts index fe8e058..3bfbb56 100644 --- a/packages/core/src/test-support/mockRelay.ts +++ b/packages/core/src/test-support/mockRelay.ts @@ -41,6 +41,10 @@ export class MockRelay { /** Frames injected before normal query results, even when they do not match * the requested filter. Models a malicious or buggy relay. */ public unsolicitedQueryEvents: unknown[] = []; + /** When true, REQ subscriptions never receive EOSE -- models a relay that + * goes silent (or dies) mid-query, leaving the client's timeout as the + * only way out. */ + public suppressEose = false; /** Addressable-event d tags whose publishes are refused. Lets ordering tests * reject recovery writes while still accepting credential writes (or vice * versa), independently of the signing identity. */ @@ -101,7 +105,7 @@ export class MockRelay { const matches = this.query(filters[0] ?? {}); for (const event of this.unsolicitedQueryEvents) socket.send(JSON.stringify(["EVENT", subId, event])); for (const event of matches) socket.send(JSON.stringify(["EVENT", subId, event])); - socket.send(JSON.stringify(["EOSE", subId])); + if (!this.suppressEose) socket.send(JSON.stringify(["EOSE", subId])); let subsForSocket = this.liveSubs.get(socket); if (!subsForSocket) { subsForSocket = new Map(); @@ -176,6 +180,10 @@ export class MockRelay { } async close(): Promise { + // Terminate live sockets first: wss.close() only stops listening and its + // callback waits for every client to leave -- a test closing the relay + // mid-query (the BL-18 scenario) would otherwise hang here forever. + for (const socket of this.wss.clients) socket.terminate(); return new Promise((resolve) => { this.wss.close(() => resolve()); }); diff --git a/packages/widget/src/element.ts b/packages/widget/src/element.ts index c9820c7..c989d1c 100644 --- a/packages/widget/src/element.ts +++ b/packages/widget/src/element.ts @@ -1045,8 +1045,13 @@ export class BitLoginAuthElement extends HTMLElement { let prf = extractPrfOutput(credential); if (!prf && kind === "create") { // Some authenticators only reveal PRF results during assertion; ask - // for one immediately against the passkey that was just created. - const assertion = (await navigator.credentials.get(buildPasskeyGetOptions())) as PublicKeyCredential | null; + // for one immediately, pinned via allowCredentials to EXACTLY the + // passkey that was just created -- an unrestricted discovery here + // would let the sheet offer an older passkey and silently bind a + // different account (audit BL-17). + const assertion = (await navigator.credentials.get( + buildPasskeyGetOptions(undefined, credential.rawId) + )) as PublicKeyCredential | null; prf = assertion ? extractPrfOutput(assertion) : null; } if (!prf) { @@ -1717,10 +1722,19 @@ export class BitLoginAuthElement extends HTMLElement { `; case "bunker-connect": { - const authNotice = this.bunkerAuthUrl - ? `
Your signer asks you to approve this connection first. - Open the approval page, then return here.
` - : ""; + // The auth URL is validated https-only in core (sanitizeAuthUrl); + // parse defensively anyway and show the HOSTNAME so the user sees + // where the signer is sending them before they click. + let authNotice = ""; + if (this.bunkerAuthUrl) { + try { + const authHost = new URL(this.bunkerAuthUrl).hostname; + authNotice = `
Your signer asks you to approve this connection first. + Open the approval page at ${escapeHtml(authHost)}, then return here.
`; + } catch { + authNotice = ""; + } + } const qrBlock = this.bunkerConnectUri ? `
${renderQrSvg(this.bunkerConnectUri, "Nostr Connect QR code")}
diff --git a/packages/widget/src/passkey.test.ts b/packages/widget/src/passkey.test.ts index a234615..94bc225 100644 --- a/packages/widget/src/passkey.test.ts +++ b/packages/widget/src/passkey.test.ts @@ -75,6 +75,22 @@ describe("ceremony options", () => { expect(buildPasskeyGetOptions("site.example").publicKey!.rpId).toBe("site.example"); expect(buildPasskeyCreateOptions("BitLogin", "site.example").publicKey!.rp.id).toBe("site.example"); }); + + it("pins the assertion to one credential when given (audit BL-17)", () => { + // The post-creation PRF fallback MUST scope discovery to the passkey + // just created; an open sheet could bind an older passkey's account. + const rawId = new Uint8Array(16).fill(3).buffer; + const options = buildPasskeyGetOptions(undefined, rawId).publicKey!; + expect(options.allowCredentials).toEqual([{ type: "public-key", id: rawId }]); + }); + + it("labels each created passkey distinguishably (audit follow-up)", () => { + const a = buildPasskeyCreateOptions("BitLogin").publicKey!; + const b = buildPasskeyCreateOptions("BitLogin").publicKey!; + expect(a.user.name).toMatch(/^BitLogin \([0-9a-f]{4}\)$/u); + expect(a.user.displayName).toBe(a.user.name); + expect(a.user.name).not.toBe(b.user.name); + }); }); describe("extractPrfOutput", () => { diff --git a/packages/widget/src/passkey.ts b/packages/widget/src/passkey.ts index 172816d..ff2e126 100644 --- a/packages/widget/src/passkey.ts +++ b/packages/widget/src/passkey.ts @@ -59,8 +59,14 @@ export function passkeySupported(w: { PublicKeyCredential?: unknown } = window): } /** Options for the sign-in ceremony: discoverable credential, user - * verification required, PRF evaluated in the same step. */ -export function buildPasskeyGetOptions(rpId?: string): CredentialRequestOptions { + * verification required, PRF evaluated in the same step. + * + * `allowCredentialId` pins the assertion to ONE specific credential. It is + * required for the post-creation PRF fallback: an unrestricted discovery + * there would let the browser's account sheet offer OLDER passkeys, and a + * user picking one would silently bind a different account than the passkey + * they just created (audit BL-17). */ +export function buildPasskeyGetOptions(rpId?: string, allowCredentialId?: ArrayBuffer): CredentialRequestOptions { return { publicKey: { // The challenge protects sign-count/liveness in server-verified @@ -68,6 +74,9 @@ export function buildPasskeyGetOptions(rpId?: string): CredentialRequestOptions // is whether the derived credential opens the capsule. Random anyway. challenge: asBufferSource(randomBytes(32)), ...(rpId ? { rpId } : {}), + ...(allowCredentialId + ? { allowCredentials: [{ type: "public-key" as const, id: allowCredentialId }] } + : {}), userVerification: "required", extensions: { prf: { eval: { first: PASSKEY_PRF_SALT } } } as AuthenticationExtensionsClientInputs } @@ -77,6 +86,10 @@ export function buildPasskeyGetOptions(rpId?: string): CredentialRequestOptions /** Options for creating the passkey. Discoverable + user-verified, so later * sign-ins need no username at all. */ export function buildPasskeyCreateOptions(rpName: string, rpId?: string): CredentialCreationOptions { + // A short suffix so several BitLogin passkeys in one password manager are + // tellable apart (audit follow-up: identical names made multiple + // passkey-derived accounts indistinguishable in the credential sheet). + const label = `${rpName} (${bytesToHex(randomBytes(2))})`; return { publicKey: { challenge: asBufferSource(randomBytes(32)), @@ -85,8 +98,8 @@ export function buildPasskeyCreateOptions(rpName: string, rpId?: string): Creden // Opaque handle; the account identity is DERIVED from the PRF, not // from this. Random so re-creates don't silently overwrite. id: asBufferSource(randomBytes(16)), - name: rpName, - displayName: rpName + name: label, + displayName: label }, pubKeyCredParams: [ { type: "public-key", alg: -7 }, // ES256 diff --git a/packages/widget/src/signers/nip07.test.ts b/packages/widget/src/signers/nip07.test.ts index 02db44b..4fb20b1 100644 --- a/packages/widget/src/signers/nip07.test.ts +++ b/packages/widget/src/signers/nip07.test.ts @@ -1,18 +1,18 @@ import { describe, expect, it } from "vitest"; +import { signNostrEvent } from "@bitlogin/core/nostr"; +import { generatePrivateKey, getPublicKeyHex } from "@bitlogin/core/crypto"; import { detectForeignNip07Provider, Nip07Signer, type ForeignNip07Provider } from "./nip07.js"; import { SignerTimeoutError, SignerUnsupportedError } from "./types.js"; -const PUBKEY = "a".repeat(64); +// A real key: the signer now VERIFIES what the "extension" returns, so the +// fake must produce genuinely signed events. +const EXTENSION_KEY = generatePrivateKey(); +const PUBKEY = getPublicKeyHex(EXTENSION_KEY); function fullProvider(overrides: Partial = {}): ForeignNip07Provider { return { getPublicKey: async () => PUBKEY, - signEvent: async (event) => ({ - ...event, - id: "e".repeat(64), - pubkey: PUBKEY, - sig: "f".repeat(128) - }), + signEvent: async (event) => signNostrEvent({ ...event, pubkey: PUBKEY }, EXTENSION_KEY), getRelays: async () => ({}), nip44: { encrypt: async (peer, plaintext) => `nip44(${peer.slice(0, 4)},${plaintext})`, @@ -90,7 +90,7 @@ describe("Nip07Signer delegation", () => { fullProvider({ signEvent: async (event) => { seen = event; - return { ...event, id: "e".repeat(64), pubkey: PUBKEY, sig: "f".repeat(128) }; + return signNostrEvent({ ...event, pubkey: PUBKEY }, EXTENSION_KEY); } }) ); @@ -103,6 +103,32 @@ describe("Nip07Signer delegation", () => { expect(seen!.created_at).toBe(1234); }); + it("rejects a returned event whose fields differ from the request", async () => { + const signer = new Nip07Signer( + fullProvider({ + signEvent: async (event) => + signNostrEvent({ ...event, pubkey: PUBKEY, content: "tampered" }, EXTENSION_KEY) + }) + ); + await expect(signer.signEvent({ kind: 1, content: "original" })).rejects.toThrow( + /different event/ + ); + }); + + it("rejects an event signed by a different identity than the session's", async () => { + const otherKey = generatePrivateKey(); + const signer = new Nip07Signer( + fullProvider({ + signEvent: async (event) => + signNostrEvent({ ...event, pubkey: getPublicKeyHex(otherKey) }, otherKey) + }) + ); + await signer.getPublicKey(); // pins the session identity + await expect(signer.signEvent({ kind: 1, content: "x" })).rejects.toThrow( + /different identity/ + ); + }); + it("passes nip44/nip04 calls through and surfaces the extension's own rejections", async () => { const signer = new Nip07Signer(fullProvider()); expect(await signer.nip44Encrypt(PUBKEY, "hi")).toBe(`nip44(${PUBKEY.slice(0, 4)},hi)`); diff --git a/packages/widget/src/signers/nip07.ts b/packages/widget/src/signers/nip07.ts index b3faad6..c1beb3f 100644 --- a/packages/widget/src/signers/nip07.ts +++ b/packages/widget/src/signers/nip07.ts @@ -7,7 +7,7 @@ * owns that slot, and fighting it (claimSigner) is reserved for BitLogin-account * sessions where the user explicitly chose BitLogin as their signer. */ -import type { NostrEvent } from "@bitlogin/core/nostr"; +import { verifyNostrEvent, type NostrEvent } from "@bitlogin/core/nostr"; import type { UnsignedEventForSigning } from "../provider.js"; import { SignerTimeoutError, @@ -96,12 +96,18 @@ export class Nip07Signer implements Signer { }; } + /** Identity this extension reported; later signEvent results must come + * from it -- a multi-profile extension switching accounts mid-session + * must surface as an error, not as silently mixed authorship. */ + private knownPublicKey: string | null = null; + async getPublicKey(): Promise { const raw = await withDeadline(this.provider.getPublicKey(), "getPublicKey", this.timeoutMs); const publicKey = typeof raw === "string" ? raw.toLowerCase() : ""; if (!/^[0-9a-f]{64}$/.test(publicKey)) { throw new Error("The extension returned an invalid public key (expected 64 hex characters)."); } + this.knownPublicKey = publicKey; return publicKey; } @@ -115,7 +121,25 @@ export class Nip07Signer implements Signer { tags: (event.tags ?? []) as string[][], created_at: event.created_at ?? Math.floor(Date.now() / 1000) }; - return withDeadline(this.provider.signEvent(normalized), "signEvent", this.timeoutMs); + const signed = await withDeadline(this.provider.signEvent(normalized), "signEvent", this.timeoutMs); + // The extension is an external signer: verify what came back before any + // caller treats it as the event they asked for. NIP-07's contract is + // "the supplied event plus id/pubkey/sig" -- enforce exactly that. + if (!verifyNostrEvent(signed)) { + throw new Error("The extension returned an event that does not verify."); + } + if (this.knownPublicKey && signed.pubkey !== this.knownPublicKey) { + throw new Error("The extension signed with a different identity than this session's."); + } + if ( + signed.kind !== normalized.kind || + signed.content !== normalized.content || + signed.created_at !== normalized.created_at || + JSON.stringify(signed.tags) !== JSON.stringify(normalized.tags) + ) { + throw new Error("The extension returned a different event than the one requested."); + } + return signed; } async nip44Encrypt(peerPublicKey: string, plaintext: string): Promise {