Skip to content

Commit

Permalink
crypto: fix crash in CCM mode without data
Browse files Browse the repository at this point in the history
  • Loading branch information
tniessen committed Apr 5, 2021
1 parent 4f387c2 commit 9c17def
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 4 deletions.
2 changes: 1 addition & 1 deletion doc/api/crypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions src/crypto/crypto_cipher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_)));
}
}

Expand Down
20 changes: 20 additions & 0 deletions test/parallel/test-crypto-authenticated.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -665,3 +666,22 @@ 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();
}, {
message: /Unsupported state/
});
}
}

0 comments on commit 9c17def

Please sign in to comment.