Add support for creating critical signature subpackets

Assign most signature subpacket types a criticality based on whether
failing to interpret their meaning would negatively impact security.

For Notation Data subpackets, let the user indicate their criticality
using the `signatureNotations[*].critical` property.

openpgp.d.ts

@@ -532,6 +532,7 @@ export interface RawNotation {
   name: string;
   value: Uint8Array;
   humanReadable: boolean;
+  critical: boolean;
 }
 
 export class TrustPacket extends BasePacket {
@@ -604,7 +605,7 @@ interface EncryptOptions {
   signingUserIDs?: MaybeArray<UserID>;
   /** (optional) array of user IDs to encrypt for, e.g. { name:'Robert Receiver', email:'robert@openpgp.org' } */
   encryptionUserIDs?: MaybeArray<UserID>;
-  /** (optional) array of notations to add to the signatures, e.g. { name: 'test@example.org', value: new TextEncoder().encode('test'), humanReadable: true } */
+  /** (optional) array of notations to add to the signatures, e.g. { name: 'test@example.org', value: new TextEncoder().encode('test'), humanReadable: true, critical: false } */
   signatureNotations?: MaybeArray<RawNotation>;
   config?: PartialConfig;
 }

src/cleartext.js

@@ -64,7 +64,7 @@ export class CleartextMessage {
    * @param {Array<module:type/keyid~KeyID>} [signingKeyIDs] - Array of key IDs to use for signing. Each signingKeyIDs[i] corresponds to privateKeys[i]
    * @param {Date} [date] - The creation time of the signature that should be created
    * @param {Array} [userIDs] - User IDs to sign with, e.g. [{ name:'Steve Sender', email:'steve@openpgp.org' }]
-   * @param {Array} [notations] - Notation Data to add to the signatures, e.g. [{ name: 'test@example.org', value: new TextEncoder().encode('test'), humanReadable: true }]
+   * @param {Array} [notations] - Notation Data to add to the signatures, e.g. [{ name: 'test@example.org', value: new TextEncoder().encode('test'), humanReadable: true, critical: false }]
    * @param {Object} [config] - Full configuration, defaults to openpgp.config
    * @returns {Promise<CleartextMessage>} New cleartext message with signed content.
    * @async

src/key/helper.js

@@ -192,7 +192,7 @@ export async function getPreferredAlgo(type, keys = [], date = new Date(), userI
  * @param {Object} [signatureProperties] - Properties to write on the signature packet before signing
  * @param {Date} [date] - Override the creationtime of the signature
  * @param {Object} [userID] - User ID
- * @param {Array} [notations] - Notation Data to add to the signature, e.g. [{ name: 'test@example.org', value: new TextEncoder().encode('test'), humanReadable: true }]
+ * @param {Array} [notations] - Notation Data to add to the signature, e.g. [{ name: 'test@example.org', value: new TextEncoder().encode('test'), humanReadable: true, critical: false }]
  * @param {Object} [detached] - Whether to create a detached signature packet
  * @param {Object} config - full configuration
  * @returns {Promise<SignaturePacket>} Signature packet.

src/message.js

@@ -476,7 +476,7 @@ export class Message {
    * @param {Array<module:type/keyid~KeyID>} [signingKeyIDs] - Array of key IDs to use for signing. Each signingKeyIDs[i] corresponds to signingKeys[i]
    * @param {Date} [date] - Override the creation time of the signature
    * @param {Array} [userIDs] - User IDs to sign with, e.g. [{ name:'Steve Sender', email:'steve@openpgp.org' }]
-   * @param {Array} [notations] - Notation Data to add to the signatures, e.g. [{ name: 'test@example.org', value: new TextEncoder().encode('test'), humanReadable: true }]
+   * @param {Array} [notations] - Notation Data to add to the signatures, e.g. [{ name: 'test@example.org', value: new TextEncoder().encode('test'), humanReadable: true, critical: false }]
    * @param {Object} [config] - Full configuration, defaults to openpgp.config
    * @returns {Promise<Message>} New message with signed content.
    * @async
@@ -564,7 +564,7 @@ export class Message {
    * @param {Array<module:type/keyid~KeyID>} [signingKeyIDs] - Array of key IDs to use for signing. Each signingKeyIDs[i] corresponds to signingKeys[i]
    * @param {Date} [date] - Override the creation time of the signature
    * @param {Array} [userIDs] - User IDs to sign with, e.g. [{ name:'Steve Sender', email:'steve@openpgp.org' }]
-   * @param {Array} [notations] - Notation Data to add to the signatures, e.g. [{ name: 'test@example.org', value: new TextEncoder().encode('test'), humanReadable: true }]
+   * @param {Array} [notations] - Notation Data to add to the signatures, e.g. [{ name: 'test@example.org', value: new TextEncoder().encode('test'), humanReadable: true, critical: false }]
    * @param {Object} [config] - Full configuration, defaults to openpgp.config
    * @returns {Promise<Signature>} New detached signature of message content.
    * @async
@@ -707,7 +707,7 @@ export class Message {
  * @param {Array<module:type/keyid~KeyID>} [signingKeyIDs] - Array of key IDs to use for signing. Each signingKeyIDs[i] corresponds to signingKeys[i]
  * @param {Date} [date] - Override the creationtime of the signature
  * @param {Array} [userIDs] - User IDs to sign with, e.g. [{ name:'Steve Sender', email:'steve@openpgp.org' }]
- * @param {Array} [notations] - Notation Data to add to the signatures, e.g. [{ name: 'test@example.org', value: new TextEncoder().encode('test'), humanReadable: true }]
+ * @param {Array} [notations] - Notation Data to add to the signatures, e.g. [{ name: 'test@example.org', value: new TextEncoder().encode('test'), humanReadable: true, critical: false }]
  * @param {Boolean} [detached] - Whether to create detached signature packets
  * @param {Object} [config] - Full configuration, defaults to openpgp.config
  * @returns {Promise<PacketList>} List of signature packets.

src/openpgp.js

@@ -256,7 +256,7 @@ export async function encryptKey({ privateKey, passphrase, config, ...rest }) {
  * @param {Date} [options.date=current date] - Override the creation date of the message signature
  * @param {Object|Object[]} [options.signingUserIDs=primary user IDs] - Array of user IDs to sign with, one per key in `signingKeys`, e.g. `[{ name: 'Steve Sender', email: 'steve@openpgp.org' }]`
  * @param {Object|Object[]} [options.encryptionUserIDs=primary user IDs] - Array of user IDs to encrypt for, one per key in `encryptionKeys`, e.g. `[{ name: 'Robert Receiver', email: 'robert@openpgp.org' }]`
- * @param {Object|Object[]} [options.signatureNotations=[]] - Array of notations to add to the signatures, e.g. `[{ name: 'test@example.org', value: new TextEncoder().encode('test'), humanReadable: true }]`
+ * @param {Object|Object[]} [options.signatureNotations=[]] - Array of notations to add to the signatures, e.g. `[{ name: 'test@example.org', value: new TextEncoder().encode('test'), humanReadable: true, critical: false }]`
  * @param {Object} [options.config] - Custom configuration settings to overwrite those in [config]{@link module:config}
  * @returns {Promise<MaybeStream<String>|MaybeStream<Uint8Array>>} Encrypted message (string if `armor` was true, the default; Uint8Array if `armor` was false).
  * @async
@@ -388,7 +388,7 @@ export async function decrypt({ message, decryptionKeys, passwords, sessionKeys,
  * @param {KeyID|KeyID[]} [options.signingKeyIDs=latest-created valid signing (sub)keys] - Array of key IDs to use for signing. Each signingKeyIDs[i] corresponds to signingKeys[i]
  * @param {Date} [options.date=current date] - Override the creation date of the signature
  * @param {Object|Object[]} [options.signingUserIDs=primary user IDs] - Array of user IDs to sign with, one per key in `signingKeys`, e.g. `[{ name: 'Steve Sender', email: 'steve@openpgp.org' }]`
- * @param {Object|Object[]} [options.signatureNotations=[]] - Array of notations to add to the signatures, e.g. `[{ name: 'test@example.org', value: new TextEncoder().encode('test'), humanReadable: true }]`
+ * @param {Object|Object[]} [options.signatureNotations=[]] - Array of notations to add to the signatures, e.g. `[{ name: 'test@example.org', value: new TextEncoder().encode('test'), humanReadable: true, critical: false }]`
  * @param {Object} [options.config] - Custom configuration settings to overwrite those in [config]{@link module:config}
  * @returns {Promise<MaybeStream<String|Uint8Array>>} Signed message (string if `armor` was true, the default; Uint8Array if `armor` was false).
  * @async

src/packet/signature.js

@@ -226,41 +226,41 @@ class SignaturePacket {
     if (this.created === null) {
       throw new Error('Missing signature creation time');
     }
-    arr.push(writeSubPacket(sub.signatureCreationTime, util.writeDate(this.created)));
+    arr.push(writeSubPacket(true, sub.signatureCreationTime, util.writeDate(this.created)));
     if (this.signatureExpirationTime !== null) {
-      arr.push(writeSubPacket(sub.signatureExpirationTime, util.writeNumber(this.signatureExpirationTime, 4)));
+      arr.push(writeSubPacket(true, sub.signatureExpirationTime, util.writeNumber(this.signatureExpirationTime, 4)));
     }
     if (this.exportable !== null) {
-      arr.push(writeSubPacket(sub.exportableCertification, new Uint8Array([this.exportable ? 1 : 0])));
+      arr.push(writeSubPacket(true, sub.exportableCertification, new Uint8Array([this.exportable ? 1 : 0])));
     }
     if (this.trustLevel !== null) {
       bytes = new Uint8Array([this.trustLevel, this.trustAmount]);
-      arr.push(writeSubPacket(sub.trustSignature, bytes));
+      arr.push(writeSubPacket(true, sub.trustSignature, bytes));
     }
     if (this.regularExpression !== null) {
-      arr.push(writeSubPacket(sub.regularExpression, this.regularExpression));
+      arr.push(writeSubPacket(true, sub.regularExpression, this.regularExpression));
     }
     if (this.revocable !== null) {
-      arr.push(writeSubPacket(sub.revocable, new Uint8Array([this.revocable ? 1 : 0])));
+      arr.push(writeSubPacket(true, sub.revocable, new Uint8Array([this.revocable ? 1 : 0])));
     }
     if (this.keyExpirationTime !== null) {
-      arr.push(writeSubPacket(sub.keyExpirationTime, util.writeNumber(this.keyExpirationTime, 4)));
+      arr.push(writeSubPacket(true, sub.keyExpirationTime, util.writeNumber(this.keyExpirationTime, 4)));
     }
     if (this.preferredSymmetricAlgorithms !== null) {
       bytes = util.stringToUint8Array(util.uint8ArrayToString(this.preferredSymmetricAlgorithms));
-      arr.push(writeSubPacket(sub.preferredSymmetricAlgorithms, bytes));
+      arr.push(writeSubPacket(false, sub.preferredSymmetricAlgorithms, bytes));
     }
     if (this.revocationKeyClass !== null) {
       bytes = new Uint8Array([this.revocationKeyClass, this.revocationKeyAlgorithm]);
       bytes = util.concat([bytes, this.revocationKeyFingerprint]);
-      arr.push(writeSubPacket(sub.revocationKey, bytes));
+      arr.push(writeSubPacket(false, sub.revocationKey, bytes));
     }
     if (!this.issuerKeyID.isNull() && this.issuerKeyVersion !== 5) {
       // If the version of [the] key is greater than 4, this subpacket
       // MUST NOT be included in the signature.
-      arr.push(writeSubPacket(sub.issuer, this.issuerKeyID.write()));
+      arr.push(writeSubPacket(true, sub.issuer, this.issuerKeyID.write()));
     }
-    this.rawNotations.forEach(({ name, value, humanReadable }) => {
+    this.rawNotations.forEach(({ name, value, humanReadable, critical }) => {
       bytes = [new Uint8Array([humanReadable ? 0x80 : 0, 0, 0, 0])];
       const encodedName = util.encodeUTF8(name);
       // 2 octets of name length
@@ -270,61 +270,61 @@ class SignaturePacket {
       bytes.push(encodedName);
       bytes.push(value);
       bytes = util.concat(bytes);
-      arr.push(writeSubPacket(sub.notationData, bytes));
+      arr.push(writeSubPacket(critical, sub.notationData, bytes));
     });
     if (this.preferredHashAlgorithms !== null) {
       bytes = util.stringToUint8Array(util.uint8ArrayToString(this.preferredHashAlgorithms));
-      arr.push(writeSubPacket(sub.preferredHashAlgorithms, bytes));
+      arr.push(writeSubPacket(false, sub.preferredHashAlgorithms, bytes));
     }
     if (this.preferredCompressionAlgorithms !== null) {
       bytes = util.stringToUint8Array(util.uint8ArrayToString(this.preferredCompressionAlgorithms));
-      arr.push(writeSubPacket(sub.preferredCompressionAlgorithms, bytes));
+      arr.push(writeSubPacket(false, sub.preferredCompressionAlgorithms, bytes));
     }
     if (this.keyServerPreferences !== null) {
       bytes = util.stringToUint8Array(util.uint8ArrayToString(this.keyServerPreferences));
-      arr.push(writeSubPacket(sub.keyServerPreferences, bytes));
+      arr.push(writeSubPacket(false, sub.keyServerPreferences, bytes));
     }
     if (this.preferredKeyServer !== null) {
-      arr.push(writeSubPacket(sub.preferredKeyServer, util.encodeUTF8(this.preferredKeyServer)));
+      arr.push(writeSubPacket(false, sub.preferredKeyServer, util.encodeUTF8(this.preferredKeyServer)));
     }
     if (this.isPrimaryUserID !== null) {
-      arr.push(writeSubPacket(sub.primaryUserID, new Uint8Array([this.isPrimaryUserID ? 1 : 0])));
+      arr.push(writeSubPacket(false, sub.primaryUserID, new Uint8Array([this.isPrimaryUserID ? 1 : 0])));
     }
     if (this.policyURI !== null) {
-      arr.push(writeSubPacket(sub.policyURI, util.encodeUTF8(this.policyURI)));
+      arr.push(writeSubPacket(false, sub.policyURI, util.encodeUTF8(this.policyURI)));
     }
     if (this.keyFlags !== null) {
       bytes = util.stringToUint8Array(util.uint8ArrayToString(this.keyFlags));
-      arr.push(writeSubPacket(sub.keyFlags, bytes));
+      arr.push(writeSubPacket(true, sub.keyFlags, bytes));
     }
     if (this.signersUserID !== null) {
-      arr.push(writeSubPacket(sub.signersUserID, util.encodeUTF8(this.signersUserID)));
+      arr.push(writeSubPacket(false, sub.signersUserID, util.encodeUTF8(this.signersUserID)));
     }
     if (this.reasonForRevocationFlag !== null) {
       bytes = util.stringToUint8Array(String.fromCharCode(this.reasonForRevocationFlag) + this.reasonForRevocationString);
-      arr.push(writeSubPacket(sub.reasonForRevocation, bytes));
+      arr.push(writeSubPacket(true, sub.reasonForRevocation, bytes));
     }
     if (this.features !== null) {
       bytes = util.stringToUint8Array(util.uint8ArrayToString(this.features));
-      arr.push(writeSubPacket(sub.features, bytes));
+      arr.push(writeSubPacket(false, sub.features, bytes));
     }
     if (this.signatureTargetPublicKeyAlgorithm !== null) {
       bytes = [new Uint8Array([this.signatureTargetPublicKeyAlgorithm, this.signatureTargetHashAlgorithm])];
       bytes.push(util.stringToUint8Array(this.signatureTargetHash));
       bytes = util.concat(bytes);
-      arr.push(writeSubPacket(sub.signatureTarget, bytes));
+      arr.push(writeSubPacket(true, sub.signatureTarget, bytes));
     }
     if (this.embeddedSignature !== null) {
-      arr.push(writeSubPacket(sub.embeddedSignature, this.embeddedSignature.write()));
+      arr.push(writeSubPacket(true, sub.embeddedSignature, this.embeddedSignature.write()));
     }
     if (this.issuerFingerprint !== null) {
       bytes = [new Uint8Array([this.issuerKeyVersion]), this.issuerFingerprint];
       bytes = util.concat(bytes);
-      arr.push(writeSubPacket(sub.issuerFingerprint, bytes));
+      arr.push(writeSubPacket(this.version === 5, sub.issuerFingerprint, bytes));
     }
     if (this.preferredAEADAlgorithms !== null) {
       bytes = util.stringToUint8Array(util.uint8ArrayToString(this.preferredAEADAlgorithms));
-      arr.push(writeSubPacket(sub.preferredAEADAlgorithms, bytes));
+      arr.push(writeSubPacket(false, sub.preferredAEADAlgorithms, bytes));
     }
 
     const result = util.concat(arr);
@@ -753,18 +753,19 @@ class SignaturePacket {
 export default SignaturePacket;
 
 /**
- * Creates a string representation of a sub signature packet
+ * Creates a Uint8Array representation of a sub signature packet
  * @see {@link https://tools.ietf.org/html/rfc4880#section-5.2.3.1|RFC4880 5.2.3.1}
  * @see {@link https://tools.ietf.org/html/rfc4880#section-5.2.3.2|RFC4880 5.2.3.2}
+ * @param {Boolean} critical - Whether the subpacket should be critical.
  * @param {Integer} type - Subpacket signature type.
  * @param {String} data - Data to be included
- * @returns {String} A string-representation of a sub signature packet.
+ * @returns {Uint8Array} The signature subpacket.
  * @private
  */
-function writeSubPacket(type, data) {
+function writeSubPacket(critical, type, data) {
   const arr = [];
   arr.push(writeSimpleLength(data.length + 1));
-  arr.push(new Uint8Array([type]));
+  arr.push(new Uint8Array([(critical ? 0x80 : 0) | type]));
   arr.push(data);
   return util.concat(arr);
 }

test/general/signature.js

@@ -1153,27 +1153,41 @@ eSvSZutLuKKbidSYMLhWROPlwKc2GU2ws6PrLZAyCAel/lU=
         {
           name: 'test@example.com',
           value: new TextEncoder().encode('test'),
-          humanReadable: true
+          humanReadable: true,
+          critical: true
         },
         {
           name: 'séparation-de-domaine@proton.ch',
           value: new Uint8Array([0, 1, 2, 3]),
-          humanReadable: false
+          humanReadable: false,
+          critical: false
         }
       ],
       config
     });
+    expect(openpgp.decrypt({
+      message: await openpgp.readMessage({ armoredMessage: message_with_notation }),
+      decryptionKeys: privKey,
+      verificationKeys: privKey,
+      expectSigned: true,
+      config
+    })).to.be.rejectedWith('Unknown critical notation: test@example.com');
     const { signatures: [sig] } = await openpgp.decrypt({
       message: await openpgp.readMessage({ armoredMessage: message_with_notation }),
       decryptionKeys: privKey,
-      verificationKeys: privKey
+      verificationKeys: privKey,
+      config: {
+        knownNotations: ['test@example.com'],
+        ...config
+      }
     });
+    expect(await sig.verified).to.be.true;
     const { packets: [{ rawNotations: notations }] } = await sig.signature;
     expect(notations).to.have.length(2);
     expect(notations[0].name).to.equal('test@example.com');
     expect(notations[0].value).to.deep.equal(new Uint8Array([116, 101, 115, 116]));
     expect(notations[0].humanReadable).to.be.true;
-    expect(notations[0].critical).to.be.false;
+    expect(notations[0].critical).to.be.true;
     expect(notations[1].name).to.equal('séparation-de-domaine@proton.ch');
     expect(notations[1].value).to.deep.equal(new Uint8Array([0, 1, 2, 3]));
     expect(notations[1].humanReadable).to.be.false;

test/typescript/definitions.ts

@@ -138,7 +138,8 @@ import {
   const textSignedWithNotations: string = await sign({ signingKeys: privateKeys, message: textMessage, signatureNotations: [{
     name: 'test@example.org',
     value: new TextEncoder().encode('test'),
-    humanReadable: true
+    humanReadable: true,
+    critical: false
   }] });
   expect(textSignedWithNotations).to.include('-----BEGIN PGP MESSAGE-----');