From 843c83328622a54ee70ac8a98d307c17be3fbbe8 Mon Sep 17 00:00:00 2001 From: Frederik Rothenberger Date: Fri, 15 Mar 2024 10:24:56 +0100 Subject: [PATCH 1/3] fix: Remove ArrayBuffer checks from WebAuthnIdentity This PR removes `instanceof ArrayBuffer` checks from the `WebAuthnIdentity`. The reason is that these checks hide errors and cause problem in conjunction with the Bitwarden password manager (which sets these fields to `Uint8Array`). See also this issue: https://github.com/dfinity/internet-identity/issues/2235 Simply removing the checks solved the issue entirely, as the fields that supposedly need to be `ArrayBuffer` are converted to `Uint8Array` anyway. This change was tested with Internet Identity and the Bitwarden extension in Chrome. --- docs/CHANGELOG.md | 1 + packages/identity/src/identity/webauthn.ts | 40 +++++++--------------- 2 files changed, 13 insertions(+), 28 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index ac9e990a1..c5559f228 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -6,6 +6,7 @@ * feat: adds `fromPem` method for `identity-secp256k1` * feat: HttpAgent tracks a watermark from the latest readState call. Queries with signatures made before the watermark will be automatically retried, and rejected if they are still behind. +* fix: remove `ArrrayBuffer` checks from `WebAuthnIdentity` to resolve issues with the Bitwarden password manager ## [1.0.1] - 2024-02-20 diff --git a/packages/identity/src/identity/webauthn.ts b/packages/identity/src/identity/webauthn.ts index 87195ac8a..7ebae02d9 100644 --- a/packages/identity/src/identity/webauthn.ts +++ b/packages/identity/src/identity/webauthn.ts @@ -105,14 +105,7 @@ async function _createCredential( }, }, )) as PublicKeyCredentialWithAttachment; - - // Validate that it's the correct type at runtime, since WebAuthn does not HAVE to - // reply with a PublicKeyCredential. - if (creds.response === undefined || !(creds.rawId instanceof ArrayBuffer)) { - return null; - } else { - return creds; - } + return creds; } // See https://www.iana.org/assignments/cose/cose.xhtml#algorithms for a complete @@ -154,9 +147,6 @@ export class WebAuthnIdentity extends SignIdentity { } const response = creds.response as AuthenticatorAttestationResponse; - if (!(response.attestationObject instanceof ArrayBuffer)) { - throw new Error('Was expecting an attestation response.'); - } // Parse the attestationObject as CBOR. const attObject = borc.decodeFirst(new Uint8Array(response.attestationObject)); @@ -214,24 +204,18 @@ export class WebAuthnIdentity extends SignIdentity { } const response = result.response as AuthenticatorAssertionResponse; - if ( - response.signature instanceof ArrayBuffer && - response.authenticatorData instanceof ArrayBuffer - ) { - const cbor = borc.encode( - new borc.Tagged(55799, { - authenticator_data: new Uint8Array(response.authenticatorData), - client_data_json: new TextDecoder().decode(response.clientDataJSON), - signature: new Uint8Array(response.signature), - }), - ); - if (!cbor) { - throw new Error('failed to encode cbor'); - } - return cbor.buffer as Signature; - } else { - throw new Error('Invalid response from WebAuthn.'); + + const cbor = borc.encode( + new borc.Tagged(55799, { + authenticator_data: new Uint8Array(response.authenticatorData), + client_data_json: new TextDecoder().decode(response.clientDataJSON), + signature: new Uint8Array(response.signature), + }), + ); + if (!cbor) { + throw new Error('failed to encode cbor'); } + return cbor.buffer as Signature; } /** From 52fbb90ef9f6cf3a902085974689d57534fb94d8 Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Fri, 15 Mar 2024 10:16:11 -0700 Subject: [PATCH 2/3] bufFromBufLike --- packages/identity/src/identity/webauthn.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/identity/src/identity/webauthn.ts b/packages/identity/src/identity/webauthn.ts index 7ebae02d9..bbc6681ea 100644 --- a/packages/identity/src/identity/webauthn.ts +++ b/packages/identity/src/identity/webauthn.ts @@ -10,6 +10,7 @@ import { } from '@dfinity/agent'; import borc from 'borc'; import { randomBytes } from '@noble/hashes/utils'; +import { bufFromBufLike } from '@dfinity/candid'; function _coseToDerEncodedBlob(cose: ArrayBuffer): DerEncodedPublicKey { return wrapDER(cose, DER_COSE_OID).buffer as DerEncodedPublicKey; @@ -104,8 +105,17 @@ async function _createCredential( }, }, }, - )) as PublicKeyCredentialWithAttachment; - return creds; + )) as PublicKeyCredentialWithAttachment | null; + + if (creds === null) { + return null; + } + + return { + ...creds, + // Some password managers will return a Uint8Array, so we ensure we return an ArrayBuffer. + rawId: bufFromBufLike(creds.rawId), + }; } // See https://www.iana.org/assignments/cose/cose.xhtml#algorithms for a complete From b3c30de8d2f5f2475c52600a65da24b97a8d2275 Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Fri, 15 Mar 2024 10:25:47 -0700 Subject: [PATCH 3/3] checking presence of attestationObject instead of checking for ArrayBuffer --- packages/identity/src/identity/webauthn.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/identity/src/identity/webauthn.ts b/packages/identity/src/identity/webauthn.ts index bbc6681ea..0bcdb9264 100644 --- a/packages/identity/src/identity/webauthn.ts +++ b/packages/identity/src/identity/webauthn.ts @@ -157,6 +157,9 @@ export class WebAuthnIdentity extends SignIdentity { } const response = creds.response as AuthenticatorAttestationResponse; + if (response.attestationObject === undefined) { + throw new Error('Was expecting an attestation response.'); + } // Parse the attestationObject as CBOR. const attObject = borc.decodeFirst(new Uint8Array(response.attestationObject));