From acf5b1e9e6860cb1a94eb7d1b46c9ca10f7af3b6 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 21 Nov 2024 15:54:19 +0100 Subject: [PATCH] [PM-7382] Add support for non-UUID credential (#11993) * feat: add tests for guidToRawFormat * feat: add support for parsing b64 credential ids * refactor: change interface to use Uint8Array for simplification Technically this deviates from the specification, but nobody is going to be using the authenticator directly but us so it shouldn't matter. We're gonna switch to `passkey-rs` anyways so * feat: change how the authenticator parses credential ids to support b64 --- ...fido2-authenticator.service.abstraction.ts | 2 +- .../fido2/credential-id-utils.spec.ts | 68 +++++++++++++++++++ .../services/fido2/credential-id-utils.ts | 31 +++++++++ .../fido2/fido2-authenticator.service.spec.ts | 14 ++-- .../fido2/fido2-authenticator.service.ts | 29 ++++---- .../services/fido2/guid-utils.spec.ts | 28 ++++++++ 6 files changed, 148 insertions(+), 24 deletions(-) create mode 100644 libs/common/src/platform/services/fido2/credential-id-utils.spec.ts create mode 100644 libs/common/src/platform/services/fido2/credential-id-utils.ts create mode 100644 libs/common/src/platform/services/fido2/guid-utils.spec.ts diff --git a/libs/common/src/platform/abstractions/fido2/fido2-authenticator.service.abstraction.ts b/libs/common/src/platform/abstractions/fido2/fido2-authenticator.service.abstraction.ts index 535248e7ecd..d878d70b56e 100644 --- a/libs/common/src/platform/abstractions/fido2/fido2-authenticator.service.abstraction.ts +++ b/libs/common/src/platform/abstractions/fido2/fido2-authenticator.service.abstraction.ts @@ -64,7 +64,7 @@ export class Fido2AuthenticatorError extends Error { } export interface PublicKeyCredentialDescriptor { - id: BufferSource; + id: Uint8Array; transports?: ("ble" | "hybrid" | "internal" | "nfc" | "usb")[]; type: "public-key"; } diff --git a/libs/common/src/platform/services/fido2/credential-id-utils.spec.ts b/libs/common/src/platform/services/fido2/credential-id-utils.spec.ts new file mode 100644 index 00000000000..76e068ac01c --- /dev/null +++ b/libs/common/src/platform/services/fido2/credential-id-utils.spec.ts @@ -0,0 +1,68 @@ +import { compareCredentialIds, parseCredentialId } from "./credential-id-utils"; + +describe("credential-id-utils", () => { + describe("parseCredentialId", () => { + it("returns credentialId in binary format when given a valid UUID string", () => { + const result = parseCredentialId("08d70b74-e9f5-4522-a425-e5dcd40107e7"); + + expect(result).toEqual( + new Uint8Array([ + 0x08, 0xd7, 0x0b, 0x74, 0xe9, 0xf5, 0x45, 0x22, 0xa4, 0x25, 0xe5, 0xdc, 0xd4, 0x01, 0x07, + 0xe7, + ]), + ); + }); + + it("returns credentialId in binary format when given a valid Base64Url string", () => { + const result = parseCredentialId("b64.CNcLdOn1RSKkJeXc1AEH5w"); + + expect(result).toEqual( + new Uint8Array([ + 0x08, 0xd7, 0x0b, 0x74, 0xe9, 0xf5, 0x45, 0x22, 0xa4, 0x25, 0xe5, 0xdc, 0xd4, 0x01, 0x07, + 0xe7, + ]), + ); + }); + + it("returns undefined when given an invalid Base64 string", () => { + const result = parseCredentialId("b64.#$%&"); + + expect(result).toBeUndefined(); + }); + + it("returns undefined when given an invalid UUID string", () => { + const result = parseCredentialId("invalid"); + + expect(result).toBeUndefined(); + }); + }); + + describe("compareCredentialIds", () => { + it("returns true when the two credential IDs are equal", () => { + const a = new Uint8Array([0x01, 0x02, 0x03]); + const b = new Uint8Array([0x01, 0x02, 0x03]); + + const result = compareCredentialIds(a, b); + + expect(result).toBe(true); + }); + + it("returns false when the two credential IDs are not equal", () => { + const a = new Uint8Array([0x01, 0x02, 0x03]); + const b = new Uint8Array([0x01, 0x02, 0x04]); + + const result = compareCredentialIds(a, b); + + expect(result).toBe(false); + }); + + it("returns false when the two credential IDs have different lengths", () => { + const a = new Uint8Array([0x01, 0x02, 0x03]); + const b = new Uint8Array([0x01, 0x02, 0x03, 0x04]); + + const result = compareCredentialIds(a, b); + + expect(result).toBe(false); + }); + }); +}); diff --git a/libs/common/src/platform/services/fido2/credential-id-utils.ts b/libs/common/src/platform/services/fido2/credential-id-utils.ts new file mode 100644 index 00000000000..a548b8befd3 --- /dev/null +++ b/libs/common/src/platform/services/fido2/credential-id-utils.ts @@ -0,0 +1,31 @@ +import { Fido2Utils } from "./fido2-utils"; +import { guidToRawFormat } from "./guid-utils"; + +export function parseCredentialId(encodedCredentialId: string): Uint8Array { + try { + if (encodedCredentialId.startsWith("b64.")) { + return Fido2Utils.stringToBuffer(encodedCredentialId.slice(4)); + } + + return guidToRawFormat(encodedCredentialId); + } catch { + return undefined; + } +} + +/** + * Compares two credential IDs for equality. + */ +export function compareCredentialIds(a: Uint8Array, b: Uint8Array): boolean { + if (a.length !== b.length) { + return false; + } + + for (let i = 0; i < a.length; i++) { + if (a[i] !== b[i]) { + return false; + } + } + + return true; +} diff --git a/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts b/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts index 5f15005d71c..e3f79ff9d58 100644 --- a/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts +++ b/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts @@ -26,9 +26,9 @@ import { import { Utils } from "../../misc/utils"; import { CBOR } from "./cbor"; +import { parseCredentialId } from "./credential-id-utils"; import { AAGUID, Fido2AuthenticatorService } from "./fido2-authenticator.service"; import { Fido2Utils } from "./fido2-utils"; -import { guidToRawFormat } from "./guid-utils"; const RpId = "bitwarden.com"; @@ -139,7 +139,7 @@ describe("FidoAuthenticatorService", () => { params = await createParams({ excludeCredentialDescriptorList: [ { - id: guidToRawFormat(excludedCipher.login.fido2Credentials[0].credentialId), + id: parseCredentialId(excludedCipher.login.fido2Credentials[0].credentialId), type: "public-key", }, ], @@ -482,7 +482,7 @@ describe("FidoAuthenticatorService", () => { credentialId = Utils.newGuid(); params = await createParams({ allowCredentialDescriptorList: [ - { id: guidToRawFormat(credentialId), type: "public-key" }, + { id: parseCredentialId(credentialId), type: "public-key" }, ], rpId: RpId, }); @@ -546,7 +546,7 @@ describe("FidoAuthenticatorService", () => { let params: Fido2AuthenticatorGetAssertionParams; beforeEach(async () => { - credentialIds = [Utils.newGuid(), Utils.newGuid()]; + credentialIds = [Utils.newGuid(), "b64.Lb5SVTumSV6gYJpeWh3laA"]; ciphers = [ await createCipherView( { type: CipherType.Login }, @@ -559,7 +559,7 @@ describe("FidoAuthenticatorService", () => { ]; params = await createParams({ allowCredentialDescriptorList: credentialIds.map((credentialId) => ({ - id: guidToRawFormat(credentialId), + id: parseCredentialId(credentialId), type: "public-key", })), rpId: RpId, @@ -667,7 +667,7 @@ describe("FidoAuthenticatorService", () => { selectedCredentialId = credentialIds[0]; params = await createParams({ allowCredentialDescriptorList: credentialIds.map((credentialId) => ({ - id: guidToRawFormat(credentialId), + id: parseCredentialId(credentialId), type: "public-key", })), rpId: RpId, @@ -723,7 +723,7 @@ describe("FidoAuthenticatorService", () => { const flags = encAuthData.slice(32, 33); const counter = encAuthData.slice(33, 37); - expect(result.selectedCredential.id).toEqual(guidToRawFormat(selectedCredentialId)); + expect(result.selectedCredential.id).toEqual(parseCredentialId(selectedCredentialId)); expect(result.selectedCredential.userHandle).toEqual( Fido2Utils.stringToBuffer(fido2Credentials[0].userHandle), ); diff --git a/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts b/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts index 8f0523769d9..e5d3685b9c2 100644 --- a/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts +++ b/libs/common/src/platform/services/fido2/fido2-authenticator.service.ts @@ -23,9 +23,10 @@ import { LogService } from "../../abstractions/log.service"; import { Utils } from "../../misc/utils"; import { CBOR } from "./cbor"; +import { compareCredentialIds, parseCredentialId } from "./credential-id-utils"; import { p1363ToDer } from "./ecdsa-utils"; import { Fido2Utils } from "./fido2-utils"; -import { guidToRawFormat, guidToStandardFormat } from "./guid-utils"; +import { guidToStandardFormat } from "./guid-utils"; // AAGUID: d548826e-79b4-db40-a3d8-11116f7e8349 export const AAGUID = new Uint8Array([ @@ -178,7 +179,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr const authData = await generateAuthData({ rpId: params.rpEntity.id, - credentialId: guidToRawFormat(credentialId), + credentialId: parseCredentialId(credentialId), counter: fido2Credential.counter, userPresence: true, userVerification: userVerified, @@ -193,7 +194,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr ); return { - credentialId: guidToRawFormat(credentialId), + credentialId: parseCredentialId(credentialId), attestationObject, authData, publicKey: pubKeyDer, @@ -313,7 +314,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr const authenticatorData = await generateAuthData({ rpId: selectedFido2Credential.rpId, - credentialId: guidToRawFormat(selectedCredentialId), + credentialId: parseCredentialId(selectedCredentialId), counter: selectedFido2Credential.counter, userPresence: true, userVerification: userVerified, @@ -328,7 +329,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr return { authenticatorData, selectedCredential: { - id: guidToRawFormat(selectedCredentialId), + id: parseCredentialId(selectedCredentialId), userHandle: Fido2Utils.stringToBuffer(selectedFido2Credential.userHandle), }, signature, @@ -412,16 +413,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr credentials: PublicKeyCredentialDescriptor[], rpId: string, ): Promise { - const ids: string[] = []; - - for (const credential of credentials) { - try { - ids.push(guidToStandardFormat(credential.id)); - // eslint-disable-next-line no-empty - } catch {} - } - - if (ids.length === 0) { + if (credentials.length === 0) { return []; } @@ -432,7 +424,12 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr cipher.type === CipherType.Login && cipher.login.hasFido2Credentials && cipher.login.fido2Credentials[0].rpId === rpId && - ids.includes(cipher.login.fido2Credentials[0].credentialId), + credentials.some((credential) => + compareCredentialIds( + credential.id, + parseCredentialId(cipher.login.fido2Credentials[0].credentialId), + ), + ), ); } diff --git a/libs/common/src/platform/services/fido2/guid-utils.spec.ts b/libs/common/src/platform/services/fido2/guid-utils.spec.ts new file mode 100644 index 00000000000..098ea4bee75 --- /dev/null +++ b/libs/common/src/platform/services/fido2/guid-utils.spec.ts @@ -0,0 +1,28 @@ +import { guidToRawFormat } from "./guid-utils"; + +describe("guid-utils", () => { + describe("guidToRawFormat", () => { + it.each([ + [ + "00000000-0000-0000-0000-000000000000", + [ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, + ], + "08d70b74-e9f5-4522-a425-e5dcd40107e7", + [ + 0x08, 0xd7, 0x0b, 0x74, 0xe9, 0xf5, 0x45, 0x22, 0xa4, 0x25, 0xe5, 0xdc, 0xd4, 0x01, 0x07, + 0xe7, + ], + ], + ])("returns UUID in binary format when given a valid UUID string", (input, expected) => { + const result = guidToRawFormat(input); + + expect(result).toEqual(new Uint8Array(expected)); + }); + + it("throws an error when given an invalid UUID string", () => { + expect(() => guidToRawFormat("invalid")).toThrow(TypeError); + }); + }); +});