diff --git a/libs/common/src/webauthn/models/api/fido2-key.api.ts b/libs/common/src/webauthn/models/api/fido2-key.api.ts index 1eb8a56ada..4b07368eb7 100644 --- a/libs/common/src/webauthn/models/api/fido2-key.api.ts +++ b/libs/common/src/webauthn/models/api/fido2-key.api.ts @@ -1,6 +1,7 @@ import { BaseResponse } from "../../../models/response/base.response"; export class Fido2KeyApi extends BaseResponse { + nonDiscoverableId: string; keyType: "public-key"; keyAlgorithm: "ECDSA"; keyCurve: "P-256"; @@ -20,6 +21,7 @@ export class Fido2KeyApi extends BaseResponse { return; } + this.nonDiscoverableId = this.getResponseProperty("NonDiscoverableId"); this.keyType = this.getResponseProperty("KeyType"); this.keyAlgorithm = this.getResponseProperty("KeyType"); this.keyCurve = this.getResponseProperty("KeyCurve"); diff --git a/libs/common/src/webauthn/models/data/fido2-key.data.ts b/libs/common/src/webauthn/models/data/fido2-key.data.ts index 180c46d1d9..6203999c3f 100644 --- a/libs/common/src/webauthn/models/data/fido2-key.data.ts +++ b/libs/common/src/webauthn/models/data/fido2-key.data.ts @@ -1,6 +1,7 @@ import { Fido2KeyApi } from "../api/fido2-key.api"; export class Fido2KeyData { + nonDiscoverableId: string; keyType: "public-key"; keyAlgorithm: "ECDSA"; keyCurve: "P-256"; @@ -19,6 +20,7 @@ export class Fido2KeyData { return; } + this.nonDiscoverableId = data.nonDiscoverableId; this.keyType = data.keyType; this.keyAlgorithm = data.keyAlgorithm; this.keyCurve = data.keyCurve; diff --git a/libs/common/src/webauthn/models/domain/fido2-key.ts b/libs/common/src/webauthn/models/domain/fido2-key.ts index b9cb7f298e..d3c0684289 100644 --- a/libs/common/src/webauthn/models/domain/fido2-key.ts +++ b/libs/common/src/webauthn/models/domain/fido2-key.ts @@ -7,6 +7,7 @@ import { Fido2KeyData } from "../data/fido2-key.data"; import { Fido2KeyView } from "../view/fido2-key.view"; export class Fido2Key extends Domain { + nonDiscoverableId: EncString | null = null; keyType: EncString; keyAlgorithm: EncString; keyCurve: EncString; @@ -30,6 +31,7 @@ export class Fido2Key extends Domain { this, obj, { + nonDiscoverableId: null, keyType: null, keyAlgorithm: null, keyCurve: null, @@ -49,6 +51,7 @@ export class Fido2Key extends Domain { return this.decryptObj( new Fido2KeyView(), { + nonDiscoverableId: null, keyType: null, keyAlgorithm: null, keyCurve: null, @@ -68,6 +71,7 @@ export class Fido2Key extends Domain { toFido2KeyData(): Fido2KeyData { const i = new Fido2KeyData(); this.buildDataModel(this, i, { + nonDiscoverableId: null, keyType: null, keyAlgorithm: null, keyCurve: null, @@ -87,6 +91,7 @@ export class Fido2Key extends Domain { return null; } + const nonDiscoverableId = EncString.fromJSON(obj.nonDiscoverableId); const keyType = EncString.fromJSON(obj.keyType); const keyAlgorithm = EncString.fromJSON(obj.keyAlgorithm); const keyCurve = EncString.fromJSON(obj.keyCurve); @@ -99,6 +104,7 @@ export class Fido2Key extends Domain { const origin = EncString.fromJSON(obj.origin); return Object.assign(new Fido2Key(), obj, { + nonDiscoverableId, keyType, keyAlgorithm, keyCurve, diff --git a/libs/common/src/webauthn/models/view/fido2-key.view.ts b/libs/common/src/webauthn/models/view/fido2-key.view.ts index 094c580469..f498ac14d4 100644 --- a/libs/common/src/webauthn/models/view/fido2-key.view.ts +++ b/libs/common/src/webauthn/models/view/fido2-key.view.ts @@ -3,6 +3,7 @@ import { Jsonify } from "type-fest"; import { ItemView } from "../../../vault/models/view/item.view"; export class Fido2KeyView extends ItemView { + nonDiscoverableId: string; keyType: "public-key"; keyAlgorithm: "ECDSA"; keyCurve: "P-256"; diff --git a/libs/common/src/webauthn/services/fido2-authenticator.service.spec.ts b/libs/common/src/webauthn/services/fido2-authenticator.service.spec.ts index 3651b541b4..82ebecfc09 100644 --- a/libs/common/src/webauthn/services/fido2-authenticator.service.spec.ts +++ b/libs/common/src/webauthn/services/fido2-authenticator.service.spec.ts @@ -92,7 +92,67 @@ describe("FidoAuthenticatorService", () => { describe.skip("when extensions parameter is present", () => undefined); - describe("when vault contains excluded credential", () => { + describe("when vault contains excluded non-discoverable credential", () => { + let excludedCipherView: CipherView; + let params: Fido2AuthenticatorMakeCredentialsParams; + + beforeEach(async () => { + const excludedCipher = createCipher({ type: CipherType.Login }); + excludedCipherView = await excludedCipher.decrypt(); + excludedCipherView.fido2Key.nonDiscoverableId = Utils.newGuid(); + params = await createParams({ + excludeCredentialDescriptorList: [ + { + id: Fido2Utils.stringToBuffer(excludedCipherView.fido2Key.nonDiscoverableId), + type: "public-key", + }, + ], + }); + cipherService.get.mockImplementation(async (id) => + id === excludedCipher.id ? excludedCipher : undefined + ); + cipherService.getAllDecrypted.mockResolvedValue([excludedCipherView]); + }); + + /** + * Spec: collect an authorization gesture confirming user consent for creating a new credential. + * Deviation: Consent is not asked and the user is simply informed of the situation. + **/ + it("should inform user", async () => { + userInterface.informExcludedCredential.mockResolvedValue(); + + try { + await authenticator.makeCredential(params); + // eslint-disable-next-line no-empty + } catch {} + + expect(userInterface.informExcludedCredential).toHaveBeenCalled(); + }); + + /** Spec: return an error code equivalent to "NotAllowedError" and terminate the operation. */ + it("should throw error", async () => { + userInterface.informExcludedCredential.mockResolvedValue(); + + const result = async () => await authenticator.makeCredential(params); + + await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.NotAllowed); + }); + + it("should not inform user of duplication when input data does not pass checks", async () => { + userInterface.informExcludedCredential.mockResolvedValue(); + const invalidParams = await createInvalidParams(); + + for (const p of Object.values(invalidParams)) { + try { + await authenticator.makeCredential(p); + // eslint-disable-next-line no-empty + } catch {} + } + expect(userInterface.informExcludedCredential).not.toHaveBeenCalled(); + }); + }); + + describe("when vault contains excluded discoverable credential", () => { let excludedCipherView: CipherView; let params: Fido2AuthenticatorMakeCredentialsParams; @@ -153,6 +213,7 @@ describe("FidoAuthenticatorService", () => { beforeEach(async () => { params = await createParams({ requireResidentKey: true }); + cipherService.getAllDecrypted.mockResolvedValue([]); }); /** @@ -193,6 +254,7 @@ describe("FidoAuthenticatorService", () => { name: params.rpEntity.name, fido2Key: expect.objectContaining({ + nonDiscoverableId: null, keyType: "public-key", keyAlgorithm: "ECDSA", keyCurve: "P-256", @@ -274,6 +336,7 @@ describe("FidoAuthenticatorService", () => { name: existingCipherView.name, fido2Key: expect.objectContaining({ + nonDiscoverableId: expect.anything(), keyType: "public-key", keyAlgorithm: "ECDSA", keyCurve: "P-256", @@ -311,56 +374,82 @@ describe("FidoAuthenticatorService", () => { }); }); - describe("attestation of new credential", () => { - const cipherId = "75280e7e-a72e-4d6c-bf1e-d37238352f9b"; - const cipherIdBytes = new Uint8Array([ - 0x75, 0x28, 0x0e, 0x7e, 0xa7, 0x2e, 0x4d, 0x6c, 0xbf, 0x1e, 0xd3, 0x72, 0x38, 0x35, 0x2f, - 0x9b, - ]); - let params: Fido2AuthenticatorMakeCredentialsParams; + for (const requireResidentKey of [true, false]) { + describe(`attestation of new ${ + requireResidentKey ? "discoverable" : "non-discoverable" + } credential`, () => { + const cipherId = "75280e7e-a72e-4d6c-bf1e-d37238352f9b"; + const cipherIdBytes = new Uint8Array([ + 0x75, 0x28, 0x0e, 0x7e, 0xa7, 0x2e, 0x4d, 0x6c, 0xbf, 0x1e, 0xd3, 0x72, 0x38, 0x35, 0x2f, + 0x9b, + ]); + const nonDiscoverableId = "52217b91-73f1-4fea-b3f2-54a7959fd5aa"; + const nonDiscoverableIdBytes = new Uint8Array([ + 0x52, 0x21, 0x7b, 0x91, 0x73, 0xf1, 0x4f, 0xea, 0xb3, 0xf2, 0x54, 0xa7, 0x95, 0x9f, 0xd5, + 0xaa, + ]); + let params: Fido2AuthenticatorMakeCredentialsParams; - beforeEach(async () => { - params = await createParams({ requireResidentKey: true }); - userInterface.confirmNewCredential.mockResolvedValue(true); - cipherService.encrypt.mockResolvedValue({} as unknown as Cipher); - cipherService.createWithServer.mockImplementation(async (cipher) => { - cipher.id = cipherId; - return cipher; + beforeEach(async () => { + const cipher = createCipher({ id: cipherId, type: CipherType.Login }); + params = await createParams({ requireResidentKey }); + userInterface.confirmNewNonDiscoverableCredential.mockResolvedValue(cipherId); + userInterface.confirmNewCredential.mockResolvedValue(true); + cipherService.get.mockImplementation(async (cipherId) => + cipherId === cipher.id ? cipher : undefined + ); + cipherService.getAllDecrypted.mockResolvedValue([await cipher.decrypt()]); + cipherService.encrypt.mockImplementation(async (cipher) => { + cipher.fido2Key.nonDiscoverableId = nonDiscoverableId; // Replace id for testability + return {} as any; + }); + cipherService.createWithServer.mockImplementation(async (cipher) => { + cipher.id = cipherId; + return cipher; + }); + cipherService.updateWithServer.mockImplementation(async (cipher) => { + cipher.id = cipherId; + return cipher; + }); + }); + + it("should return attestation object", async () => { + const result = await authenticator.makeCredential(params); + + const attestationObject = CBOR.decode(result.buffer); + + const encAuthData: Uint8Array = attestationObject.authData; + const rpIdHash = encAuthData.slice(0, 32); + const flags = encAuthData.slice(32, 33); + const counter = encAuthData.slice(33, 37); + const aaguid = encAuthData.slice(37, 53); + const credentialIdLength = encAuthData.slice(53, 55); + const credentialId = encAuthData.slice(55, 71); + // Public key format is not tested here since it will be tested + // by the assertion tests. + // const publicKey = encAuthData.slice(87); + + expect(attestationObject.fmt).toBe("none"); + expect(attestationObject.attStmt).toEqual({}); + expect(rpIdHash).toEqual( + new Uint8Array([ + 0x22, 0x6b, 0xb3, 0x92, 0x02, 0xff, 0xf9, 0x22, 0xdc, 0x74, 0x05, 0xcd, 0x28, 0xa8, + 0x34, 0x5a, 0xc4, 0xf2, 0x64, 0x51, 0xd7, 0x3d, 0x0b, 0x40, 0xef, 0xf3, 0x1d, 0xc1, + 0xd0, 0x5c, 0x3d, 0xc3, + ]) + ); + expect(flags).toEqual(new Uint8Array([0b00000001])); // UP = true + expect(counter).toEqual(new Uint8Array([0, 0, 0, 0])); // 0 because of new counter + expect(aaguid).toEqual(AAGUID); + expect(credentialIdLength).toEqual(new Uint8Array([0, 16])); // 16 bytes because we're using GUIDs + if (requireResidentKey) { + expect(credentialId).toEqual(cipherIdBytes); + } else { + expect(credentialId).toEqual(nonDiscoverableIdBytes); + } }); }); - - it("should throw error if user denies creation request", async () => { - const result = await authenticator.makeCredential(params); - - const attestationObject = CBOR.decode(result.buffer); - - const encAuthData: Uint8Array = attestationObject.authData; - const rpIdHash = encAuthData.slice(0, 32); - const flags = encAuthData.slice(32, 33); - const counter = encAuthData.slice(33, 37); - const aaguid = encAuthData.slice(37, 53); - const credentialIdLength = encAuthData.slice(53, 55); - const credentialId = encAuthData.slice(55, 71); - // Public key format is not tested here since it will be tested - // by the assertion tests. - // const publicKey = encAuthData.slice(87); - - expect(attestationObject.fmt).toBe("none"); - expect(attestationObject.attStmt).toEqual({}); - expect(rpIdHash).toEqual( - new Uint8Array([ - 0x22, 0x6b, 0xb3, 0x92, 0x02, 0xff, 0xf9, 0x22, 0xdc, 0x74, 0x05, 0xcd, 0x28, 0xa8, - 0x34, 0x5a, 0xc4, 0xf2, 0x64, 0x51, 0xd7, 0x3d, 0x0b, 0x40, 0xef, 0xf3, 0x1d, 0xc1, - 0xd0, 0x5c, 0x3d, 0xc3, - ]) - ); - expect(flags).toEqual(new Uint8Array([0b00000001])); // UP = true - expect(counter).toEqual(new Uint8Array([0, 0, 0, 0])); // 0 because of new counter - expect(aaguid).toEqual(AAGUID); - expect(credentialIdLength).toEqual(new Uint8Array([0, 16])); // 16 bytes because we're using GUIDs - expect(credentialId).toEqual(cipherIdBytes); - }); - }); + } async function createParams( params: Partial = {} @@ -469,6 +558,7 @@ function createCipher(data: Partial = {}): Cipher { const cipher = new Cipher(); cipher.id = data.id ?? Utils.newGuid(); cipher.type = data.type ?? CipherType.Fido2Key; + cipher.login = data.type ?? data.type === CipherType.Login ? new Login() : null; cipher.fido2Key = data.fido2Key ?? new Fido2Key(); return cipher; } diff --git a/libs/common/src/webauthn/services/fido2-authenticator.service.ts b/libs/common/src/webauthn/services/fido2-authenticator.service.ts index f5ac35a379..36a37648eb 100644 --- a/libs/common/src/webauthn/services/fido2-authenticator.service.ts +++ b/libs/common/src/webauthn/services/fido2-authenticator.service.ts @@ -123,7 +123,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr attStmt: {}, authData: await generateAuthData({ rpId: params.rpEntity.id, - credentialId: cipher.id, + credentialId: params.requireResidentKey ? cipher.id : cipher.fido2Key.nonDiscoverableId, counter: cipher.fido2Key.counter, userPresence: true, userVerification: false, @@ -153,13 +153,15 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr } private async vaultContainsId(ids: string[]): Promise { - for (const id of ids) { - if ((await this.cipherService.get(id)) != undefined) { - return true; - } - } + const ciphers = await this.cipherService.getAllDecrypted(); - return false; + return ciphers.some( + (cipher) => + (cipher.type === CipherType.Fido2Key && ids.includes(cipher.id)) || + (cipher.type === CipherType.Login && + cipher.fido2Key != undefined && + ids.includes(cipher.fido2Key.nonDiscoverableId)) + ); } private async createKeyPair() { @@ -180,6 +182,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr const pcks8Key = await crypto.subtle.exportKey("pkcs8", keyValue); const fido2Key = new Fido2KeyView(); + fido2Key.nonDiscoverableId = params.requireResidentKey ? null : Utils.newGuid(); fido2Key.keyType = "public-key"; fido2Key.keyAlgorithm = "ECDSA"; fido2Key.keyCurve = "P-256";