Rename config.ignoreMdcError, drop config.integrityProtect and allow V4 keys to be AEAD-encrypted (#1261)

* Rename `config.ignoreMdcError` to `config.allowUnauthenticatedMessages`

* Do not support creating sym. enc. messages without integrity protection

* Use `config.aeadProtect` to determine SKESK encryption mode

openpgp.d.ts

@@ -305,10 +305,10 @@ interface Config {
   compression: enums.compression;
   showVersion: boolean;
   showComment: boolean;
-  integrityProtect: boolean;
   deflateLevel: number;
   aeadProtect: boolean;
-  ignoreMdcError: boolean;
+  allowUnauthenticatedMessages: boolean;
+  allowUnauthenticatedStream: boolean;
   checksumRequired: boolean;
   minRsaBits: number;
   passwordCollisionCheck: boolean;

src/config/config.js

@@ -82,19 +82,20 @@ export default {
    * @property {Integer} s2kIterationCountByte
    */
   s2kIterationCountByte: 224,
-  /** Use integrity protection for symmetric encryption
-   * @memberof module:config
-   * @property {Boolean} integrityProtect
-   */
-  integrityProtect: true,
   /**
+   * Allow decryption of messages without integrity protection.
+   * This is an **insecure** setting:
+   *  - message modifications cannot be detected, thus processing the decrypted data is potentially unsafe.
+   *  - it enables downgrade attacks against integrity-protected messages.
    * @memberof module:config
-   * @property {Boolean} ignoreMdcError Fail on decrypt if message is not integrity protected
+   * @property {Boolean} allowUnauthenticatedMessages
    */
-  ignoreMdcError: false,
+  allowUnauthenticatedMessages: false,
   /**
+   * Allow streaming unauthenticated data before its integrity has been checked.
+   * This setting is **insecure** if the partially decrypted message is processed further or displayed to the user.
    * @memberof module:config
-   * @property {Boolean} allowUnauthenticatedStream Stream unauthenticated data before integrity has been checked
+   * @property {Boolean} allowUnauthenticatedStream
    */
   allowUnauthenticatedStream: false,
   /**

src/key/factory.js

@@ -181,16 +181,13 @@ async function wrapKeyObject(secretKeyPacket, secretSubkeyPackets, options, conf
     if (index === 0) {
       signaturePacket.isPrimaryUserID = true;
     }
-    if (config.integrityProtect) {
-      signaturePacket.features = [0];
-      signaturePacket.features[0] |= enums.features.modificationDetection;
-    }
+    // integrity protection always enabled
+    signaturePacket.features = [0];
+    signaturePacket.features[0] |= enums.features.modificationDetection;
     if (config.aeadProtect) {
-      signaturePacket.features || (signaturePacket.features = [0]);
       signaturePacket.features[0] |= enums.features.aead;
     }
     if (config.v5Keys) {
-      signaturePacket.features || (signaturePacket.features = [0]);
       signaturePacket.features[0] |= enums.features.v5Keys;
     }
     if (options.keyExpirationTime > 0) {

src/message.js

@@ -326,10 +326,8 @@ export class Message {
     if (aeadAlgorithm) {
       symEncryptedPacket = new AEADEncryptedDataPacket();
       symEncryptedPacket.aeadAlgorithm = aeadAlgorithm;
-    } else if (config.integrityProtect) {
-      symEncryptedPacket = new SymEncryptedIntegrityProtectedDataPacket();
     } else {
-      symEncryptedPacket = new SymmetricallyEncryptedDataPacket();
+      symEncryptedPacket = new SymEncryptedIntegrityProtectedDataPacket();
     }
     symEncryptedPacket.packets = this.packets;
 

src/packet/secret_key.js

@@ -296,7 +296,7 @@ class SecretKeyPacket extends PublicKeyPacket {
     const blockLen = crypto.cipher[this.symmetric].blockSize;
     this.iv = await crypto.random.getRandomBytes(blockLen);
 
-    if (this.version === 5) {
+    if (config.aeadProtect) {
       this.s2k_usage = 253;
       this.aead = 'eax';
       const mode = crypto[this.aead];

src/packet/symmetrically_encrypted_data.js

@@ -75,8 +75,8 @@ class SymmetricallyEncryptedDataPacket {
    */
   async decrypt(sessionKeyAlgorithm, key, streaming, config = defaultConfig) {
     // If MDC errors are not being ignored, all missing MDC packets in symmetrically encrypted data should throw an error
-    if (!config.ignoreMdcError) {
-      throw new Error('Decryption failed due to missing MDC.');
+    if (!config.allowUnauthenticatedMessages) {
+      throw new Error('Message is not authenticated.');
     }
 
     this.encrypted = await stream.readToEnd(this.encrypted);

test/general/packet.js

@@ -67,9 +67,9 @@ module.exports = () => describe("Packet", function() {
 
   it('Symmetrically encrypted packet without integrity protection - allow decryption', async function() {
     const aeadProtectVal = openpgp.config.aeadProtect;
-    const ignoreMdcErrorVal = openpgp.config.ignoreMdcError;
+    const allowUnauthenticatedMessagesVal = openpgp.config.allowUnauthenticatedMessages;
     openpgp.config.aeadProtect = false;
-    openpgp.config.ignoreMdcError = true;
+    openpgp.config.allowUnauthenticatedMessages = true;
 
     const message = new openpgp.PacketList();
     const testText = input.createSomeMessage();
@@ -94,7 +94,7 @@ module.exports = () => describe("Packet", function() {
       expect(await stringify(msg2[0].packets[0].data)).to.equal(stringify(literal.data));
     } finally {
       openpgp.config.aeadProtect = aeadProtectVal;
-      openpgp.config.ignoreMdcError = ignoreMdcErrorVal;
+      openpgp.config.allowUnauthenticatedMessages = allowUnauthenticatedMessagesVal;
     }
   });
 
@@ -120,7 +120,7 @@ module.exports = () => describe("Packet", function() {
 
       const msg2 = new openpgp.PacketList();
       await msg2.read(message.write(), { SymmetricallyEncryptedDataPacket: openpgp.SymmetricallyEncryptedDataPacket });
-      await expect(msg2[0].decrypt(algo, key, undefined, openpgp.config)).to.eventually.be.rejectedWith('Decryption failed due to missing MDC.');
+      await expect(msg2[0].decrypt(algo, key, undefined, openpgp.config)).to.eventually.be.rejectedWith('Message is not authenticated.');
     } finally {
       openpgp.config.aeadProtect = aeadProtectVal;
     }
@@ -838,12 +838,12 @@ V+HOQJQxXJkVRYa3QrFUehiMzTeqqMdgC6ZqJy7+
     const rsa = openpgp.enums.publicKey.rsaEncryptSign;
     const { privateParams, publicParams } = await crypto.generateParams(rsa, 1024, 65537);
 
-    const secretKeyPacket = new openpgp.SecretKeyPacket(undefined, { ...openpgp.config, v5Keys: true });
+    const secretKeyPacket = new openpgp.SecretKeyPacket();
     secretKeyPacket.privateParams = privateParams;
     secretKeyPacket.publicParams = publicParams;
     secretKeyPacket.algorithm = "rsaSign";
     secretKeyPacket.isEncrypted = false;
-    await secretKeyPacket.encrypt('hello', openpgp.config);
+    await secretKeyPacket.encrypt('hello', { ...openpgp.config, aeadProtect: true });
     expect(secretKeyPacket.s2k_usage).to.equal(253);
 
     const raw = new openpgp.PacketList();
@@ -860,12 +860,12 @@ V+HOQJQxXJkVRYa3QrFUehiMzTeqqMdgC6ZqJy7+
   it('Writing and encryption of a secret key packet (CFB)', async function() {
     const rsa = openpgp.enums.publicKey.rsaEncryptSign;
     const { privateParams, publicParams } = await crypto.generateParams(rsa, 1024, 65537);
-    const secretKeyPacket = new openpgp.SecretKeyPacket(undefined, { ...openpgp.config, v5Keys: false });
+    const secretKeyPacket = new openpgp.SecretKeyPacket();
     secretKeyPacket.privateParams = privateParams;
     secretKeyPacket.publicParams = publicParams;
     secretKeyPacket.algorithm = "rsaSign";
     secretKeyPacket.isEncrypted = false;
-    await secretKeyPacket.encrypt('hello', openpgp.config);
+    await secretKeyPacket.encrypt('hello', { ...openpgp.config, aeadProtect: false });
     expect(secretKeyPacket.s2k_usage).to.equal(254);
 
     const raw = new openpgp.PacketList();