Throw UnsupportedError on unknown algorithm in keys, signatures and…

… encrypted session keys (#1523) The relevant packets will be considered unsupported instead of malformed. Hence, parsing them will succeed by default (based on `config.ignoreUnsupportedPackets`).
openpgpjs · Jun 7, 2022 · ef06618 · ef06618
1 parent a822dd8
commit ef06618
Show file tree

Hide file tree

Showing 9 changed files with 81 additions and 22 deletions.
diff --git a/src/crypto/crypto.js b/src/crypto/crypto.js
@@ -34,6 +34,7 @@ import enums from '../enums';
 import util from '../util';
 import OID from '../type/oid';
 import { Curve } from './public_key/elliptic/curves';
+import { UnsupportedError } from '../packet/packet';
 
 /**
  * Encrypts data using specified algorithm and public key parameters.
@@ -105,7 +106,7 @@ export async function publicKeyDecrypt(algo, publicKeyParams, privateKeyParams,
         oid, kdfParams, V, C.data, Q, d, fingerprint);
     }
     default:
-      throw new Error('Invalid public key encryption algorithm.');
+      throw new Error('Unknown public key encryption algorithm.');
   }
 }
 
@@ -140,23 +141,26 @@ export function parsePublicKeyParams(algo, bytes) {
     }
     case enums.publicKey.ecdsa: {
       const oid = new OID(); read += oid.read(bytes);
+      checkSupportedCurve(oid);
       const Q = util.readMPI(bytes.subarray(read)); read += Q.length + 2;
       return { read: read, publicParams: { oid, Q } };
     }
     case enums.publicKey.eddsa: {
       const oid = new OID(); read += oid.read(bytes);
+      checkSupportedCurve(oid);
       let Q = util.readMPI(bytes.subarray(read)); read += Q.length + 2;
       Q = util.leftPad(Q, 33);
       return { read: read, publicParams: { oid, Q } };
     }
     case enums.publicKey.ecdh: {
       const oid = new OID(); read += oid.read(bytes);
+      checkSupportedCurve(oid);
       const Q = util.readMPI(bytes.subarray(read)); read += Q.length + 2;
       const kdfParams = new KDFParams(); read += kdfParams.read(bytes.subarray(read));
       return { read: read, publicParams: { oid, Q, kdfParams } };
     }
     default:
-      throw new Error('Invalid public key encryption algorithm.');
+      throw new UnsupportedError('Unknown public key encryption algorithm.');
   }
 }
 
@@ -192,12 +196,13 @@ export function parsePrivateKeyParams(algo, bytes, publicParams) {
       return { read, privateParams: { d } };
     }
     case enums.publicKey.eddsa: {
+      const curve = new Curve(publicParams.oid);
       let seed = util.readMPI(bytes.subarray(read)); read += seed.length + 2;
-      seed = util.leftPad(seed, 32);
+      seed = util.leftPad(seed, curve.payloadSize);
       return { read, privateParams: { seed } };
     }
     default:
-      throw new Error('Invalid public key encryption algorithm.');
+      throw new UnsupportedError('Unknown public key encryption algorithm.');
   }
 }
 
@@ -234,7 +239,7 @@ export function parseEncSessionKeyParams(algo, bytes) {
       return { V, C };
     }
     default:
-      throw new Error('Invalid public key encryption algorithm.');
+      throw new UnsupportedError('Unknown public key encryption algorithm.');
   }
 }
 
@@ -293,7 +298,7 @@ export function generateParams(algo, bits, oid) {
     case enums.publicKey.elgamal:
       throw new Error('Unsupported algorithm for key generation.');
     default:
-      throw new Error('Invalid public key algorithm.');
+      throw new Error('Unknown public key algorithm.');
   }
 }
 
@@ -340,7 +345,7 @@ export async function validateParams(algo, publicParams, privateParams) {
       return publicKey.elliptic.eddsa.validateParams(oid, Q, seed);
     }
     default:
-      throw new Error('Invalid public key algorithm.');
+      throw new Error('Unknown public key algorithm.');
   }
 }
 
@@ -391,3 +396,16 @@ export function getCipher(algo) {
   const algoName = enums.read(enums.symmetric, algo);
   return cipher[algoName];
 }
+
+/**
+ * Check whether the given curve OID is supported
+ * @param {module:type/oid} oid - EC object identifier
+ * @throws {UnsupportedError} if curve is not supported
+ */
+function checkSupportedCurve(oid) {
+  try {
+    oid.getName();
+  } catch (e) {
+    throw new UnsupportedError('Unknown curve OID');
+  }
+}
diff --git a/src/crypto/public_key/elliptic/curves.js b/src/crypto/public_key/elliptic/curves.js
@@ -28,6 +28,7 @@ import util from '../../../util';
 import { uint8ArrayToB64, b64ToUint8Array } from '../../../encoding/base64';
 import OID from '../../../type/oid';
 import { keyFromPublic, keyFromPrivate, getIndutnyCurve } from './indutnyKey';
+import { UnsupportedError } from '../../../packet/packet';
 
 const webCrypto = util.getWebCrypto();
 const nodeCrypto = util.getNodeCrypto();
@@ -145,7 +146,7 @@ class Curve {
       // by curve name or oid string
       this.name = enums.write(enums.curve, oidOrName);
     } catch (err) {
-      throw new Error('Not valid curve');
+      throw new UnsupportedError('Unknown curve');
     }
     params = params || curves[this.name];
 

diff --git a/src/crypto/signature.js b/src/crypto/signature.js
@@ -7,6 +7,7 @@
 import publicKey from './public_key';
 import enums from '../enums';
 import util from '../util';
+import { UnsupportedError } from '../packet/packet';
 
 /**
  * Parse signature in binary form to get the parameters.
@@ -55,7 +56,7 @@ export function parseSignatureParams(algo, signature) {
       return { r, s };
     }
     default:
-      throw new Error('Invalid signature algorithm.');
+      throw new UnsupportedError('Unknown signature algorithm.');
   }
 }
 
@@ -101,7 +102,7 @@ export async function verify(algo, hashAlgo, signature, publicParams, data, hash
       return publicKey.elliptic.eddsa.verify(oid, hashAlgo, signature, data, Q, hashed);
     }
     default:
-      throw new Error('Invalid signature algorithm.');
+      throw new Error('Unknown signature algorithm.');
   }
 }
 
@@ -151,6 +152,6 @@ export async function sign(algo, hashAlgo, publicKeyParams, privateKeyParams, da
       return publicKey.elliptic.eddsa.sign(oid, hashAlgo, data, Q, seed, hashed);
     }
     default:
-      throw new Error('Invalid signature algorithm.');
+      throw new Error('Unknown signature algorithm.');
   }
 }
diff --git a/src/key/helper.js b/src/key/helper.js
@@ -340,7 +340,7 @@ export function sanitizeKeyOptions(options, subkeyDefaults = {}) {
       try {
         options.curve = enums.write(enums.curve, options.curve);
       } catch (e) {
-        throw new Error('Invalid curve');
+        throw new Error('Unknown curve');
       }
       if (options.curve === enums.curve.ed25519 || options.curve === enums.curve.curve25519) {
         options.curve = options.sign ? enums.curve.ed25519 : enums.curve.curve25519;

diff --git a/src/packet/public_key.js b/src/packet/public_key.js
@@ -123,13 +123,9 @@ class PublicKeyPacket {
       }
 
       // - A series of values comprising the key material.
-      try {
-        const { read, publicParams } = crypto.parsePublicKeyParams(this.algorithm, bytes.subarray(pos));
-        this.publicParams = publicParams;
-        pos += read;
-      } catch (err) {
-        throw new Error('Error reading MPIs');
-      }
+      const { read, publicParams } = crypto.parsePublicKeyParams(this.algorithm, bytes.subarray(pos));
+      this.publicParams = publicParams;
+      pos += read;
 
       // we set the fingerprint and keyID already to make it possible to put together the key packets directly in the Key constructor
       await this.computeFingerprintAndKeyID();

diff --git a/src/packet/secret_key.js b/src/packet/secret_key.js
@@ -21,6 +21,7 @@ import crypto from '../crypto';
 import enums from '../enums';
 import util from '../util';
 import defaultConfig from '../config';
+import { UnsupportedError } from './packet';
 
 /**
  * A Secret-Key packet contains all the information that is found in a
@@ -155,6 +156,8 @@ class SecretKeyPacket extends PublicKeyPacket {
         const { privateParams } = crypto.parsePrivateKeyParams(this.algorithm, cleartext, this.publicParams);
         this.privateParams = privateParams;
       } catch (err) {
+        if (err instanceof UnsupportedError) throw err;
+        // avoid throwing potentially sensitive errors
         throw new Error('Error reading MPIs');
       }
     }

diff --git a/test/crypto/ecdh.js b/test/crypto/ecdh.js
@@ -69,7 +69,7 @@ module.exports = () => describe('ECDH key exchange @lightweight', function () {
   it('Invalid curve oid', function (done) {
     expect(decrypt_message(
       '', 2, 7, [], [], [], [], []
-    )).to.be.rejectedWith(Error, /Not valid curve/).notify(done);
+    )).to.be.rejectedWith(Error, /Unknown curve/).notify(done);
   });
   it('Invalid ephemeral key', function (done) {
     if (!openpgp.config.useIndutnyElliptic && !util.getNodeCrypto()) {

diff --git a/test/crypto/elliptic.js b/test/crypto/elliptic.js
@@ -167,10 +167,10 @@ module.exports = () => describe('Elliptic Curve Cryptography @lightweight', func
       return Promise.all([
         expect(verify_signature(
           'invalid oid', 8, [], [], [], []
-        )).to.be.rejectedWith(Error, /Not valid curve/),
+        )).to.be.rejectedWith(Error, /Unknown curve/),
         expect(verify_signature(
           '\x00', 8, [], [], [], []
-        )).to.be.rejectedWith(Error, /Not valid curve/)
+        )).to.be.rejectedWith(Error, /Unknown curve/)
       ]);
     });
     it('Invalid public key', async function () {

diff --git a/test/general/packet.js b/test/general/packet.js
@@ -996,6 +996,46 @@ kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu
       ).to.be.rejectedWith(/Version 1 of the signature packet is unsupported/);
     });
 
+    it('Ignores unknown signature algorithm only with `config.ignoreUnsupportedPackets` enabled', async function() {
+      const binarySignature = util.hexToUint8Array('c2750401630a00060502628b8e2200210910f30ddfc2310b3560162104b9b0045c1930f842cb245566f30ddfc2310b35602ded0100bd69fe6a9f52499cd8b2fd2493dae91c997979890df4467cf31b197901590ff10100ead4c671487535b718a8428c8e6099e3873a41610aad9fcdaa06f6df5f404002');
+
+      const parsed = await openpgp.PacketList.fromBinary(binarySignature, allAllowedPackets, { ...openpgp.config, ignoreUnsupportedPackets: true });
+      expect(parsed.length).to.equal(1);
+      expect(parsed[0]).instanceOf(openpgp.UnparseablePacket);
+      expect(parsed[0].tag).to.equal(openpgp.enums.packet.signature);
+
+      await expect(
+        openpgp.PacketList.fromBinary(binarySignature, allAllowedPackets, { ...openpgp.config, ignoreUnsupportedPackets: false })
+      ).to.be.rejectedWith(/Unknown signature algorithm/);
+    });
+
+    it('Ignores unknown key algorithm only with `config.ignoreUnsupportedPackets` enabled', async function() {
+      const binaryKey = util.hexToUint8Array('c55804628b944e63092b06010401da470f01010740d01ab8619b6dc6a36da5bff62ff416a974900f5a8c74d1bd1760d717d0aad8d50000ff516f8e3190aa5b394597655d7c32e16392e638da0e2a869fb7b1f429d9de263d1062cd0f3c7465737440746573742e636f6d3ec28c0410160a001d0502628b944e040b0907080315080a0416000201021901021b03021e01002109104803e40df201fa5b16210496dc42e91cc585e2f5e331644803e40df201fa5b340b0100812c47b60fa509e12e329fc37cc9c437cc6a6500915caa03ad8703db849846f900ff571b9a0d9e1dcc087d9fae04ec2906e60ef40ca02a387eb07ce1c37bedeecd0a');
+
+      const parsed = await openpgp.PacketList.fromBinary(binaryKey, allAllowedPackets, { ...openpgp.config, ignoreUnsupportedPackets: true });
+      expect(parsed.length).to.equal(3);
+      expect(parsed[0]).instanceOf(openpgp.UnparseablePacket);
+      expect(parsed[0].tag).to.equal(openpgp.enums.packet.secretKey);
+
+      await expect(
+        openpgp.PacketList.fromBinary(binaryKey, allAllowedPackets, { ...openpgp.config, ignoreUnsupportedPackets: false })
+      ).to.be.rejectedWith(/Unknown public key encryption algorithm/);
+    });
+
+    it('Ignores unknown PKESK algorithm only with `config.ignoreUnsupportedPackets` enabled', async function() {
+      const binaryMessage = util.hexToUint8Array('c15e03c6a6737124ef0f5e63010740282956b4db64ea79e1b4b8e5c528241b5e1cf40b2f5df2a619692755d532353d30a8e044e7c96f51741c73e6c5c8f73db08daf66e49240afe90c9b50705d51e71ec2e7630c5bd86b002e1f6dbd638f61e2d23501830d9bb3711c66963363a6e5f8d9294210a0cd194174c3caa3f29865d33c6be4c09b437f906ca8d35e666f3ef53fd22e0d8ceade');
+
+      const parsed = await openpgp.PacketList.fromBinary(binaryMessage, allAllowedPackets, { ...openpgp.config, ignoreUnsupportedPackets: true });
+      expect(parsed.length).to.equal(2);
+      expect(parsed[0]).instanceOf(openpgp.UnparseablePacket);
+      expect(parsed[0].tag).to.equal(openpgp.enums.packet.publicKeyEncryptedSessionKey);
+
+      await expect(
+        openpgp.PacketList.fromBinary(binaryMessage, allAllowedPackets, { ...openpgp.config, ignoreUnsupportedPackets: false })
+      ).to.be.rejectedWith(/Unknown public key encryption algorithm/);
+    });
+
+
     it('Throws on disallowed packet even with tolerant mode enabled', async function() {
       const packets = new openpgp.PacketList();
       packets.push(new openpgp.LiteralDataPacket());