Fix ECDSA Signature Unwrapping

packages/server/src/helpers/iso/isoCrypto/unwrapEC2Signature.ts

@@ -1,36 +1,66 @@
 import { AsnParser, ECDSASigValue } from '../../../deps.ts';
+import { COSECRV } from '../../cose.ts';
 import { isoUint8Array } from '../index.ts';
 
 /**
  * In WebAuthn, EC2 signatures are wrapped in ASN.1 structure so we need to peel r and s apart.
  *
  * See https://www.w3.org/TR/webauthn-2/#sctn-signature-attestation-types
  */
-export function unwrapEC2Signature(signature: Uint8Array): Uint8Array {
+export function unwrapEC2Signature(signature: Uint8Array, crv: COSECRV): Uint8Array {
   const parsedSignature = AsnParser.parse(signature, ECDSASigValue);
-  let rBytes = new Uint8Array(parsedSignature.r);
-  let sBytes = new Uint8Array(parsedSignature.s);
+  const n = getSignatureComponentLength(crv);
 
-  if (shouldRemoveLeadingZero(rBytes)) {
-    rBytes = rBytes.slice(1);
-  }
-
-  if (shouldRemoveLeadingZero(sBytes)) {
-    sBytes = sBytes.slice(1);
-  }
+  const rBytes = toNormalizedBytes(parsedSignature.r, n);
+  const sBytes = toNormalizedBytes(parsedSignature.s, n);
 
   const finalSignature = isoUint8Array.concat([rBytes, sBytes]);
 
   return finalSignature;
 }
 
 /**
- * Determine if the DER-specific `00` byte at the start of an ECDSA signature byte sequence
- * should be removed based on the following logic:
+ * ECDSA signatures with in the subtle crypto API expect signatures with `r` and `s` values
+ * encoded to a specific length depending on the order of the curve.
  *
- * "If the leading byte is 0x0, and the the high order bit on the second byte is not set to 0,
- * then remove the leading 0x0 byte"
+ * See <https://www.w3.org/TR/WebCryptoAPI/#ecdsa-operations>
  */
-function shouldRemoveLeadingZero(bytes: Uint8Array): boolean {
-  return bytes[0] === 0x0 && (bytes[1] & (1 << 7)) !== 0;
+function getSignatureComponentLength(crv: COSECRV): number {
+  switch (crv) {
+  case COSECRV.P256:
+    return 32;
+  case COSECRV.P384:
+    return 48;
+  case COSECRV.P521:
+    return 66;
+  default:
+    throw new Error(`Unexpected COSE crv value of ${crv} (EC2)`);
+  }
+}
+
+/**
+ * Converts the ASN.1 integer representation to bytes of a specific length `n`.
+ *
+ * DER encodes integers as big-endian byte arrays, with as small as possible representation and
+ * require leading `0` bytes to disambiguate between negative and positive numbers. This means
+ * that `r` and `s` can potentially not be the expected length `n` that is needed by the WebCrypto
+ * subtle API: if there it leading `0`s it can be shorter than expected, and if it has a leading
+ * `1` bit, it can be one byte longer.
+ *
+ * See <https://www.itu.int/rec/T-REC-X.690-202102-I/en>
+ * See <https://www.w3.org/TR/WebCryptoAPI/#ecdsa-operations>
+ */
+function toNormalizedBytes(i: ArrayBuffer, n: number): Uint8Array {
+  const iBytes = new Uint8Array(i);
+
+  const normalizedBytes = new Uint8Array(n);
+  if (iBytes.length <= n) {
+    normalizedBytes.set(iBytes, n - iBytes.length);
+  } else if (iBytes.length === n + 1 && iBytes[0] === 0 && (iBytes[1] & 0x80) === 0x80) {
+    normalizedBytes.set(iBytes.slice(1));
+  } else {
+    throw new Error("invalid signature component length");
+  }
+
+  return normalizedBytes;
 }

packages/server/src/helpers/iso/isoCrypto/verify.ts

@@ -2,6 +2,7 @@ import {
   COSEALG,
   COSEKEYS,
   COSEPublicKey,
+  isCOSECrv,
   isCOSEPublicKeyEC2,
   isCOSEPublicKeyOKP,
   isCOSEPublicKeyRSA,
@@ -23,7 +24,11 @@ export function verify(opts: {
   const { cosePublicKey, signature, data, shaHashOverride } = opts;
 
   if (isCOSEPublicKeyEC2(cosePublicKey)) {
-    const unwrappedSignature = unwrapEC2Signature(signature);
+    const crv = cosePublicKey.get(COSEKEYS.crv);
+    if (!isCOSECrv(crv)) {
+      throw new Error(`unknown COSE curve ${crv}`);
+    }
+    const unwrappedSignature = unwrapEC2Signature(signature, crv);
     return verifyEC2({
       cosePublicKey,
       signature: unwrappedSignature,

packages/server/src/helpers/iso/isoCrypto/verifyEC2.test.ts

@@ -0,0 +1,102 @@
+import { assert } from 'https://deno.land/std@0.198.0/assert/mod.ts';
+
+import { COSEALG, COSECRV, COSEKEYS, COSEKTY, COSEPublicKeyEC2 } from '../../cose.ts';
+import { verifyEC2 } from './verifyEC2.ts';
+import { unwrapEC2Signature } from './unwrapEC2Signature.ts';
+import { isoBase64URL } from '../index.ts';
+
+Deno.test(
+  'should verify a signature signed with an P-256 public key',
+  async () => {
+    const cosePublicKey: COSEPublicKeyEC2 = new Map();
+    cosePublicKey.set(COSEKEYS.kty, COSEKTY.EC2);
+    cosePublicKey.set(COSEKEYS.alg, COSEALG.ES256);
+    cosePublicKey.set(COSEKEYS.crv, COSECRV.P256);
+    cosePublicKey.set(
+      COSEKEYS.x,
+      isoBase64URL.toBuffer('_qRi-kwOVobsqJ_1GAHZYfC77QoIdsVFYkx2Mw20UM4'),
+    );
+    cosePublicKey.set(
+      COSEKEYS.y,
+      isoBase64URL.toBuffer('BXEathwyOK_uQRmlZ_m4wReHLujSXk_-e3-9co5B2MY'),
+    );
+
+    const data = isoBase64URL.toBuffer('Bt81jmu3ieajF4w1at8HmieVOTDymHd7xJguJCUsL-Q');
+    const signature = isoBase64URL.toBuffer(
+      'MEQCH1h_F7TPTMVh_kwb_ssjD0_2U77bbXazz2ux-P6khLQCIQCutHs9eCBkCIMP3yA9mmNRKEfFd-REmhGY2GbHozaC7w'
+    );
+
+    const verified = await verifyEC2({
+      cosePublicKey,
+      data,
+      signature: unwrapEC2Signature(signature, COSECRV.P256),
+    });
+
+    assert(verified);
+  },
+);
+
+Deno.test(
+  'should verify a signature signed with an P-384 public key',
+  async () => {
+    const cosePublicKey: COSEPublicKeyEC2 = new Map();
+    cosePublicKey.set(COSEKEYS.kty, COSEKTY.EC2);
+    cosePublicKey.set(COSEKEYS.alg, COSEALG.ES384);
+    cosePublicKey.set(COSEKEYS.crv, COSECRV.P384);
+    cosePublicKey.set(
+      COSEKEYS.x,
+      isoBase64URL.toBuffer('pm-0exykk1x0O72S9sm6fl-iXxFrGikjQHi1CgONIiEz_yDJdCPxN453qg6HLkOx'),
+    );
+    cosePublicKey.set(
+      COSEKEYS.y,
+      isoBase64URL.toBuffer('2B7yW7sgza8Sf7ifznQlGJqmJxgupkAevUqqOJTWaWBZiQ7sAf-TfAaNBukiz12K'),
+    );
+
+    const data = isoBase64URL.toBuffer('D7mI8UwWXv4rpfSQUNqtUXAhZEPbRLugmWclPpJ9m7c');
+    const signature = isoBase64URL.toBuffer(
+      'MGMCL3lZ2Rjxo5WcmTCdWyB6jTE9PVuduOR_AsJu956J9S_mFNbHP_-MbyWem4dfb5iqAjABJhTRltNl5Y0O4XC7YLNsYKq2WxYQ1HFOMGsr6oNkUPsX3UAr2zeeWL_Tp1VgHeM'
+    );
+
+    const verified = await verifyEC2({
+      cosePublicKey,
+      data,
+      signature: unwrapEC2Signature(signature, COSECRV.P384),
+    });
+
+    assert(verified);
+  },
+);
+
+Deno.test({
+  // This test is currently ignored, as Deno's implementation of `WebCrypto.subtle` API does not
+  // support the P-521 curve at the moment.
+  ignore: true,
+  name: 'should verify a signature signed with an P-521 public key',
+  async fn() {
+    const cosePublicKey: COSEPublicKeyEC2 = new Map();
+    cosePublicKey.set(COSEKEYS.kty, COSEKTY.EC2);
+    cosePublicKey.set(COSEKEYS.alg, COSEALG.ES512);
+    cosePublicKey.set(COSEKEYS.crv, COSECRV.P521);
+    cosePublicKey.set(
+      COSEKEYS.x,
+      isoBase64URL.toBuffer('AaLbnrCvCuQivbknRW50FjdqPQv4NRF9tHsN4QuVQ3sw8uSspd33o-NTBfjg5JzX9rnpbkKDigb6NugmrVjzNMNK'),
+    );
+    cosePublicKey.set(
+      COSEKEYS.y,
+      isoBase64URL.toBuffer('AE64axa8L8PkLX5Td0GaX79cLOW9E2-8-ObhL9XT_ih-1XxbGQcA5VhL1gI0xIQq5zYAxgZYey6PmbbqgtcUPRVt'),
+    );
+
+    const data = isoBase64URL.toBuffer('5p0h9RZTjLoBlnL2nY5pqOnhGy4q60NzbjDe2rVDR7o');
+    const signature = isoBase64URL.toBuffer(
+      'MIGHAkFRpbGknlgpETORypMprGBXMkJMfuqgJupy3NcgCOaJJdj3Voz74kV2pjPqkLNpuO9FqVtXeEsUw-jYsBHcMqHZhwJCAQ88uFDJS5g81XVBcLMIgf6ro-F-5jgRAmHx3CRVNGdk81MYbFJhT3hd2w9RdhT8qBG0zzRBXYAcHrKo0qJwQZot'
+    );
+
+    const verified = await verifyEC2({
+      cosePublicKey,
+      data,
+      signature: unwrapEC2Signature(signature, COSECRV.P521),
+    });
+
+    assert(verified);
+  },
+});