From 909d44f43687d10bcc89506c6e24b0a4eecee81b Mon Sep 17 00:00:00 2001
From: larabr <7375870+larabr@users.noreply.github.com>
Date: Fri, 6 Oct 2023 17:21:50 +0200
Subject: [PATCH] Add back support for verification of some invalid ECDSA sigs
 affected by old lib bug

At some point we used to generate invalid ECDSA sigs with the js (non-native) elliptic lib,
if the signature digest had leading zeros: https://github.com/openpgpjs/openpgpjs/pull/948 .

Brainpool curves are the most likely to have been affected by the bug, since they do not
have WebCrypto support (unlike NIST curves).
This commit reintroduces support on web to verify such invalid signatures
(support for this was previously built-in in the indutny-elliptic library).
It also expands the fix to work in Node.
---
 src/crypto/public_key/elliptic/ecdsa.js | 37 +++++++++++++++++++++----
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/src/crypto/public_key/elliptic/ecdsa.js b/src/crypto/public_key/elliptic/ecdsa.js
index d000bf01..33f22232 100644
--- a/src/crypto/public_key/elliptic/ecdsa.js
+++ b/src/crypto/public_key/elliptic/ecdsa.js
@@ -97,12 +97,25 @@ export async function sign(oid, hashAlgo, message, publicKey, privateKey, hashed
  */
 export async function verify(oid, hashAlgo, signature, message, publicKey, hashed) {
   const curve = new CurveWithOID(oid);
+  // See https://github.com/openpgpjs/openpgpjs/pull/948.
+  // NB: the impact was more likely limited to Brainpool curves, since thanks
+  // to WebCrypto availability, NIST curve should not have been affected.
+  // Similarly, secp256k1 should have been used rarely enough.
+  // However, we implement the fix for all curves, since it's only needed in case of
+  // verification failure, which is unexpected, hence a minor slowdown is acceptable.
+  const tryFallbackVerificationForOldBug = () => (
+    hashed[0] === 0 ?
+      jsVerify(curve, signature, hashed.subarray(1), publicKey) :
+      false
+  );
+
   if (message && !util.isStream(message)) {
     switch (curve.type) {
       case 'web':
         try {
           // Need to await to make sure browser succeeds
-          return await webVerify(curve, hashAlgo, signature, message, publicKey);
+          const verified = await webVerify(curve, hashAlgo, signature, message, publicKey);
+          return verified || tryFallbackVerificationForOldBug();
         } catch (err) {
           // We do not fallback if the error is related to key integrity
           // Unfortunately Safari does not support p521 and throws a DataError when using it
@@ -113,14 +126,15 @@ export async function verify(oid, hashAlgo, signature, message, publicKey, hashe
           util.printDebugError('Browser did not support verifying: ' + err.message);
         }
         break;
-      case 'node':
-        return nodeVerify(curve, hashAlgo, signature, message, publicKey);
+      case 'node': {
+        const verified = await nodeVerify(curve, hashAlgo, signature, message, publicKey);
+        return verified || tryFallbackVerificationForOldBug();
+      }
     }
   }
 
-  const nobleCurve = getNobleCurve(curve.name);
-  // lowS: non-canonical sig: https://stackoverflow.com/questions/74338846/ecdsa-signature-verification-mismatch
-  return nobleCurve.verify(util.concatUint8Array([signature.r, signature.s]), hashed, publicKey, { lowS: false });
+  const verified = jsVerify(curve, signature, hashed, publicKey);
+  return verified || tryFallbackVerificationForOldBug();
 }
 
 /**
@@ -164,6 +178,17 @@ export async function validateParams(oid, Q, d) {
 //   Helper functions   //
 //                      //
 //////////////////////////
+
+/**
+ * Fallback javascript implementation of ECDSA verification.
+ * To be used if no native implementation is available for the given curve/operation.
+ */
+function jsVerify(curve, signature, hashed, publicKey) {
+  const nobleCurve = getNobleCurve(curve.name);
+  // lowS: non-canonical sig: https://stackoverflow.com/questions/74338846/ecdsa-signature-verification-mismatch
+  return nobleCurve.verify(util.concatUint8Array([signature.r, signature.s]), hashed, publicKey, { lowS: false });
+}
+
 async function webSign(curve, hashAlgo, message, keyPair) {
   const len = curve.payloadSize;
   const jwk = privateToJWK(curve.payloadSize, webCurves[curve.name], keyPair.publicKey, keyPair.privateKey);