diff --git a/libs/common/src/webauthn/services/domain-utils.spec.ts b/libs/common/src/webauthn/services/domain-utils.spec.ts new file mode 100644 index 00000000000..7c9c27869a2 --- /dev/null +++ b/libs/common/src/webauthn/services/domain-utils.spec.ts @@ -0,0 +1,53 @@ +import { isValidRpId } from "./domain-utils"; + +// Spec: If options.rp.id is not a registrable domain suffix of and is not equal to effectiveDomain, return a DOMException whose name is "SecurityError", and terminate this algorithm. +describe("validateRpId", () => { + it("should not be valid when rpId is more specific than origin", () => { + const rpId = "sub.login.bitwarden.com"; + const origin = "https://login.bitwarden.com:1337"; + + expect(isValidRpId(rpId, origin)).toBe(false); + }); + + it("should not be valid when effective domains of rpId and origin do not match", () => { + const rpId = "passwordless.dev"; + const origin = "https://login.bitwarden.com:1337"; + + expect(isValidRpId(rpId, origin)).toBe(false); + }); + + it("should not be valid when subdomains are the same but effective domains of rpId and origin do not match", () => { + const rpId = "login.passwordless.dev"; + const origin = "https://login.bitwarden.com:1337"; + + expect(isValidRpId(rpId, origin)).toBe(false); + }); + + it("should be valid when domains of rpId and origin are the same", () => { + const rpId = "bitwarden.com"; + const origin = "https://bitwarden.com"; + + expect(isValidRpId(rpId, origin)).toBe(true); + }); + + it("should be valid when origin is a subdomain of rpId", () => { + const rpId = "bitwarden.com"; + const origin = "https://login.bitwarden.com:1337"; + + expect(isValidRpId(rpId, origin)).toBe(true); + }); + + it("should be valid when domains of rpId and origin are the same and they are both subdomains", () => { + const rpId = "login.bitwarden.com"; + const origin = "https://login.bitwarden.com:1337"; + + expect(isValidRpId(rpId, origin)).toBe(true); + }); + + it("should be valid when origin is a subdomain of rpId and they are both subdomains", () => { + const rpId = "login.bitwarden.com"; + const origin = "https://sub.login.bitwarden.com:1337"; + + expect(isValidRpId(rpId, origin)).toBe(true); + }); +}); diff --git a/libs/common/src/webauthn/services/domain-utils.ts b/libs/common/src/webauthn/services/domain-utils.ts new file mode 100644 index 00000000000..20b6e41700d --- /dev/null +++ b/libs/common/src/webauthn/services/domain-utils.ts @@ -0,0 +1,11 @@ +import { parse } from "tldts"; + +export function isValidRpId(rpId: string, origin: string) { + const parsedOrigin = parse(origin, { allowPrivateDomains: true }); + const parsedRpId = parse(rpId, { allowPrivateDomains: true }); + + return ( + parsedOrigin.domain === parsedRpId.domain && + parsedOrigin.subdomain.endsWith(parsedRpId.subdomain) + ); +} diff --git a/libs/common/src/webauthn/services/fido2-authenticator.service.ts b/libs/common/src/webauthn/services/fido2-authenticator.service.ts index 1817c1c645f..6b23c55ee70 100644 --- a/libs/common/src/webauthn/services/fido2-authenticator.service.ts +++ b/libs/common/src/webauthn/services/fido2-authenticator.service.ts @@ -115,7 +115,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr cipher.fido2Key = await createKeyView(params, keyPair.privateKey); const reencrypted = await this.cipherService.encrypt(cipher); await this.cipherService.updateWithServer(reencrypted); - } catch { + } catch (error) { throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.Unknown); } } diff --git a/libs/common/src/webauthn/services/fido2-client.service.spec.ts b/libs/common/src/webauthn/services/fido2-client.service.spec.ts index 6996e317326..c719650366f 100644 --- a/libs/common/src/webauthn/services/fido2-client.service.spec.ts +++ b/libs/common/src/webauthn/services/fido2-client.service.spec.ts @@ -28,7 +28,7 @@ describe("FidoAuthenticatorService", () => { }); describe("createCredential", () => { - describe("invalid input parameters", () => { + describe("input parameters validation", () => { // Spec: If sameOriginWithAncestors is false, return a "NotAllowedError" DOMException. it("should throw error if sameOriginWithAncestors is false", async () => { const params = createParams({ sameOriginWithAncestors: false }); @@ -81,10 +81,23 @@ describe("FidoAuthenticatorService", () => { }); // Spec: If options.rp.id is not a registrable domain suffix of and is not equal to effectiveDomain, return a DOMException whose name is "SecurityError", and terminate this algorithm. - it("should throw error if rp.id does not match origin effective domain", async () => { + it("should throw error if rp.id is not valid for this origin", async () => { const params = createParams({ - origin: "passwordless.dev", - rp: { id: "bitwarden.com", name: "Bitwarden" }, + origin: "https://passwordless.dev", + rp: { id: "bitwarden.com", name: "Bitwraden" }, + }); + + const result = async () => await client.createCredential(params); + + const rejects = expect(result).rejects; + await rejects.toMatchObject({ name: "SecurityError" }); + await rejects.toBeInstanceOf(DOMException); + }); + + it("should throw error if origin is not an https domain", async () => { + const params = createParams({ + origin: "http://passwordless.dev", + rp: { id: "bitwarden.com", name: "Bitwraden" }, }); const result = async () => await client.createCredential(params); @@ -179,7 +192,7 @@ describe("FidoAuthenticatorService", () => { function createParams(params: Partial = {}): CreateCredentialParams { return { - origin: params.origin ?? "bitwarden.com", + origin: params.origin ?? "https://bitwarden.com", sameOriginWithAncestors: params.sameOriginWithAncestors ?? true, attestation: params.attestation, authenticatorSelection: params.authenticatorSelection, @@ -234,9 +247,22 @@ describe("FidoAuthenticatorService", () => { }); // Spec: If options.rp.id is not a registrable domain suffix of and is not equal to effectiveDomain, return a DOMException whose name is "SecurityError", and terminate this algorithm. - it("should throw error if rp.id does not match origin effective domain", async () => { + it("should throw error if rp.id is not valid for this origin", async () => { const params = createParams({ - origin: "passwordless.dev", + origin: "https://passwordless.dev", + rpId: "bitwarden.com", + }); + + const result = async () => await client.assertCredential(params); + + const rejects = expect(result).rejects; + await rejects.toMatchObject({ name: "SecurityError" }); + await rejects.toBeInstanceOf(DOMException); + }); + + it("should throw error if origin is not an http domain", async () => { + const params = createParams({ + origin: "http://passwordless.dev", rpId: "bitwarden.com", }); @@ -345,7 +371,7 @@ describe("FidoAuthenticatorService", () => { return { allowedCredentialIds: params.allowedCredentialIds ?? [], challenge: params.challenge ?? Fido2Utils.bufferToString(randomBytes(16)), - origin: params.origin ?? RpId, + origin: params.origin ?? "https://bitwarden.com", rpId: params.rpId ?? RpId, timeout: params.timeout, userVerification: params.userVerification, diff --git a/libs/common/src/webauthn/services/fido2-client.service.ts b/libs/common/src/webauthn/services/fido2-client.service.ts index ed50f76cbff..3244fd2b586 100644 --- a/libs/common/src/webauthn/services/fido2-client.service.ts +++ b/libs/common/src/webauthn/services/fido2-client.service.ts @@ -20,6 +20,8 @@ import { } from "../abstractions/fido2-client.service.abstraction"; import { Fido2Utils } from "../abstractions/fido2-utils"; +import { isValidRpId } from "./domain-utils"; + export class Fido2ClientService implements Fido2ClientServiceAbstraction { constructor(private authenticator: Fido2AuthenticatorService) {} @@ -36,14 +38,15 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction { throw new TypeError("Invalid 'user.id' length"); } - const { domain: effectiveDomain } = parse(params.origin, { allowPrivateDomains: true }); - if (effectiveDomain == undefined) { - throw new DOMException("'origin' is not a valid domain", "SecurityError"); + const parsedOrigin = parse(params.origin, { allowPrivateDomains: true }); + const rpId = params.rp.id ?? parsedOrigin.domain; + + if (parsedOrigin.domain == undefined || !params.origin.startsWith("https://")) { + throw new DOMException("'origin' is not a valid https origin", "SecurityError"); } - const rpId = params.rp.id ?? effectiveDomain; - if (effectiveDomain !== rpId) { - throw new DOMException("'rp.id' does not match origin effective domain", "SecurityError"); + if (!isValidRpId(rpId, params.origin)) { + throw new DOMException("'rp.id' cannot be used with the current origin", "SecurityError"); } let credTypesAndPubKeyAlgs: PublicKeyCredentialParam[]; @@ -72,19 +75,15 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction { const clientDataJSON = JSON.stringify(collectedClientData); const clientDataJSONBytes = Utils.fromByteStringToArray(clientDataJSON); const clientDataHash = await crypto.subtle.digest({ name: "SHA-256" }, clientDataJSONBytes); - if (abortController.signal.aborted) { throw new DOMException(undefined, "AbortError"); } - const timeout = setAbortTimeout( abortController, params.authenticatorSelection?.userVerification, params.timeout ); - const excludeCredentialDescriptorList: PublicKeyCredentialDescriptor[] = []; - if (params.excludeCredentials !== undefined) { for (const credential of params.excludeCredentials) { try { @@ -97,7 +96,6 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction { } catch {} } } - const makeCredentialParams: Fido2AuthenticatorMakeCredentialsParams = { requireResidentKey: params.authenticatorSelection?.residentKey === "required" || @@ -117,7 +115,6 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction { displayName: params.user.displayName, }, }; - let makeCredentialResult; try { makeCredentialResult = await this.authenticator.makeCredential( @@ -133,12 +130,10 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction { } throw new DOMException(undefined, "NotAllowedError"); } - if (abortController.signal.aborted) { throw new DOMException(undefined, "AbortError"); } clearTimeout(timeout); - return { credentialId: Fido2Utils.bufferToString(makeCredentialResult.credentialId), attestationObject: Fido2Utils.bufferToString(makeCredentialResult.attestationObject), @@ -158,9 +153,15 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction { throw new DOMException("'origin' is not a valid domain", "SecurityError"); } - const rpId = params.rpId ?? effectiveDomain; - if (effectiveDomain !== rpId) { - throw new DOMException("'rp.id' does not match origin effective domain", "SecurityError"); + const parsedOrigin = parse(params.origin, { allowPrivateDomains: true }); + const rpId = params.rpId ?? parsedOrigin.domain; + + if (parsedOrigin.domain == undefined || !params.origin.startsWith("https://")) { + throw new DOMException("'origin' is not a valid https origin", "SecurityError"); + } + + if (!isValidRpId(rpId, params.origin)) { + throw new DOMException("'rp.id' cannot be used with the current origin", "SecurityError"); } const collectedClientData = {