Throw intelligible error on GCM authentication failure, fix/refactor test for modification detection on decryption

Also, address race condition in error handling as part of AEAD message decryption,
which would cause non-uniform errors during testing.

src/crypto/mode/gcm.js

@@ -84,8 +84,14 @@ async function GCM(cipher, key) {
           if (webcryptoEmptyMessagesUnsupported && ct.length === tagLength) {
             return AES_GCM.decrypt(ct, key, iv, adata);
           }
-          const pt = await webCrypto.decrypt({ name: ALGO, iv, additionalData: adata, tagLength: tagLength * 8 }, _key, ct);
+          try {
-          return new Uint8Array(pt);
+            const pt = await webCrypto.decrypt({ name: ALGO, iv, additionalData: adata, tagLength: tagLength * 8 }, _key, ct);
+            return new Uint8Array(pt);
+          } catch (e) {
+            if (e.name === 'OperationError') {
+              throw new Error('Authentication tag mismatch');
+            }
+          }
         }
       };
     } catch (err) {

src/packet/sym_encrypted_integrity_protected_data.js

@@ -260,6 +260,7 @@ export async function runAEAD(packet, fn, key, data) {
         if (!chunkIndex || chunk.length) {
           reader.unshift(finalChunk);
           cryptedPromise = modeInstance[fn](chunk, nonce, adataArray);
+          cryptedPromise.catch(() => {});
           queuedBytes += chunk.length - tagLengthIfDecrypting + tagLengthIfEncrypting;
         } else {
           // After the last chunk, we either encrypt a final, empty
@@ -267,6 +268,7 @@ export async function runAEAD(packet, fn, key, data) {
           // validate that final authentication tag.
           adataView.setInt32(5 + chunkIndexSizeIfAEADEP + 4, cryptedBytes); // Should be setInt64(5 + chunkIndexSizeIfAEADEP, ...)
           cryptedPromise = modeInstance[fn](finalChunk, nonce, adataTagArray);
+          cryptedPromise.catch(() => {});
           queuedBytes += tagLengthIfEncrypting;
           done = true;
         }

test/general/openpgp.js

@@ -15,7 +15,6 @@ import { getPreferredCipherSuite } from '../../src/key';
 
 import * as input from './testInputs.js';
 
-const detectNode = () => typeof globalThis.process === 'object' && typeof globalThis.process.versions === 'object';
 const detectBrowser = () => typeof navigator === 'object';
 
 const pub_key = [
@@ -2331,7 +2330,7 @@ XfA3pqV4mTzF
       if: true,
       beforeEach: function() {
         openpgp.config.aeadProtect = true;
-        openpgp.config.preferredAEADAlgorithm = openpgp.enums.aead.experimentalGCM;
+        openpgp.config.preferredAEADAlgorithm = openpgp.enums.aead.gcm;
         openpgp.config.v6Keys = true;
 
         // Monkey-patch SEIPD V2 feature flag
@@ -2346,6 +2345,7 @@ XfA3pqV4mTzF
       beforeEach: function() {
         openpgp.config.aeadProtect = true;
         openpgp.config.aeadChunkSizeByte = 0;
+        openpgp.config.preferredAEADAlgorithm = openpgp.enums.aead.eax;
 
         // Monkey-patch SEIPD V2 feature flag
         publicKey.users[0].selfCertifications[0].features = [9];
@@ -2355,7 +2355,7 @@ XfA3pqV4mTzF
     });
 
     tryTests('OCB mode', tests, {
-      if: !openpgp.config.ci,
+      if: true,
       beforeEach: function() {
         openpgp.config.aeadProtect = true;
         openpgp.config.preferredAEADAlgorithm = openpgp.enums.aead.ocb;
@@ -3086,60 +3086,92 @@ XfA3pqV4mTzF
         });
 
         it('should fail to decrypt modified message', async function() {
-          const allowUnauthenticatedStream = openpgp.config.allowUnauthenticatedStream;
-          const { privateKey: key } = await openpgp.generateKey({ userIDs: [{ email: 'test@email.com' }], format: 'object' });
-          const { aeadAlgo } = await getPreferredCipherSuite([key], undefined, undefined, openpgp.config);
-          expect(!!aeadAlgo).to.equal(openpgp.config.aeadProtect);
-
-          const data = await openpgp.encrypt({ message: await openpgp.createMessage({ binary: new Uint8Array(500) }), encryptionKeys: [key.toPublic()] });
-          const encrypted = data.substr(0, 500) + (data[500] === 'a' ? 'b' : 'a') + data.substr(501);
           await loadStreamsPolyfill();
-          try {
+          // need to generate new key with AEAD support
-            for (const allowStreaming of [true, false]) {
+          const { privateKey } = await openpgp.generateKey({ userIDs: [{ email: 'test@email.com' }], type: 'rsa', format: 'object' });
-              openpgp.config.allowUnauthenticatedStream = allowStreaming;
+          const { aeadAlgo } = await getPreferredCipherSuite([privateKey], undefined, undefined, openpgp.config);
-              for (const [i, encryptedData] of [
+          // sanity check
-                encrypted,
+          expect(aeadAlgo).to.equal(openpgp.config.aeadProtect ? openpgp.config.preferredAEADAlgorithm : undefined);
-                new ReadableStream({
+
-                  start(controller) {
+          const encrypted = await openpgp.encrypt({
-                    controller.enqueue(encrypted);
+            message: await openpgp.createMessage({ binary: new Uint8Array(500) }),
-                    controller.close();
+            encryptionKeys: privateKey
-                  }
+          });
-                }),
+          // corrupt the SEIPD packet
-                new ReadableStream({
+          const encryptedCorrupted = encrypted.substr(0, 1000) + (encrypted[1000] === 'a' ? 'b' : 'a') + encrypted.substr(1001);
-                  start() {
+
-                    this.remaining = encrypted.split('\n');
+          const generateSingleChunkStream = () => (
-                  },
+            new ReadableStream({
-                  async pull(controller) {
+              start(controller) {
-                    if (this.remaining.length) {
+                controller.enqueue(encryptedCorrupted);
-                      await new Promise(res => setTimeout(res));
+                controller.close();
-                      controller.enqueue(this.remaining.shift() + '\n');
-                    } else {
-                      controller.close();
-                    }
-                  }
-                })
-              ].entries()) {
-                let stepReached = 0;
-                try {
-                  const message = await openpgp.readMessage({ armoredMessage: encryptedData });
-                  stepReached = 1;
-                  const { data: decrypted } = await openpgp.decrypt({ message: message, decryptionKeys: [key] });
-                  stepReached = 2;
-                  await stream.readToEnd(decrypted);
-                } catch (e) {
-                  expect(e.message).to.match(/Modification detected|Authentication tag mismatch|Unsupported state or unable to authenticate data/);
-                  expect(stepReached).to.equal(
-                    i === 0 ? 1 :
-                      (openpgp.config.aeadChunkSizeByte === 0 && (i === 2 || detectNode() || util.getHardwareConcurrency() < 8)) || (!openpgp.config.aeadProtect && openpgp.config.allowUnauthenticatedStream) ? 2 :
-                        1
-                  );
-                  continue;
-                }
-                throw new Error(`Expected "Modification detected" error in subtest ${i}`);
               }
+            })
+          );
+          const generateMultiChunkStream = () => (
+            new ReadableStream({
+              start() {
+                this.remaining = encryptedCorrupted.split('\n');
+              },
+              async pull(controller) {
+                if (this.remaining.length) {
+                  // sleep to slow down enqeueing
+                  await new Promise(resolve => { setTimeout(resolve); });
+                  controller.enqueue(this.remaining.shift() + '\n');
+                } else {
+                  controller.close();
+                }
+              }
+            })
+          );
+
+          if (openpgp.config.aeadProtect) {
+            const expectedError = /Authentication tag mismatch|Unsupported state or unable to authenticate data/;
+            // AEAD fails either on AEAD chunk decryption or when reading the decrypted stream:
+            // if the corruption is in the first AEAD chunk, then `openpgp.decrypt` will throw
+            // when reading the decrypted stream to parse the packet list.
+            await Promise.all([
+              testStreamingDecryption(encryptedCorrupted, true, expectedError, true),
+              testStreamingDecryption(encryptedCorrupted, false, expectedError, true),
+              // `config.allowUnauthenticatedStream` does not apply to AEAD
+              testStreamingDecryption(generateSingleChunkStream(), true, expectedError, openpgp.config.aeadChunkSizeByte > 0),
+              testStreamingDecryption(generateSingleChunkStream(), false, expectedError, openpgp.config.aeadChunkSizeByte > 0),
+              // Increasing number of streaming chunks should not affect the result
+              testStreamingDecryption(generateMultiChunkStream(), true, expectedError, openpgp.config.aeadChunkSizeByte > 0),
+              testStreamingDecryption(generateMultiChunkStream(), false, expectedError, openpgp.config.aeadChunkSizeByte > 0)
+            ]);
+          } else {
+            const expectedError = /Modification detected/;
+            await Promise.all([
+              testStreamingDecryption(encryptedCorrupted, true, expectedError, true),
+              testStreamingDecryption(encryptedCorrupted, false, expectedError, true),
+              testStreamingDecryption(generateSingleChunkStream(), true, expectedError, false),
+              testStreamingDecryption(generateSingleChunkStream(), false, expectedError, true),
+              // Increasing number of streaming chunks should not affect the result
+              testStreamingDecryption(generateMultiChunkStream(), true, expectedError, false),
+              testStreamingDecryption(generateMultiChunkStream(), false, expectedError, true)
+            ]);
+          }
+
+          async function testStreamingDecryption(encryptedDataOrStream, allowUnauthenticatedStream, expectedErrorMessage, expectedFailureOnDecrypt = null) {
+            // parsing the message won't fail since armor checksum is ignored
+            const message = await openpgp.readMessage({ armoredMessage: encryptedDataOrStream });
+            let didFailOnDecrypt = true;
+
+            try {
+              const { data: decrypted } = await openpgp.decrypt({
+                message,
+                decryptionKeys: [privateKey],
+                config: { allowUnauthenticatedStream }
+              });
+              didFailOnDecrypt = false;
+              await stream.readToEnd(decrypted);
+              // expected to have thrown
+              throw new Error(`Expected decryption to fail with error ${expectedErrorMessage}`);
+            } catch (e) {
+              expect(e.message).to.match(expectedErrorMessage);
+              expect(didFailOnDecrypt).to.equal(expectedFailureOnDecrypt);
             }
-          } finally {
-            openpgp.config.allowUnauthenticatedStream = allowUnauthenticatedStream;
           }
         });