Throw UnsupportedError on unknown algorithm in keys, signatures and encrypted session keys

openpgp.d.ts

@@ -74,11 +74,11 @@ export abstract class Key {
 
 type AllowedKeyPackets = PublicKeyPacket | PublicSubkeyPacket | SecretKeyPacket | SecretSubkeyPacket | UserIDPacket | UserAttributePacket | SignaturePacket;
 export class PublicKey extends Key {
-  constructor(packetlist: PacketList<AnyKeyPacket>);
+  constructor(packetlist: PacketList<AnyPacket>);
 }
 
 export class PrivateKey extends PublicKey {
-  constructor(packetlist: PacketList<AnyKeyPacket>);
+  constructor(packetlist: PacketList<AnyPacket>);
   public revoke(reason?: ReasonForRevocation, date?: Date, config?: Config): Promise<PrivateKey>;
   public isDecrypted(): boolean;
   public addSubkey(options: SubkeyOptions): Promise<PrivateKey>;
@@ -527,7 +527,12 @@ export class TrustPacket extends BasePacket {
   static readonly tag: enums.packet.trust;
 }
 
-export type AnyPacket = BasePacket;
+export class UnparseablePacket {
+  tag: enums.packet;
+  write: () => Uint8Array;
+}
+
+export type AnyPacket = BasePacket | UnparseablePacket;
 export type AnySecretKeyPacket = SecretKeyPacket | SecretSubkeyPacket;
 export type AnyKeyPacket = BasePublicKeyPacket;
 

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.');
   }
 }
 
@@ -156,7 +157,7 @@ export function parsePublicKeyParams(algo, bytes) {
       return { read: read, publicParams: { oid, Q, kdfParams } };
     }
     default:
-      throw new Error('Invalid public key encryption algorithm.');
+      throw new UnsupportedError('Unknown public key encryption algorithm.');
   }
 }
 
@@ -197,7 +198,7 @@ export function parsePrivateKeyParams(algo, bytes, publicParams) {
       return { read, privateParams: { seed } };
     }
     default:
-      throw new Error('Invalid public key encryption algorithm.');
+      throw new UnsupportedError('Unknown public key encryption algorithm.');
   }
 }
 
@@ -234,7 +235,7 @@ export function parseEncSessionKeyParams(algo, bytes) {
       return { V, C };
     }
     default:
-      throw new Error('Invalid public key encryption algorithm.');
+      throw new Error('Unknown public key encryption algorithm.');
   }
 }
 
@@ -293,7 +294,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 +341,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.');
   }
 }
 

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];
 

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.');
   }
 }

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;

src/key/key.js

@@ -28,9 +28,15 @@ import Subkey from './subkey';
 import * as helper from './helper';
 import PrivateKey from './private_key';
 import PublicKey from './public_key';
+import { UnparseablePacket } from '../packet/packet';
 
 // A key revocation certificate can contain the following packets
 const allowedRevocationPackets = /*#__PURE__*/ util.constructAllowedPackets([SignaturePacket]);
+const mainKeyPacketTags = new Set([enums.packet.publicKey, enums.packet.privateKey]);
+const keyPacketTags = new Set([
+  enums.packet.publicKey, enums.packet.privateKey,
+  enums.packet.publicSubkey, enums.packet.privateSubkey
+]);
 
 /**
  * Abstract class that represents an OpenPGP key. Must contain a primary key.
@@ -51,8 +57,29 @@ class Key {
     let user;
     let primaryKeyID;
     let subkey;
+    let ignoreUntil;
+
     for (const packet of packetlist) {
+
+      if (packet instanceof UnparseablePacket) {
+        const isUnparseableKeyPacket = keyPacketTags.has(packet.tag);
+        if (isUnparseableKeyPacket && !ignoreUntil){
+          // Since non-key packets apply to the preceding key packet, if a (sub)key is Unparseable we must
+          // discard all non-key packets that follow, until another (sub)key packet is found.
+          if (mainKeyPacketTags.has(packet.tag)) {
+            ignoreUntil = mainKeyPacketTags;
+          } else {
+            ignoreUntil = keyPacketTags;
+          }
+        }
+        continue;
+      }
+
       const tag = packet.constructor.tag;
+      if (ignoreUntil) {
+        if (!ignoreUntil.has(tag)) continue;
+        ignoreUntil = null;
+      }
       if (disallowedPackets.has(tag)) {
         throw new Error(`Unexpected packet type: ${tag}`);
       }

src/packet/index.js

@@ -1,2 +1,3 @@
 export * from './all_packets';
 export { default as PacketList } from './packetlist';
+export { UnparseablePacket } from './packet';

src/packet/packet.js

@@ -309,3 +309,13 @@ export class UnsupportedError extends Error {
   }
 }
 
+export class UnparseablePacket {
+  constructor(tag, rawContent) {
+    this.tag = tag;
+    this.rawContent = rawContent;
+  }
+
+  write() {
+    return this.rawContent;
+  }
+}

src/packet/packetlist.js

@@ -3,6 +3,7 @@ import {
   readPackets, supportsStreaming,
   writeTag, writeHeader,
   writePartialLength, writeSimpleLength,
+  UnparseablePacket,
   UnsupportedError
 } from './packet';
 import util from '../util';
@@ -89,6 +90,9 @@ class PacketList extends Array {
                 // Those are also the ones we want to be more strict about and throw on parse errors
                 // (since we likely cannot process the message without these packets anyway).
                 await writer.abort(e);
+              } else {
+                const unparsedPacket = new UnparseablePacket(parsed.tag, parsed.packet);
+                await writer.write(unparsedPacket);
               }
               util.printDebugError(e);
             }
@@ -129,12 +133,13 @@ class PacketList extends Array {
     const arr = [];
 
     for (let i = 0; i < this.length; i++) {
+      const tag = this[i] instanceof UnparseablePacket ? this[i].tag : this[i].constructor.tag;
       const packetbytes = this[i].write();
       if (util.isStream(packetbytes) && supportsStreaming(this[i].constructor.tag)) {
         let buffer = [];
         let bufferLength = 0;
         const minLength = 512;
-        arr.push(writeTag(this[i].constructor.tag));
+        arr.push(writeTag(tag));
         arr.push(stream.transform(packetbytes, value => {
           buffer.push(value);
           bufferLength += value.length;
@@ -152,9 +157,9 @@ class PacketList extends Array {
           let length = 0;
           arr.push(stream.transform(stream.clone(packetbytes), value => {
             length += value.length;
-          }, () => writeHeader(this[i].constructor.tag, length)));
+          }, () => writeHeader(tag, length)));
         } else {
-          arr.push(writeHeader(this[i].constructor.tag, packetbytes.length));
+          arr.push(writeHeader(tag, packetbytes.length));
         }
         arr.push(packetbytes);
       }

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();

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');
       }
     }

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()) {

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 () {