From 6e2a787ff85f25a6a78d63b0372681a6789b2461 Mon Sep 17 00:00:00 2001 From: larabr <7375870+larabr@users.noreply.github.com> Date: Wed, 3 Mar 2021 18:05:40 +0100 Subject: [PATCH] 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 | 4 ++-- src/config/config.js | 17 +++++++++-------- src/key/factory.js | 9 +++------ src/message.js | 4 +--- src/packet/secret_key.js | 2 +- src/packet/symmetrically_encrypted_data.js | 4 ++-- test/general/packet.js | 16 ++++++++-------- 7 files changed, 26 insertions(+), 30 deletions(-) diff --git a/openpgp.d.ts b/openpgp.d.ts index fc0c6191..139d25e0 100644 --- a/openpgp.d.ts +++ b/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; diff --git a/src/config/config.js b/src/config/config.js index 6082809f..27e83463 100644 --- a/src/config/config.js +++ b/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, /** diff --git a/src/key/factory.js b/src/key/factory.js index 92e53f2a..af3389a5 100644 --- a/src/key/factory.js +++ b/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) { diff --git a/src/message.js b/src/message.js index e4b0c4f2..314eb517 100644 --- a/src/message.js +++ b/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; diff --git a/src/packet/secret_key.js b/src/packet/secret_key.js index 4d0fa98c..bef72d17 100644 --- a/src/packet/secret_key.js +++ b/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]; diff --git a/src/packet/symmetrically_encrypted_data.js b/src/packet/symmetrically_encrypted_data.js index 3cf2b9e6..76c9bc10 100644 --- a/src/packet/symmetrically_encrypted_data.js +++ b/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); diff --git a/test/general/packet.js b/test/general/packet.js index f153000f..167ce769 100644 --- a/test/general/packet.js +++ b/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();