Only ignore unsupported packets when config.tolerant is set (#1298)

Don't ignore parse errors if `config.tolerant` is enabled. This leads to
more useful error messages in most cases, as ignoring these errors will
most likely still lead to an error down the line (e.g. if a key binding
signature is missing). Unsupported and unknown packets and packets with
an unsupported or unknown version are still ignored, for forward
compatibility.

Also, make `PKESK.encrypt`/`decrypt` void.

openpgp.d.ts

@@ -366,10 +366,10 @@ export class AEADEncryptedDataPacket extends BasePacket {
   private crypt(fn: Function, sessionKey: Uint8Array, data: MaybeStream<Uint8Array>): MaybeStream<Uint8Array>
 }
 
-export class PublicKeyEncryptedSessionKeyPaclet extends BasePacket {
+export class PublicKeyEncryptedSessionKeyPacket extends BasePacket {
   static readonly tag: enums.packet.publicKeyEncryptedSessionKey;
-  private decrypt(keyPacket: SecretKeyPacket): Promise<true>; // throws on error
-  private encrypt(keyPacket: PublicKeyPacket): Promise<true>; // throws on error
+  private decrypt(keyPacket: SecretKeyPacket): void; // throws on error
+  private encrypt(keyPacket: PublicKeyPacket): void; // throws on error
 }
 
 export class SymEncryptedSessionKey extends BasePacket {

src/packet/aead_encrypted_data.js

@@ -20,6 +20,7 @@ import crypto from '../crypto';
 import enums from '../enums';
 import util from '../util';
 import defaultConfig from '../config';
+import { UnsupportedError } from './packet';
 
 import LiteralDataPacket from './literal_data';
 import CompressedDataPacket from './compressed_data';
@@ -66,8 +67,9 @@ class AEADEncryptedDataPacket {
    */
   async read(bytes) {
     await stream.parse(bytes, async reader => {
-      if (await reader.readByte() !== VERSION) { // The only currently defined value is 1.
-        throw new Error('Invalid packet version.');
+      const version = await reader.readByte();
+      if (version !== VERSION) { // The only currently defined value is 1.
+        throw new UnsupportedError(`Version ${version} of the AEAD-encrypted data packet is not supported.`);
       }
       this.cipherAlgo = await reader.readByte();
       this.aeadAlgo = await reader.readByte();

src/packet/one_pass_signature.js

@@ -20,6 +20,9 @@ import SignaturePacket from './signature';
 import KeyID from '../type/keyid';
 import enums from '../enums';
 import util from '../util';
+import { UnsupportedError } from './packet';
+
+const VERSION = 3;
 
 /**
  * Implementation of the One-Pass Signature Packets (Tag 4)
@@ -74,6 +77,9 @@ class OnePassSignaturePacket {
     let mypos = 0;
     // A one-octet version number.  The current version is 3.
     this.version = bytes[mypos++];
+    if (this.version !== VERSION) {
+      throw new UnsupportedError(`Version ${this.version} of the one-pass signature packet is unsupported.`);
+    }
 
     // A one-octet signature type.  Signature types are described in
     //   Section 5.2.1.

src/packet/packet.js

@@ -97,17 +97,17 @@ export function writeHeader(tag_type, length) {
 
 /**
  * Whether the packet type supports partial lengths per RFC4880
- * @param {Integer} tag_type - Tag type
+ * @param {Integer} tag - Tag type
  * @returns {Boolean} String of the header.
  */
-export function supportsStreaming(tag_type) {
+export function supportsStreaming(tag) {
   return [
     enums.packet.literalData,
     enums.packet.compressedData,
     enums.packet.symmetricallyEncryptedData,
     enums.packet.symEncryptedIntegrityProtectedData,
     enums.packet.aeadEncryptedData
-  ].includes(tag_type);
+  ].includes(tag);
 }
 
 /**
@@ -296,3 +296,16 @@ export async function readPackets(input, callback) {
     reader.releaseLock();
   }
 }
+
+export class UnsupportedError extends Error {
+  constructor(...params) {
+    super(...params);
+
+    if (Error.captureStackTrace) {
+      Error.captureStackTrace(this, UnsupportedError);
+    }
+
+    this.name = 'UnsupportedError';
+  }
+}
+

src/packet/packetlist.js

@@ -2,7 +2,8 @@ import * as stream from '@openpgp/web-stream-tools';
 import {
   readPackets, supportsStreaming,
   writeTag, writeHeader,
-  writePartialLength, writeSimpleLength
+  writePartialLength, writeSimpleLength,
+  UnsupportedError
 } from './packet';
 import util from '../util';
 import enums from '../enums';
@@ -17,7 +18,14 @@ import defaultConfig from '../config';
  */
 export function newPacketFromTag(tag, allowedPackets) {
   if (!allowedPackets[tag]) {
-    throw new Error(`Packet not allowed in this context: ${enums.read(enums.packet, tag)}`);
+    // distinguish between disallowed packets and unknown ones
+    let packetType;
+    try {
+      packetType = enums.read(enums.packet, tag);
+    } catch (e) {
+      throw new UnsupportedError(`Unknown packet type with tag: ${tag}`);
+    }
+    throw new UnsupportedError(`Packet not allowed in this context: ${packetType}`);
   }
   return new allowedPackets[tag]();
 }
@@ -67,10 +75,11 @@ class PacketList extends Array {
               await packet.read(parsed.packet, config);
               await writer.write(packet);
             } catch (e) {
-              if (!config.tolerant || supportsStreaming(parsed.tag)) {
-                // The packets that support streaming are the ones that contain
-                // message data. Those are also the ones we want to be more strict
-                // about and throw on parse errors for.
+              const isTolerableError = config.tolerant && e instanceof UnsupportedError;
+              if (!isTolerableError || supportsStreaming(parsed.tag)) {
+                // The packets that support streaming are the ones that contain message data.
+                // 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);
               }
               util.printDebugError(e);

src/packet/public_key.js

@@ -22,6 +22,7 @@ import defaultConfig from '../config';
 import crypto from '../crypto';
 import enums from '../enums';
 import util from '../util';
+import { UnsupportedError } from './packet';
 
 /**
  * Implementation of the Key Material Packet (Tag 5,6,7,14)
@@ -137,7 +138,7 @@ class PublicKeyPacket {
       await this.computeFingerprintAndKeyID();
       return pos;
     }
-    throw new Error('Version ' + this.version + ' of the key packet is unsupported.');
+    throw new UnsupportedError(`Version ${this.version} of the key packet is unsupported.`);
   }
 
   /**

src/packet/public_key_encrypted_session_key.js

@@ -19,6 +19,9 @@ import KeyID from '../type/keyid';
 import crypto from '../crypto';
 import enums from '../enums';
 import util from '../util';
+import { UnsupportedError } from './packet';
+
+const VERSION = 3;
 
 /**
  * Public-Key Encrypted Session Key Packets (Tag 1)
@@ -61,6 +64,9 @@ class PublicKeyEncryptedSessionKeyPacket {
    */
   read(bytes) {
     this.version = bytes[0];
+    if (this.version !== VERSION) {
+      throw new UnsupportedError(`Version ${this.version} of the PKESK packet is unsupported.`);
+    }
     this.publicKeyID.read(bytes.subarray(1, bytes.length));
     this.publicKeyAlgorithm = enums.read(enums.publicKey, bytes[9]);
 
@@ -89,7 +95,7 @@ class PublicKeyEncryptedSessionKeyPacket {
   /**
    * Encrypt session key packet
    * @param {PublicKeyPacket} key - Public key
-   * @returns {Promise<Boolean>}
+   * @throws {Error} if encryption failed
    * @async
    */
   async encrypt(key) {
@@ -101,16 +107,13 @@ class PublicKeyEncryptedSessionKeyPacket {
     const algo = enums.write(enums.publicKey, this.publicKeyAlgorithm);
     this.encrypted = await crypto.publicKeyEncrypt(
       algo, key.publicParams, data, key.getFingerprintBytes());
-    return true;
   }
 
   /**
    * Decrypts the session key (only for public key encrypted session key
    * packets (tag 1)
-   *
-   * @param {SecretKeyPacket} key
-   *            Private key with secret params unlocked
-   * @returns {Promise<Boolean>}
+   * @param {SecretKeyPacket} key - decrypted private key
+   * @throws {Error} if decryption failed
    * @async
    */
   async decrypt(key) {
@@ -129,7 +132,6 @@ class PublicKeyEncryptedSessionKeyPacket {
       this.sessionKey = sessionKey;
       this.sessionKeyAlgorithm = enums.read(enums.symmetric, decoded[0]);
     }
-    return true;
   }
 }
 

src/packet/signature.js

@@ -16,7 +16,7 @@
 // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
 
 import * as stream from '@openpgp/web-stream-tools';
-import { readSimpleLength, writeSimpleLength } from './packet';
+import { readSimpleLength, UnsupportedError, writeSimpleLength } from './packet';
 import KeyID from '../type/keyid.js';
 import crypto from '../crypto';
 import enums from '../enums';
@@ -99,7 +99,7 @@ class SignaturePacket {
     this.version = bytes[i++];
 
     if (this.version !== 4 && this.version !== 5) {
-      throw new Error('Version ' + this.version + ' of the signature is unsupported.');
+      throw new UnsupportedError(`Version ${this.version} of the signature packet is unsupported.`);
     }
 
     this.signatureType = bytes[i++];

src/packet/sym_encrypted_integrity_protected_data.js

@@ -26,6 +26,7 @@ import CompressedDataPacket from './compressed_data';
 import OnePassSignaturePacket from './one_pass_signature';
 import SignaturePacket from './signature';
 import PacketList from './packetlist';
+import { UnsupportedError } from './packet';
 
 // A SEIP packet can contain the following packet types
 const allowedPackets = /*#__PURE__*/ util.constructAllowedPackets([
@@ -60,10 +61,10 @@ class SymEncryptedIntegrityProtectedDataPacket {
 
   async read(bytes) {
     await stream.parse(bytes, async reader => {
-
+      const version = await reader.readByte();
       // - A one-octet version number. The only currently defined value is 1.
-      if (await reader.readByte() !== VERSION) {
-        throw new Error('Invalid packet version.');
+      if (version !== VERSION) {
+        throw new UnsupportedError(`Version ${version} of the SEIP packet is unsupported.`);
       }
 
       // - Encrypted data, the output of the selected symmetric-key cipher

src/packet/sym_encrypted_session_key.js

@@ -20,6 +20,7 @@ import defaultConfig from '../config';
 import crypto from '../crypto';
 import enums from '../enums';
 import util from '../util';
+import { UnsupportedError } from './packet';
 
 /**
  * Symmetric-Key Encrypted Session Key Packets (Tag 3)
@@ -63,6 +64,9 @@ class SymEncryptedSessionKeyPacket {
 
     // A one-octet version number. The only currently defined version is 4.
     this.version = bytes[offset++];
+    if (this.version !== 4 && this.version !== 5) {
+      throw new UnsupportedError(`Version ${this.version} of the SKESK packet is unsupported.`);
+    }
 
     // A one-octet number describing the symmetric algorithm used.
     const algo = enums.read(enums.symmetric, bytes[offset++]);

src/packet/trust.js

@@ -1,6 +1,7 @@
 /* eslint class-methods-use-this: ["error", { "exceptMethods": ["read"] }] */
 
 import enums from '../enums';
+import { UnsupportedError } from './packet';
 
 /**
  * Implementation of the Trust Packet (Tag 12)
@@ -25,13 +26,14 @@ class TrustPacket {
   /**
    * Parsing function for a trust packet (tag 12).
    * Currently not implemented as we ignore trust packets
-   * @param {String} byptes - Payload of a tag 12 packet
    */
-  read() {} // TODO
+  read() {
+    throw new UnsupportedError('Trust packets are not supported');
+  }
 
   // eslint-disable-next-line class-methods-use-this
   write() {
-    throw new Error('Trust packets are not supported');
+    throw new UnsupportedError('Trust packets are not supported');
   }
 }
 

test/general/config.js

@@ -359,5 +359,4 @@ qDEdLyNWF30o6wD/fZYCV8aS4dAu2U3fpN5y5+PbuXFRYljA5gQ/1zrGN/UA
     await expect(sig4.valid).to.be.false;
     await expect(sig4.error).to.match(/eddsa keys are considered too weak/);
   });
-
 });

test/general/packet.js

@@ -67,21 +67,6 @@ module.exports = () => describe("Packet", function() {
       '=KXkj\n' +
       '-----END PGP PRIVATE KEY BLOCK-----';
 
-  it('Ignores disallowed packet with tolerant mode enabled', async function() {
-    const packets = new openpgp.PacketList();
-    packets.push(new openpgp.MarkerPacket());
-    const bytes = packets.write();
-    const parsed = await openpgp.PacketList.fromBinary(bytes, {}, { ...openpgp.config, tolerant: true });
-    expect(parsed.length).to.equal(0);
-  });
-
-  it('Throws on disallowed packet with tolerant mode disabled', async function() {
-    const packets = new openpgp.PacketList();
-    packets.push(new openpgp.MarkerPacket());
-    const bytes = packets.write();
-    await expect(openpgp.PacketList.fromBinary(bytes, {}, { ...openpgp.config, tolerant: false })).to.be.rejectedWith(/Packet not allowed in this context/);
-  });
-
   it('Symmetrically encrypted packet without integrity protection - allow decryption', async function() {
     const aeadProtectVal = openpgp.config.aeadProtect;
     const allowUnauthenticatedMessagesVal = openpgp.config.allowUnauthenticatedMessages;
@@ -963,4 +948,28 @@ V+HOQJQxXJkVRYa3QrFUehiMzTeqqMdgC6ZqJy7+
       });
     });
   });
+
+  describe('PacketList parsing', function () {
+    it('Ignores disallowed packet with tolerant mode enabled', async function() {
+      const packets = new openpgp.PacketList();
+      packets.push(new openpgp.MarkerPacket());
+      const bytes = packets.write();
+      const parsed = await openpgp.PacketList.fromBinary(bytes, {}, { ...openpgp.config, tolerant: true });
+      expect(parsed.length).to.equal(0);
+    });
+
+    it('Throws on disallowed packet with tolerant mode disabled', async function() {
+      const packets = new openpgp.PacketList();
+      packets.push(new openpgp.MarkerPacket());
+      const bytes = packets.write();
+      await expect(openpgp.PacketList.fromBinary(bytes, {}, { ...openpgp.config, tolerant: false })).to.be.rejectedWith(/Packet not allowed in this context/);
+    });
+
+    it('Throws on parsing errors even with tolerant mode enabled', async function () {
+      const { privateKeyArmored: armoredKey } = await openpgp.generateKey({ userIDs:[{ name:'test', email:'test@a.it' }] });
+      await expect(
+        openpgp.readKey({ armoredKey, config: { tolerant: true, maxUserIDLength: 2 } })
+      ).to.be.rejectedWith(/User ID string is too long/);
+    });
+  });
 });