Merge pull request #251 from MasterKale/fix/metadata-attstmt-alg-check

fix/metadata-attstmt-alg-check

7 files changed, 70 insertions, 25 deletions

diff --git a/packages/server/src/metadata/mdsTypes.ts b/packages/server/src/metadata/mdsTypes.ts
index 0421383..1bf9f80 100644
--- a/packages/server/src/metadata/mdsTypes.ts
+++ b/packages/server/src/metadata/mdsTypes.ts
@@ -292,4 +292,5 @@ export type AuthenticatorGetInfo = {
   };
   maxMsgSize?: number;
   pinProtocols?: number[];
+  algorithms?: { type: 'public-key', alg: number }[];
 };
diff --git a/packages/server/src/metadata/verifyAttestationWithMetadata.test.ts b/packages/server/src/metadata/verifyAttestationWithMetadata.test.ts
index 9ba01fd..b48ef2e 100644
--- a/packages/server/src/metadata/verifyAttestationWithMetadata.test.ts
+++ b/packages/server/src/metadata/verifyAttestationWithMetadata.test.ts
@@ -47,11 +47,11 @@ test('should verify attestation with metadata (android-safetynet)', async () =>
   const credentialPublicKey =
     'pQECAyYgASFYIAKH2NrGZT-lUEA3tbBXR9owjW_7OnA1UqoL1UuKY_VCIlggpjeOH0xyBCpGDya55JLXXKrzyOieQN3dvG1pV-Qs-Gs';
 
-  const verified = await verifyAttestationWithMetadata(
-    metadataStatementJSONSafetyNet,
-    base64url.toBuffer(credentialPublicKey),
+  const verified = await verifyAttestationWithMetadata({
+    statement: metadataStatementJSONSafetyNet,
+    credentialPublicKey: base64url.toBuffer(credentialPublicKey),
     x5c,
-  );
+  });
 
   expect(verified).toEqual(true);
 });
@@ -97,11 +97,11 @@ test('should verify attestation with rsa_emsa_pkcs1_sha256_raw authenticator alg
   ];
   const credentialPublicKey = 'pAEDAzkBACBZAQC3X5SKwYUkxFxxyvCnz_37Z57eSdsgQuiBLDaBOd1R6VEZReAV3nVr_7jiRgmWfu1C-S3Aro65eSG5shcDCgIvY3KdEI8K5ENEPlmucjnFILBAE_MZtPmZlkEDmVCDcVspHX2iKqiVWYV6IFzVX1QUf0SAlWijV9NEfKDbij34ddV0qfG2nEMA0_xVpN2OK2BVXonFg6tS3T00XlFh4MdzIauIHTDT63eAdHlkFrMqU53T5IqDvL3VurBmBjYRJ3VDT9mA2sm7fSrJNXhSVLPst-ZsiOioVKrpzFE9sJmyCQvq2nGZ2RhDo8FfAKiw0kvJRkCSSe1ddxryk9_VSCprIUMBAAE';
 
-  const verified = await verifyAttestationWithMetadata(
-    metadataStatement,
-    base64url.toBuffer(credentialPublicKey),
+  const verified = await verifyAttestationWithMetadata({
+    statement: metadataStatement,
+    credentialPublicKey: base64url.toBuffer(credentialPublicKey),
     x5c,
-  );
+  });
 
   expect(verified).toEqual(true);
 });
@@ -154,11 +154,11 @@ test('should not validate certificate path when authenticator is self-referencin
   ];
   const credentialPublicKey = 'pQECAyYgASFYIBdmUVOxrn-OOtkVwGP_vAspH3VkgzcGXVlu3-acb7EZIlggKgDTs0fr2d51sLR6uL3KP2cqR3iIUkKMCjyMJhYOkf4';
 
-  const verified = await verifyAttestationWithMetadata(
-    metadataStatement,
-    base64url.toBuffer(credentialPublicKey),
+  const verified = await verifyAttestationWithMetadata({
+    statement: metadataStatement,
+    credentialPublicKey: base64url.toBuffer(credentialPublicKey),
     x5c,
-  );
+  });
 
   expect(verified).toEqual(true);
 });
diff --git a/packages/server/src/metadata/verifyAttestationWithMetadata.ts b/packages/server/src/metadata/verifyAttestationWithMetadata.ts
index e068a05..5193135 100644
--- a/packages/server/src/metadata/verifyAttestationWithMetadata.ts
+++ b/packages/server/src/metadata/verifyAttestationWithMetadata.ts
@@ -10,14 +10,26 @@ import { COSEKEYS, COSEKTY } from '../helpers/convertCOSEtoPKCS';
  * Match properties of the authenticator's attestation statement against expected values as
  * registered with the FIDO Alliance Metadata Service
  */
-export async function verifyAttestationWithMetadata(
-  statement: MetadataStatement,
-  credentialPublicKey: Buffer,
-  x5c: Buffer[] | Base64URLString[],
-): Promise<boolean> {
+export async function verifyAttestationWithMetadata({
+  statement,
+  credentialPublicKey,
+  x5c,
+  attestationStatementAlg,
+}: {
+  statement: MetadataStatement;
+  credentialPublicKey: Buffer;
+  x5c: Buffer[] | Base64URLString[];
+  attestationStatementAlg?: number;
+}): Promise<boolean> {
+  const {
+    authenticationAlgorithms,
+    authenticatorGetInfo,
+    attestationRootCertificates,
+  } = statement;
+
   // Make sure the alg in the attestation statement matches one of the ones specified in metadata
   const keypairCOSEAlgs: Set<COSEInfo> = new Set();
-  statement.authenticationAlgorithms.forEach(algSign => {
+  authenticationAlgorithms.forEach(algSign => {
     // Map algSign string to { kty, alg, crv }
     const algSignCOSEINFO = algSignToCOSEInfoMap[algSign];
 
@@ -78,7 +90,7 @@ export async function verifyAttestationWithMetadata(
      * ]
      * ```
      */
-    const debugMDSAlgs = statement.authenticationAlgorithms
+    const debugMDSAlgs = authenticationAlgorithms
       .map((algSign) => `'${algSign}' (COSE info: ${stringifyCOSEInfo(algSignToCOSEInfoMap[algSign])})`);
     const strMDSAlgs = JSON.stringify(debugMDSAlgs, null, 2).replace(/"/g, '');
 
@@ -92,9 +104,21 @@ export async function verifyAttestationWithMetadata(
     );
   }
 
+  /**
+   * Confirm the attestation statement's algorithm is one supported according to metadata
+   */
+  if (attestationStatementAlg !== undefined && authenticatorGetInfo?.algorithms !== undefined) {
+    const getInfoAlgs = authenticatorGetInfo.algorithms.map(_alg => _alg.alg);
+    if (getInfoAlgs.indexOf(attestationStatementAlg) < 0) {
+      throw new Error(
+        `Attestation statement alg ${attestationStatementAlg} did not match one of ${getInfoAlgs}`,
+      );
+    }
+  }
+
   // Prepare to check the certificate chain
   const authenticatorCerts = x5c.map(convertCertBufferToPEM);
-  const statementRootCerts = statement.attestationRootCertificates.map(convertCertBufferToPEM);
+  const statementRootCerts = attestationRootCertificates.map(convertCertBufferToPEM);
 
   /**
    * If an authenticator returns exactly one certificate in its x5c, and that cert is found in the
diff --git a/packages/server/src/registration/verifications/tpm/verifyAttestationTPM.ts b/packages/server/src/registration/verifications/tpm/verifyAttestationTPM.ts
index 2afd0a4..c74a7fe 100644
--- a/packages/server/src/registration/verifications/tpm/verifyAttestationTPM.ts
+++ b/packages/server/src/registration/verifications/tpm/verifyAttestationTPM.ts
@@ -257,7 +257,12 @@ export async function verifyAttestationTPM(options: AttestationFormatVerifierOpt
   const statement = await MetadataService.getStatement(aaguid);
   if (statement) {
     try {
-      await verifyAttestationWithMetadata(statement, credentialPublicKey, x5c);
+      await verifyAttestationWithMetadata({
+        statement,
+        credentialPublicKey,
+        x5c,
+        attestationStatementAlg: alg,
+      });
     } catch (err) {
       const _err = err as Error;
       throw new Error(`${_err.message} (TPM)`);
diff --git a/packages/server/src/registration/verifications/verifyAttestationAndroidKey.ts b/packages/server/src/registration/verifications/verifyAttestationAndroidKey.ts
index dcc3670..0930eb8 100644
--- a/packages/server/src/registration/verifications/verifyAttestationAndroidKey.ts
+++ b/packages/server/src/registration/verifications/verifyAttestationAndroidKey.ts
@@ -78,7 +78,12 @@ export async function verifyAttestationAndroidKey(
   const statement = await MetadataService.getStatement(aaguid);
   if (statement) {
     try {
-      await verifyAttestationWithMetadata(statement, credentialPublicKey, x5c);
+      await verifyAttestationWithMetadata({
+        statement,
+        credentialPublicKey,
+        x5c,
+        attestationStatementAlg: alg,
+      });
     } catch (err) {
       const _err = err as Error;
       throw new Error(`${_err.message} (AndroidKey)`);
diff --git a/packages/server/src/registration/verifications/verifyAttestationAndroidSafetyNet.ts b/packages/server/src/registration/verifications/verifyAttestationAndroidSafetyNet.ts
index 14be27e..4b8c31f 100644
--- a/packages/server/src/registration/verifications/verifyAttestationAndroidSafetyNet.ts
+++ b/packages/server/src/registration/verifications/verifyAttestationAndroidSafetyNet.ts
@@ -25,7 +25,7 @@ export async function verifyAttestationAndroidSafetyNet(
     verifyTimestampMS = true,
     credentialPublicKey,
   } = options;
-  const { response, ver } = attStmt;
+  const { alg, response, ver } = attStmt;
 
   if (!ver) {
     throw new Error('No ver value in attestation (SafetyNet)');
@@ -95,7 +95,12 @@ export async function verifyAttestationAndroidSafetyNet(
   const statement = await MetadataService.getStatement(aaguid);
   if (statement) {
     try {
-      await verifyAttestationWithMetadata(statement, credentialPublicKey, HEADER.x5c);
+      await verifyAttestationWithMetadata({
+        statement,
+        credentialPublicKey,
+        x5c: HEADER.x5c,
+        attestationStatementAlg: alg,
+      });
     } catch (err) {
       const _err = err as Error;
       throw new Error(`${_err.message} (SafetyNet)`);
diff --git a/packages/server/src/registration/verifications/verifyAttestationPacked.ts b/packages/server/src/registration/verifications/verifyAttestationPacked.ts
index 844a1ed..415c814 100644
--- a/packages/server/src/registration/verifications/verifyAttestationPacked.ts
+++ b/packages/server/src/registration/verifications/verifyAttestationPacked.ts
@@ -99,7 +99,12 @@ export async function verifyAttestationPacked(
       }
 
       try {
-        await verifyAttestationWithMetadata(statement, credentialPublicKey, x5c);
+        await verifyAttestationWithMetadata({
+          statement,
+          credentialPublicKey,
+          x5c,
+          attestationStatementAlg: alg,
+        });
       } catch (err) {
         const _err = err as Error;
         throw new Error(`${_err.message} (Packed|Full)`);