From 16b12d7f55dcc900d8c235ab561e1039788e0d59 Mon Sep 17 00:00:00 2001 From: Wiktor Kwapisiewicz Date: Thu, 9 May 2019 12:12:22 +0200 Subject: [PATCH 1/2] Expose all signature notations Previous implementation used an object to hold notations so if multiple notations had the same key name only the last one was visible. After this change notations are exposed as an array of key-value pairs that can be converted to a map through `new Map(notations)`. See #897. --- src/packet/signature.js | 27 +++++++++++-------------- test/general/packet.js | 45 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 15 deletions(-) diff --git a/src/packet/signature.js b/src/packet/signature.js index 249e3a6e..2dc13719 100644 --- a/src/packet/signature.js +++ b/src/packet/signature.js @@ -70,7 +70,7 @@ function Signature(date=new Date()) { this.revocationKeyAlgorithm = null; this.revocationKeyFingerprint = null; this.issuerKeyId = new type_keyid(); - this.notation = null; + this.notations = []; this.preferredHashAlgorithms = null; this.preferredCompressionAlgorithms = null; this.keyServerPreferences = null; @@ -228,18 +228,16 @@ Signature.prototype.write_hashed_sub_packets = function () { bytes = util.concat([bytes, this.revocationKeyFingerprint]); arr.push(write_sub_packet(sub.revocation_key, bytes)); } - if (this.notation !== null) { - Object.entries(this.notation).forEach(([name, value]) => { - bytes = [new Uint8Array([0x80, 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 = util.concat(bytes); - arr.push(write_sub_packet(sub.notation_data, bytes)); - }); - } + this.notations.forEach(([name, value]) => { + bytes = [new Uint8Array([0x80, 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 = util.concat(bytes); + arr.push(write_sub_packet(sub.notation_data, bytes)); + }); if (this.preferredHashAlgorithms !== null) { bytes = util.str_to_Uint8Array(util.Uint8Array_to_str(this.preferredHashAlgorithms)); arr.push(write_sub_packet(sub.preferred_hash_algorithms, bytes)); @@ -447,8 +445,7 @@ Signature.prototype.read_sub_packet = function (bytes, trusted=true) { 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.notation = this.notation || {}; - this.notation[name] = value; + this.notations.push([name, value]); } else { util.print_debug("Unsupported notation flag "+bytes[mypos]); } diff --git a/test/general/packet.js b/test/general/packet.js index a7b23554..53a40fb8 100644 --- a/test/general/packet.js +++ b/test/general/packet.js @@ -801,6 +801,51 @@ kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu expect(signature.packets[0].signersUserId).to.equal('test-wkd@metacode.biz'); }); + it('Reading notations from armored key', async function() { + const pubkey = +`-----BEGIN PGP PUBLIC KEY BLOCK----- + +mQENBFzQOToBCADd0Pwh8edZ6gR3x49L1PaBPtiAQUr1QDUDWeNes8co5MTFl5hG +lHzptt+VD0JGucuIkvi34f5z2ZbInAV/xYDX3kSYefy6LB8XJD527I/o9bqY1P7T +PjtTZ4emcqNGkGhV2hNGV+hFcTevUS9Ty4vGg6P7X6RjfjeTrClHelJT8+9IiH+4 +0h4X/Y1hwoijRWanYnZjuAUIrOXnG76iknXQRGc8th8iI0oIZfKQomfF0K5lXFhH +SU8Yvmik3vCTLHC6Ce0GVRCTIcU0/Xi2MK/Yrg9bGzSblHxomLU0NT6pee+2UjqR +BZXOAPLY66Lsh1oqxQ6ihVnOmbraU9glAGm1ABEBAAG0EFRlc3R0IDx0ZXN0QGV4 +YT6JAYoEEwEIAHQCGwMFCQPCZwAFCwkIBwIGFQoJCAsCBBYCAwECHgECF4AWIQQZ +jHIqS6wzbp2qrkRXnQGzq+FUDgUCXNA5VBoUgAAAAAAQAAF0ZXN0QGV4YW1wbGUu +Y29tMhoUgAAAAAAQAAF0ZXN0QGV4YW1wbGUuY29tMwAKCRBXnQGzq+FUDoLiCACn +ls1iy0hT59Xt3o3tmmxe1jLzkbQEprR6MMfZamtex5/BHViu2HPAu5i13mXyBRnJ +4Zvd/HUxJukP3tdQyJIlZFe8XwloMoRAA37KOZ5QGyKH8Jxq3LcAcQOOkFtWgr+Z +JbjUKF1IuqCsK6SYB8f7SVKgpZk/kqG3HE3gk72ONnqdvwOa9cIhAuZScdgZ+PLC +6W/0+IrnQIasvKeEWeK4u6/NYT35HUsUE/9Z6WKF+qxJnp5Pi2Q5cio6bFlGDNQb ++MiuiEb3Mzb3ev2PVg7WELBRXOg8QlCxrghqfi1SH791mmiyGK+GIQgnjRwMejTh +dNsnHYag/KAewag55AQvuQENBFzQOToBCADJD+auK+Opo1q3ZLxODMyw5//XHJH4 +0vQPNawyBiOdBuneWHF3jfDwGa+lOftUx1abSwsq+Qs955THgLVSiJvivHWVy8pN +tPv0XLa9rMj2wh/OmckbcgzSMeJJIz09bTj095ONPGYW2D4AcpkOc+b5bkqV6r+N +yk9nopPJNCNqYYJtecTClDT5haRKBP5XjXRVsIXva/nHZGXKQLX8iWG2D5DOJNDP +ZkAEoIPg+7J85Q3u2iSFPnLPzKHlMAoQW8d9RAEYyJ6WqiILUIDShhvXg+RIkzri +wY/WkvhB/Kpj0r1SRbNhWRpmOWCR+0a2uHaLz9X0KTP7WMqQbmIdpRgZABEBAAGJ +ATwEGAEIACYWIQQZjHIqS6wzbp2qrkRXnQGzq+FUDgUCXNA5OgIbDAUJA8JnAAAK +CRBXnQGzq+FUDgI6B/9Far0CUR6rWvUiviBY4P5oe44I9P9P7ilWmum1cIQWxMyF +0sc5tRcVLpMomURlrDz0TR5GNs+nuGAHTRBfN7VO0Y+R/LyEd1Rf80ONObXOqzMp +vF9CdW3a7W4WicZwnGgUOImTICazR2VmR+RREdZshqrOCaOnuKmN3QwGH1zzFwJA +sTbLoNMdBv8SEARaRVOWPM1HwJ701mMYF48FqhHd5uinH/ZCeBhqrBfhmXa68FWx +xuyJz6ttl5Fp4nsB3waQdgPGZJ9NUrGfopLUZ44xDuJjBONd7rbYOh71TWbHd8wG +V+HOQJQxXJkVRYa3QrFUehiMzTeqqMdgC6ZqJy7+ +=et/d +-----END PGP PUBLIC KEY BLOCK-----`; + + 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('test@example.com'); + expect(notations[0][1]).to.equal('2'); + expect(notations[1][0]).to.equal('test@example.com'); + expect(notations[1][1]).to.equal('3'); + }); + it('Writing and encryption of a secret key packet.', function() { const key = new openpgp.packet.List(); key.push(new openpgp.packet.SecretKey()); From 82799390ded7bd023555f481fc6992a560e7924b Mon Sep 17 00:00:00 2001 From: Wiktor Kwapisiewicz Date: Mon, 13 May 2019 20:56:23 +0200 Subject: [PATCH 2/2] 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-----',