From a57bffc84a68dba9bb75d2c406353805a5a3bd55 Mon Sep 17 00:00:00 2001 From: larabr Date: Mon, 14 Oct 2024 12:15:33 +0200 Subject: [PATCH] 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 | 21 ++++++++++------- src/util.js | 3 +++ test/general/key.js | 51 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 8 deletions(-) diff --git a/src/crypto/signature.js b/src/crypto/signature.js index d9574ad1..4f78a1ad 100644 --- a/src/crypto/signature.js +++ b/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: { diff --git a/src/util.js b/src/util.js index d4635cad..d13052c0 100644 --- a/src/util.js +++ b/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); diff --git a/test/general/key.js b/test/general/key.js index 995c32b5..151a7116 100644 --- a/test/general/key.js +++ b/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-----