crypto: fix crash in CCM mode without data

Fixes: #38035 PR-URL: #38102 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Apr 8, 2021 · dfe3f95 · dfe3f95
1 parent 8930eba
commit dfe3f95
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 4 deletions.
diff --git a/doc/api/crypto.md b/doc/api/crypto.md
@@ -5218,7 +5218,7 @@ mode must adhere to certain restrictions when using the cipher API:
   `plaintextLength + authTagLength`. Node.js does not include the authentication
   tag, so the ciphertext length is always `plaintextLength`.
   This is not necessary if no AAD is used.
-* As CCM processes the whole message at once, `update()` can only be called
+* As CCM processes the whole message at once, `update()` must be called exactly
   once.
 * Even though calling `update()` is sufficient to encrypt/decrypt the message,
   applications *must* call `final()` to compute or verify the

diff --git a/src/crypto/crypto_cipher.cc b/src/crypto/crypto_cipher.cc
@@ -844,9 +844,9 @@ bool CipherBase::Final(AllocatedBuffer* out) {
         CHECK(mode == EVP_CIPH_GCM_MODE);
         auth_tag_len_ = sizeof(auth_tag_);
       }
-      CHECK_EQ(1, EVP_CIPHER_CTX_ctrl(ctx_.get(), EVP_CTRL_AEAD_GET_TAG,
-                      auth_tag_len_,
-                      reinterpret_cast<unsigned char*>(auth_tag_)));
+      ok = (1 == EVP_CIPHER_CTX_ctrl(ctx_.get(), EVP_CTRL_AEAD_GET_TAG,
+                     auth_tag_len_,
+                     reinterpret_cast<unsigned char*>(auth_tag_)));
     }
   }
 

diff --git a/test/parallel/test-crypto-authenticated.js b/test/parallel/test-crypto-authenticated.js
@@ -70,6 +70,7 @@ const expectedWarnings = common.hasFipsCrypto ?
     ['Use Cipheriv for counter mode of aes-256-ccm'],
     ['Use Cipheriv for counter mode of aes-256-ccm'],
     ['Use Cipheriv for counter mode of aes-256-ccm'],
+    ['Use Cipheriv for counter mode of aes-128-ccm'],
   ];
 
 const expectedDeprecationWarnings = [
@@ -665,3 +666,24 @@ for (const test of TEST_CASES) {
     function H(length) { return '00'.repeat(length); }
   }
 }
+
+{
+  // CCM cipher without data should not crash, see https://github.com/nodejs/node/issues/38035.
+  const algo = 'aes-128-ccm';
+  const key = Buffer.alloc(16);
+  const iv = Buffer.alloc(12);
+  const opts = { authTagLength: 10 };
+
+  for (const cipher of [
+    crypto.createCipher(algo, 'foo', opts),
+    crypto.createCipheriv(algo, key, iv, opts),
+  ]) {
+    assert.throws(() => {
+      cipher.final();
+    }, common.hasOpenSSL3 ? {
+      code: 'ERR_OSSL_TAG_NOT_SET'
+    } : {
+      message: /Unsupported state/
+    });
+  }
+}