Support non-human-readable notation values (#983)

This change adds support for binary (non-human-readable) values in signature notations through `rawNotations` property on signature objects. Human-readable notations will additionally appear in `notations` object where the value of the notation will be deserialized into a string. Additionally the check for human-readable flag was modified to check the existence of the flag instead of comparison with the whole value.
TogaTech · Aug 18, 2020 · 0712e8a · 0712e8a
1 parent 25bf080
commit 0712e8a
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 31 deletions.
diff --git a/src/packet/signature.js b/src/packet/signature.js
@@ -71,7 +71,8 @@ function Signature(date = new Date()) {
   this.revocationKeyAlgorithm = null;
   this.revocationKeyFingerprint = null;
   this.issuerKeyId = new type_keyid();
-  this.notations = [];
+  this.rawNotations = [];
+  this.notations = {};
   this.preferredHashAlgorithms = null;
   this.preferredCompressionAlgorithms = null;
   this.keyServerPreferences = null;
@@ -233,13 +234,14 @@ Signature.prototype.write_hashed_sub_packets = function () {
     bytes = util.concat([bytes, this.revocationKeyFingerprint]);
     arr.push(write_sub_packet(sub.revocation_key, bytes));
   }
-  this.notations.forEach(([name, value]) => {
-    bytes = [new Uint8Array([0x80, 0, 0, 0])];
+  this.rawNotations.forEach(([{ name, value, humanReadable }]) => {
+    bytes = [new Uint8Array([humanReadable ? 0x80 : 0, 0, 0, 0])];
     // 2 octets of name length
     bytes.push(util.writeNumber(name.length, 2));
     // 2 octets of value length
     bytes.push(util.writeNumber(value.length, 2));
-    bytes.push(util.str_to_Uint8Array(name + value));
+    bytes.push(util.str_to_Uint8Array(name));
+    bytes.push(value);
     bytes = util.concat(bytes);
     arr.push(write_sub_packet(sub.notation_data, bytes));
   });
@@ -436,29 +438,31 @@ Signature.prototype.read_sub_packet = function (bytes, trusted = true) {
       this.issuerKeyId.read(bytes.subarray(mypos, bytes.length));
       break;
 
-    case 20:
+    case 20: {
       // Notation Data
-      // We don't know how to handle anything but a text flagged data.
-      if (bytes[mypos] === 0x80) {
-        // We extract key/value tuple from the byte stream.
-        mypos += 4;
-        const m = util.readNumber(bytes.subarray(mypos, mypos + 2));
-        mypos += 2;
-        const n = util.readNumber(bytes.subarray(mypos, mypos + 2));
-        mypos += 2;
-
-        const name = util.Uint8Array_to_str(bytes.subarray(mypos, mypos + m));
-        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]);
+      const humanReadable = !!(bytes[mypos] & 0x80);
+
+      // We extract key/value tuple from the byte stream.
+      mypos += 4;
+      const m = util.readNumber(bytes.subarray(mypos, mypos + 2));
+      mypos += 2;
+      const n = util.readNumber(bytes.subarray(mypos, mypos + 2));
+      mypos += 2;
+
+      const name = util.Uint8Array_to_str(bytes.subarray(mypos, mypos + m));
+      const value = bytes.subarray(mypos + m, mypos + m + n);
+
+      this.rawNotations.push({ name, humanReadable, value });
+
+      if (humanReadable) {
+        this.notations[name] = util.Uint8Array_to_str(value);
+      }
+
+      if (critical && (config.known_notations.indexOf(name) === -1)) {
+        throw new Error("Unknown critical notation: " + name);
       }
       break;
+    }
     case 21:
       // Preferred Hash Algorithms
       read_array('preferredHashAlgorithms', bytes.subarray(mypos, bytes.length));

diff --git a/test/general/packet.js b/test/general/packet.js
@@ -819,13 +819,26 @@ V+HOQJQxXJkVRYa3QrFUehiMzTeqqMdgC6ZqJy7+
 
     const key = (await openpgp.key.readArmored(pubkey)).keys[0];
 
-    const notations = key.users[0].selfCertifications[0].notations;
-
-    expect(notations.length).to.equal(2);
-    expect(notations[0][0]).to.equal('[email protected]');
-    expect(notations[0][1]).to.equal('2');
-    expect(notations[1][0]).to.equal('[email protected]');
-    expect(notations[1][1]).to.equal('3');
+    const { notations, rawNotations } = key.users[0].selfCertifications[0];
+
+    // Even though there are two notations with the same keys
+    // the `notations` property reads only the single one:
+    // the last one encountered during parse
+    expect(Object.keys(notations).length).to.equal(1);
+    expect(notations['[email protected]']).to.equal('3');
+
+    // On the other hand `rawNotations` property provides access to all
+    // notations, even non human-readable. The values are not deserialized
+    // and they are byte-arrays.
+    expect(rawNotations.length).to.equal(2);
+
+    expect(rawNotations[0].name).to.equal('[email protected]');
+    expect(rawNotations[0].value).to.deep.equal(Uint8Array.from(['2'.charCodeAt(0)]));
+    expect(rawNotations[0].humanReadable).to.equal(true);
+
+    expect(rawNotations[1].name).to.equal('[email protected]');
+    expect(rawNotations[1].value).to.deep.equal(Uint8Array.from(['3'.charCodeAt(0)]));
+    expect(rawNotations[1].humanReadable).to.equal(true);
   });
 
   it('Writing and encryption of a secret key packet.', function() {

diff --git a/test/general/signature.js b/test/general/signature.js
@@ -831,6 +831,15 @@ vwjE8mqJXetNMfj8r2SCyvkEnlVRYR+/mnge+ib56FdJ8uKtqSxyvgA=
 =fRXs
 -----END PGP MESSAGE-----`;
 
+const signature_with_non_human_readable_notations = `-----BEGIN PGP SIGNATURE-----
+
+wncEARYKAB8FAl2TS9MYFAAAAAAADAADdGVzdEBrZXkuY29tAQIDAAoJEGZ9
+gtV/iL8hrhMBAOQ/UgqRTbx1Z8inGmRdUx1cJU1SR4Pnq/eJNH/CFk5DAP0Q
+hUhMKMuiM3pRwdIyDOItkUWQmjEEw7/XmhgInkXsCw==
+=ZGXr
+-----END PGP SIGNATURE-----
+`;
+
   it('Testing signature checking on CAST5-enciphered message', async function() {
     const { reject_message_hash_algorithms } = openpgp.config;
     Object.assign(openpgp.config, { reject_message_hash_algorithms: new Set([openpgp.enums.hash.md5, openpgp.enums.hash.ripemd]) });
@@ -887,6 +896,21 @@ vwjE8mqJXetNMfj8r2SCyvkEnlVRYR+/mnge+ib56FdJ8uKtqSxyvgA=
     expect(sig.data).to.match(/-----END PGP MESSAGE-----\r\n$/);
   });
 
+  it('Supports non-human-readable notations', async function() {
+    const { packets: [signature] } = await openpgp.message.readArmored(signature_with_non_human_readable_notations);
+    // There are no human-readable notations so `notations` property does not
+    // expose the `[email protected]` notation.
+    expect(Object.keys(signature.notations).length).to.equal(0);
+    expect(signature.notations['[email protected]']).to.equal(undefined);
+
+    // The notation is readable through `rawNotations` property:
+    expect(signature.rawNotations.length).to.equal(1);
+    const notation = signature.rawNotations[0];
+    expect(notation.name).to.equal('[email protected]');
+    expect(notation.value).to.deep.equal(Uint8Array.from([0x01, 0x02, 0x03]));
+    expect(notation.humanReadable).to.equal(false);
+  });
+
   it('Verify V4 signature. Hash: SHA1. PK: RSA. Signature Type: 0x00 (binary document)', async function() {
     const { reject_message_hash_algorithms } = openpgp.config;
     Object.assign(openpgp.config, { reject_message_hash_algorithms: new Set([openpgp.enums.hash.md5, openpgp.enums.hash.ripemd]) });