From 6bcf84c79b497137357edf26cec21323072f7fb9 Mon Sep 17 00:00:00 2001 From: huggingbot <83656073+huggingbot@users.noreply.github.com> Date: Wed, 28 Oct 2026 17:12:58 +0800 Subject: [PATCH 1/4] feat: prove new SIWE identifiers with recovery mutations Send identifierAuthorizations as a list sharing one challenge per escrow (escrow #7): one proof per SIWE identifier the escrow has not enrolled, plus the enrolled-identifier proof for updates. Co-authored-by: Cursor --- .../mfa-recovery-controller/ARCHITECTURE.md | 33 +++-- .../CUBIST_TEST_SEQUENCE.md | 49 ++++--- packages/mfa-recovery-controller/README.md | 10 +- .../src/MfaRecoveryController.test.ts | 73 ++++++++++ .../src/MfaRecoveryController.ts | 125 +++++++++++------ .../cubist-escrow-provider.test.ts | 18 +-- .../cubist-escrow-provider.ts | 10 +- .../src/identifier-auth.ts | 44 +++--- .../src/pending-operation-validation.ts | 11 +- packages/mfa-recovery-controller/src/types.ts | 7 +- .../mfa-recovery-controller/tests/stubs.ts | 131 +++++++++++++----- 11 files changed, 354 insertions(+), 157 deletions(-) diff --git a/packages/mfa-recovery-controller/ARCHITECTURE.md b/packages/mfa-recovery-controller/ARCHITECTURE.md index 18f22a4627f..3af1f82f524 100644 --- a/packages/mfa-recovery-controller/ARCHITECTURE.md +++ b/packages/mfa-recovery-controller/ARCHITECTURE.md @@ -118,8 +118,8 @@ Anything that fails is rejected rather than acted on: an identifier. - `writing` only: the approval binds `sub` to `profileId` and `ext.request_hash` to `requestHash`, with `aal >= 2`. `register` / `updateIdentifiers` also need - `identifiers_hash` to match `approvedIdentifiers`, and every payload - identifier to equal one of them exactly. This runs before `writing` is + `identifiers_hash` to match `approvedIdentifiers`, and every non-SIWE + payload identifier to equal one of them exactly. This runs before `writing` is persisted: an escrow rejection is not a `Deny`, so a mismatch found there would leave an unabortable write. Receipts must be well-formed with at most one per escrow. @@ -147,9 +147,12 @@ sequenceDiagram M->>M: skip escrows with a stored receipt, re-verify stored receipts M->>E: isAvailable (remaining escrows) M->>M: reuse token, or re-issue it if exp <= now() - alt not register - M->>I: getKeyBoundIdentifierToken - M->>E: generateChallenge + PoP per remaining escrow (all required) + opt updateIdentifiers with SIWE identifiers + M->>E: getIdentifiers (enrolled set at expectedVersion) + end + opt update identifier, or SIWE identifiers not yet enrolled + M->>I: getKeyBoundIdentifierToken per identifier + M->>E: generateChallenge + PoPs per remaining escrow (all required) end M->>M: wrap secret per escrow, persist writing par write every remaining replica @@ -178,8 +181,12 @@ Auth approvals: - The approval's `identifiers` are persisted with its token as `approvedIdentifiers` and refreshed with it. `register` and `updateIdentifiers` send them to every escrow, which checks them against - `identifiers_hash` and rejects payload identifiers outside them. Hashing - sorts object keys, not arrays. + `identifiers_hash` and rejects non-SIWE payload identifiers outside them. + Hashing sorts object keys, not arrays. +- Auth does not vouch for SIWE wallets. Each SIWE identifier an escrow has not + enrolled needs its own proof in `identifierAuthorizations`; updates add one + proof for the `identifier` argument. Escrows past `expectedVersion` replay + their receipt, so they get no SIWE proofs. - Missing or stale MFA rejects with `AuthenticationController`'s `StepUpRequiredError`. - Escrows verify signatures. The controller decodes claims only for pending-state @@ -211,8 +218,8 @@ Versioning: `resume()` is the same write path without creating a new mutation: authorizing pending gets a fresh token then replicates; writing pending -retries only escrows that do not already have a stored receipt. Updates -re-run identifier auth (a new key-bound token and PoP) for those escrows on +retries only escrows that do not already have a stored receipt. Updates and +mutations adding SIWE wallets re-run identifier auth (a new key-bound token and PoP) for those escrows on every attempt, so the identifier provider must be able to authenticate again. ## Cubist integration @@ -301,13 +308,13 @@ the highest one. The next mutation still requires `resume()` first. flowchart TD Start[identifier + requestHash] --> Mode{trusted type?} Mode -->|passkey / social / siwe| KB[fresh proof key, IdP key-bound token] - KB --> Chal[each escrow generateChallenge] - Chal --> PoP[sign PoP over hash of token + challengeId + requestHash] + KB --> Chal[each escrow generateChallenge, one per escrow] + Chal --> PoP[each proof key signs hash of token + challengeId + requestHash] Mode -->|emailOtp / smsOtp| Todo[TODO MFA-605 / MFA-606 / MFA-568] Mode -->|anything else| Reject[unknown_identifier_type] ``` On **mutation**, every remaining target escrow must authorize or the write does not start (`identifier_auth_failed`). On **read**, escrows whose -challenge fails are skipped. `register` sends no identifier authorization -(`null`). +challenge fails are skipped. A `register` without SIWE identifiers sends no +identifier authorizations (`[]`). diff --git a/packages/mfa-recovery-controller/CUBIST_TEST_SEQUENCE.md b/packages/mfa-recovery-controller/CUBIST_TEST_SEQUENCE.md index 57c2a324207..292567b44c0 100644 --- a/packages/mfa-recovery-controller/CUBIST_TEST_SEQUENCE.md +++ b/packages/mfa-recovery-controller/CUBIST_TEST_SEQUENCE.md @@ -71,22 +71,30 @@ sequenceDiagram Auth-->>C: JWT bound to requestHash C->>Cubist: isAvailable - opt updates only (not register) + opt updateIdentifiers adding SIWE identifiers + C->>Cubist: getIdentifiers(JWT) + Cubist-->>C: enrolled identifiers, version + end + opt updates, or SIWE identifiers not yet enrolled Note over C,Cubist: Proof of possession for requestHash (see below) end Note over C: Encrypt the secret to the Cubist wrap key - C->>Cubist: applyMutation(mutation, JWT, { token, proof } or null, payload, approvedIdentifiers) + C->>Cubist: applyMutation(mutation, JWT, [{ token, proof }, ...], payload, approvedIdentifiers) Cubist-->>C: signed receipt Note over C: Verify the receipt with the pinned receipt key C-->>Mobile: done ``` -| Operation | Identifier proof | Payload | -| ---------------------- | ----------------------- | -------------------- | -| `register` | none | identifiers + secret | -| `updateRecoverySecret` | the identifier argument | secret | -| `updateIdentifiers` | the identifier argument | identifiers | +| Operation | Identifier proofs | Payload | +| ---------------------- | ------------------------------------------------------ | -------------------- | +| `register` | each SIWE identifier | identifiers + secret | +| `updateRecoverySecret` | the identifier argument | secret | +| `updateIdentifiers` | the identifier argument + each SIWE identifier it adds | identifiers | + +Auth does not vouch for SIWE wallets, so `approvedIdentifiers` need not +include them; each new wallet proves control instead. In step 1 the primary +account signs a SIWE proof for `register`. If an escrow refuses the mutation (`MutationRejectedError`) and every replica does the same, the controller clears the pending mutation. A transport failure @@ -95,18 +103,21 @@ to finish that write. ## Proof of possession -Updates and reads prove control of a registered identifier: - -1. The controller generates an Ed25519 proof key and requests an identifier token - binding `hash([proofPublicKey, requestHash])`: the SIWE nonce or WebAuthn challenge. -2. The proof key signs `hash([token, challenge.id, requestHash])` for each escrow. - The controller sends the approval and `{ kind: 'key-bound', token, proof }` - with `applyMutation` or `getSecret`. -3. The escrow C2F verifies the provider assertion, registered identifier, request - binding, challenge, and proof signature. - -The proof private key never leaves the controller/caller; read sessions keep it -in memory. `register` sends `null` instead of an identifier proof. Implementation: +Updates and reads prove control of a registered identifier; mutations also prove +each SIWE identifier the escrow has not enrolled: + +1. The controller generates an Ed25519 proof key per identifier and requests an + identifier token binding `hash([proofPublicKey, requestHash])`: the SIWE nonce + or WebAuthn challenge. +2. Each escrow issues one challenge, keyed to the approval's profile. Every proof + key signs `hash([token, challenge.id, requestHash])` for that challenge. The + controller sends the approval and the `{ kind: 'key-bound', token, proof }` + list with `applyMutation`, or the single proof with `getSecret`. +3. The escrow C2F verifies the provider assertions, identifiers, request + binding, challenge, and proof signatures, then consumes the challenge once. + +The proof private keys never leave the controller/caller; read sessions keep +theirs in memory. A `register` without SIWE identifiers sends no proofs. Implementation: `requestKeyBoundIdentifierToken` and `authorizeEscrowsWithToken` in `src/identifier-auth.ts`. ## Read: `authenticateIdentifier`, `getRecoverySecret` diff --git a/packages/mfa-recovery-controller/README.md b/packages/mfa-recovery-controller/README.md index 47af0960dbc..5e7b0c0c184 100644 --- a/packages/mfa-recovery-controller/README.md +++ b/packages/mfa-recovery-controller/README.md @@ -16,10 +16,12 @@ Without a recent MFA verification, approvals reject with `StepUpRequiredError`: verify, then call `resume()` or retry the read. `register` and `updateIdentifiers` send the approval's `identifiers` to each -escrow as `approvedIdentifiers`; the escrow rejects payload identifiers outside -that list. Build payload identifiers from those entries unchanged (verifier -included): the controller rejects any other identifier before writing, and the -mutation can then be aborted. Set `CubistEscrowProviderOptions.authAudience` to the escrow's +escrow as `approvedIdentifiers`; the escrow rejects non-SIWE payload +identifiers outside that list. Build those from Auth's entries unchanged +(verifier included): the controller rejects any other before writing, and the +mutation can then be aborted. Each SIWE wallet a mutation adds is proven +through `identifierAuthProvider` instead, including at `register`. Set +`CubistEscrowProviderOptions.authAudience` to the escrow's `AUTH_CONTROLLER_AUDIENCE`. Before `updateIdentifiers`, call `getIdentifiers()` (AAL1) for the enrolled diff --git a/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts b/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts index 94f63c754c1..5976b1b5bb1 100644 --- a/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts +++ b/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts @@ -51,6 +51,12 @@ const SOCIAL: Identifier = { const OTHER_PASSKEY: Identifier = { ...PASSKEY, value: 'cred-2' }; +const WALLET: Identifier = { + type: 'siwe', + namespace: 'example.com:1', + value: '0xAbC0000000000000000000000000000000000001', +}; + const IDENTIFIERS = [PASSKEY, SOCIAL]; const SECRET = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]); @@ -901,6 +907,73 @@ describe('MfaRecoveryController', () => { ); }); + describe('SIWE identifiers', () => { + it('proves each SIWE identifier at register', async () => { + await withController(async ({ controller, escrowA }) => { + const apply = vi.spyOn(escrowA, 'applyMutation'); + await controller.register(SECRET, [PASSKEY, WALLET]); + expect( + apply.mock.calls[0][2].map(({ token }) => token.identifier), + ).toStrictEqual([WALLET]); + }); + }); + + it('proves an added SIWE identifier with the enrolled identifier on one challenge', async () => { + await withController(async ({ controller, escrowA }) => { + await controller.register(SECRET, IDENTIFIERS); + const apply = vi.spyOn(escrowA, 'applyMutation'); + await controller.updateIdentifiers( + PASSKEY, + [SOCIAL, WALLET], + REGISTERED_EPOCH, + ); + const authorizations = apply.mock.calls[0][2]; + expect( + authorizations.map(({ token }) => token.identifier), + ).toStrictEqual([PASSKEY, WALLET]); + expect( + new Set(authorizations.map(({ proof }) => proof.challengeId)).size, + ).toBe(1); + }); + }); + + it('does not prove an enrolled SIWE identifier in another address case', async () => { + await withController(async ({ controller, escrowA }) => { + await controller.register(SECRET, [PASSKEY, WALLET]); + const apply = vi.spyOn(escrowA, 'applyMutation'); + await controller.updateIdentifiers( + PASSKEY, + [SOCIAL, { ...WALLET, value: WALLET.value.toLowerCase() }], + REGISTERED_EPOCH, + ); + expect( + apply.mock.calls[0][2].map(({ token }) => token.identifier), + ).toStrictEqual([PASSKEY]); + }); + }); + + it('does not prove SIWE identifiers to an escrow that already applied the mutation', async () => { + await withController(async ({ controller, escrowA }) => { + await controller.register(SECRET, IDENTIFIERS); + escrowA.invalidReceipts = true; + await expect( + controller.updateIdentifiers( + PASSKEY, + [SOCIAL, WALLET], + REGISTERED_EPOCH, + ), + ).rejects.toThrow('Invalid mutation receipt'); + escrowA.invalidReceipts = false; + const apply = vi.spyOn(escrowA, 'applyMutation'); + await controller.resume(); + expect( + apply.mock.calls[0][2].map(({ token }) => token.identifier), + ).toStrictEqual([PASSKEY]); + expect(await controller.getPhase()).toBe('idle'); + }); + }); + }); + describe('resume and abort', () => { it('resumes a persisted writing mutation after a replica failure', async () => { await withController(async ({ controller, escrowA }) => { diff --git a/packages/mfa-recovery-controller/src/MfaRecoveryController.ts b/packages/mfa-recovery-controller/src/MfaRecoveryController.ts index fd1af208cce..2859649d5c1 100644 --- a/packages/mfa-recovery-controller/src/MfaRecoveryController.ts +++ b/packages/mfa-recovery-controller/src/MfaRecoveryController.ts @@ -35,7 +35,6 @@ import { MIN_IDENTIFIERS, requestKeyBoundIdentifierToken, } from './identifier-auth.js'; -import type { AuthorizedEscrow } from './identifier-auth.js'; import type { MfaRecoveryControllerMethodActions } from './MfaRecoveryController-method-action-types.js'; import { assertValidPendingOperation, @@ -44,9 +43,10 @@ import { } from './pending-operation-validation.js'; import { assertAbortAllowed, getRecoveryPhase } from './state-machine.js'; import type { - AuthControllerToken, Identifier, + IdentifierAuthorization, IdentifierSession, + KeyBoundCredential, Mutation, MutationPayload, MutationReceipt, @@ -393,11 +393,12 @@ export class MfaRecoveryController extends BaseController< }); const authorizedEscrows = await authorizeEscrowsWithToken({ escrows: available, - credential: session, + credentials: [session], authControllerToken: approval, }); const results = await Promise.allSettled( - authorizedEscrows.map(async ({ escrow, authorization }) => { + authorizedEscrows.map(async ({ escrow, authorizations }) => { + const [authorization] = authorizations; const response = await escrow.getSecret( approval, authorization, @@ -593,7 +594,7 @@ export class MfaRecoveryController extends BaseController< } async #replicateMutation(pending: WritingPendingOperation): Promise { - const { mutation, identifier } = pending; + const { mutation } = pending; const remainingEscrows = await this.#getRemainingEscrows(pending); if (remainingEscrows.length === 0) { await this.#clearPending(); @@ -608,24 +609,10 @@ export class MfaRecoveryController extends BaseController< ? pending : { ...pending, ...(await this.#authorizeMutation(mutation)) }; await this.#assertPending(writing); - const authorizedEscrows = - mutation.operation === 'register' - ? null - : await this.#authorizeEscrows({ - escrows: remainingEscrows, - identifier: identifier as Identifier, - requestHash: mutation.requestHash, - authControllerToken: writing.authControllerToken, - }); - if ( - authorizedEscrows !== null && - authorizedEscrows.length !== remainingEscrows.length - ) { - throw new MfaRecoveryError( - 'Unable to authorize every escrow', - 'identifier_auth_failed', - ); - } + const authorizations = await this.#authorizeEscrows( + writing, + remainingEscrows, + ); const payloads = await Promise.all( remainingEscrows.map(async (escrow) => this.#payloadForEscrow(writing, escrow), @@ -638,7 +625,7 @@ export class MfaRecoveryController extends BaseController< escrow.applyMutation( mutation, writing.authControllerToken, - authorizedEscrows?.[index]?.authorization ?? null, + authorizations[index], payloads[index], writing.approvedIdentifiers, ), @@ -741,28 +728,78 @@ export class MfaRecoveryController extends BaseController< return { receipts, hasInvalidReceipt }; } - async #authorizeEscrows({ - escrows, - identifier, - requestHash, - authControllerToken, - }: { - escrows: RecoveryEscrowProvider[]; - identifier: Identifier; - requestHash: string; - authControllerToken: AuthControllerToken; - }): Promise { - getIdentifierAuthMode(identifier.type); - const credential = await requestKeyBoundIdentifierToken({ - identifier, - requestHash, - identifierAuthProvider: this.#identifierAuthProvider, - }); - return await authorizeEscrowsWithToken({ + /** + * Proves the update's enrolled identifier and each SIWE identifier the + * escrows have not enrolled, all against one challenge per escrow. + * + * @param writing - Writing pending mutation. + * @param escrows - Escrows to authorize. + * @returns Each escrow's identifier authorizations, in `escrows` order. + */ + async #authorizeEscrows( + writing: WritingPendingOperation, + escrows: RecoveryEscrowProvider[], + ): Promise { + const identifiers = [ + ...(writing.identifier ? [writing.identifier] : []), + ...(await this.#unenrolledSiweIdentifiers(writing, escrows)), + ]; + if (identifiers.length === 0) { + return escrows.map(() => []); + } + const credentials: KeyBoundCredential[] = []; + for (const identifier of identifiers) { + getIdentifierAuthMode(identifier.type); + credentials.push( + await requestKeyBoundIdentifierToken({ + identifier, + requestHash: writing.mutation.requestHash, + identifierAuthProvider: this.#identifierAuthProvider, + }), + ); + } + const authorized = await authorizeEscrowsWithToken({ escrows, - credential, - authControllerToken, + credentials, + authControllerToken: writing.authControllerToken, }); + if (authorized.length !== escrows.length) { + throw new MfaRecoveryError( + 'Unable to authorize every escrow', + 'identifier_auth_failed', + ); + } + return authorized.map(({ authorizations }) => authorizations); + } + + async #unenrolledSiweIdentifiers( + { mutation, payload, authControllerToken }: WritingPendingOperation, + escrows: RecoveryEscrowProvider[], + ): Promise { + const siwe = pendingPayloadHasIdentifiers(payload) + ? payload.identifiers.filter(({ type }) => type === 'siwe') + : []; + if (mutation.operation === 'register' || siwe.length === 0) { + return siwe; + } + const responses = await Promise.all( + escrows.map( + async (escrow) => await escrow.getIdentifiers(authControllerToken), + ), + ); + // Escrows past `expectedVersion` replay their receipt without checking proofs. + const enrolled = + responses.find(({ version }) => version === mutation.expectedVersion) + ?.identifiers ?? siwe; + return siwe.filter( + (identifier) => + !enrolled.some( + (other) => + other.type === identifier.type && + other.namespace === identifier.namespace && + other.value.toLowerCase() === identifier.value.toLowerCase(), + ), + ); } async #payloadForEscrow( diff --git a/packages/mfa-recovery-controller/src/escrow-providers/cubist-escrow-provider.test.ts b/packages/mfa-recovery-controller/src/escrow-providers/cubist-escrow-provider.test.ts index 0a9c9afae98..ec3c1289d05 100644 --- a/packages/mfa-recovery-controller/src/escrow-providers/cubist-escrow-provider.test.ts +++ b/packages/mfa-recovery-controller/src/escrow-providers/cubist-escrow-provider.test.ts @@ -173,18 +173,18 @@ describe('CubistEscrowProvider', () => { }); describe('applyMutation', () => { - it('invokes applyMutation, including a null identifier authorization', async () => { + it('invokes applyMutation, including empty identifier authorizations', async () => { const receipt = await signedReceipt(); const { provider, invoke } = createProvider({ stdout: receipt }); expect( - await provider.applyMutation(MUTATION, TOKEN, null, PAYLOAD, APPROVED), + await provider.applyMutation(MUTATION, TOKEN, [], PAYLOAD, APPROVED), ).toStrictEqual(receipt); expect(invoke).toHaveBeenCalledWith('cubist_secret_escrow', 'latest', { request: { cmd: 'applyMutation', mutation: MUTATION, authControllerToken: TOKEN, - identifierAuthorization: null, + identifierAuthorizations: [], payload: WIRE_PAYLOAD, approvedIdentifiers: APPROVED, }, @@ -198,16 +198,16 @@ describe('CubistEscrowProvider', () => { await provider.applyMutation( MUTATION, TOKEN, - AUTHORIZATION, + [AUTHORIZATION], PAYLOAD, APPROVED, ); const [, , { request }] = invoke.mock.calls[0] as [ string, string, - { request: { identifierAuthorization: { token: unknown } } }, + { request: { identifierAuthorizations: [{ token: unknown }] } }, ]; - expect(request.identifierAuthorization.token).toStrictEqual({ + expect(request.identifierAuthorizations[0].token).toStrictEqual({ identifier: AUTHORIZATION.token.identifier, proofPublicKey: bytesToHex(new Uint8Array(32).fill(1)), requestHash: '0x1', @@ -221,7 +221,7 @@ describe('CubistEscrowProvider', () => { provider.applyMutation( MUTATION, TOKEN, - AUTHORIZATION, + [AUTHORIZATION], PAYLOAD, APPROVED, ), @@ -241,7 +241,7 @@ describe('CubistEscrowProvider', () => { provider.applyMutation( MUTATION, TOKEN, - AUTHORIZATION, + [AUTHORIZATION], PAYLOAD, APPROVED, ), @@ -253,7 +253,7 @@ describe('CubistEscrowProvider', () => { policy: { response: 'Deny', reason: 'version mismatch' }, }); await expect( - provider.applyMutation(MUTATION, TOKEN, null, PAYLOAD, APPROVED), + provider.applyMutation(MUTATION, TOKEN, [], PAYLOAD, APPROVED), ).rejects.toBeInstanceOf(MutationRejectedError); }); }); diff --git a/packages/mfa-recovery-controller/src/escrow-providers/cubist-escrow-provider.ts b/packages/mfa-recovery-controller/src/escrow-providers/cubist-escrow-provider.ts index 8429085e0dd..9cc2fee60e3 100644 --- a/packages/mfa-recovery-controller/src/escrow-providers/cubist-escrow-provider.ts +++ b/packages/mfa-recovery-controller/src/escrow-providers/cubist-escrow-provider.ts @@ -173,16 +173,16 @@ export class CubistEscrowProvider implements RecoveryEscrowProvider { /** * @param mutation - Mutation to apply. * @param authControllerToken - Request-bound AuthController token. - * @param identifierAuthorization - Identifier proof. `null` for register. + * @param identifierAuthorizations - Identifier proofs sharing one challenge. * @param payload - Per-escrow mutation payload. - * @param approvedIdentifiers - The approval's identifiers, which payload - * identifiers must be drawn from. + * @param approvedIdentifiers - The approval's identifiers, which non-SIWE + * payload identifiers must be drawn from. * @returns The escrow's mutation receipt. */ async applyMutation( mutation: Mutation, authControllerToken: AuthControllerToken, - identifierAuthorization: IdentifierAuthorization | null, + identifierAuthorizations: IdentifierAuthorization[], payload: MutationPayload, approvedIdentifiers: Identifier[], ): Promise { @@ -191,7 +191,7 @@ export class CubistEscrowProvider implements RecoveryEscrowProvider { { mutation, authControllerToken, - identifierAuthorization, + identifierAuthorizations, payload, approvedIdentifiers, }, diff --git a/packages/mfa-recovery-controller/src/identifier-auth.ts b/packages/mfa-recovery-controller/src/identifier-auth.ts index cbc6c57e13d..997a0eddaf8 100644 --- a/packages/mfa-recovery-controller/src/identifier-auth.ts +++ b/packages/mfa-recovery-controller/src/identifier-auth.ts @@ -45,7 +45,7 @@ export const IDENTIFIER_AUTH_MODES: Record = { export type AuthorizedEscrow = { escrow: RecoveryEscrowProvider; - authorization: IdentifierAuthorization; + authorizations: IdentifierAuthorization[]; }; /** @@ -77,25 +77,25 @@ export async function requestKeyBoundIdentifierToken({ } /** - * Presents a key-bound credential to each escrow that provides a challenge. + * Presents key-bound credentials to each escrow that provides a challenge. All + * of an escrow's proofs share its one challenge. * * @param params - Escrow authorization parameters. * @param params.escrows - Escrows to authorize. - * @param params.credential - Key-bound identifier credential. + * @param params.credentials - Key-bound identifier credentials. * @param params.authControllerToken - Approval each escrow keys its challenge * to. * @returns Successful escrow authorizations. */ export async function authorizeEscrowsWithToken({ escrows, - credential, + credentials, authControllerToken, }: { escrows: RecoveryEscrowProvider[]; - credential: KeyBoundCredential; + credentials: KeyBoundCredential[]; authControllerToken: AuthControllerToken; }): Promise { - const { token, proofPrivateKey } = credential; const challengeResults = await Promise.allSettled( escrows.map((escrow) => escrow.generateChallenge(authControllerToken)), ); @@ -105,21 +105,23 @@ export async function authorizeEscrowsWithToken({ : [], ); const authorizationResults = await Promise.allSettled( - challenges.map(async ({ escrow, challenge }) => { - const message = await hash([token, challenge.id, token.requestHash]); - return { - escrow, - authorization: { - kind: 'key-bound' as const, - token, - proof: { - challengeId: challenge.id, - requestHash: token.requestHash, - signature: await sign(proofPrivateKey, message), - }, - }, - }; - }), + challenges.map(async ({ escrow, challenge }) => ({ + escrow, + authorizations: await Promise.all( + credentials.map(async ({ token, proofPrivateKey }) => { + const message = await hash([token, challenge.id, token.requestHash]); + return { + kind: 'key-bound' as const, + token, + proof: { + challengeId: challenge.id, + requestHash: token.requestHash, + signature: await sign(proofPrivateKey, message), + }, + }; + }), + ), + })), ); return authorizationResults .filter(isFulfilledResult) diff --git a/packages/mfa-recovery-controller/src/pending-operation-validation.ts b/packages/mfa-recovery-controller/src/pending-operation-validation.ts index f6cd76225cd..baf2f92c770 100644 --- a/packages/mfa-recovery-controller/src/pending-operation-validation.ts +++ b/packages/mfa-recovery-controller/src/pending-operation-validation.ts @@ -128,19 +128,22 @@ export async function assertValidPendingOperation( } /** - * Escrows reject payload identifiers that are not exactly (verifier included) - * in the approval's list, and the rejection leaves the write unabortable. + * Escrows reject non-SIWE payload identifiers that are not exactly (verifier + * included) in the approval's list, and the rejection leaves the write + * unabortable. SIWE identifiers are proven by their own authorizations. * * @param identifiers - Payload identifiers. * @param approvedIdentifiers - The approval's identifiers. - * @returns Whether every payload identifier is in the approval. + * @returns Whether every non-SIWE payload identifier is in the approval. */ async function isApprovedSubset( identifiers: Identifier[], approvedIdentifiers: Identifier[], ): Promise { const approved = new Set(await Promise.all(approvedIdentifiers.map(hash))); - const hashes = await Promise.all(identifiers.map(hash)); + const hashes = await Promise.all( + identifiers.filter(({ type }) => type !== 'siwe').map(hash), + ); return hashes.every((identifierHash) => approved.has(identifierHash)); } diff --git a/packages/mfa-recovery-controller/src/types.ts b/packages/mfa-recovery-controller/src/types.ts index 0ce5a7460d4..8b512c638e6 100644 --- a/packages/mfa-recovery-controller/src/types.ts +++ b/packages/mfa-recovery-controller/src/types.ts @@ -328,10 +328,15 @@ export type RecoveryEscrowProvider = { requestId: string, pkE: Uint8Array, ) => Promise; + /** + * `identifierAuthorizations` share one challenge: one proof per SIWE + * identifier the escrow has not enrolled, plus one enrolled-identifier proof + * for updates. + */ applyMutation: ( mutation: Mutation, authControllerToken: AuthControllerToken, - identifierAuthorization: IdentifierAuthorization | null, + identifierAuthorizations: IdentifierAuthorization[], payload: MutationPayload, approvedIdentifiers: Identifier[], ) => Promise; diff --git a/packages/mfa-recovery-controller/tests/stubs.ts b/packages/mfa-recovery-controller/tests/stubs.ts index 54f98ddcc29..c634fa067f0 100644 --- a/packages/mfa-recovery-controller/tests/stubs.ts +++ b/packages/mfa-recovery-controller/tests/stubs.ts @@ -288,10 +288,8 @@ export class StubEscrowProvider implements RecoveryEscrowProvider { requestId, pkE, }); - const { profileId } = await this.#verifyIdentifierAuthorization( - authorization, - requestHash, - ); + await this.#verifyProofs([authorization], requestHash); + const profileId = this.#profileIdFor(authorization.token.identifier); const claims = decodeAuthControllerToken(authControllerToken); if ( claims?.sub !== profileId || @@ -322,7 +320,7 @@ export class StubEscrowProvider implements RecoveryEscrowProvider { async applyMutation( mutation: Mutation, authControllerToken: AuthControllerToken, - identifierAuthorization: IdentifierAuthorization | null, + identifierAuthorizations: IdentifierAuthorization[], payload: MutationPayload, approvedIdentifiers: Identifier[], ): Promise { @@ -336,7 +334,7 @@ export class StubEscrowProvider implements RecoveryEscrowProvider { return await this.#applyMutation( mutation, authControllerToken, - identifierAuthorization, + identifierAuthorizations, payload, approvedIdentifiers, ); @@ -351,7 +349,7 @@ export class StubEscrowProvider implements RecoveryEscrowProvider { async #applyMutation( mutation: Mutation, authControllerToken: AuthControllerToken, - identifierAuthorization: IdentifierAuthorization | null, + identifierAuthorizations: IdentifierAuthorization[], payload: MutationPayload, approvedIdentifiers: Identifier[], ): Promise { @@ -382,8 +380,17 @@ export class StubEscrowProvider implements RecoveryEscrowProvider { throw new MfaRecoveryError('Version mismatch', 'version_mismatch'); } + const enrolledAuthorizations = this.#splitSiweAuthorizations( + identifierAuthorizations, + (payload as { identifiers?: Identifier[] }).identifiers ?? [], + record?.identifiers ?? [], + ); + if (identifierAuthorizations.length > 0) { + await this.#verifyProofs(identifierAuthorizations, mutation.requestHash); + } + if (mutation.operation === 'register') { - if (identifierAuthorization !== null || record !== undefined) { + if (enrolledAuthorizations.length > 0 || record !== undefined) { throw new MfaRecoveryError('Invalid registration', 'invalid_register'); } const registerPayload = payload as RegisterPayload; @@ -432,17 +439,16 @@ export class StubEscrowProvider implements RecoveryEscrowProvider { 'update_unauthorized', ); } - if (identifierAuthorization === null) { + if (enrolledAuthorizations.length !== 1) { throw new MfaRecoveryError( - 'Identifier authorization required', + 'Exactly one enrolled identifier authorization required', 'missing_identifier_auth', ); } - const authorization = await this.#verifyIdentifierAuthorization( - identifierAuthorization, - mutation.requestHash, - ); - if (authorization.profileId !== mutation.profileId) { + if ( + this.#profileIdFor(enrolledAuthorizations[0].token.identifier) !== + mutation.profileId + ) { throw new MfaRecoveryError('Profile mismatch', 'profile_mismatch'); } @@ -612,7 +618,9 @@ export class StubEscrowProvider implements RecoveryEscrowProvider { if ( decodeAuthControllerToken(token)?.ext.identifiersHash !== (await hash(approvedIdentifiers)) || - !identifiers.every((identifier) => approved.has(canonicalize(identifier))) + !identifiers + .filter(({ type }) => type !== 'siwe') + .every((identifier) => approved.has(canonicalize(identifier))) ) { throw new MfaRecoveryError( 'Identifier ownership not approved', @@ -624,23 +632,48 @@ export class StubEscrowProvider implements RecoveryEscrowProvider { } } - async #verifyIdentifierAuthorization( - authorization: IdentifierAuthorization, - requestHash: string, - ): Promise<{ profileId: string }> { - const { token, proof } = authorization; - const { identifier } = token; - getIdentifierAuthMode(identifier.type); - if ( - token.requestHash !== requestHash || - proof.requestHash !== requestHash - ) { + /** + * Pairs each SIWE identifier this replica has not enrolled with exactly one + * authorization. + * + * @param authorizations - Authorizations sent with the mutation. + * @param identifiers - Payload identifiers. + * @param enrolled - Identifiers this replica has enrolled. + * @returns The remaining, enrolled-identifier authorizations. + */ + #splitSiweAuthorizations( + authorizations: IdentifierAuthorization[], + identifiers: Identifier[], + enrolled: Identifier[], + ): IdentifierAuthorization[] { + const enrolledKeys = new Set(enrolled.map(canonicalIdentifier)); + const newSiwe = new Set( + identifiers + .filter(({ type }) => type === 'siwe') + .map(canonicalIdentifier) + .filter((key) => !enrolledKeys.has(key)), + ); + const siwe = authorizations + .map(({ token }) => canonicalIdentifier(token.identifier)) + .filter((key) => newSiwe.has(key)); + if (siwe.length !== newSiwe.size || new Set(siwe).size !== siwe.length) { throw new MfaRecoveryError( - 'Request hash mismatch', - 'request_hash_mismatch', + 'Each new SIWE identifier requires one authorization', + 'missing_siwe_auth', ); } - const challenge = this.#popChallenges.get(proof.challengeId); + return authorizations.filter( + ({ token }) => !newSiwe.has(canonicalIdentifier(token.identifier)), + ); + } + + async #verifyProofs( + authorizations: IdentifierAuthorization[], + requestHash: string, + ): Promise { + const challenge = this.#popChallenges.get( + authorizations[0].proof.challengeId, + ); if ( challenge === undefined || challenge.consumed || @@ -649,21 +682,41 @@ export class StubEscrowProvider implements RecoveryEscrowProvider { ) { throw new MfaRecoveryError('Invalid PoP challenge', 'invalid_challenge'); } - const message = await hash([token, proof.challengeId, requestHash]); - if ( - !(await verifySignature(token.proofPublicKey, proof.signature, message)) - ) { - throw new MfaRecoveryError('Invalid PoP signature', 'invalid_pop'); + for (const { token, proof } of authorizations) { + getIdentifierAuthMode(token.identifier.type); + if ( + token.requestHash !== requestHash || + proof.requestHash !== requestHash + ) { + throw new MfaRecoveryError( + 'Request hash mismatch', + 'request_hash_mismatch', + ); + } + if (proof.challengeId !== challenge.id) { + throw new MfaRecoveryError( + 'Invalid PoP challenge', + 'invalid_challenge', + ); + } + const message = await hash([token, proof.challengeId, requestHash]); + if ( + !(await verifySignature(token.proofPublicKey, proof.signature, message)) + ) { + throw new MfaRecoveryError('Invalid PoP signature', 'invalid_pop'); + } } challenge.consumed = true; + } + #profileIdFor(identifier: Identifier): string { const profileId = this.#identifierIndex.get( canonicalIdentifier(identifier), ); if (profileId === undefined) { throw new MfaRecoveryError('Unknown identifier', 'unknown_identifier'); } - return { profileId }; + return profileId; } #assertIdentifiersAvailable( @@ -734,5 +787,9 @@ export const passthroughEncryptor: PendingOperationEncryptor = { * @returns Index key. */ function canonicalIdentifier(identifier: Identifier): string { - return `${identifier.type}:${identifier.namespace}:${identifier.value}`; + const value = + identifier.type === 'siwe' + ? identifier.value.toLowerCase() + : identifier.value; + return `${identifier.type}:${identifier.namespace}:${value}`; } From 01d7d4324d048d2c0f92f4413e6a3683fd430750 Mon Sep 17 00:00:00 2001 From: huggingbot <83656073+huggingbot@users.noreply.github.com> Date: Wed, 28 Oct 2026 17:13:38 +0800 Subject: [PATCH 2/4] docs: add changelog entry for SIWE proofs Co-authored-by: Cursor --- packages/mfa-recovery-controller/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/mfa-recovery-controller/CHANGELOG.md b/packages/mfa-recovery-controller/CHANGELOG.md index 2a3be6bdb05..6fff04aa871 100644 --- a/packages/mfa-recovery-controller/CHANGELOG.md +++ b/packages/mfa-recovery-controller/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Sends the approval's identifiers to escrows as `approvedIdentifiers` with `register` and `updateIdentifiers` mutations, and rejects payload identifiers that are not exactly among them before writing ([#10883](https://github.com/MetaMask/core/pull/10883)) - Adds `getIdentifiers`, which returns the enrolled identifiers at the highest escrow version as `epoch` along with Auth's identifiers, for building `updateIdentifiers` calls ([#11149](https://github.com/MetaMask/core/pull/11149)) - Passes the operation's AuthController token to `RecoveryEscrowProvider.generateChallenge`, and requires `RecoveryEscrowProvider.getIdentifiers` ([#11149](https://github.com/MetaMask/core/pull/11149)) + - Proves each SIWE identifier an escrow has not enrolled through `identifierAuthProvider`, including at `register`, and passes `RecoveryEscrowProvider.applyMutation` a list of identifier authorizations sharing one challenge ([#11151](https://github.com/MetaMask/core/pull/11151)) - Exports `MfaRecoveryController`, `getDefaultMfaRecoveryControllerState`, and the controller's action, event, messenger, state, and options types. - Exports `AuthApiRecoveryAuthProvider`, `AuthApiRecoveryAuthProviderOptions` and `AuthApiRecoveryAuthProviderMessenger` for requesting approvals through `AuthenticationController:requestMfaApproval`, using the profile id from `AuthenticationController:refreshCanonicalProfileId` ([#10883](https://github.com/MetaMask/core/pull/10883)) - Exports `CubistEscrowProvider` and `CubistEscrowProviderOptions` for storing recovery secrets with CubeSigner. From 3436a6d4bbff3b723cb3752ec0ab50416bebe75a Mon Sep 17 00:00:00 2001 From: huggingbot <83656073+huggingbot@users.noreply.github.com> Date: Wed, 28 Oct 2026 17:49:51 +0800 Subject: [PATCH 3/4] docs: note the SIWE approvedIdentifiers exemption Co-authored-by: Cursor --- packages/mfa-recovery-controller/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mfa-recovery-controller/CHANGELOG.md b/packages/mfa-recovery-controller/CHANGELOG.md index 6fff04aa871..90deb24fbcf 100644 --- a/packages/mfa-recovery-controller/CHANGELOG.md +++ b/packages/mfa-recovery-controller/CHANGELOG.md @@ -14,7 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Sends the approval's identifiers to escrows as `approvedIdentifiers` with `register` and `updateIdentifiers` mutations, and rejects payload identifiers that are not exactly among them before writing ([#10883](https://github.com/MetaMask/core/pull/10883)) - Adds `getIdentifiers`, which returns the enrolled identifiers at the highest escrow version as `epoch` along with Auth's identifiers, for building `updateIdentifiers` calls ([#11149](https://github.com/MetaMask/core/pull/11149)) - Passes the operation's AuthController token to `RecoveryEscrowProvider.generateChallenge`, and requires `RecoveryEscrowProvider.getIdentifiers` ([#11149](https://github.com/MetaMask/core/pull/11149)) - - Proves each SIWE identifier an escrow has not enrolled through `identifierAuthProvider`, including at `register`, and passes `RecoveryEscrowProvider.applyMutation` a list of identifier authorizations sharing one challenge ([#11151](https://github.com/MetaMask/core/pull/11151)) + - Proves each SIWE identifier an escrow has not enrolled through `identifierAuthProvider`, including at `register`, instead of requiring it in `approvedIdentifiers`, and passes `RecoveryEscrowProvider.applyMutation` a list of identifier authorizations sharing one challenge ([#11151](https://github.com/MetaMask/core/pull/11151)) - Exports `MfaRecoveryController`, `getDefaultMfaRecoveryControllerState`, and the controller's action, event, messenger, state, and options types. - Exports `AuthApiRecoveryAuthProvider`, `AuthApiRecoveryAuthProviderOptions` and `AuthApiRecoveryAuthProviderMessenger` for requesting approvals through `AuthenticationController:requestMfaApproval`, using the profile id from `AuthenticationController:refreshCanonicalProfileId` ([#10883](https://github.com/MetaMask/core/pull/10883)) - Exports `CubistEscrowProvider` and `CubistEscrowProviderOptions` for storing recovery secrets with CubeSigner. From 023dfc5b789f3cc1b30258acbea5b947c2bdfeb4 Mon Sep 17 00:00:00 2001 From: huggingbot <83656073+huggingbot@users.noreply.github.com> Date: Wed, 28 Oct 2026 22:47:53 +0800 Subject: [PATCH 4/4] fix: simplify new SIWE identifier lookup and validate SIWE namespace --- packages/mfa-recovery-controller/CHANGELOG.md | 1 + .../src/MfaRecoveryController.test.ts | 15 +++++ .../src/MfaRecoveryController.ts | 62 ++++++++++++------- .../src/identifier-auth.ts | 14 +++++ 4 files changed, 68 insertions(+), 24 deletions(-) diff --git a/packages/mfa-recovery-controller/CHANGELOG.md b/packages/mfa-recovery-controller/CHANGELOG.md index 90deb24fbcf..5acf9a76b9c 100644 --- a/packages/mfa-recovery-controller/CHANGELOG.md +++ b/packages/mfa-recovery-controller/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Adds `getIdentifiers`, which returns the enrolled identifiers at the highest escrow version as `epoch` along with Auth's identifiers, for building `updateIdentifiers` calls ([#11149](https://github.com/MetaMask/core/pull/11149)) - Passes the operation's AuthController token to `RecoveryEscrowProvider.generateChallenge`, and requires `RecoveryEscrowProvider.getIdentifiers` ([#11149](https://github.com/MetaMask/core/pull/11149)) - Proves each SIWE identifier an escrow has not enrolled through `identifierAuthProvider`, including at `register`, instead of requiring it in `approvedIdentifiers`, and passes `RecoveryEscrowProvider.applyMutation` a list of identifier authorizations sharing one challenge ([#11151](https://github.com/MetaMask/core/pull/11151)) + - Rejects SIWE identifiers whose `namespace` is not their verifier's `domain:chainId` with `invalid_identifier` ([#11151](https://github.com/MetaMask/core/pull/11151)) - Exports `MfaRecoveryController`, `getDefaultMfaRecoveryControllerState`, and the controller's action, event, messenger, state, and options types. - Exports `AuthApiRecoveryAuthProvider`, `AuthApiRecoveryAuthProviderOptions` and `AuthApiRecoveryAuthProviderMessenger` for requesting approvals through `AuthenticationController:requestMfaApproval`, using the profile id from `AuthenticationController:refreshCanonicalProfileId` ([#10883](https://github.com/MetaMask/core/pull/10883)) - Exports `CubistEscrowProvider` and `CubistEscrowProviderOptions` for storing recovery secrets with CubeSigner. diff --git a/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts b/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts index 5976b1b5bb1..147ad0908f9 100644 --- a/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts +++ b/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts @@ -55,6 +55,7 @@ const WALLET: Identifier = { type: 'siwe', namespace: 'example.com:1', value: '0xAbC0000000000000000000000000000000000001', + verifier: { domain: 'example.com', chainId: 1 }, }; const IDENTIFIERS = [PASSKEY, SOCIAL]; @@ -1856,6 +1857,20 @@ describe('MfaRecoveryController', () => { }); }, ); + + it.each([ + { ...WALLET, namespace: 'example.com:2' }, + { ...WALLET, verifier: undefined }, + ])( + 'rejects a SIWE namespace that does not match its verifier', + async (wallet) => { + await withController(async ({ controller }) => { + await expect( + controller.register(SECRET, [PASSKEY, wallet]), + ).rejects.toMatchObject({ code: 'invalid_identifier' }); + }); + }, + ); }); }); diff --git a/packages/mfa-recovery-controller/src/MfaRecoveryController.ts b/packages/mfa-recovery-controller/src/MfaRecoveryController.ts index 2859649d5c1..c9461c710fe 100644 --- a/packages/mfa-recovery-controller/src/MfaRecoveryController.ts +++ b/packages/mfa-recovery-controller/src/MfaRecoveryController.ts @@ -32,6 +32,7 @@ import { authorizeEscrowsWithToken, countDistinctIdentifiers, getIdentifierAuthMode, + getSiweNamespace, MIN_IDENTIFIERS, requestKeyBoundIdentifierToken, } from './identifier-auth.js'; @@ -263,7 +264,7 @@ export class MfaRecoveryController extends BaseController< recoverySecret: Uint8Array, identifiers: Identifier[], ): Promise { - this.#assertKnownIdentifierTypes(identifiers); + this.#assertValidIdentifiers(identifiers); this.#assertMinIdentifiers(identifiers); await this.#mutate({ operation: 'register', @@ -288,7 +289,7 @@ export class MfaRecoveryController extends BaseController< recoverySecret: Uint8Array, epoch: number, ): Promise { - this.#assertKnownIdentifierTypes([identifier]); + this.#assertValidIdentifiers([identifier]); await this.#mutate({ operation: 'updateRecoverySecret', payload: { @@ -312,7 +313,7 @@ export class MfaRecoveryController extends BaseController< identifiers: Identifier[], epoch: number, ): Promise { - this.#assertKnownIdentifierTypes([identifier, ...identifiers]); + this.#assertValidIdentifiers([identifier, ...identifiers]); this.#assertMinIdentifiers(identifiers); await this.#mutate({ operation: 'updateIdentifiers', @@ -331,7 +332,7 @@ export class MfaRecoveryController extends BaseController< async authenticateIdentifier( identifier: Identifier, ): Promise { - this.#assertKnownIdentifierTypes([identifier]); + this.#assertValidIdentifiers([identifier]); const requestId = randomId(); const ephemeral = await generateWrapKey(); const pkE = ephemeral.publicKey; @@ -367,7 +368,7 @@ export class MfaRecoveryController extends BaseController< ): Promise { return await this.#withLock(async () => { const { token, requestId, pkE } = session; - this.#assertKnownIdentifierTypes([token.identifier]); + this.#assertValidIdentifiers([token.identifier]); const requestHash = await hash({ operation: 'getRecoverySecret', requestId, @@ -742,7 +743,7 @@ export class MfaRecoveryController extends BaseController< ): Promise { const identifiers = [ ...(writing.identifier ? [writing.identifier] : []), - ...(await this.#unenrolledSiweIdentifiers(writing, escrows)), + ...(await this.#findNewSiweIdentifiers(writing, escrows)), ]; if (identifiers.length === 0) { return escrows.map(() => []); @@ -772,33 +773,37 @@ export class MfaRecoveryController extends BaseController< return authorized.map(({ authorizations }) => authorizations); } - async #unenrolledSiweIdentifiers( + // SIWE identifiers some escrow has not enrolled. Escrows that already + // applied the mutation replay their receipt and ignore the extra proofs. + async #findNewSiweIdentifiers( { mutation, payload, authControllerToken }: WritingPendingOperation, escrows: RecoveryEscrowProvider[], ): Promise { - const siwe = pendingPayloadHasIdentifiers(payload) + const siweIdentifiers = pendingPayloadHasIdentifiers(payload) ? payload.identifiers.filter(({ type }) => type === 'siwe') : []; - if (mutation.operation === 'register' || siwe.length === 0) { - return siwe; + if (mutation.operation === 'register' || siweIdentifiers.length === 0) { + return siweIdentifiers; } const responses = await Promise.all( - escrows.map( - async (escrow) => await escrow.getIdentifiers(authControllerToken), - ), + escrows.map((escrow) => escrow.getIdentifiers(authControllerToken)), ); - // Escrows past `expectedVersion` replay their receipt without checking proofs. - const enrolled = - responses.find(({ version }) => version === mutation.expectedVersion) - ?.identifiers ?? siwe; - return siwe.filter( - (identifier) => - !enrolled.some( + const enrolledPerEscrow = responses.map(({ identifiers }) => identifiers); + // Escrows compare SIWE addresses case-insensitively. + const isEnrolledInEveryEscrow = ({ + namespace, + value, + }: Identifier): boolean => + enrolledPerEscrow.every((enrolled) => + enrolled.some( (other) => - other.type === identifier.type && - other.namespace === identifier.namespace && - other.value.toLowerCase() === identifier.value.toLowerCase(), + other.type === 'siwe' && + other.namespace === namespace && + other.value.toLowerCase() === value.toLowerCase(), ), + ); + return siweIdentifiers.filter( + (identifier) => !isEnrolledInEveryEscrow(identifier), ); } @@ -848,9 +853,18 @@ export class MfaRecoveryController extends BaseController< ); } - #assertKnownIdentifierTypes(identifiers: Identifier[]): void { + #assertValidIdentifiers(identifiers: Identifier[]): void { for (const identifier of identifiers) { getIdentifierAuthMode(identifier.type); + if ( + identifier.type === 'siwe' && + identifier.namespace !== getSiweNamespace(identifier) + ) { + throw new MfaRecoveryError( + 'SIWE namespace must be the verifier domain:chainId', + 'invalid_identifier', + ); + } } } diff --git a/packages/mfa-recovery-controller/src/identifier-auth.ts b/packages/mfa-recovery-controller/src/identifier-auth.ts index 997a0eddaf8..2594668a631 100644 --- a/packages/mfa-recovery-controller/src/identifier-auth.ts +++ b/packages/mfa-recovery-controller/src/identifier-auth.ts @@ -1,3 +1,5 @@ +import { isObject } from '@metamask/utils'; + import { generateSigningKey, hash, sign } from './crypto.js'; import { MfaRecoveryError } from './errors.js'; import { isFulfilledResult } from './escrow-utils.js'; @@ -30,6 +32,18 @@ export function countDistinctIdentifiers(identifiers: Identifier[]): number { ).size; } +/** + * @param identifier - SIWE identifier. + * @returns The verifier's `domain:chainId`, which escrows use as the namespace. + */ +export function getSiweNamespace(identifier: Identifier): string | undefined { + const { verifier } = identifier; + if (!isObject(verifier)) { + return undefined; + } + return `${String(verifier.domain)}:${String(verifier.chainId)}`; +} + /** * Trusted identifier-type registry. Escrows and the controller derive the * authentication mode from this table, never from a client-selected flag.