diff options
-rw-r--r-- | .github/ISSUE_TEMPLATE/library-problem.md | 2 | ||||
-rw-r--r-- | .vscode/settings.json | 3 | ||||
-rw-r--r-- | README.md | 2 | ||||
-rwxr-xr-x | bootstrap.sh | 10 | ||||
-rw-r--r-- | package.json | 1 | ||||
-rw-r--r-- | packages/browser/src/methods/startRegistration.test.ts | 55 | ||||
-rw-r--r-- | packages/browser/src/methods/startRegistration.ts | 26 | ||||
-rw-r--r-- | packages/server/deno.lock | 37 | ||||
-rw-r--r-- | packages/server/src/helpers/parseAuthenticatorData.test.ts | 20 | ||||
-rw-r--r-- | packages/server/src/helpers/parseAuthenticatorData.ts | 17 | ||||
-rw-r--r-- | pnpm-lock.yaml | 26 |
11 files changed, 154 insertions, 45 deletions
diff --git a/.github/ISSUE_TEMPLATE/library-problem.md b/.github/ISSUE_TEMPLATE/library-problem.md index 4e81309..2270ede 100644 --- a/.github/ISSUE_TEMPLATE/library-problem.md +++ b/.github/ISSUE_TEMPLATE/library-problem.md @@ -30,7 +30,7 @@ assignees: '' - **OS:** (e.g. macOS 13.2) - **Browser:** (e.g. Chrome 110) -- **Authenticator:** (e.g. YubiKey 5, iPhone 12 @ iOS 16.3) +- **Authenticator:** (e.g. YubiKey 5, iPhone 12 @ iOS 16.3, 1Password browser extension 2.15.1) ### SimpleWebAuthn Libraries diff --git a/.vscode/settings.json b/.vscode/settings.json index aa3e177..04bb916 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,6 +1,9 @@ { "typescript.tsdk": "node_modules/typescript/lib", "editor.formatOnSave": true, + // `deno.enable` is superfluous but necessary pre-Deno 1.37 (we're currently using 1.36) + // See https://github.com/denoland/deno/issues/20411#issuecomment-1725920550 + "deno.enable": true, "deno.path": "/opt/homebrew/bin/deno", "deno.enablePaths": [ "./packages/server", @@ -64,7 +64,7 @@ Install the following before proceeding: After pulling down the code, set up dependencies: ```sh -$> pnpm install +$> pnpm run bootstrap-monorepo ``` To run unit tests for all workspace packages, use the `test` series of scripts: diff --git a/bootstrap.sh b/bootstrap.sh new file mode 100755 index 0000000..6928605 --- /dev/null +++ b/bootstrap.sh @@ -0,0 +1,10 @@ +# Install root dependencies without trying to link workspace packages +pnpm install --ignore-workspace +# Built typescript-types so server can build +pnpm run build:types --skip-nx-cache +# If we can't run tests on a fresh pull then it might mean we need to refresh the Deno lock file +pnpm run test:server --skip-nx-cache +# If we can build server then the repo is probably ready for dev +pnpm run build:server --skip-nx-cache +# Link all the workspace packages as usual with a standard install +pnpm install diff --git a/package.json b/package.json index 9cf5cbe..c74ed3a 100644 --- a/package.json +++ b/package.json @@ -3,6 +3,7 @@ "version": "0.0.0", "private": true, "scripts": { + "bootstrap-monorepo": "./bootstrap.sh", "clean": "rm -rf ./packages/**/node_modules && rm -rf ./packages/**/dist && rm -rf ./packages/**/npm", "lint": "deno lint packages/**/src/**/*.ts example/**/*.ts", "test": "pnpm run test:browser; pnpm run test:server", diff --git a/packages/browser/src/methods/startRegistration.test.ts b/packages/browser/src/methods/startRegistration.test.ts index b8ca081..97878c6 100644 --- a/packages/browser/src/methods/startRegistration.test.ts +++ b/packages/browser/src/methods/startRegistration.test.ts @@ -309,6 +309,61 @@ test('should not return convenience values if getters missing', async () => { expect(response.response.authenticatorData).toBeUndefined(); }); +test('should survive browser extensions that intercept WebAuthn and incorrectly implement public key value getters', async () => { + /** + * 1Password browser extension v2.15.1 (the one that introduced passkeys support) seemed to have + * implemented the following methods on AuthenticatorAttestationResponse... + * + * - getPublicKeyAlgorithm() + * - getPublicKey() + * - getAuthenticatorData() + * + * ...But when you attempt to call them as methods they'll error out with `TypeError`'s: + * + * Safari: + * > TypeError: Can only call AuthenticatorAttestationResponse.getPublicKeyAlgorithm on instances + * > of AuthenticatorAttestationResponse + * + * Chrome: + * > TypeError: Illegal invocation + * + * Firefox: + * > N/A (it handled it fine for some reason) + * + * Make sure `startRegistration()` can survive this scenario. + * + * See https://github.com/MasterKale/SimpleWebAuthn/issues/438 for more context. + */ + + // Mock extension return values from the browser extension intercepting WebAuthn + mockNavigatorCreate.mockImplementation((): Promise<unknown> => { + return new Promise((resolve) => { + resolve({ + response: { + getPublicKeyAlgorithm: () => { + throw new Error('I throw for some reason'); + }, + getPublicKey: () => { + throw new Error('I also throw for some reason'); + }, + getAuthenticatorData: () => { + throw new Error('I throw for some reason too'); + }, + }, + getClientExtensionResults: () => {}, + }); + }); + }); + + await expect(startRegistration(goodOpts1)).resolves; + + const response = await startRegistration(goodOpts1); + + expect(response.response.publicKeyAlgorithm).toBeUndefined(); + expect(response.response.publicKey).toBeUndefined(); + expect(response.response.authenticatorData).toBeUndefined(); +}); + describe('WebAuthnError', () => { describe('AbortError', () => { const AbortError = generateCustomError('AbortError'); diff --git a/packages/browser/src/methods/startRegistration.ts b/packages/browser/src/methods/startRegistration.ts index 74da7fd..9f4a104 100644 --- a/packages/browser/src/methods/startRegistration.ts +++ b/packages/browser/src/methods/startRegistration.ts @@ -67,23 +67,35 @@ export async function startRegistration( // L3 says this is required, but browser and webview support are still not guaranteed. let responsePublicKeyAlgorithm: number | undefined = undefined; if (typeof response.getPublicKeyAlgorithm === 'function') { - responsePublicKeyAlgorithm = response.getPublicKeyAlgorithm(); + try { + responsePublicKeyAlgorithm = response.getPublicKeyAlgorithm(); + } catch (error) { + // pass + } } let responsePublicKey: string | undefined = undefined; if (typeof response.getPublicKey === 'function') { - const _publicKey = response.getPublicKey(); - if (_publicKey !== null) { - responsePublicKey = bufferToBase64URLString(_publicKey); + try { + const _publicKey = response.getPublicKey(); + if (_publicKey !== null) { + responsePublicKey = bufferToBase64URLString(_publicKey); + } + } catch (error) { + // pass } } // L3 says this is required, but browser and webview support are still not guaranteed. let responseAuthenticatorData: string | undefined; if (typeof response.getAuthenticatorData === 'function') { - responseAuthenticatorData = bufferToBase64URLString( - response.getAuthenticatorData(), - ); + try { + responseAuthenticatorData = bufferToBase64URLString( + response.getAuthenticatorData(), + ); + } catch (error) { + // pass + } } return { diff --git a/packages/server/deno.lock b/packages/server/deno.lock index ea6ba9c..eb4e600 100644 --- a/packages/server/deno.lock +++ b/packages/server/deno.lock @@ -137,26 +137,23 @@ "https://deno.land/x/wasmbuild@0.14.1/cache.ts": "89eea5f3ce6035a1164b3e655c95f21300498920575ade23161421f5b01967f4", "https://deno.land/x/wasmbuild@0.14.1/loader.ts": "d98d195a715f823151cbc8baa3f32127337628379a02d9eb2a3c5902dbccfc02", "https://deno.land/x/xhr@0.3.0/mod.ts": "094aacd627fd9635cd942053bf8032b5223b909858fa9dc8ffa583752ff63b20", - "https://esm.sh/@peculiar/asn1-android@2.3.6": "ad561af8e9c98ca24edea2a06b78126388d30e8d7563aa8ff019539e63ad1409", - "https://esm.sh/@peculiar/asn1-ecc@2.3.6": "09b28e664b009f8682fbbc6bfaae9c6738ede103cc7eb2250304687fc611cb45", - "https://esm.sh/@peculiar/asn1-rsa@2.3.6": "28c4bbbc2cdde75e0899a0a97b1d332be7f60c1b096c43749cc9f02b248be025", - "https://esm.sh/@peculiar/asn1-schema@2.3.6": "c19e745f7409b7f09be724c8b6a9e58b0399f5c58d86290a1a44bac2285991d5", - "https://esm.sh/@peculiar/asn1-x509@2.3.6": "38b5e7aa6ba5da5c13cc20550d35bbe8190738b34436faf4c2f3b1f81588a4ca", - "https://esm.sh/cross-fetch@4.0.0": "c7c8c2e11976c1815ac47c08e8195a674513c4b82df0dcc296e759ceaf8f8c21", - "https://esm.sh/debug@4.3.4": "c6bcd0430fb5d0a499bbacce8bbe8f051616cda3c0696337b0749247e451f0db", - "https://esm.sh/v131/@peculiar/asn1-android@2.3.6/denonext/asn1-android.mjs": "34764b794e10675a2567bbe7bdc7fde073353e8abc413fbcd9c3fde2ed1c5c3b", - "https://esm.sh/v131/@peculiar/asn1-ecc@2.3.6/denonext/asn1-ecc.mjs": "be8ea819764a103e2ba44c38764079d760a1568fbb54b26bc402d2ce471a8ff7", - "https://esm.sh/v131/@peculiar/asn1-rsa@2.3.6/denonext/asn1-rsa.mjs": "728598dab7aa294feb67304a87d21e854050b29f72c7230f45fd798b1fcf8e86", - "https://esm.sh/v131/@peculiar/asn1-schema@2.3.6/denonext/asn1-schema.mjs": "04aed5d13f20547dae79896a6d50797e08fe3c6b1de529f756f48a6da591a66e", - "https://esm.sh/v131/@peculiar/asn1-x509@2.3.6/denonext/asn1-x509.mjs": "fb3dc85d465d8b7594ba9ae91c109143e330af6c2ef814d3d112ab7d63417450", - "https://esm.sh/v131/asn1js@3.0.5/denonext/asn1js.mjs": "c430a2ca4adfbb1d38611c8d3b1c040aefa7d9bafd7d15f552667026427a88b7", - "https://esm.sh/v131/cross-fetch@4.0.0/denonext/cross-fetch.mjs": "238eae32034ab217cdbef10b812c7357f61ab2730c58207f4cf9135e9dec7a61", - "https://esm.sh/v131/debug@4.3.4/denonext/debug.mjs": "892826bb4505deb6337df2f0f72b1e355e5377e702dd739b78774539d7482f5c", - "https://esm.sh/v131/ipaddr.js@2.1.0/denonext/ipaddr.mjs": "214c1ee845e73e7c51ba673b36e7e6b9c5d8e8fb1fc19443444ad9cec44adf7e", - "https://esm.sh/v131/ms@2.1.2/denonext/ms.mjs": "aa4dc45ba72554c5011168f8910cc646c37af53cfff1a15a4decced838b8eb14", - "https://esm.sh/v131/pvtsutils@1.3.2/denonext/pvtsutils.mjs": "1018ae12b1c67a8a314a265834bf3a7252c2105611a6699f215b543ee7f006b0", - "https://esm.sh/v131/pvutils@1.1.3/denonext/pvutils.mjs": "ad156a239ec0f1aa99f123fab44abc292c87d4fd6969d7af264436630457523c", - "https://esm.sh/v131/tslib@2.6.1/denonext/tslib.mjs": "4694ecbc32a05aa4a37497b65f3e55018c24b2a4e4f05462a7746fc4850b4791" + "https://esm.sh/@peculiar/asn1-android@2.3.6": "fb4dd2b9fe423ad0f98c5f7c8b365404f56827b3cfc1b9abbce6d419f72908c3", + "https://esm.sh/@peculiar/asn1-ecc@2.3.6": "6e259ae1390d9d6f0bb3055b4d06b467925747f8db9f78156c34a5ea79beaac1", + "https://esm.sh/@peculiar/asn1-rsa@2.3.6": "9c2a9481c9515ea860378742ad255bce7424a0d2f24f87e7d390aac669135dc8", + "https://esm.sh/@peculiar/asn1-schema@2.3.6": "c15028356c71536e543f9112a04efc56aa9b47d739e1d57d162fe1a6e5abc6ce", + "https://esm.sh/@peculiar/asn1-x509@2.3.6": "701eadefdaac96a504cb024efb38dec25c1fa2c61b3963b2b6d17c324fcc54e5", + "https://esm.sh/cross-fetch@4.0.0": "455ae44f37acd1ed33e9449c428750b6129152532aeccd3de69bd19005a38331", + "https://esm.sh/v132/@peculiar/asn1-android@2.3.6/denonext/asn1-android.mjs": "5e01acec4a325066184c8a2f2f8d4864a571b9039b5679078b3893a93e90a1b5", + "https://esm.sh/v132/@peculiar/asn1-ecc@2.3.6/denonext/asn1-ecc.mjs": "5989ad7184187eb1b16e2133deecf1d2b96c31ef96927b1892b2c2653890cb69", + "https://esm.sh/v132/@peculiar/asn1-rsa@2.3.6/denonext/asn1-rsa.mjs": "f687c324c0d89aa12e450a2b33aaf7271bb77e05e2fef9a83544a94ecb7f8c5b", + "https://esm.sh/v132/@peculiar/asn1-schema@2.3.6/denonext/asn1-schema.mjs": "f4cc2f4f847fff6b9789e13efee1e8c4a3d3f67c16caa90d532619b67ea46241", + "https://esm.sh/v132/@peculiar/asn1-x509@2.3.6/denonext/asn1-x509.mjs": "34745efad09645b27c6c3125aca6b91c3478f2f417eff74d43d5bf5fb412e5bf", + "https://esm.sh/v132/asn1js@3.0.5/denonext/asn1js.mjs": "06e1a6a2bbe1ab615ab45fa4c5351749fb47a24bd8a9b72f490df8bd17f30cb3", + "https://esm.sh/v132/cross-fetch@4.0.0/denonext/cross-fetch.mjs": "238eae32034ab217cdbef10b812c7357f61ab2730c58207f4cf9135e9dec7a61", + "https://esm.sh/v132/ipaddr.js@2.1.0/denonext/ipaddr.mjs": "214c1ee845e73e7c51ba673b36e7e6b9c5d8e8fb1fc19443444ad9cec44adf7e", + "https://esm.sh/v132/pvtsutils@1.3.5/denonext/pvtsutils.mjs": "016782b990e2652a326cf1d8984c2adc4552171da902cc73dc8e12ffdbe3a100", + "https://esm.sh/v132/pvutils@1.1.3/denonext/pvutils.mjs": "ad156a239ec0f1aa99f123fab44abc292c87d4fd6969d7af264436630457523c", + "https://esm.sh/v132/tslib@2.6.2/denonext/tslib.mjs": "29782bcd3139f77ec063dc5a9385c0fff4a8d0a23b6765c73d9edeb169a04bf1" }, "npm": { "specifiers": { diff --git a/packages/server/src/helpers/parseAuthenticatorData.test.ts b/packages/server/src/helpers/parseAuthenticatorData.test.ts index 0e4b112..e1f5be2 100644 --- a/packages/server/src/helpers/parseAuthenticatorData.test.ts +++ b/packages/server/src/helpers/parseAuthenticatorData.test.ts @@ -2,7 +2,7 @@ import { assertEquals } from 'https://deno.land/std@0.198.0/assert/mod.ts'; import { parseAuthenticatorData } from './parseAuthenticatorData.ts'; import { AuthenticationExtensionsAuthenticatorOutputs } from './decodeAuthenticatorExtensions.ts'; -import { isoBase64URL } from './iso/index.ts'; +import { isoBase64URL, isoUint8Array } from './iso/index.ts'; // Grabbed this from a Conformance test, contains attestation data const authDataWithAT = isoBase64URL.toBuffer( @@ -64,3 +64,21 @@ Deno.test('should parse extension data', () => { } as AuthenticationExtensionsAuthenticatorOutputs, ); }); + +Deno.test('should parse malformed authenticator data from Firefox 117', () => { + /** + * Firefox 117 is incorrectly serializing authenticator data, and using string values for kty and + * crv at the same time. See the following issues for more context (I've dealt with this issue + * before, over in the py_webauthn project): + * + * - https://github.com/duo-labs/py_webauthn/issues/175 + * - https://github.com/mozilla/authenticator-rs/pull/292 + */ + const authDataBadKty = + 'b40499b0271a68957267de4ec40056a74c8758c6582e1e01fcf357d73101e7ba450000000400000000000000000000000000000000008072d3a1a3fa7cf32f44367df847585ff0850c7bd62c338ab45be1fda6fdb79982f96c20efc0bb6ed9347e8c1e77690e67b225b485a098f6f46fde3f2a85acd0177a04d6bb5c7566fb89881dfe48ea7abc361f7acaf86a5966adef557930fa5c045c636f50cf938e508a81b845134eb2988dc3af0ab6f98cfc615532684b4a6363a301634f4b50032720674564323535313921982018d51858187318e6188918eb18ab187e18fd18fd185d184b08184b187318e818e118f818c71518ff18f5183a18fd18a3186b185f1109183e183b14'; + + const parsed = parseAuthenticatorData(isoUint8Array.fromHex(authDataBadKty)); + + // If we can assert this then it means we could parse the bad auth data above + assertEquals(parsed.flags.at, true); +}); diff --git a/packages/server/src/helpers/parseAuthenticatorData.ts b/packages/server/src/helpers/parseAuthenticatorData.ts index 497f2d4..28bd469 100644 --- a/packages/server/src/helpers/parseAuthenticatorData.ts +++ b/packages/server/src/helpers/parseAuthenticatorData.ts @@ -53,6 +53,23 @@ export function parseAuthenticatorData( credentialID = authData.slice(pointer, pointer += credIDLen); + /** + * Firefox 117 incorrectly CBOR-encodes authData when EdDSA (-8) is used for the public key. + * A CBOR "Map of 3 items" (0xa3) should be "Map of 4 items" (0xa4), and if we manually adjust + * the single byte there's a good chance the authData can be correctly parsed. + * + * This browser release also incorrectly uses the string labels "OKP" and "Ed25519" instead of + * their integer representations for kty and crv respectively. That's why the COSE public key + * in the hex below looks so odd. + */ + // Bytes decode to `{ 1: "OKP", 3: -8, -1: "Ed25519" }` (it's missing key -2 a.k.a. COSEKEYS.x) + const badEdDSACBOR = isoUint8Array.fromHex('a301634f4b500327206745643235353139'); + const bytesAtCurrentPosition = authData.slice(pointer, pointer + badEdDSACBOR.byteLength); + if (isoUint8Array.areEqual(badEdDSACBOR, bytesAtCurrentPosition)) { + // Change the bad CBOR 0xa3 to 0xa4 so that the credential public key can be recognized + authData[pointer] = 0xa4; + } + // Decode the next CBOR item in the buffer, then re-encode it back to a Buffer const firstDecoded = isoCBOR.decodeFirst<COSEPublicKey>( authData.slice(pointer), diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index beec721..1ff76db 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -101,7 +101,7 @@ importers: version: link:../../typescript-types/npm cbor-x: specifier: ^1.5.2 - version: 1.5.2 + version: 1.5.4 cross-fetch: specifier: ^4.0.0 version: 4.0.0 @@ -111,7 +111,7 @@ importers: version: 0.4.0 '@types/node': specifier: ^18.11.9 - version: 18.11.9 + version: 18.17.8 picocolors: specifier: ^1.0.0 version: 1.0.0 @@ -1558,10 +1558,6 @@ packages: resolution: {integrity: sha512-jhuKLIRrhvCPLqwPcx6INqmKeiA5EWrsCOPhrlFSrbrmU4ZMPjj5Ul/oLCMDO98XRUIwVm78xICz4EPCektzeQ==} dev: true - /@types/node@18.11.9: - resolution: {integrity: sha512-CRpX21/kGdzjOpFsZSkcrXMGIBWMGNIHXXBVFSH+ggkftxg+XYP20TESbh+zFvFj3EQOl5byk0HTRn1IL6hbqg==} - dev: true - /@types/node@18.17.8: resolution: {integrity: sha512-Av/7MqX/iNKwT9Tr60V85NqMnsmh8ilfJoBlIVibkXfitk9Q22D9Y5mSpm+FvG5DET7EbVfB40bOiLzKgYFgPw==} dev: true @@ -2074,8 +2070,8 @@ packages: dev: false optional: true - /cbor-x@1.5.2: - resolution: {integrity: sha512-JArE6xcgj3eo13fpnShO42QFBUuXP2uG12RLeF2Nb+dJcETFYxkUa27gXQrRYp67Ahtaxyfbg+ihc62XTyQqsQ==} + /cbor-x@1.5.4: + resolution: {integrity: sha512-PVKILDn+Rf6MRhhcyzGXi5eizn1i0i3F8Fe6UMMxXBnWkalq9+C5+VTmlIjAYM4iF2IYF2N+zToqAfYOp+3rfw==} optionalDependencies: cbor-extract: 2.1.1 dev: false @@ -2423,7 +2419,7 @@ packages: /cross-fetch@4.0.0: resolution: {integrity: sha512-e4a5N8lVvuLgAWgnCrLr2PP0YyDOTHa9H/Rj54dirp61qXnNq46m82bRhNqIA5VccJtWBvPTFRV3TtvHUKPB1g==} dependencies: - node-fetch: 2.6.13 + node-fetch: 2.7.0 transitivePeerDependencies: - encoding dev: false @@ -4740,8 +4736,8 @@ packages: resolution: {integrity: sha512-mmcei9JghVNDYydghQmeDX8KoAm0FAiYyIcUt/N4nhyAipB17pllZQDOJD2fotxABnt4Mdz+dKTO7eftLg4d0A==} dev: true - /node-fetch@2.6.13: - resolution: {integrity: sha512-StxNAxh15zr77QvvkmveSQ8uCQ4+v5FkvNTj0OESmiHu+VRi/gXArXtkWMElOsOUNLtUEvI4yS+rdtOHZTwlQA==} + /node-fetch@2.6.7: + resolution: {integrity: sha512-ZjMPFEfVx5j+y2yF35Kzx5sF7kDzxuDj6ziH4FFbOp87zKDZNx8yExJIb05OGF4Nlt9IHFIMBkRl41VdvcNdbQ==} engines: {node: 4.x || >=6.0.0} peerDependencies: encoding: ^0.1.0 @@ -4750,10 +4746,10 @@ packages: optional: true dependencies: whatwg-url: 5.0.0 - dev: false + dev: true - /node-fetch@2.6.7: - resolution: {integrity: sha512-ZjMPFEfVx5j+y2yF35Kzx5sF7kDzxuDj6ziH4FFbOp87zKDZNx8yExJIb05OGF4Nlt9IHFIMBkRl41VdvcNdbQ==} + /node-fetch@2.7.0: + resolution: {integrity: sha512-c4FRfUm/dbcWZ7U+1Wq0AwCyFL+3nt2bEw05wfxSz+DWpWsitgmSgYmy2dQdWyKC1694ELPqMs/YzUSNozLt8A==} engines: {node: 4.x || >=6.0.0} peerDependencies: encoding: ^0.1.0 @@ -4762,7 +4758,7 @@ packages: optional: true dependencies: whatwg-url: 5.0.0 - dev: true + dev: false /node-gyp-build-optional-packages@5.0.3: resolution: {integrity: sha512-k75jcVzk5wnnc/FMxsf4udAoTEUv2jY3ycfdSd3yWu6Cnd1oee6/CfZJApyscA4FJOmdoixWwiwOyf16RzD5JA==} |