From 58e2804cb0802a751f3b4069252b904eedf17f56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Stan=C4=9Bk?= Date: Wed, 13 Jul 2022 14:26:36 +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 - Use non-const signature for X509 name functions Signed-off-by: Jan Staněk --- src/node_crypto.cc | 26 ++++++++++++++++++++++++-- src/node_crypto.h | 4 ++++ src/node_crypto_bio.cc | 4 ++++ src/node_crypto_common.cc | 14 +++++++++++--- 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index d7c7d06646..de8b26930b 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -133,7 +133,11 @@ template int SSLWrap::SetCACerts(SecureContext* sc); template void SSLWrap::MemoryInfo(MemoryTracker* tracker) const; template SSL_SESSION* SSLWrap::GetSessionCallback( SSL* s, +#if OPENSSL_IS_LEGACY + unsigned char *key, +#else const unsigned char* key, +#endif int len, int* copy); template int SSLWrap::NewSessionCallback(SSL* s, @@ -1785,7 +1789,11 @@ void SSLWrap::ConfigureSecureContext(SecureContext* sc) { template SSL_SESSION* SSLWrap::GetSessionCallback(SSL* s, +#if OPENSSL_IS_LEGACY + unsigned char* key, +#else const unsigned char* key, +#endif int len, int* copy) { Base* w = static_cast(SSL_get_app_data(s)); @@ -5908,9 +5916,23 @@ struct PBKDF2Job : public CryptoJob { } inline void DoThreadPoolWork() override { - auto salt_data = reinterpret_cast(salt.data()); + static const char * const empty = ""; + + auto pass_data = reinterpret_cast(empty); + auto pass_size = int(0); + auto salt_data = reinterpret_cast(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(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 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(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 d43e5af2b5..7d313dd3df 100644 --- a/src/node_crypto_common.cc +++ b/src/node_crypto_common.cc @@ -337,7 +337,15 @@ MaybeLocal GetCipherStandardName( } MaybeLocal 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(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, @@ -845,7 +853,7 @@ v8::MaybeLocal GetInfoAccessString( return ToV8Value(env, bio); } -template +template static MaybeLocal GetX509NameObject(Environment* env, X509* cert) { X509_NAME* name = get_name(cert); CHECK_NOT_NULL(name); @@ -868,7 +876,7 @@ static MaybeLocal GetX509NameObject(Environment* env, X509* cert) { // anyway, and multi-value RDNs are rare, i.e., the vast majority of // Relative Distinguished Names contains a single type-value pair only. const ASN1_OBJECT* type = X509_NAME_ENTRY_get_object(entry); - const ASN1_STRING* value = X509_NAME_ENTRY_get_data(entry); + ASN1_STRING* value = X509_NAME_ENTRY_get_data(entry); // If OpenSSL knows the type, use the short name of the type as the key, and // the numeric representation of the type's OID otherwise. -- 2.36.1