From 82799390ded7bd023555f481fc6992a560e7924b Mon Sep 17 00:00:00 2001
From: Wiktor Kwapisiewicz <wiktor@metacode.biz>
Date: Mon, 13 May 2019 20:56:23 +0200
Subject: [PATCH] Fix signatures with critical notations

Previously the signature parsing function ignored critical bit on
notations.

This change checks for notations that are marked "critical" but are not
on the known notations list (controlled by config array
`openpgp.config.known_notations`) and triggers parse error if such
a notation have been encountered.

See: #897.
---
 src/config/config.js      |  9 +++++++-
 src/packet/signature.js   |  5 +++++
 test/general/signature.js | 43 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/src/config/config.js b/src/config/config.js
index a59218ea..9e53b925 100644
--- a/src/config/config.js
+++ b/src/config/config.js
@@ -183,5 +183,12 @@ export default {
    * @memberof module:config
    * @property {Integer} max_userid_length
    */
-  max_userid_length: 1024 * 5
+  max_userid_length: 1024 * 5,
+  /**
+   * Contains notatations that are considered "known". Known notations do not trigger
+   * validation error when the notation is marked as critical.
+   * @memberof module:config
+   * @property {Array} known_notations
+   */
+  known_notations: ["preferred-email-encoding@pgp.com", "pka-address@gnupg.org"]
 };
diff --git a/src/packet/signature.js b/src/packet/signature.js
index 2dc13719..5b01da2a 100644
--- a/src/packet/signature.js
+++ b/src/packet/signature.js
@@ -32,6 +32,7 @@ import type_mpi from '../type/mpi.js';
 import crypto from '../crypto';
 import enums from '../enums';
 import util from '../util';
+import config from '../config';
 
 /**
  * Implementation of the Signature Packet (Tag 2)
@@ -446,6 +447,10 @@ Signature.prototype.read_sub_packet = function (bytes, trusted=true) {
         const value = util.Uint8Array_to_str(bytes.subarray(mypos + m, mypos + m + n));
 
         this.notations.push([name, value]);
+
+        if (critical && (config.known_notations.indexOf(name) === -1)) {
+          throw new Error("Unknown critical notation: " + name);
+        }
       } else {
         util.print_debug("Unsupported notation flag "+bytes[mypos]);
       }
diff --git a/test/general/signature.js b/test/general/signature.js
index 9c4f2af7..e4f8163b 100644
--- a/test/general/signature.js
+++ b/test/general/signature.js
@@ -467,6 +467,16 @@ describe("Signature", function() {
         '=NbaL',
         '-----END PGP PRIVATE KEY BLOCK-----'].join("\n");
 
+  const signature_with_critical_notation = `-----BEGIN PGP MESSAGE-----
+
+owGbwMvMwMH4oOW7S46CznTG09xJDDE3Wl1KUotLuDousDAwcjBYiSmyXL+48d6x
+U1PSGUxcj8IUszKBVMpMaWAAAgEGZpAeh9SKxNyCnFS95PzcytRiBi5OAZjyXXzM
+f8WYLqv7TXP61Sa4rqT12CI3xaN73YS2pt089f96odCKaEPnWJ3iSGmzJaW/ug10
+2Zo8Wj2k4s7t8wt4H3HtTu+y5UZfV3VOO+l//sdE/o+Lsub8FZH7/eOq7OnbNp4n
+vwjE8mqJXetNMfj8r2SCyvkEnlVRYR+/mnge+ib56FdJ8uKtqSxyvgA=
+=fRXs
+-----END PGP MESSAGE-----`;
+
   it('Testing signature checking on CAST5-enciphered message', async function() {
     const priv_key = (await openpgp.key.readArmored(priv_key_arm1)).keys[0];
     const pub_key = (await openpgp.key.readArmored(pub_key_arm1)).keys[0];
@@ -606,6 +616,39 @@ describe("Signature", function() {
     });
   });
 
+  it('Verify fails with signed message with critical notations', async function() {
+    let testFailed = true;
+    try {
+      openpgp.config.tolerant = false;
+      const sMsg = await openpgp.message.readArmored(signature_with_critical_notation);
+      const pub_key = (await openpgp.key.readArmored(pub_key_arm2)).keys[0];
+      const verified = await sMsg.verify([pub_key]);
+      await verified[0].verified;
+      testFailed = false;
+    } catch (e) {
+      expect(e.message).to.equal('Unknown critical notation: test@example.com');
+    } finally {
+      openpgp.config.tolerant = true;
+    }
+    // fail the test if execution does not throw an exception
+    expect(testFailed).to.be.true;
+  });
+
+  it('Verify succeeds with known signed message with critical notations', async function() {
+    openpgp.config.tolerant = false;
+    openpgp.config.known_notations.push('test@example.com');
+    try {
+      const sMsg = await openpgp.message.readArmored(signature_with_critical_notation);
+      const pub_key = (await openpgp.key.readArmored(pub_key_arm2)).keys[0];
+      const verified = await sMsg.verify([pub_key]);
+      openpgp.stream.pipe(sMsg.getLiteralData(), new WritableStream());
+      expect(await verified[0].verified).to.be.true;
+    } finally {
+      openpgp.config.known_notations.pop();
+      openpgp.config.tolerant = true;
+    }
+  });
+
   it('Verify cleartext signed message with two signatures with openpgp.verify', async function() {
     const msg_armor =
       ['-----BEGIN PGP SIGNED MESSAGE-----',