From 661190af13a82301c61d761c0949b4148e1761f3 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Sun, 19 Jan 2014 23:38:15 +0000 Subject: [PATCH] crypto: throw only in direct C++ methods Do not throw in internal C++ methods, that clobbers logic and may lead to the situations, where both exception was thrown and the value was returned (via `args.GetReturnValue().Set()`). That doesn't play nicely with v8. fix #6912 --- src/node_crypto.cc | 193 ++++++++++++++++++++++++++------------------- src/node_crypto.h | 86 ++++++++++---------- 2 files changed, 158 insertions(+), 121 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index b6eabc5c429d6e..cf1b6379fbf616 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2692,6 +2692,46 @@ void Hash::HashDigest(const FunctionCallbackInfo& args) { } +void SignBase::CheckThrow(SignBase::Error error) { + HandleScope scope(node_isolate); + + switch (error) { + case kSignUnknownDigest: + return ThrowError("Unknown message digest"); + + case kSignNotInitialised: + return ThrowError("Not initialised"); + + case kSignInit: + case kSignUpdate: + case kSignPrivateKey: + case kSignPublicKey: + { + unsigned long err = ERR_get_error(); + if (err) + return ThrowCryptoError(err); + switch (error) { + case kSignInit: + return ThrowError("EVP_SignInit_ex failed"); + case kSignUpdate: + return ThrowError("EVP_SignUpdate failed"); + case kSignPrivateKey: + return ThrowError("PEM_read_bio_PrivateKey failed"); + case kSignPublicKey: + return ThrowError("PEM_read_bio_PUBKEY failed"); + default: + abort(); + } + } + + case kSignOk: + return; + } +} + + + + void Sign::Initialize(Environment* env, v8::Handle target) { Local t = FunctionTemplate::New(New); @@ -2712,17 +2752,18 @@ void Sign::New(const FunctionCallbackInfo& args) { } -void Sign::SignInit(const char* sign_type) { - HandleScope scope(node_isolate); - +SignBase::Error Sign::SignInit(const char* sign_type) { assert(md_ == NULL); md_ = EVP_get_digestbyname(sign_type); - if (!md_) { - return ThrowError("Uknown message digest"); - } + if (!md_) + return kSignUnknownDigest; + EVP_MD_CTX_init(&mdctx_); - EVP_SignInit_ex(&mdctx_, md_, NULL); + if (!EVP_SignInit_ex(&mdctx_, md_, NULL)) + return kSignInit; initialised_ = true; + + return kSignOk; } @@ -2736,15 +2777,16 @@ void Sign::SignInit(const FunctionCallbackInfo& args) { } const String::Utf8Value sign_type(args[0]); - sign->SignInit(*sign_type); + CheckThrow(sign->SignInit(*sign_type)); } -bool Sign::SignUpdate(const char* data, int len) { +SignBase::Error Sign::SignUpdate(const char* data, int len) { if (!initialised_) - return false; - EVP_SignUpdate(&mdctx_, data, len); - return true; + return kSignNotInitialised; + if (!EVP_SignUpdate(&mdctx_, data, len)) + return kSignUpdate; + return kSignOk; } @@ -2756,7 +2798,7 @@ void Sign::SignUpdate(const FunctionCallbackInfo& args) { ASSERT_IS_STRING_OR_BUFFER(args[0]); // Only copy the data if we have to, because it's a string - int r; + Error err; if (args[0]->IsString()) { Local string = args[0].As(); enum encoding encoding = ParseEncoding(args[1], BINARY); @@ -2765,29 +2807,25 @@ void Sign::SignUpdate(const FunctionCallbackInfo& args) { size_t buflen = StringBytes::StorageSize(string, encoding); char* buf = new char[buflen]; size_t written = StringBytes::Write(buf, buflen, string, encoding); - r = sign->SignUpdate(buf, written); + err = sign->SignUpdate(buf, written); delete[] buf; } else { char* buf = Buffer::Data(args[0]); size_t buflen = Buffer::Length(args[0]); - r = sign->SignUpdate(buf, buflen); + err = sign->SignUpdate(buf, buflen); } - if (!r) { - return ThrowTypeError("SignUpdate fail"); - } + CheckThrow(err); } -bool Sign::SignFinal(const char* key_pem, - int key_pem_len, - const char* passphrase, - unsigned char** sig, - unsigned int *sig_len) { - if (!initialised_) { - ThrowError("Sign not initalised"); - return false; - } +SignBase::Error Sign::SignFinal(const char* key_pem, + int key_pem_len, + const char* passphrase, + unsigned char** sig, + unsigned int *sig_len) { + if (!initialised_) + return kSignNotInitialised; BIO* bp = NULL; EVP_PKEY* pkey = NULL; @@ -2820,17 +2858,10 @@ bool Sign::SignFinal(const char* key_pem, EVP_MD_CTX_cleanup(&mdctx_); - if (fatal) { - unsigned long err = ERR_get_error(); - if (err) { - ThrowCryptoError(err); - } else { - ThrowError("PEM_read_bio_PrivateKey"); - } - return false; - } + if (fatal) + return kSignPrivateKey; - return true; + return kSignOk; } @@ -2857,15 +2888,17 @@ void Sign::SignFinal(const FunctionCallbackInfo& args) { md_len = 8192; // Maximum key size is 8192 bits md_value = new unsigned char[md_len]; - bool r = sign->SignFinal(buf, - buf_len, - len >= 3 && !args[2]->IsNull() ? *passphrase : NULL, - &md_value, - &md_len); - if (!r) { + Error err = sign->SignFinal( + buf, + buf_len, + len >= 3 && !args[2]->IsNull() ? *passphrase : NULL, + &md_value, + &md_len); + if (err != kSignOk) { delete[] md_value; md_value = NULL; md_len = 0; + return CheckThrow(err); } Local rc = StringBytes::Encode( @@ -2896,18 +2929,18 @@ void Verify::New(const FunctionCallbackInfo& args) { } -void Verify::VerifyInit(const char* verify_type) { - HandleScope scope(node_isolate); - +SignBase::Error Verify::VerifyInit(const char* verify_type) { assert(md_ == NULL); md_ = EVP_get_digestbyname(verify_type); - if (md_ == NULL) { - return ThrowError("Unknown message digest"); - } + if (md_ == NULL) + return kSignUnknownDigest; EVP_MD_CTX_init(&mdctx_); - EVP_VerifyInit_ex(&mdctx_, md_, NULL); + if (!EVP_VerifyInit_ex(&mdctx_, md_, NULL)) + return kSignInit; initialised_ = true; + + return kSignOk; } @@ -2921,15 +2954,18 @@ void Verify::VerifyInit(const FunctionCallbackInfo& args) { } const String::Utf8Value verify_type(args[0]); - verify->VerifyInit(*verify_type); + CheckThrow(verify->VerifyInit(*verify_type)); } -bool Verify::VerifyUpdate(const char* data, int len) { +SignBase::Error Verify::VerifyUpdate(const char* data, int len) { if (!initialised_) - return false; - EVP_VerifyUpdate(&mdctx_, data, len); - return true; + return kSignNotInitialised; + + if (!EVP_VerifyUpdate(&mdctx_, data, len)) + return kSignUpdate; + + return kSignOk; } @@ -2941,7 +2977,7 @@ void Verify::VerifyUpdate(const FunctionCallbackInfo& args) { ASSERT_IS_STRING_OR_BUFFER(args[0]); // Only copy the data if we have to, because it's a string - bool r; + Error err; if (args[0]->IsString()) { Local string = args[0].As(); enum encoding encoding = ParseEncoding(args[1], BINARY); @@ -2950,30 +2986,25 @@ void Verify::VerifyUpdate(const FunctionCallbackInfo& args) { size_t buflen = StringBytes::StorageSize(string, encoding); char* buf = new char[buflen]; size_t written = StringBytes::Write(buf, buflen, string, encoding); - r = verify->VerifyUpdate(buf, written); + err = verify->VerifyUpdate(buf, written); delete[] buf; } else { char* buf = Buffer::Data(args[0]); size_t buflen = Buffer::Length(args[0]); - r = verify->VerifyUpdate(buf, buflen); + err = verify->VerifyUpdate(buf, buflen); } - if (!r) { - return ThrowTypeError("VerifyUpdate fail"); - } + CheckThrow(err); } -bool Verify::VerifyFinal(const char* key_pem, - int key_pem_len, - const char* sig, - int siglen) { - HandleScope scope(node_isolate); - - if (!initialised_) { - ThrowError("Verify not initalised"); - return false; - } +SignBase::Error Verify::VerifyFinal(const char* key_pem, + int key_pem_len, + const char* sig, + int siglen, + bool* verify_result) { + if (!initialised_) + return kSignNotInitialised; ClearErrorOnReturn clear_error_on_return; (void) &clear_error_on_return; // Silence compiler warning. @@ -3036,13 +3067,11 @@ bool Verify::VerifyFinal(const char* key_pem, EVP_MD_CTX_cleanup(&mdctx_); initialised_ = false; - if (fatal) { - unsigned long err = ERR_get_error(); - ThrowCryptoError(err); - return false; - } + if (fatal) + return kSignPublicKey; - return r == 1; + *verify_result = r == 1; + return kSignOk; } @@ -3074,11 +3103,13 @@ void Verify::VerifyFinal(const FunctionCallbackInfo& args) { hbuf = Buffer::Data(args[1]); } - bool rc = verify->VerifyFinal(kbuf, klen, hbuf, hlen); - if (args[1]->IsString()) { + bool verify_result; + Error err = verify->VerifyFinal(kbuf, klen, hbuf, hlen, &verify_result); + if (args[1]->IsString()) delete[] hbuf; - } - args.GetReturnValue().Set(rc); + if (err != kSignOk) + return CheckThrow(err); + args.GetReturnValue().Set(verify_result); } diff --git a/src/node_crypto.h b/src/node_crypto.h index 7f29e8959046cc..aa670dbf10fc3a 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -446,23 +446,50 @@ class Hash : public BaseObject { bool initialised_; }; -class Sign : public BaseObject { +class SignBase : public BaseObject { public: - ~Sign() { + typedef enum { + kSignOk, + kSignUnknownDigest, + kSignInit, + kSignNotInitialised, + kSignUpdate, + kSignPrivateKey, + kSignPublicKey + } Error; + + SignBase(Environment* env, v8::Local wrap) + : BaseObject(env, wrap), + md_(NULL), + initialised_(false) { + } + + ~SignBase() { if (!initialised_) return; EVP_MD_CTX_cleanup(&mdctx_); } + protected: + static void CheckThrow(Error error); + + EVP_MD_CTX mdctx_; /* coverity[member_decl] */ + const EVP_MD* md_; /* coverity[member_decl] */ + bool initialised_; +}; + +class Sign : public SignBase { + public: + static void Initialize(Environment* env, v8::Handle target); - void SignInit(const char* sign_type); - bool SignUpdate(const char* data, int len); - bool SignFinal(const char* key_pem, - int key_pem_len, - const char* passphrase, - unsigned char** sig, - unsigned int *sig_len); + Error SignInit(const char* sign_type); + Error SignUpdate(const char* data, int len); + Error SignFinal(const char* key_pem, + int key_pem_len, + const char* passphrase, + unsigned char** sig, + unsigned int *sig_len); protected: static void New(const v8::FunctionCallbackInfo& args); @@ -470,35 +497,22 @@ class Sign : public BaseObject { static void SignUpdate(const v8::FunctionCallbackInfo& args); static void SignFinal(const v8::FunctionCallbackInfo& args); - Sign(Environment* env, v8::Local wrap) - : BaseObject(env, wrap), - md_(NULL), - initialised_(false) { + Sign(Environment* env, v8::Local wrap) : SignBase(env, wrap) { MakeWeak(this); } - - private: - EVP_MD_CTX mdctx_; /* coverity[member_decl] */ - const EVP_MD* md_; /* coverity[member_decl] */ - bool initialised_; }; -class Verify : public BaseObject { +class Verify : public SignBase { public: - ~Verify() { - if (!initialised_) - return; - EVP_MD_CTX_cleanup(&mdctx_); - } - static void Initialize(Environment* env, v8::Handle target); - void VerifyInit(const char* verify_type); - bool VerifyUpdate(const char* data, int len); - bool VerifyFinal(const char* key_pem, - int key_pem_len, - const char* sig, - int siglen); + Error VerifyInit(const char* verify_type); + Error VerifyUpdate(const char* data, int len); + Error VerifyFinal(const char* key_pem, + int key_pem_len, + const char* sig, + int siglen, + bool* verify_result); protected: static void New(const v8::FunctionCallbackInfo& args); @@ -506,17 +520,9 @@ class Verify : public BaseObject { static void VerifyUpdate(const v8::FunctionCallbackInfo& args); static void VerifyFinal(const v8::FunctionCallbackInfo& args); - Verify(Environment* env, v8::Local wrap) - : BaseObject(env, wrap), - md_(NULL), - initialised_(false) { + Verify(Environment* env, v8::Local wrap) : SignBase(env, wrap) { MakeWeak(this); } - - private: - EVP_MD_CTX mdctx_; /* coverity[member_decl] */ - const EVP_MD* md_; /* coverity[member_decl] */ - bool initialised_; }; class DiffieHellman : public BaseObject {