From ea610f38a05ca2b256e1f8b1d0dd8b33abc521ec Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Jan=20Stan=C4=9Bk?= <jstanek@redhat.com>

Date: Wed, 7 Jul 2021 13:37:46 +0200

Subject: [PATCH] Use OpenSSL 1.0 API

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

- Pass non-const pointer to BIO_new

  In legacy OpenSSL, the method parameter for BIO_new is not marked const,

  although the function does not need it to be mutable.

  This is likely an oversight in the interface.

  The provided "fix" is potentially dangerous,

  as casting away `const`-ness is potentially an undefined behaviour.

  Since the code around assumes it is constant anyway,

  it *should* be fine here – but use with care.

- Remove const-classifier for SSL_SESSION callback argument

  In legacy OpenSSL, the parameter is expected to be mutable.

  Using `const` prevents passing the method as a function pointer

  to other OpenSSL API functions.

- Sanitize inputs into PBKDF2

- Return const char from SSL_CIPHER_get_version

Signed-off-by: Jan Staněk <jstanek@redhat.com>

---

 src/node_crypto.cc        | 26 ++++++++++++++++++++++++--

 src/node_crypto.h         |  4 ++++

 src/node_crypto_bio.cc    |  4 ++++

 src/node_crypto_common.cc | 10 +++++++++-

 4 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/src/node_crypto.cc b/src/node_crypto.cc

index dbef9d42f0..c9de7d8a19 100644

--- a/src/node_crypto.cc

+++ b/src/node_crypto.cc

@@ -127,7 +127,11 @@ template int SSLWrap<TLSWrap>::SetCACerts(SecureContext* sc);

 template void SSLWrap<TLSWrap>::MemoryInfo(MemoryTracker* tracker) const;

 template SSL_SESSION* SSLWrap<TLSWrap>::GetSessionCallback(

     SSL* s,

+#if OPENSSL_IS_LEGACY

+    unsigned char *key,

+#else

     const unsigned char* key,

+#endif

     int len,

     int* copy);

 template int SSLWrap<TLSWrap>::NewSessionCallback(SSL* s,

@@ -1769,7 +1773,11 @@ void SSLWrap<Base>::ConfigureSecureContext(SecureContext* sc) {

 template <class Base>

 SSL_SESSION* SSLWrap<Base>::GetSessionCallback(SSL* s,

+#if OPENSSL_IS_LEGACY

+                                               unsigned char* key,

+#else

                                                const unsigned char* key,

+#endif

                                                int len,

                                                int* copy) {

   Base* w = static_cast<Base*>(SSL_get_app_data(s));

@@ -5898,9 +5906,23 @@ struct PBKDF2Job : public CryptoJob {

   }

   inline void DoThreadPoolWork() override {

-    auto salt_data = reinterpret_cast<const unsigned char*>(salt.data());

+    static const char * const empty = "";

+

+    auto pass_data = reinterpret_cast<const char *>(empty);

+    auto pass_size = int(0);

+    auto salt_data = reinterpret_cast<const unsigned char *>(empty);

+    auto salt_size = int(0);

+

+    if (pass.size() > 0) {

+      pass_data = pass.data(), pass_size = pass.size();

+    }

+    if (salt.size() > 0) {

+      salt_data = reinterpret_cast<const unsigned char *>(salt.data());

+      salt_size = salt.size();

+    }

+

     const bool ok =

-        PKCS5_PBKDF2_HMAC(pass.data(), pass.size(), salt_data, salt.size(),

+        PKCS5_PBKDF2_HMAC(pass_data, pass_size, salt_data, salt_size,

                           iteration_count, digest, keybuf_size, keybuf_data);

     success = Just(ok);

     Cleanse();

diff --git a/src/node_crypto.h b/src/node_crypto.h

index d46730c9ba..dbc46fbec8 100644

--- a/src/node_crypto.h

+++ b/src/node_crypto.h

@@ -235,7 +235,11 @@ class SSLWrap {

   static void AddMethods(Environment* env, v8::Local<v8::FunctionTemplate> t);

   static SSL_SESSION* GetSessionCallback(SSL* s,

+#if OPENSSL_IS_LEGACY

+                                         unsigned char* key,

+#else // OPENSSL_IS_LEGACY

                                          const unsigned char* key,

+#endif // OPENSSL_IS_LEGACY

                                          int len,

                                          int* copy);

   static int NewSessionCallback(SSL* s, SSL_SESSION* sess);

diff --git a/src/node_crypto_bio.cc b/src/node_crypto_bio.cc

index 8c58e31f86..319580c9b6 100644

--- a/src/node_crypto_bio.cc

+++ b/src/node_crypto_bio.cc

@@ -32,7 +32,11 @@ namespace node {

 namespace crypto {

 BIOPointer NodeBIO::New(Environment* env) {

+#if OPENSSL_IS_LEGACY

+  BIOPointer bio(BIO_new(const_cast<BIO_METHOD *>(GetMethod())));

+#else

   BIOPointer bio(BIO_new(GetMethod()));

+#endif

   if (bio && env != nullptr)

     NodeBIO::FromBIO(bio.get())->env_ = env;

   return bio;

diff --git a/src/node_crypto_common.cc b/src/node_crypto_common.cc

index 6473b652ac..da1033fdef 100644

--- a/src/node_crypto_common.cc

+++ b/src/node_crypto_common.cc

@@ -405,7 +405,15 @@ MaybeLocal<Value> GetCipherStandardName(

 }

 MaybeLocal<Value> GetCipherVersion(Environment* env, const SSL_CIPHER* cipher) {

-  return GetCipherValue(env, cipher, SSL_CIPHER_get_version);

+#if OPENSSL_IS_LEGACY

+  auto get_version = [](const SSL_CIPHER *cipher){

+    return const_cast<const char *>(SSL_CIPHER_get_version(cipher));

+  };

+#else // OPENSSL_IS_LEGACY

+  auto get_version = SSL_CIPHER_get_version;

+#endif // OPENSSL_IS_LEGACY

+

+  return GetCipherValue(env, cipher, get_version);

 }

 StackOfX509 CloneSSLCerts(X509Pointer&& cert,

-- 

2.31.1