crypto: make SignBase compatible with OpenSSL 1.1.0

davidben · evanlucas · commit fa1fc16c3e38 · 2017-11-13T07:59:44.000-06:00
1.1.0 requires EVP_MD_CTX be heap-allocated. In doing so, move the Init and Update hooks to shared code because they are the same between Verify and Sign. PR-URL: #16130 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Rod Vagg <rod@vagg.org>
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -4052,6 +4052,38 @@ void Hash::HashDigest(const FunctionCallbackInfo<Value>& args) {
 }
 
 
+SignBase::~SignBase() {
+  EVP_MD_CTX_free(mdctx_);
+}
+
+
+SignBase::Error SignBase::Init(const char* sign_type) {
+  CHECK_EQ(mdctx_, nullptr);
+  const EVP_MD* md = EVP_get_digestbyname(sign_type);
+  if (md == nullptr)
+    return kSignUnknownDigest;
+
+  mdctx_ = EVP_MD_CTX_new();
+  if (mdctx_ == nullptr ||
+      !EVP_DigestInit_ex(mdctx_, md, nullptr)) {
+    EVP_MD_CTX_free(mdctx_);
+    mdctx_ = nullptr;
+    return kSignInit;
+  }
+
+  return kSignOk;
+}
+
+
+SignBase::Error SignBase::Update(const char* data, int len) {
+  if (mdctx_ == nullptr)
+    return kSignNotInitialised;
+  if (!EVP_DigestUpdate(mdctx_, data, len))
+    return kSignUpdate;
+  return kSignOk;
+}
+
+
 void SignBase::CheckThrow(SignBase::Error error) {
   HandleScope scope(env()->isolate());
 
@@ -4125,36 +4157,12 @@ void Sign::New(const FunctionCallbackInfo<Value>& args) {
 }
 
 
-SignBase::Error Sign::SignInit(const char* sign_type) {
-  CHECK_EQ(initialised_, false);
-  const EVP_MD* md = EVP_get_digestbyname(sign_type);
-  if (md == nullptr)
-    return kSignUnknownDigest;
-
-  EVP_MD_CTX_init(&mdctx_);
-  if (!EVP_DigestInit_ex(&mdctx_, md, nullptr))
-    return kSignInit;
-  initialised_ = true;
-
-  return kSignOk;
-}
-
-
 void Sign::SignInit(const FunctionCallbackInfo<Value>& args) {
   Sign* sign;
   ASSIGN_OR_RETURN_UNWRAP(&sign, args.Holder());
 
   const node::Utf8Value sign_type(args.GetIsolate(), args[0]);
-  sign->CheckThrow(sign->SignInit(*sign_type));
-}
-
-
-SignBase::Error Sign::SignUpdate(const char* data, int len) {
-  if (!initialised_)
-    return kSignNotInitialised;
-  if (!EVP_DigestUpdate(&mdctx_, data, len))
-    return kSignUpdate;
-  return kSignOk;
+  sign->CheckThrow(sign->Init(*sign_type));
 }
 
 
@@ -4165,7 +4173,7 @@ void Sign::SignUpdate(const FunctionCallbackInfo<Value>& args) {
   Error err;
   char* buf = Buffer::Data(args[0]);
   size_t buflen = Buffer::Length(args[0]);
-  err = sign->SignUpdate(buf, buflen);
+  err = sign->Update(buf, buflen);
 
   sign->CheckThrow(err);
 }
@@ -4208,7 +4216,7 @@ SignBase::Error Sign::SignFinal(const char* key_pem,
                                 unsigned int* sig_len,
                                 int padding,
                                 int salt_len) {
-  if (!initialised_)
+  if (!mdctx_)
     return kSignNotInitialised;
 
   BIO* bp = nullptr;
@@ -4253,18 +4261,17 @@ SignBase::Error Sign::SignFinal(const char* key_pem,
   }
 #endif  // NODE_FIPS_MODE
 
-  if (Node_SignFinal(&mdctx_, sig, sig_len, pkey, padding, salt_len))
+  if (Node_SignFinal(mdctx_, sig, sig_len, pkey, padding, salt_len))
     fatal = false;
 
-  initialised_ = false;
-
  exit:
   if (pkey != nullptr)
     EVP_PKEY_free(pkey);
   if (bp != nullptr)
     BIO_free_all(bp);
 
-  EVP_MD_CTX_cleanup(&mdctx_);
+  EVP_MD_CTX_free(mdctx_);
+  mdctx_ = nullptr;
 
   if (fatal)
     return kSignPrivateKey;
@@ -4338,38 +4345,12 @@ void Verify::New(const FunctionCallbackInfo<Value>& args) {
 }
 
 
-SignBase::Error Verify::VerifyInit(const char* verify_type) {
-  CHECK_EQ(initialised_, false);
-  const EVP_MD* md = EVP_get_digestbyname(verify_type);
-  if (md == nullptr)
-    return kSignUnknownDigest;
-
-  EVP_MD_CTX_init(&mdctx_);
-  if (!EVP_DigestInit_ex(&mdctx_, md, nullptr))
-    return kSignInit;
-  initialised_ = true;
-
-  return kSignOk;
-}
-
-
 void Verify::VerifyInit(const FunctionCallbackInfo<Value>& args) {
   Verify* verify;
   ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder());
 
   const node::Utf8Value verify_type(args.GetIsolate(), args[0]);
-  verify->CheckThrow(verify->VerifyInit(*verify_type));
-}
-
-
-SignBase::Error Verify::VerifyUpdate(const char* data, int len) {
-  if (!initialised_)
-    return kSignNotInitialised;
-
-  if (!EVP_DigestUpdate(&mdctx_, data, len))
-    return kSignUpdate;
-
-  return kSignOk;
+  verify->CheckThrow(verify->Init(*verify_type));
 }
 
 
@@ -4380,7 +4361,7 @@ void Verify::VerifyUpdate(const FunctionCallbackInfo<Value>& args) {
   Error err;
   char* buf = Buffer::Data(args[0]);
   size_t buflen = Buffer::Length(args[0]);
-  err = verify->VerifyUpdate(buf, buflen);
+  err = verify->Update(buf, buflen);
 
   verify->CheckThrow(err);
 }
@@ -4393,7 +4374,7 @@ SignBase::Error Verify::VerifyFinal(const char* key_pem,
                                     int padding,
                                     int saltlen,
                                     bool* verify_result) {
-  if (!initialised_)
+  if (!mdctx_)
     return kSignNotInitialised;
 
   EVP_PKEY* pkey = nullptr;
@@ -4438,7 +4419,7 @@ SignBase::Error Verify::VerifyFinal(const char* key_pem,
       goto exit;
   }
 
-  if (!EVP_DigestFinal_ex(&mdctx_, m, &m_len)) {
+  if (!EVP_DigestFinal_ex(mdctx_, m, &m_len)) {
     goto exit;
   }
 
@@ -4451,7 +4432,7 @@ SignBase::Error Verify::VerifyFinal(const char* key_pem,
     goto err;
   if (!ApplyRSAOptions(pkey, pkctx, padding, saltlen))
     goto err;
-  if (EVP_PKEY_CTX_set_signature_md(pkctx, mdctx_.digest) <= 0)
+  if (EVP_PKEY_CTX_set_signature_md(pkctx, EVP_MD_CTX_md(mdctx_)) <= 0)
     goto err;
   r = EVP_PKEY_verify(pkctx,
                       reinterpret_cast<const unsigned char*>(sig),
@@ -4470,8 +4451,8 @@ SignBase::Error Verify::VerifyFinal(const char* key_pem,
   if (x509 != nullptr)
     X509_free(x509);
 
-  EVP_MD_CTX_cleanup(&mdctx_);
-  initialised_ = false;
+  EVP_MD_CTX_free(mdctx_);
+  mdctx_ = nullptr;
 
   if (fatal)
     return kSignPublicKey;
diff --git a/src/node_crypto.h b/src/node_crypto.h
@@ -562,28 +562,24 @@ class SignBase : public BaseObject {
 
   SignBase(Environment* env, v8::Local<v8::Object> wrap)
       : BaseObject(env, wrap),
-        initialised_(false) {
+        mdctx_(nullptr) {
   }
 
-  ~SignBase() override {
-    if (!initialised_)
-      return;
-    EVP_MD_CTX_cleanup(&mdctx_);
-  }
+  ~SignBase() override;
+
+  Error Init(const char* sign_type);
+  Error Update(const char* data, int len);
 
  protected:
   void CheckThrow(Error error);
 
-  EVP_MD_CTX mdctx_; /* coverity[member_decl] */
-  bool initialised_;
+  EVP_MD_CTX* mdctx_;
 };
 
 class Sign : public SignBase {
  public:
   static void Initialize(Environment* env, v8::Local<v8::Object> target);
 
-  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,
@@ -607,8 +603,6 @@ class Verify : public SignBase {
  public:
   static void Initialize(Environment* env, v8::Local<v8::Object> target);
 
-  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,
