From dfe3f952a3ecbb9c046497e7e0b73e6104082072 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Mon, 5 Apr 2021 23:37:47 +0200 Subject: [PATCH] crypto: fix crash in CCM mode without data Fixes: https://github.com/nodejs/node/issues/38035 PR-URL: https://github.com/nodejs/node/pull/38102 Reviewed-By: Colin Ihrig Reviewed-By: Anna Henningsen Reviewed-By: James M Snell --- doc/api/crypto.md | 2 +- src/crypto/crypto_cipher.cc | 6 +++--- test/parallel/test-crypto-authenticated.js | 22 ++++++++++++++++++++++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/doc/api/crypto.md b/doc/api/crypto.md index cd9bc251481ecd..5fff5d7c0683f0 100644 --- 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 index c9c3563ef917ec..b15795b691ea0b 100644 --- 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(auth_tag_))); + ok = (1 == EVP_CIPHER_CTX_ctrl(ctx_.get(), EVP_CTRL_AEAD_GET_TAG, + auth_tag_len_, + reinterpret_cast(auth_tag_))); } } diff --git a/test/parallel/test-crypto-authenticated.js b/test/parallel/test-crypto-authenticated.js index d6b09b037739c8..21c5af6cfe3e5e 100644 --- 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/ + }); + } +}