From 146e8f8340d2b6b4fc08e235f522a53848d01290 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 20 Sep 2017 18:54:30 -0400 Subject: [PATCH] crypto: make CipherBase 1.1.0-compatible In OpenSSL 1.1.0, EVP_CIPHER_CTX must be heap-allocated. Once we're heap-allocating them, there's no need in a separate initialised_ bit. The presence of ctx_ is sufficient. PR-URL: https://github.com/nodejs/node/pull/16130 Reviewed-By: Ben Noordhuis Reviewed-By: Rod Vagg --- src/node_crypto.cc | 72 +++++++++++++++++++++++----------------------- src/node_crypto.h | 11 ++----- 2 files changed, 39 insertions(+), 44 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index b26b6f76d85..00001889672 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -3454,7 +3454,7 @@ void CipherBase::Init(const char* cipher_type, } #endif // NODE_FIPS_MODE - CHECK_EQ(initialised_, false); + CHECK_EQ(ctx_, nullptr); const EVP_CIPHER* const cipher = EVP_get_cipherbyname(cipher_type); if (cipher == nullptr) { return env()->ThrowError("Unknown cipher"); @@ -3472,11 +3472,11 @@ void CipherBase::Init(const char* cipher_type, key, iv); - EVP_CIPHER_CTX_init(&ctx_); + ctx_ = EVP_CIPHER_CTX_new(); const bool encrypt = (kind_ == kCipher); - EVP_CipherInit_ex(&ctx_, cipher, nullptr, nullptr, nullptr, encrypt); + EVP_CipherInit_ex(ctx_, cipher, nullptr, nullptr, nullptr, encrypt); - int mode = EVP_CIPHER_CTX_mode(&ctx_); + int mode = EVP_CIPHER_CTX_mode(ctx_); if (encrypt && (mode == EVP_CIPH_CTR_MODE || mode == EVP_CIPH_GCM_MODE || mode == EVP_CIPH_CCM_MODE)) { ProcessEmitWarning(env(), "Use Cipheriv for counter mode of %s", @@ -3484,17 +3484,16 @@ void CipherBase::Init(const char* cipher_type, } if (mode == EVP_CIPH_WRAP_MODE) - EVP_CIPHER_CTX_set_flags(&ctx_, EVP_CIPHER_CTX_FLAG_WRAP_ALLOW); + EVP_CIPHER_CTX_set_flags(ctx_, EVP_CIPHER_CTX_FLAG_WRAP_ALLOW); - CHECK_EQ(1, EVP_CIPHER_CTX_set_key_length(&ctx_, key_len)); + CHECK_EQ(1, EVP_CIPHER_CTX_set_key_length(ctx_, key_len)); - EVP_CipherInit_ex(&ctx_, + EVP_CipherInit_ex(ctx_, nullptr, nullptr, reinterpret_cast(key), reinterpret_cast(iv), kind_ == kCipher); - initialised_ = true; } @@ -3531,32 +3530,33 @@ void CipherBase::InitIv(const char* cipher_type, return env()->ThrowError("Invalid IV length"); } - EVP_CIPHER_CTX_init(&ctx_); + ctx_ = EVP_CIPHER_CTX_new(); if (mode == EVP_CIPH_WRAP_MODE) - EVP_CIPHER_CTX_set_flags(&ctx_, EVP_CIPHER_CTX_FLAG_WRAP_ALLOW); + EVP_CIPHER_CTX_set_flags(ctx_, EVP_CIPHER_CTX_FLAG_WRAP_ALLOW); const bool encrypt = (kind_ == kCipher); - EVP_CipherInit_ex(&ctx_, cipher, nullptr, nullptr, nullptr, encrypt); + EVP_CipherInit_ex(ctx_, cipher, nullptr, nullptr, nullptr, encrypt); if (is_gcm_mode && - !EVP_CIPHER_CTX_ctrl(&ctx_, EVP_CTRL_GCM_SET_IVLEN, iv_len, nullptr)) { - EVP_CIPHER_CTX_cleanup(&ctx_); + !EVP_CIPHER_CTX_ctrl(ctx_, EVP_CTRL_GCM_SET_IVLEN, iv_len, nullptr)) { + EVP_CIPHER_CTX_free(ctx_); + ctx_ = nullptr; return env()->ThrowError("Invalid IV length"); } - if (!EVP_CIPHER_CTX_set_key_length(&ctx_, key_len)) { - EVP_CIPHER_CTX_cleanup(&ctx_); + if (!EVP_CIPHER_CTX_set_key_length(ctx_, key_len)) { + EVP_CIPHER_CTX_free(ctx_); + ctx_ = nullptr; return env()->ThrowError("Invalid key length"); } - EVP_CipherInit_ex(&ctx_, + EVP_CipherInit_ex(ctx_, nullptr, nullptr, reinterpret_cast(key), reinterpret_cast(iv), kind_ == kCipher); - initialised_ = true; } @@ -3578,8 +3578,8 @@ void CipherBase::InitIv(const FunctionCallbackInfo& args) { bool CipherBase::IsAuthenticatedMode() const { // Check if this cipher operates in an AEAD mode that we support. - CHECK_EQ(initialised_, true); - const EVP_CIPHER* const cipher = EVP_CIPHER_CTX_cipher(&ctx_); + CHECK_NE(ctx_, nullptr); + const EVP_CIPHER* const cipher = EVP_CIPHER_CTX_cipher(ctx_); int mode = EVP_CIPHER_mode(cipher); return mode == EVP_CIPH_GCM_MODE; } @@ -3591,7 +3591,7 @@ void CipherBase::GetAuthTag(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); // Only callable after Final and if encrypting. - if (cipher->initialised_ || + if (cipher->ctx_ != nullptr || cipher->kind_ != kCipher || cipher->auth_tag_len_ == 0) { return args.GetReturnValue().SetUndefined(); @@ -3608,7 +3608,7 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo& args) { CipherBase* cipher; ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); - if (!cipher->initialised_ || + if (cipher->ctx_ == nullptr || !cipher->IsAuthenticatedMode() || cipher->kind_ != kDecipher) { return args.GetReturnValue().Set(false); @@ -3626,10 +3626,10 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo& args) { bool CipherBase::SetAAD(const char* data, unsigned int len) { - if (!initialised_ || !IsAuthenticatedMode()) + if (ctx_ == nullptr || !IsAuthenticatedMode()) return false; int outlen; - if (!EVP_CipherUpdate(&ctx_, + if (!EVP_CipherUpdate(ctx_, nullptr, &outlen, reinterpret_cast(data), @@ -3653,21 +3653,21 @@ bool CipherBase::Update(const char* data, int len, unsigned char** out, int* out_len) { - if (!initialised_) + if (ctx_ == nullptr) return 0; // on first update: if (kind_ == kDecipher && IsAuthenticatedMode() && auth_tag_len_ > 0) { - EVP_CIPHER_CTX_ctrl(&ctx_, + EVP_CIPHER_CTX_ctrl(ctx_, EVP_CTRL_GCM_SET_TAG, auth_tag_len_, reinterpret_cast(auth_tag_)); auth_tag_len_ = 0; } - *out_len = len + EVP_CIPHER_CTX_block_size(&ctx_); + *out_len = len + EVP_CIPHER_CTX_block_size(ctx_); *out = Malloc(static_cast(*out_len)); - return EVP_CipherUpdate(&ctx_, + return EVP_CipherUpdate(ctx_, *out, out_len, reinterpret_cast(data), @@ -3713,9 +3713,9 @@ void CipherBase::Update(const FunctionCallbackInfo& args) { bool CipherBase::SetAutoPadding(bool auto_padding) { - if (!initialised_) + if (ctx_ == nullptr) return false; - return EVP_CIPHER_CTX_set_padding(&ctx_, auto_padding); + return EVP_CIPHER_CTX_set_padding(ctx_, auto_padding); } @@ -3729,22 +3729,22 @@ void CipherBase::SetAutoPadding(const FunctionCallbackInfo& args) { bool CipherBase::Final(unsigned char** out, int *out_len) { - if (!initialised_) + if (ctx_ == nullptr) return false; *out = Malloc( - static_cast(EVP_CIPHER_CTX_block_size(&ctx_))); - int r = EVP_CipherFinal_ex(&ctx_, *out, out_len); + static_cast(EVP_CIPHER_CTX_block_size(ctx_))); + int r = EVP_CipherFinal_ex(ctx_, *out, out_len); if (r == 1 && kind_ == kCipher && IsAuthenticatedMode()) { auth_tag_len_ = sizeof(auth_tag_); - r = EVP_CIPHER_CTX_ctrl(&ctx_, EVP_CTRL_GCM_GET_TAG, auth_tag_len_, + r = EVP_CIPHER_CTX_ctrl(ctx_, EVP_CTRL_GCM_GET_TAG, auth_tag_len_, reinterpret_cast(auth_tag_)); CHECK_EQ(r, 1); } - EVP_CIPHER_CTX_cleanup(&ctx_); - initialised_ = false; + EVP_CIPHER_CTX_free(ctx_); + ctx_ = nullptr; return r == 1; } @@ -3755,7 +3755,7 @@ void CipherBase::Final(const FunctionCallbackInfo& args) { CipherBase* cipher; ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); - if (!cipher->initialised_) return env->ThrowError("Unsupported state"); + if (cipher->ctx_ == nullptr) return env->ThrowError("Unsupported state"); unsigned char* out_value = nullptr; int out_len = -1; diff --git a/src/node_crypto.h b/src/node_crypto.h index c0ebfd1ead9..7ed1066c6c9 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -51,8 +51,6 @@ #include #include -#define EVP_F_EVP_DECRYPTFINAL 101 - #if !defined(OPENSSL_NO_TLSEXT) && defined(SSL_CTX_set_tlsext_status_cb) # define NODE__HAVE_TLSEXT_STATUS_CB #endif // !defined(OPENSSL_NO_TLSEXT) && defined(SSL_CTX_set_tlsext_status_cb) @@ -442,9 +440,7 @@ class Connection : public AsyncWrap, public SSLWrap { class CipherBase : public BaseObject { public: ~CipherBase() override { - if (!initialised_) - return; - EVP_CIPHER_CTX_cleanup(&ctx_); + EVP_CIPHER_CTX_free(ctx_); } static void Initialize(Environment* env, v8::Local target); @@ -483,15 +479,14 @@ class CipherBase : public BaseObject { v8::Local wrap, CipherKind kind) : BaseObject(env, wrap), - initialised_(false), + ctx_(nullptr), kind_(kind), auth_tag_len_(0) { MakeWeak(this); } private: - EVP_CIPHER_CTX ctx_; /* coverity[member_decl] */ - bool initialised_; + EVP_CIPHER_CTX* ctx_; const CipherKind kind_; unsigned int auth_tag_len_; char auth_tag_[EVP_GCM_TLS_TAG_LEN];