crypto: fix crash in CCM mode without data

tniessen · tniessen · commit dfe3f952a3ec · 2021-04-08T22:18:52.000+02:00
Fixes: #38035 PR-URL: #38102 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
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/
+    });
+  }
+}

Original file line number	Diff line number	Diff line change
`@@ -844,9 +844,9 @@ bool CipherBase::Final(AllocatedBuffer* out) {`
`844`	`844`	`CHECK(mode == EVP_CIPH_GCM_MODE);`
`845`	`845`	`auth_tag_len_ = sizeof(auth_tag_);`
`846`	`846`	`}`
`847`		`- CHECK_EQ(1, EVP_CIPHER_CTX_ctrl(ctx_.get(), EVP_CTRL_AEAD_GET_TAG,`
`848`		`- auth_tag_len_,`
`849`		`- reinterpret_cast<unsigned char*>(auth_tag_)));`
	`847`	`+ ok = (1 == EVP_CIPHER_CTX_ctrl(ctx_.get(), EVP_CTRL_AEAD_GET_TAG,`
	`848`	`+ auth_tag_len_,`
	`849`	`+ reinterpret_cast<unsigned char*>(auth_tag_)));`
`850`	`850`	`}`
`851`	`851`	`}`
`852`	`852`