Merge pull request #256 from MasterKale/feat/better-signature-verification

feat/better-signature-verification

10 files changed, 220 insertions, 279 deletions

diff --git a/packages/server/src/authentication/verifyAuthenticationResponse.test.ts b/packages/server/src/authentication/verifyAuthenticationResponse.test.ts
index 8a5b2fa..3c9a5b3 100644
--- a/packages/server/src/authentication/verifyAuthenticationResponse.test.ts
+++ b/packages/server/src/authentication/verifyAuthenticationResponse.test.ts
@@ -22,8 +22,8 @@ afterEach(() => {
   mockParseAuthData.mockRestore();
 });
 
-test('should verify an assertion response', () => {
-  const verification = verifyAuthenticationResponse({
+test('should verify an assertion response', async () => {
+  const verification = await verifyAuthenticationResponse({
     credential: assertionResponse,
     expectedChallenge: assertionChallenge,
     expectedOrigin: assertionOrigin,
@@ -34,8 +34,8 @@ test('should verify an assertion response', () => {
   expect(verification.verified).toEqual(true);
 });
 
-test('should return authenticator info after verification', () => {
-  const verification = verifyAuthenticationResponse({
+test('should return authenticator info after verification', async () => {
+  const verification = await verifyAuthenticationResponse({
     credential: assertionResponse,
     expectedChallenge: assertionChallenge,
     expectedOrigin: assertionOrigin,
@@ -47,31 +47,31 @@ test('should return authenticator info after verification', () => {
   expect(verification.authenticationInfo.credentialID).toEqual(authenticator.credentialID);
 });
 
-test('should throw when response challenge is not expected value', () => {
-  expect(() => {
+test('should throw when response challenge is not expected value', async () => {
+  await expect(
     verifyAuthenticationResponse({
       credential: assertionResponse,
       expectedChallenge: 'shouldhavebeenthisvalue',
       expectedOrigin: 'https://different.address',
       expectedRPID: 'dev.dontneeda.pw',
       authenticator: authenticator,
-    });
-  }).toThrow(/authentication response challenge/i);
+    }),
+  ).rejects.toThrow(/authentication response challenge/i);
 });
 
-test('should throw when response origin is not expected value', () => {
-  expect(() => {
+test('should throw when response origin is not expected value', async () => {
+  await expect(
     verifyAuthenticationResponse({
       credential: assertionResponse,
       expectedChallenge: assertionChallenge,
       expectedOrigin: 'https://different.address',
       expectedRPID: 'dev.dontneeda.pw',
       authenticator: authenticator,
-    });
-  }).toThrow(/authentication response origin/i);
+    }),
+  ).rejects.toThrow(/authentication response origin/i);
 });
 
-test('should throw when assertion type is not webauthn.create', () => {
+test('should throw when assertion type is not webauthn.create', async () => {
   // @ts-ignore 2345
   mockDecodeClientData.mockReturnValue({
     origin: assertionOrigin,
@@ -79,35 +79,35 @@ test('should throw when assertion type is not webauthn.create', () => {
     challenge: assertionChallenge,
   });
 
-  expect(() => {
+  await expect(
     verifyAuthenticationResponse({
       credential: assertionResponse,
       expectedChallenge: assertionChallenge,
       expectedOrigin: assertionOrigin,
       expectedRPID: 'dev.dontneeda.pw',
       authenticator: authenticator,
-    });
-  }).toThrow(/authentication response type/i);
+    }),
+  ).rejects.toThrow(/authentication response type/i);
 });
 
-test('should throw error if user was not present', () => {
+test('should throw error if user was not present', async () => {
   mockParseAuthData.mockReturnValue({
     rpIdHash: toHash(Buffer.from('dev.dontneeda.pw', 'ascii')),
     flags: 0,
   });
 
-  expect(() => {
+  await expect(
     verifyAuthenticationResponse({
       credential: assertionResponse,
       expectedChallenge: assertionChallenge,
       expectedOrigin: assertionOrigin,
       expectedRPID: 'dev.dontneeda.pw',
       authenticator: authenticator,
-    });
-  }).toThrow(/not present/i);
+    }),
+  ).rejects.toThrow(/not present/i);
 });
 
-test('should throw error if previous counter value is not less than in response', () => {
+test('should throw error if previous counter value is not less than in response', async () => {
   // This'll match the `counter` value in `assertionResponse`, simulating a potential replay attack
   const badCounter = 144;
   const badDevice = {
@@ -115,36 +115,36 @@ test('should throw error if previous counter value is not less than in response'
     counter: badCounter,
   };
 
-  expect(() => {
+  await expect(
     verifyAuthenticationResponse({
       credential: assertionResponse,
       expectedChallenge: assertionChallenge,
       expectedOrigin: assertionOrigin,
       expectedRPID: 'dev.dontneeda.pw',
       authenticator: badDevice,
-    });
-  }).toThrow(/counter value/i);
+    }),
+  ).rejects.toThrow(/counter value/i);
 });
 
-test('should throw error if assertion RP ID is unexpected value', () => {
+test('should throw error if assertion RP ID is unexpected value', async () => {
   mockParseAuthData.mockReturnValue({
     rpIdHash: toHash(Buffer.from('bad.url', 'ascii')),
     flags: 0,
   });
 
-  expect(() => {
+  await expect(
     verifyAuthenticationResponse({
       credential: assertionResponse,
       expectedChallenge: assertionChallenge,
       expectedOrigin: assertionOrigin,
       expectedRPID: 'dev.dontneeda.pw',
       authenticator: authenticator,
-    });
-  }).toThrow(/rp id/i);
+    }),
+  ).rejects.toThrow(/rp id/i);
 });
 
-test('should not compare counters if both are 0', () => {
-  const verification = verifyAuthenticationResponse({
+test('should not compare counters if both are 0', async () => {
+  const verification = await verifyAuthenticationResponse({
     credential: assertionFirstTimeUsedResponse,
     expectedChallenge: assertionFirstTimeUsedChallenge,
     expectedOrigin: assertionFirstTimeUsedOrigin,
@@ -155,7 +155,7 @@ test('should not compare counters if both are 0', () => {
   expect(verification.verified).toEqual(true);
 });
 
-test('should throw an error if user verification is required but user was not verified', () => {
+test('should throw an error if user verification is required but user was not verified', async () => {
   const actualData = esmParseAuthenticatorData.parseAuthenticatorData(
     base64url.toBuffer(assertionResponse.response.authenticatorData),
   );
@@ -168,7 +168,7 @@ test('should throw an error if user verification is required but user was not ve
     },
   });
 
-  expect(() => {
+  await expect(
     verifyAuthenticationResponse({
       credential: assertionResponse,
       expectedChallenge: assertionChallenge,
@@ -176,15 +176,15 @@ test('should throw an error if user verification is required but user was not ve
       expectedRPID: 'dev.dontneeda.pw',
       authenticator: authenticator,
       requireUserVerification: true,
-    });
-  }).toThrow(/user could not be verified/i);
+    }),
+  ).rejects.toThrow(/user could not be verified/i);
 });
 
 // TODO: Get a real TPM authentication response in here
-test.skip('should verify TPM assertion', () => {
+test.skip('should verify TPM assertion', async () => {
   const expectedChallenge = 'dG90YWxseVVuaXF1ZVZhbHVlRXZlcnlBc3NlcnRpb24';
   jest.spyOn(base64url, 'encode').mockReturnValueOnce(expectedChallenge);
-  const verification = verifyAuthenticationResponse({
+  const verification = await verifyAuthenticationResponse({
     credential: {
       id: 'YJ8FMM-AmcUt73XPX341WXWd7ypBMylGjjhu0g3VzME',
       rawId: 'YJ8FMM-AmcUt73XPX341WXWd7ypBMylGjjhu0g3VzME',
@@ -212,8 +212,8 @@ test.skip('should verify TPM assertion', () => {
   expect(verification.verified).toEqual(true);
 });
 
-test('should support multiple possible origins', () => {
-  const verification = verifyAuthenticationResponse({
+test('should support multiple possible origins', async () => {
+  const verification = await verifyAuthenticationResponse({
     credential: assertionResponse,
     expectedChallenge: assertionChallenge,
     expectedOrigin: ['https://simplewebauthn.dev', assertionOrigin],
@@ -225,19 +225,19 @@ test('should support multiple possible origins', () => {
 });
 
 test('should throw an error if origin not in list of expected origins', async () => {
-  expect(() => {
+  await expect(
     verifyAuthenticationResponse({
       credential: assertionResponse,
       expectedChallenge: assertionChallenge,
       expectedOrigin: ['https://simplewebauthn.dev', 'https://fizz.buzz'],
       expectedRPID: 'dev.dontneeda.pw',
       authenticator: authenticator,
-    });
-  }).toThrow(/unexpected authentication response origin/i);
+    }),
+  ).rejects.toThrow(/unexpected authentication response origin/i);
 });
 
 test('should support multiple possible RP IDs', async () => {
-  const verification = verifyAuthenticationResponse({
+  const verification = await verifyAuthenticationResponse({
     credential: assertionResponse,
     expectedChallenge: assertionChallenge,
     expectedOrigin: assertionOrigin,
@@ -249,19 +249,19 @@ test('should support multiple possible RP IDs', async () => {
 });
 
 test('should throw an error if RP ID not in list of possible RP IDs', async () => {
-  expect(() => {
+  await expect(
     verifyAuthenticationResponse({
       credential: assertionResponse,
       expectedChallenge: assertionChallenge,
       expectedOrigin: assertionOrigin,
       expectedRPID: ['simplewebauthn.dev'],
       authenticator: authenticator,
-    });
-  }).toThrow(/unexpected rp id/i);
+    }),
+  ).rejects.toThrow(/unexpected rp id/i);
 });
 
-test('should pass verification if custom challenge verifier returns true', () => {
-  const verification = verifyAuthenticationResponse({
+test('should pass verification if custom challenge verifier returns true', async () => {
+  const verification = await verifyAuthenticationResponse({
     credential: {
       id: 'AaIBxnYfL2pDWJmIii6CYgHBruhVvFGHheWamphVioG_TnEXxKA9MW4FWnJh21zsbmRpRJso9i2JmAtWOtXfVd4oXTgYVusXwhWWsA',
       rawId:
@@ -299,26 +299,26 @@ test('should pass verification if custom challenge verifier returns true', () =>
   expect(verification.verified).toEqual(true);
 });
 
-test('should fail verification if custom challenge verifier returns false', () => {
-  expect(() => {
+test('should fail verification if custom challenge verifier returns false', async () => {
+  await expect(
     verifyAuthenticationResponse({
       credential: assertionResponse,
       expectedChallenge: challenge => challenge === 'willNeverMatch',
       expectedOrigin: assertionOrigin,
       expectedRPID: 'dev.dontneeda.pw',
       authenticator: authenticator,
-    });
-  }).toThrow(/custom challenge verifier returned false/i);
+    }),
+  ).rejects.toThrow(/custom challenge verifier returned false/i);
 });
 
 test('should return authenticator extension output', async () => {
-  const verification = verifyAuthenticationResponse({
+  const verification = await verifyAuthenticationResponse({
     credential: {
       response: {
         clientDataJSON:
           'eyJ0eXBlIjoid2ViYXV0aG4uZ2V0IiwiY2hhbGxlbmdlIjoiaVpzVkN6dHJEVzdEMlVfR0hDSWxZS0x3VjJiQ3NCVFJxVlFVbkpYbjlUayIsIm9yaWdpbiI6ImFuZHJvaWQ6YXBrLWtleS1oYXNoOmd4N3NxX3B4aHhocklRZEx5ZkcwcHhLd2lKN2hPazJESlE0eHZLZDQzOFEiLCJhbmRyb2lkUGFja2FnZU5hbWUiOiJjb20uZmlkby5leGFtcGxlLmZpZG8yYXBpZXhhbXBsZSJ9',
         authenticatorData:
-          'DXX8xWP9p3nbLjQ-6kiYiHWLeFSdSTpP2-oc2WqjHMSFAAAAAKFsZGV2aWNlUHViS2V5pWNkcGtYTaUBAgMmIAEhWCCZGqvtneQnGp7erYgG-dyW1tzNDEdiU6VRBInsg3m-WyJYIKCXPP3tu3nif-9O50gWc_szElBN3KVDTP0jQx1q0p7aY3NpZ1hHMEUCIElSbNKK72tOYhp9WTbStQSVL8CuIxOk8DV6r_-uqWR0AiEAnVE6yu-wsyx2Wq5v66jClGhe_2P_HL8R7PIQevT-uPhlbm9uY2VAZXNjb3BlQQBmYWFndWlkULk_2WHy5kYvsSKCACJH3ng',
+          'DXX8xWP9p3nbLjQ-6kiYiHWLeFSdSTpP2-oc2WqjHMSFAAAAAKFvZGV2aWNlUHVibGljS2V5pWNkcGtYTaUBAgMmIAEhWCCZGqvtneQnGp7erYgG-dyW1tzNDEdiU6VRBInsg3m-WyJYIKCXPP3tu3nif-9O50gWc_szElBN3KVDTP0jQx1q0p7aY3NpZ1hHMEUCIElSbNKK72tOYhp9WTbStQSVL8CuIxOk8DV6r_-uqWR0AiEAnVE6yu-wsyx2Wq5v66jClGhe_2P_HL8R7PIQevT-uPhlbm9uY2VAZXNjb3BlQQBmYWFndWlkULk_2WHy5kYvsSKCACJH3ng=',
         signature:
           'MEYCIQDlRuxY7cYre0sb3T6TovQdfYIUb72cRZYOQv_zS9wN_wIhAOvN-fwjtyIhWRceqJV4SX74-z6oALERbC7ohk8EdVPO',
         userHandle: 'b2FPajFxcmM4MWo3QkFFel9RN2lEakh5RVNlU2RLNDF0Sl92eHpQYWV5UQ==',
@@ -343,7 +343,7 @@ test('should return authenticator extension output', async () => {
   });
 
   expect(verification.authenticationInfo?.authenticatorExtensionResults).toMatchObject({
-    devicePubKey: {
+    devicePublicKey: {
       dpk: Buffer.from(
         'A5010203262001215820991AABED9DE4271A9EDEAD8806F9DC96D6DCCD0C476253A5510489EC8379BE5B225820A0973CFDEDBB79E27FEF4EE7481673FB3312504DDCA5434CFD23431D6AD29EDA',
         'hex',
@@ -360,7 +360,7 @@ test('should return authenticator extension output', async () => {
 });
 
 test('should return credential backup info', async () => {
-  const verification = verifyAuthenticationResponse({
+  const verification = await verifyAuthenticationResponse({
     credential: assertionResponse,
     expectedChallenge: assertionChallenge,
     expectedOrigin: assertionOrigin,
@@ -372,115 +372,6 @@ test('should return credential backup info', async () => {
   expect(verification.authenticationInfo?.credentialBackedUp).toEqual(false);
 });
 
-test('[FIDO Conformance] should verify if user verification is required and user was verified but not present', () => {
-  const actualData = esmParseAuthenticatorData.parseAuthenticatorData(
-    base64url.toBuffer(assertionResponse.response.authenticatorData),
-  );
-
-  mockParseAuthData.mockReturnValue({
-    ...actualData,
-    flags: {
-      up: false,
-      uv: true,
-    },
-  });
-
-  const verification = verifyAuthenticationResponse({
-    credential: assertionResponse,
-    expectedChallenge: assertionChallenge,
-    expectedOrigin: assertionOrigin,
-    expectedRPID: 'dev.dontneeda.pw',
-    authenticator: authenticator,
-    advancedFIDOConfig: {
-      userVerification: 'required',
-    }
-  });
-
-  expect(verification.verified).toEqual(true);
-});
-
-test('[FIDO Conformance] should verify if user verification is preferred and user was not verified or present', () => {
-  const actualData = esmParseAuthenticatorData.parseAuthenticatorData(
-    base64url.toBuffer(assertionResponse.response.authenticatorData),
-  );
-
-  mockParseAuthData.mockReturnValue({
-    ...actualData,
-    flags: {
-      up: false,
-      uv: false,
-    },
-  });
-
-  const verification = verifyAuthenticationResponse({
-    credential: assertionResponse,
-    expectedChallenge: assertionChallenge,
-    expectedOrigin: assertionOrigin,
-    expectedRPID: 'dev.dontneeda.pw',
-    authenticator: authenticator,
-    requireUserVerification: false,
-    advancedFIDOConfig: {
-      userVerification: 'preferred',
-    },
-  });
-
-  expect(verification.verified).toEqual(true);
-});
-
-test('[FIDO Conformance] should verify if user verification is discouraged and user was verified but not present', () => {
-  const actualData = esmParseAuthenticatorData.parseAuthenticatorData(
-    base64url.toBuffer(assertionResponse.response.authenticatorData),
-  );
-
-  mockParseAuthData.mockReturnValue({
-    ...actualData,
-    flags: {
-      up: false,
-      uv: true,
-    },
-  });
-
-  const verification = verifyAuthenticationResponse({
-    credential: assertionResponse,
-    expectedChallenge: assertionChallenge,
-    expectedOrigin: assertionOrigin,
-    expectedRPID: 'dev.dontneeda.pw',
-    authenticator: authenticator,
-    advancedFIDOConfig: {
-      userVerification: 'discouraged',
-    },
-  });
-
-  expect(verification.verified).toEqual(true);
-});
-
-test('[FIDO Conformance] should verify if user verification is discouraged and user was not verified or present', () => {
-  const actualData = esmParseAuthenticatorData.parseAuthenticatorData(
-    base64url.toBuffer(assertionResponse.response.authenticatorData),
-  );
-
-  mockParseAuthData.mockReturnValue({
-    ...actualData,
-    flags: {
-      up: false,
-      uv: false,
-    },
-  });
-
-  const verification = verifyAuthenticationResponse({
-    credential: assertionResponse,
-    expectedChallenge: assertionChallenge,
-    expectedOrigin: assertionOrigin,
-    expectedRPID: 'dev.dontneeda.pw',
-    authenticator: authenticator,
-    advancedFIDOConfig: {
-      userVerification: 'discouraged',
-    },
-  });
-
-  expect(verification.verified).toEqual(true);
-});
-
 /**
  * Assertion examples below
  */
diff --git a/packages/server/src/authentication/verifyAuthenticationResponse.ts b/packages/server/src/authentication/verifyAuthenticationResponse.ts
index f6a437e..d25d521 100644
--- a/packages/server/src/authentication/verifyAuthenticationResponse.ts
+++ b/packages/server/src/authentication/verifyAuthenticationResponse.ts
@@ -8,7 +8,6 @@ import {
 
 import { decodeClientDataJSON } from '../helpers/decodeClientDataJSON';
 import { toHash } from '../helpers/toHash';
-import { convertPublicKeyToPEM } from '../helpers/convertPublicKeyToPEM';
 import { verifySignature } from '../helpers/verifySignature';
 import { parseAuthenticatorData } from '../helpers/parseAuthenticatorData';
 import { isBase64URLString } from '../helpers/isBase64URLString';
@@ -23,8 +22,8 @@ export type VerifyAuthenticationResponseOpts = {
   authenticator: AuthenticatorDevice;
   requireUserVerification?: boolean;
   advancedFIDOConfig?: {
-    userVerification?: UserVerificationRequirement,
-  },
+    userVerification?: UserVerificationRequirement;
+  };
 };
 
 /**
@@ -46,9 +45,9 @@ export type VerifyAuthenticationResponseOpts = {
  * User Presence and User Verified flags in authenticator data: UV (and UP) flags are optional
  * unless this value is `"required"`
  */
-export function verifyAuthenticationResponse(
+export async function verifyAuthenticationResponse(
   options: VerifyAuthenticationResponseOpts,
-): VerifiedAuthenticationResponse {
+): Promise<VerifiedAuthenticationResponse> {
   const {
     credential,
     expectedChallenge,
@@ -166,9 +165,7 @@ export function verifyAuthenticationResponse(
   }
 
   if (advancedFIDOConfig !== undefined) {
-    const {
-      userVerification: fidoUserVerification,
-    } = advancedFIDOConfig;
+    const { userVerification: fidoUserVerification } = advancedFIDOConfig;
 
     /**
      * Use FIDO Conformance-defined rules for verifying UP and UV flags
@@ -199,7 +196,6 @@ export function verifyAuthenticationResponse(
   const clientDataHash = toHash(base64url.toBuffer(response.clientDataJSON));
   const signatureBase = Buffer.concat([authDataBuffer, clientDataHash]);
 
-  const publicKey = convertPublicKeyToPEM(authenticator.credentialPublicKey);
   const signature = base64url.toBuffer(response.signature);
 
   if ((counter > 0 || authenticator.counter > 0) && counter <= authenticator.counter) {
@@ -215,7 +211,11 @@ export function verifyAuthenticationResponse(
   const { credentialDeviceType, credentialBackedUp } = parseBackupFlags(flags);
 
   const toReturn = {
-    verified: verifySignature(signature, signatureBase, publicKey),
+    verified: await verifySignature({
+      signature,
+      signatureBase,
+      credentialPublicKey: authenticator.credentialPublicKey,
+    }),
     authenticationInfo: {
       newCounter: counter,
       credentialID: authenticator.credentialID,
diff --git a/packages/server/src/helpers/convertCOSEtoPKCS.ts b/packages/server/src/helpers/convertCOSEtoPKCS.ts
index 5108209..618a0dc 100644
--- a/packages/server/src/helpers/convertCOSEtoPKCS.ts
+++ b/packages/server/src/helpers/convertCOSEtoPKCS.ts
@@ -1,4 +1,3 @@
-import type { SigningSchemeHash } from 'node-rsa';
 import { COSEAlgorithmIdentifier } from '@simplewebauthn/typescript-types';
 import { decodeCborFirst } from './decodeCbor';
 
@@ -64,14 +63,38 @@ export const COSECRV: { [key: number]: string } = {
 };
 
 export const COSEALGHASH: { [key: string]: string } = {
-  '-257': 'sha256',
-  '-258': 'sha384',
-  '-259': 'sha512',
   '-65535': 'sha1',
+  '-259': 'sha512',
+  '-258': 'sha384',
+  '-257': 'sha256',
   '-39': 'sha512',
   '-38': 'sha384',
   '-37': 'sha256',
-  '-7': 'sha256',
-  '-8': 'sha512',
   '-36': 'sha512',
+  '-35': 'sha384',
+  '-8': 'sha512',
+  '-7': 'sha256',
 };
+
+/**
+ * Imported from node-rsa's types
+ */
+type SigningSchemeHash =
+  | 'pkcs1-ripemd160'
+  | 'pkcs1-md4'
+  | 'pkcs1-md5'
+  | 'pkcs1-sha'
+  | 'pkcs1-sha1'
+  | 'pkcs1-sha224'
+  | 'pkcs1-sha256'
+  | 'pkcs1-sha384'
+  | 'pkcs1-sha512'
+  | 'pss-ripemd160'
+  | 'pss-md4'
+  | 'pss-md5'
+  | 'pss-sha'
+  | 'pss-sha1'
+  | 'pss-sha224'
+  | 'pss-sha256'
+  | 'pss-sha384'
+  | 'pss-sha512';
diff --git a/packages/server/src/helpers/convertPublicKeyToPEM.test.ts b/packages/server/src/helpers/convertPublicKeyToPEM.test.ts
index 6a52577..353a9eb 100644
--- a/packages/server/src/helpers/convertPublicKeyToPEM.test.ts
+++ b/packages/server/src/helpers/convertPublicKeyToPEM.test.ts
@@ -67,7 +67,7 @@ test('should return pem when input is base64URLString', () => {
   }
 });
 
-test('should return pem when input is base64URLString', () => {
+test('should raise error when kty is OKP (1)', () => {
   const mockCOSEKey = new Map<number, number | Buffer>();
 
   mockCOSEKey.set(COSEKEYS.kty, 1);
diff --git a/packages/server/src/helpers/verifySignature.ts b/packages/server/src/helpers/verifySignature.ts
index 14089a2..de8a56e 100644
--- a/packages/server/src/helpers/verifySignature.ts
+++ b/packages/server/src/helpers/verifySignature.ts
@@ -1,4 +1,24 @@
 import crypto from 'crypto';
+import cbor from 'cbor';
+import { verify as ed25519Verify } from '@noble/ed25519';
+
+import { COSEKEYS, COSEKTY } from './convertCOSEtoPKCS';
+import { convertCertBufferToPEM } from './convertCertBufferToPEM';
+import { convertPublicKeyToPEM } from './convertPublicKeyToPEM';
+
+type VerifySignatureOptsLeafCert = {
+  signature: Buffer;
+  signatureBase: Buffer;
+  leafCert: Buffer;
+  hashAlgorithm?: string;
+};
+
+type VerifySignatureOptsCredentialPublicKey = {
+  signature: Buffer;
+  signatureBase: Buffer;
+  credentialPublicKey: Buffer;
+  hashAlgorithm?: string;
+};
 
 /**
  * Verify an authenticator's signature
@@ -8,11 +28,75 @@ import crypto from 'crypto';
  * @param publicKey Authenticator's public key as a PEM certificate
  * @param algo Which algorithm to use to verify the signature (default: `'sha256'`)
  */
-export function verifySignature(
-  signature: Buffer,
-  signatureBase: Buffer,
-  publicKey: string,
-  algo = 'sha256',
-): boolean {
-  return crypto.createVerify(algo).update(signatureBase).verify(publicKey, signature);
+export async function verifySignature(
+  opts: VerifySignatureOptsLeafCert | VerifySignatureOptsCredentialPublicKey,
+): Promise<boolean> {
+  const { signature, signatureBase, hashAlgorithm = 'sha256' } = opts;
+  const _isLeafcertOpts = isLeafCertOpts(opts);
+  const _isCredPubKeyOpts = isCredPubKeyOpts(opts);
+
+  if (!_isLeafcertOpts && !_isCredPubKeyOpts) {
+    throw new Error('Must declare either "leafCert" or "credentialPublicKey"');
+  }
+
+  if (_isLeafcertOpts && _isCredPubKeyOpts) {
+    throw new Error('Must not declare both "leafCert" and "credentialPublicKey"');
+  }
+
+  let publicKeyPEM = '';
+
+  if (_isCredPubKeyOpts) {
+    const { credentialPublicKey } = opts;
+
+    // Decode CBOR to COSE
+    let struct;
+    try {
+      struct = cbor.decodeAllSync(credentialPublicKey)[0];
+    } catch (err) {
+      const _err = err as Error;
+      throw new Error(`Error decoding public key while converting to PEM: ${_err.message}`);
+    }
+
+    const kty = struct.get(COSEKEYS.kty);
+
+    if (!kty) {
+      throw new Error('Public key was missing kty');
+    }
+
+    // Check key type
+    if (kty === COSEKTY.OKP) {
+      // Verify Ed25519 slightly differently
+      const x = struct.get(COSEKEYS.x);
+
+      if (!x) {
+        throw new Error('Public key was missing x (OKP)');
+      }
+
+      return ed25519Verify(signature, signatureBase, x);
+    } else {
+      // Convert pubKey to PEM for ECC and RSA
+      publicKeyPEM = convertPublicKeyToPEM(credentialPublicKey);
+    }
+  }
+
+  if (_isLeafcertOpts) {
+    const { leafCert } = opts;
+    publicKeyPEM = convertCertBufferToPEM(leafCert);
+  }
+
+  return crypto.createVerify(hashAlgorithm).update(signatureBase).verify(publicKeyPEM, signature);
+}
+
+function isLeafCertOpts(
+  opts: VerifySignatureOptsLeafCert | VerifySignatureOptsCredentialPublicKey,
+): opts is VerifySignatureOptsLeafCert {
+  return Object.keys(opts as VerifySignatureOptsLeafCert).indexOf('leafCert') >= 0;
+}
+
+function isCredPubKeyOpts(
+  opts: VerifySignatureOptsLeafCert | VerifySignatureOptsCredentialPublicKey,
+): opts is VerifySignatureOptsCredentialPublicKey {
+  return (
+    Object.keys(opts as VerifySignatureOptsCredentialPublicKey).indexOf('credentialPublicKey') >= 0
+  );
 }
diff --git a/packages/server/src/registration/verifications/tpm/verifyAttestationTPM.ts b/packages/server/src/registration/verifications/tpm/verifyAttestationTPM.ts
index c74a7fe..fd2375c 100644
--- a/packages/server/src/registration/verifications/tpm/verifyAttestationTPM.ts
+++ b/packages/server/src/registration/verifications/tpm/verifyAttestationTPM.ts
@@ -279,8 +279,12 @@ export async function verifyAttestationTPM(options: AttestationFormatVerifierOpt
 
   // Verify signature over certInfo with the public key extracted from AIK certificate.
   // In the wise words of Yuriy Ackermann: "Get Martini friend, you are done!"
-  const leafCertPEM = convertCertBufferToPEM(x5c[0]);
-  return verifySignature(sig, certInfo, leafCertPEM, hashAlg);
+  return verifySignature({
+    signature: sig,
+    signatureBase: certInfo,
+    leafCert: x5c[0],
+    hashAlgorithm: hashAlg
+  });
 }
 
 /**
diff --git a/packages/server/src/registration/verifications/verifyAttestationAndroidKey.ts b/packages/server/src/registration/verifications/verifyAttestationAndroidKey.ts
index 0930eb8..55a0612 100644
--- a/packages/server/src/registration/verifications/verifyAttestationAndroidKey.ts
+++ b/packages/server/src/registration/verifications/verifyAttestationAndroidKey.ts
@@ -99,8 +99,12 @@ export async function verifyAttestationAndroidKey(
   }
 
   const signatureBase = Buffer.concat([authData, clientDataHash]);
-  const leafCertPEM = convertCertBufferToPEM(x5c[0]);
   const hashAlg = COSEALGHASH[alg as number];
 
-  return verifySignature(sig, signatureBase, leafCertPEM, hashAlg);
+  return verifySignature({
+    signature: sig,
+    signatureBase,
+    leafCert: x5c[0],
+    hashAlgorithm: hashAlg
+  });
 }
diff --git a/packages/server/src/registration/verifications/verifyAttestationAndroidSafetyNet.ts b/packages/server/src/registration/verifications/verifyAttestationAndroidSafetyNet.ts
index 4b8c31f..4c1e685 100644
--- a/packages/server/src/registration/verifications/verifyAttestationAndroidSafetyNet.ts
+++ b/packages/server/src/registration/verifications/verifyAttestationAndroidSafetyNet.ts
@@ -124,8 +124,11 @@ export async function verifyAttestationAndroidSafetyNet(
   const signatureBaseBuffer = Buffer.from(`${jwtParts[0]}.${jwtParts[1]}`);
   const signatureBuffer = base64url.toBuffer(SIGNATURE);
 
-  const leafCertPEM = convertCertBufferToPEM(leafCertBuffer);
-  const verified = verifySignature(signatureBuffer, signatureBaseBuffer, leafCertPEM);
+  const verified = await verifySignature({
+    signature: signatureBuffer,
+    signatureBase: signatureBaseBuffer,
+    leafCert: leafCertBuffer,
+  });
   /**
    * END Verify Signature
    */
diff --git a/packages/server/src/registration/verifications/verifyAttestationFIDOU2F.ts b/packages/server/src/registration/verifications/verifyAttestationFIDOU2F.ts
index bd6ac8e..3c79b9e 100644
--- a/packages/server/src/registration/verifications/verifyAttestationFIDOU2F.ts
+++ b/packages/server/src/registration/verifications/verifyAttestationFIDOU2F.ts
@@ -56,7 +56,9 @@ export async function verifyAttestationFIDOU2F(
     throw new Error(`${_err.message} (FIDOU2F)`);
   }
 
-  const leafCertPEM = convertCertBufferToPEM(x5c[0]);
-
-  return verifySignature(sig, signatureBase, leafCertPEM);
+  return verifySignature({
+    signature: sig,
+    signatureBase,
+    leafCert: x5c[0],
+  });
 }
diff --git a/packages/server/src/registration/verifications/verifyAttestationPacked.ts b/packages/server/src/registration/verifications/verifyAttestationPacked.ts
index 415c814..02beebc 100644
--- a/packages/server/src/registration/verifications/verifyAttestationPacked.ts
+++ b/packages/server/src/registration/verifications/verifyAttestationPacked.ts
@@ -1,22 +1,10 @@
-import elliptic from 'elliptic';
-import NodeRSA from 'node-rsa';
-
 import type { AttestationFormatVerifierOpts } from '../verifyRegistrationResponse';
 
-import {
-  COSEKEYS,
-  COSEALGHASH,
-  COSECRV,
-  COSEKTY,
-  COSERSASCHEME,
-  convertCOSEtoPKCS,
-} from '../../helpers/convertCOSEtoPKCS';
-import { toHash } from '../../helpers/toHash';
+import { COSEALGHASH } from '../../helpers/convertCOSEtoPKCS';
 import { convertCertBufferToPEM } from '../../helpers/convertCertBufferToPEM';
 import { validateCertificatePath } from '../../helpers/validateCertificatePath';
 import { getCertificateInfo } from '../../helpers/getCertificateInfo';
 import { verifySignature } from '../../helpers/verifySignature';
-import { decodeCredentialPublicKey } from '../../helpers/decodeCredentialPublicKey';
 import { MetadataService } from '../../services/metadataService';
 import { verifyAttestationWithMetadata } from '../../metadata/verifyAttestationWithMetadata';
 
@@ -42,10 +30,8 @@ export async function verifyAttestationPacked(
   const signatureBase = Buffer.concat([authData, clientDataHash]);
 
   let verified = false;
-  const pkcsPublicKey = convertCOSEtoPKCS(credentialPublicKey);
 
   if (x5c) {
-    const leafCert = convertCertBufferToPEM(x5c[0]);
     const { subject, basicConstraintsCA, version, notBefore, notAfter } = getCertificateInfo(
       x5c[0],
     );
@@ -119,76 +105,20 @@ export async function verifyAttestationPacked(
       }
     }
 
-    verified = verifySignature(sig, signatureBase, leafCert);
+    verified = await verifySignature({
+      signature: sig,
+      signatureBase,
+      leafCert: x5c[0],
+    });
   } else {
-    const cosePublicKey = decodeCredentialPublicKey(credentialPublicKey);
-
-    const kty = cosePublicKey.get(COSEKEYS.kty);
-
-    if (!kty) {
-      throw new Error('COSE public key was missing kty (Packed|Self)');
-    }
-
     const hashAlg: string = COSEALGHASH[alg as number];
 
-    if (kty === COSEKTY.EC2) {
-      const crv = cosePublicKey.get(COSEKEYS.crv);
-
-      if (!crv) {
-        throw new Error('COSE public key was missing kty crv (Packed|EC2)');
-      }
-
-      const signatureBaseHash = toHash(signatureBase, hashAlg);
-
-      /**
-       * Instantiating the curve here is _very_ computationally heavy - a bit of profiling
-       * (in compiled JS, not TS) reported an average of ~125ms to execute this line. The elliptic
-       * README states, "better do it once and reuse it", so maybe there's a better way to handle
-       * this in a server context, when we can re-use an existing instance.
-       *
-       * For now, it's worth noting that this line is probably the reason why it can take
-       * 5-6 seconds to run tests.
-       */
-      const ec = new elliptic.ec(COSECRV[crv as number]);
-      const key = ec.keyFromPublic(pkcsPublicKey);
-
-      verified = key.verify(signatureBaseHash, sig);
-    } else if (kty === COSEKTY.RSA) {
-      const n = cosePublicKey.get(COSEKEYS.n);
-
-      if (!n) {
-        throw new Error('COSE public key was missing n (Packed|RSA)');
-      }
-
-      const signingScheme = COSERSASCHEME[alg as number];
-
-      // TODO: Verify this works
-      const key = new NodeRSA();
-      key.setOptions({ signingScheme });
-      key.importKey(
-        {
-          n: n as Buffer,
-          e: 65537,
-        },
-        'components-public',
-      );
-
-      verified = key.verify(signatureBase, sig);
-    } else if (kty === COSEKTY.OKP) {
-      const x = cosePublicKey.get(COSEKEYS.x);
-
-      if (!x) {
-        throw new Error('COSE public key was missing x (Packed|OKP)');
-      }
-
-      const signatureBaseHash = toHash(signatureBase, hashAlg);
-
-      const key = new elliptic.eddsa('ed25519');
-      key.keyFromPublic(x as Buffer);
-
-      // TODO: is `publicKey` right here?
-      verified = key.verify(signatureBaseHash, sig, pkcsPublicKey);
-    }
+    verified = await verifySignature({
+      signature: sig,
+      signatureBase,
+      credentialPublicKey,
+      hashAlgorithm: hashAlg
+    });
   }
 
   return verified;