Fix key and signature parsing of EdDSALegacy entities with unsupported curves (e.g. Curve448Legacy) (#1798)

Signature parsing would fail in case of unexpected payload sizes, causing key parsing to always throw
when processing e.g. an (unsupported) Curve448Legacy subkey instead of ignoring it.

To address this, we now throw on signature verification instead of parsing (as done for ECDSA).

NB: the bug and this fix are not relevant for the new Ed25519/Ed448 entities as standardized by the crypto-refresh.

src/crypto/signature.js

@@ -38,6 +38,8 @@ export function parseSignatureParams(algo, signature) {
     case enums.publicKey.dsa:
     case enums.publicKey.ecdsa:
     {
+      // If the signature payload sizes are unexpected, we will throw on verification,
+      // where we also have access to the OID curve from the key.
       const r = util.readMPI(signature.subarray(read)); read += r.length + 2;
       const s = util.readMPI(signature.subarray(read)); read += s.length + 2;
       return { read, signatureParams: { r, s } };
@@ -46,12 +48,11 @@ export function parseSignatureParams(algo, signature) {
     // -  MPI of an EC point r.
     // -  EdDSA value s, in MPI, in the little endian representation
     case enums.publicKey.eddsaLegacy: {
-      // When parsing little-endian MPI data, we always need to left-pad it, as done with big-endian values:
-      // https://www.ietf.org/archive/id/draft-ietf-openpgp-rfc4880bis-10.html#section-3.2-9
-      let r = util.readMPI(signature.subarray(read)); read += r.length + 2;
-      r = util.leftPad(r, 32);
-      let s = util.readMPI(signature.subarray(read)); read += s.length + 2;
-      s = util.leftPad(s, 32);
+      // Only Curve25519Legacy is supported (no Curve448Legacy), but the relevant checks are done on key parsing and signature
+      // verification: if the signature payload sizes are unexpected, we will throw on verification,
+      // where we also have access to the OID curve from the key.
+      const r = util.readMPI(signature.subarray(read)); read += r.length + 2;
+      const s = util.readMPI(signature.subarray(read)); read += s.length + 2;
       return { read, signatureParams: { r, s } };
     }
     // Algorithm-Specific Fields for Ed25519 signatures:
@@ -108,8 +109,12 @@ export async function verify(algo, hashAlgo, signature, publicParams, data, hash
     }
     case enums.publicKey.eddsaLegacy: {
       const { oid, Q } = publicParams;
-      // signature already padded on parsing
-      return publicKey.elliptic.eddsaLegacy.verify(oid, hashAlgo, signature, data, Q, hashed);
+      const curveSize = new publicKey.elliptic.CurveWithOID(oid).payloadSize;
+      // When dealing little-endian MPI data, we always need to left-pad it, as done with big-endian values:
+      // https://www.ietf.org/archive/id/draft-ietf-openpgp-rfc4880bis-10.html#section-3.2-9
+      const r = util.leftPad(signature.r, curveSize);
+      const s = util.leftPad(signature.s, curveSize);
+      return publicKey.elliptic.eddsaLegacy.verify(oid, hashAlgo, { r, s }, data, Q, hashed);
     }
     case enums.publicKey.ed25519:
     case enums.publicKey.ed448: {

src/util.js

@@ -148,6 +148,9 @@ const util = {
    * @returns {Uint8Array} Padded bytes.
    */
   leftPad(bytes, length) {
+    if (bytes.length > length) {
+      throw new Error('Input array too long');
+    }
     const padded = new Uint8Array(length);
     const offset = length - bytes.length;
     padded.set(bytes, offset);

test/general/key.js

@@ -3299,6 +3299,57 @@ ruh8m7Xo2ehSSFyWRSuTSZe5tm/KXgYG
     }
   });
 
+  it('Parsing EdDSALegacy key with unsupported OID (Curve448Legacy)', async function() {
+    // Key with unknown OID (corresponding to curve448) which is not supported for EdDSALegacy
+    const armoredEdDSALegacyCurve448Key = `-----BEGIN PGP PRIVATE KEY BLOCK-----
+
+xYUEYV2UmRYDK2VxAc9AFyxgh5xnSbyt50TWl558mw9xdMN+/UBLr5+UMP8IsrvV
+MdXuTIE8CyaUQKSotHtH2RkYEXj5nsMAAAHPQIbTMSzjIWug8UFECzAex5FHgAgH
+gYF3RK+TS8D24wX8kOu2C/NoVxwGY+p+i0JHaB+7yljriSKAGxs6wsBEBB8WCgCD
+BYJhXZSZBYkFpI+9AwsJBwkQppmYlfq6zlJHFAAAAAAAHgAgc2FsdEBub3RhdGlv
+bnMuc2VxdW9pYS1wZ3Aub3Jn5wSpIutJ5HncJWk4ruUV8GzQF390rR5+qWEAnAoY
+akcDFQoIApsBAh4BFiEEwdtl1YDXuSJyVEseppmYlfq6zlIAALzdA5dA/fsgYg/J
+qaQriYKaPUkyHL7EB3BXhV2d1h/gk+qJLvXQuU2WEJ/XSs3GrsBRiiZwvPH4o+7b
+mleAxjy5wpS523vqrrBR2YZ5FwIku7WS4litSdn4AtVam/TlLdMNIf41CtFeZKBe
+c5R5VNdQy8y7qy8AAADNEUN1cnZlNDQ4IE9wdGlvbiA4wsBHBBMWCgCGBYJhXZSZ
+BYkFpI+9AwsJBwkQppmYlfq6zlJHFAAAAAAAHgAgc2FsdEBub3RhdGlvbnMuc2Vx
+dW9pYS1wZ3Aub3JnD55UsYMzE6OACP+mgw5zvT+BBgol8/uFQjHg4krjUCMDFQoI
+ApkBApsBAh4BFiEEwdtl1YDXuSJyVEseppmYlfq6zlIAAPQJA5dA0Xqwzn/0uwCq
+RlsOVCB3f5NOj1exKnlBvRw0xT1VBee1yxvlUt5eIAoCxWoRlWBJob3TTkhm9AEA
+8dyhwPmyGfWHzPw5NFG3xsXrZdNXNvit9WMVAPcmsyR7teXuDlJItxRAdJJc/qfJ
+YVbBFoaNrhYAAADHhQRhXZSZFgMrZXEBz0BL7THZ9MnCLfSPJ1FMLim9eGkQ3Bfn
+M3he5rOwO3t14QI1LjI96OjkeJipMgcFAmEP1Bq/ZHGO7oAAAc9AFnE8iNBaT3OU
+EFtxkmWHXtdaYMmGGRdopw9JPXr/UxuunDln5o9dxPxf7q7z26zXrZen+qed/Isa
+HsDCwSwEGBYKAWsFgmFdlJkFiQWkj70JEKaZmJX6us5SRxQAAAAAAB4AIHNhbHRA
+bm90YXRpb25zLnNlcXVvaWEtcGdwLm9yZxREUizdTcepBzgSMOv2VWQCWbl++3CZ
+EbgAWDryvSsyApsCwDGgBBkWCgBvBYJhXZSZCRBKo3SL4S5djkcUAAAAAAAeACBz
+YWx0QG5vdGF0aW9ucy5zZXF1b2lhLXBncC5vcmemoGTDjmNQiIzw6HOEddvS0OB7
+UZ/P07jM/EVmnYxTlBYhBAxsnkGpx1UCiH6gUUqjdIvhLl2OAAALYQOXQAMB1oKq
+OWxSFmvmgCKNcbAAyA3piF5ERIqs4z07oJvqDYrOWt75UsEIH/04gU/vHc4EmfG2
+JDLJgOLlyTUPkL/08f0ydGZPofFQBhn8HkuFFjnNtJ5oz3GIP4cdWMQFaUw0uvjb
+PM9Tm3ptENGd6Ts1AAAAFiEEwdtl1YDXuSJyVEseppmYlfq6zlIAAGpTA5dATR6i
+U2GrpUcQgpG+JqfAsGmF4yAOhgFxc1UfidFk3nTup3fLgjipkYY170WLRNbyKkVO
+Sodx93GAs58rizO1acDAWiLq3cyEPBFXbyFThbcNPcLl+/77Uk/mgkYrPQFAQWdK
+1kSRm4SizDBK37K8ChAAAADHhwRhXZSZEgMrZW8Bx0DMhzvhQo+OsXeqQ6QVw4sF
+CaexHh6rLohh7TzL3hQSjoJ27fV6JBkIWdn0LfrMlJIDbSv2SLdlgQMBCgkAAcdA
+MO7Dc1myF6Co1fAH+EuP+OxhxP/7V6ljuSCZENDfA49tQkzTta+PniG+pOVB2LHb
+huyaKBkqiaogo8LAOQQYFgoAeAWCYV2UmQWJBaSPvQkQppmYlfq6zlJHFAAAAAAA
+HgAgc2FsdEBub3RhdGlvbnMuc2VxdW9pYS1wZ3Aub3JnEjBMQAmc/2u45u5FQGmB
+QAytjSG2LM3JQN+PPVl5vEkCmwwWIQTB22XVgNe5InJUSx6mmZiV+rrOUgAASdYD
+l0DXEHQ9ykNP2rZP35ET1dmiFagFtTj/hLQcWlg16LqvJNGqOgYXuqTerbiOOt02
+XLCBln+wdewpU4ChEffMUDRBfqfQco/YsMqWV7bHJHAO0eC/DMKCjyU90xdH7R/d
+QgqsfguR1PqPuJxpXV4bSr6CGAAAAA==
+=MSvh
+-----END PGP PRIVATE KEY BLOCK-----
+`;
+    await expect(openpgp.readKey({ armoredKey: armoredEdDSALegacyCurve448Key })).to.be.rejectedWith(/No key packet found/);
+
+    await expect(openpgp.readKey({
+      armoredKey: armoredEdDSALegacyCurve448Key,
+      config: { ignoreUnsupportedPackets: false }
+    })).to.be.rejectedWith(/Unknown curve OID/);
+  });
+
   it('Parsing ECDH key with unknown kdf param version', async function() {
     // subkey with unknown kdfParam version 255. Parsing should not fail, the subkey should simply dropped
     const key = await openpgp.readKey({ armoredKey: `-----BEGIN PGP PRIVATE KEY BLOCK-----