From 5c904820483aed80dd0fe81fc1f8b3b338e90120 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Mon, 7 Apr 2025 23:10:13 +0200 Subject: [PATCH] crypto: remove CipherBase::Init As far as I can tell, the `iv` parameter can never be `undefined` (but it can be `null`!), so this code appears to have been dead since Node.js 22. This change removes dead code and adds a tiny test case for passing `undefined` as the IV. Refs: https://github.com/nodejs/node/pull/50973 --- lib/internal/crypto/cipher.js | 6 +- src/crypto/crypto_cipher.cc | 65 ------------------- src/crypto/crypto_cipher.h | 4 -- .../test-crypto-cipheriv-decipheriv.js | 8 +++ 4 files changed, 9 insertions(+), 74 deletions(-) diff --git a/lib/internal/crypto/cipher.js b/lib/internal/crypto/cipher.js index 24ee937e355d10..e2b863cdc5749a 100644 --- a/lib/internal/crypto/cipher.js +++ b/lib/internal/crypto/cipher.js @@ -115,11 +115,7 @@ function getUIntOption(options, key) { function createCipherBase(cipher, credential, options, decipher, iv) { const authTagLength = getUIntOption(options, 'authTagLength'); this[kHandle] = new CipherBase(decipher); - if (iv === undefined) { - this[kHandle].init(cipher, credential, authTagLength); - } else { - this[kHandle].initiv(cipher, credential, iv, authTagLength); - } + this[kHandle].initiv(cipher, credential, iv, authTagLength); this._decoder = null; ReflectApply(LazyTransform, this, [options]); diff --git a/src/crypto/crypto_cipher.cc b/src/crypto/crypto_cipher.cc index 7d7f78d329c074..1660b2f7429918 100644 --- a/src/crypto/crypto_cipher.cc +++ b/src/crypto/crypto_cipher.cc @@ -231,7 +231,6 @@ void CipherBase::Initialize(Environment* env, Local target) { t->InstanceTemplate()->SetInternalFieldCount(CipherBase::kInternalFieldCount); - SetProtoMethod(isolate, t, "init", Init); SetProtoMethod(isolate, t, "initiv", InitIv); SetProtoMethod(isolate, t, "update", Update); SetProtoMethod(isolate, t, "final", Final); @@ -275,7 +274,6 @@ void CipherBase::RegisterExternalReferences( ExternalReferenceRegistry* registry) { registry->Register(New); - registry->Register(Init); registry->Register(InitIv); registry->Register(Update); registry->Register(Final); @@ -347,69 +345,6 @@ void CipherBase::CommonInit(std::string_view cipher_type, } } -void CipherBase::Init(std::string_view cipher_type, - const ArrayBufferOrViewContents& key_buf, - unsigned int auth_tag_len) { - HandleScope scope(env()->isolate()); - MarkPopErrorOnReturn mark_pop_error_on_return; - const auto cipher = Cipher::FromName(cipher_type); - if (!cipher) { - return THROW_ERR_CRYPTO_UNKNOWN_CIPHER(env()); - } - - unsigned char key[Cipher::MAX_KEY_LENGTH]; - unsigned char iv[Cipher::MAX_IV_LENGTH]; - - ncrypto::Buffer keyBuf{ - .data = key_buf.data(), - .len = key_buf.size(), - }; - int key_len = cipher.bytesToKey(Digest::MD5, keyBuf, key, iv); - CHECK_NE(key_len, 0); - - if (kind_ == kCipher && - (cipher.isCtrMode() || cipher.isGcmMode() || cipher.isCcmMode())) { - // Ignore the return value (i.e. possible exception) because we are - // not calling back into JS anyway. - ProcessEmitWarning(env(), - "Use Cipheriv for counter mode of %s", - cipher_type); - } - - CommonInit(cipher_type, - cipher, - key, - key_len, - iv, - cipher.getIvLength(), - auth_tag_len); -} - -void CipherBase::Init(const FunctionCallbackInfo& args) { - CipherBase* cipher; - ASSIGN_OR_RETURN_UNWRAP(&cipher, args.This()); - Environment* env = Environment::GetCurrent(args); - - CHECK_GE(args.Length(), 3); - - const Utf8Value cipher_type(args.GetIsolate(), args[0]); - ArrayBufferOrViewContents key_buf(args[1]); - if (!key_buf.CheckSizeInt32()) - return THROW_ERR_OUT_OF_RANGE(env, "password is too large"); - - // Don't assign to cipher->auth_tag_len_ directly; the value might not - // represent a valid length at this point. - unsigned int auth_tag_len; - if (args[2]->IsUint32()) { - auth_tag_len = args[2].As()->Value(); - } else { - CHECK(args[2]->IsInt32() && args[2].As()->Value() == -1); - auth_tag_len = kNoAuthTagLength; - } - - cipher->Init(cipher_type.ToStringView(), key_buf, auth_tag_len); -} - void CipherBase::InitIv(std::string_view cipher_type, const ByteSource& key_buf, const ArrayBufferOrViewContents& iv_buf, diff --git a/src/crypto/crypto_cipher.h b/src/crypto/crypto_cipher.h index fa26f5aa864760..0b5eb8771bdafa 100644 --- a/src/crypto/crypto_cipher.h +++ b/src/crypto/crypto_cipher.h @@ -50,9 +50,6 @@ class CipherBase : public BaseObject { const unsigned char* iv, int iv_len, unsigned int auth_tag_len); - void Init(std::string_view cipher_type, - const ArrayBufferOrViewContents& key_buf, - unsigned int auth_tag_len); void InitIv(std::string_view cipher_type, const ByteSource& key_buf, const ArrayBufferOrViewContents& iv_buf, @@ -73,7 +70,6 @@ class CipherBase : public BaseObject { bool MaybePassAuthTagToOpenSSL(); static void New(const v8::FunctionCallbackInfo& args); - static void Init(const v8::FunctionCallbackInfo& args); static void InitIv(const v8::FunctionCallbackInfo& args); static void Update(const v8::FunctionCallbackInfo& args); static void Final(const v8::FunctionCallbackInfo& args); diff --git a/test/parallel/test-crypto-cipheriv-decipheriv.js b/test/parallel/test-crypto-cipheriv-decipheriv.js index 88d07c3b957f57..6742722f9e9091 100644 --- a/test/parallel/test-crypto-cipheriv-decipheriv.js +++ b/test/parallel/test-crypto-cipheriv-decipheriv.js @@ -171,6 +171,14 @@ for (let n = 1; n < 256; n += 1) { errMessage); } +// And so should undefined be (regardless of mode). +assert.throws( + () => crypto.createCipheriv('aes-128-ecb', Buffer.alloc(16)), + { code: 'ERR_INVALID_ARG_TYPE' }); +assert.throws( + () => crypto.createCipheriv('aes-128-ecb', Buffer.alloc(16), undefined), + { code: 'ERR_INVALID_ARG_TYPE' }); + // Correctly sized IV should be accepted in CBC mode. crypto.createCipheriv('aes-128-cbc', Buffer.alloc(16), Buffer.alloc(16));