From 76a36980372b8dbf82a0ada18a1ebae3d94c5fa0 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:48 +0200
Subject: [PATCH 4/7] Disable unsupported OpenSSL features
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Disable no-certificate PSK authentication

  There is no obvious way to reimplement it using only OpenSSL 1.0 public APIs.

- Disable queries for standard cipher name

  OpenSSL 1.0 does not record said names.

- Remove ClientHello getters

  The disabled functions internally use
  `SSL_client_hello_get0_ext`/`SSL_client_hello_get0_ciphers`,
  which are not available on legacy OpenSSL.
  There may be another way to get to the same data,
  but nothing jumps out in the OpenSSL 1.0.2 documentation.

- Remove TLSv1.3 CLI options

- Remove usage of OPENSSL_secure_{malloc,clear_free}

  Unsupported in OpenSSL 1.0.

  The expected semantics is the same as using the regular versions,
  so the possibility of using the secure heap was simply removed.

Signed-off-by: Jan Staněk <jstanek@redhat.com>
---
 doc/api/cli.md                                 | 18 ------------------
 doc/api/tls.md                                 | 15 +++++++--------
 src/env.h                                      | 11 ++++++++++-
 src/node_crypto.cc                             |  8 ++++++--
 src/node_crypto_common.cc                      | 12 ++++++++++++
 src/node_crypto_common.h                       |  6 ++++++
 src/node_options.cc                            | 10 +++++++++-
 .../test-tls-cli-max-version-1.3.js            |  0
 .../test-tls-cli-min-max-conflict.js           |  0
 .../test-tls-cli-min-version-1.3.js            |  0
 10 files changed, 50 insertions(+), 30 deletions(-)
 rename test/{parallel => known_issues}/test-tls-cli-max-version-1.3.js (100%)
 rename test/{parallel => known_issues}/test-tls-cli-min-max-conflict.js (100%)
 rename test/{parallel => known_issues}/test-tls-cli-min-version-1.3.js (100%)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 3f3e5e4eeb..48e51fcdfa 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -893,14 +893,6 @@ added:
 Set [`tls.DEFAULT_MAX_VERSION`][] to 'TLSv1.2'. Use to disable support for
 TLSv1.3.
 
-### `--tls-max-v1.3`
-<!-- YAML
-added: v12.0.0
--->
-
-Set default [`tls.DEFAULT_MAX_VERSION`][] to 'TLSv1.3'. Use to enable support
-for TLSv1.3.
-
 ### `--tls-min-v1.0`
 <!-- YAML
 added:
@@ -932,14 +924,6 @@ Set default [`tls.DEFAULT_MIN_VERSION`][] to 'TLSv1.2'. This is the default for
 12.x and later, but the option is supported for compatibility with older Node.js
 versions.
 
-### `--tls-min-v1.3`
-<!-- YAML
-added: v12.0.0
--->
-
-Set default [`tls.DEFAULT_MIN_VERSION`][] to 'TLSv1.3'. Use to disable support
-for TLSv1.2, which is not as secure as TLSv1.3.
-
 ### `--trace-atomics-wait`
 <!-- YAML
 added: v14.3.0
@@ -1381,11 +1365,9 @@ Node.js options that are allowed are:
 * `--tls-cipher-list`
 * `--tls-keylog`
 * `--tls-max-v1.2`
-* `--tls-max-v1.3`
 * `--tls-min-v1.0`
 * `--tls-min-v1.1`
 * `--tls-min-v1.2`
-* `--tls-min-v1.3`
 * `--trace-atomics-wait`
 * `--trace-deprecation`
 * `--trace-event-categories`
diff --git a/doc/api/tls.md b/doc/api/tls.md
index b40273a83a..cff62d0fd6 100644
--- a/doc/api/tls.md
+++ b/doc/api/tls.md
@@ -1977,10 +1977,10 @@ added: v11.4.0
 
 * {string} The default value of the `maxVersion` option of
   [`tls.createSecureContext()`][]. It can be assigned any of the supported TLS
-  protocol versions, `'TLSv1.3'`, `'TLSv1.2'`, `'TLSv1.1'`, or `'TLSv1'`.
-  **Default:** `'TLSv1.3'`, unless changed using CLI options. Using
-  `--tls-max-v1.2` sets the default to `'TLSv1.2'`. Using `--tls-max-v1.3` sets
-  the default to `'TLSv1.3'`. If multiple of the options are provided, the
+  protocol versions, `'TLSv1.2'`, `'TLSv1.1'`, or `'TLSv1'`.
+  **Default:** `'TLSv1.2'`, unless changed using CLI options. Using
+  `--tls-max-v1.2` sets the default to `'TLSv1.2'`.
+  If multiple of the options are provided, the
   highest maximum is used.
 
 ## `tls.DEFAULT_MIN_VERSION`
@@ -1990,12 +1990,11 @@ added: v11.4.0
 
 * {string} The default value of the `minVersion` option of
   [`tls.createSecureContext()`][]. It can be assigned any of the supported TLS
-  protocol versions, `'TLSv1.3'`, `'TLSv1.2'`, `'TLSv1.1'`, or `'TLSv1'`.
+  protocol versions, `'TLSv1.2'`, `'TLSv1.1'`, or `'TLSv1'`.
   **Default:** `'TLSv1.2'`, unless changed using CLI options. Using
   `--tls-min-v1.0` sets the default to `'TLSv1'`. Using `--tls-min-v1.1` sets
-  the default to `'TLSv1.1'`. Using `--tls-min-v1.3` sets the default to
-  `'TLSv1.3'`. If multiple of the options are provided, the lowest minimum is
-  used.
+  the default to `'TLSv1.1'`. If multiple of the options are provided,
+  the lowest minimum is used.
 
 [Chrome's 'modern cryptography' setting]: https://www.chromium.org/Home/chromium-security/education/tls#TOC-Cipher-Suites
 [DHE]: https://en.wikipedia.org/wiki/Diffie%E2%80%93Hellman_key_exchange
diff --git a/src/env.h b/src/env.h
index b6967afb89..1f8c3b3eff 100644
--- a/src/env.h
+++ b/src/env.h
@@ -50,6 +50,8 @@
 #include <unordered_set>
 #include <vector>
 
+#include <node-ssl-shim/ssl-shim.h>
+
 namespace node {
 
 namespace contextify {
@@ -141,6 +143,13 @@ constexpr size_t kFsStatsBufferLength =
 // Make sure that any macro V defined for use with the PER_ISOLATE_* macros is
 // undefined again after use.
 
+// Some symbols/strings are not defined when using legacy OpenSSL
+#if OPENSSL_IS_LEGACY
+#   define NODE_ENV_STANDARD_NAME_STRING
+#else // OPENSSL_IS_LEGACY
+#   define NODE_ENV_STANDARD_NAME_STRING V(standard_name_string, "standardName")
+#endif // OPENSSL_IS_LEGACY
+
 // Private symbols are per-isolate primitives but Environment proxies them
 // for the sake of convenience.  Strings should be ASCII-only and have a
 // "node:" prefix to avoid name clashes with third-party code.
@@ -368,7 +377,7 @@ constexpr size_t kFsStatsBufferLength =
   V(sni_context_string, "sni_context")                                         \
   V(source_string, "source")                                                   \
   V(stack_string, "stack")                                                     \
-  V(standard_name_string, "standardName")                                      \
+  NODE_ENV_STANDARD_NAME_STRING                                                \
   V(start_time_string, "startTime")                                            \
   V(status_string, "status")                                                   \
   V(stdio_string, "stdio")                                                     \
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 81ac3b5dd4..b0affa1fe9 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -7119,11 +7119,15 @@ namespace {
 // make use of secure heap, this has the same semantics as
 // using OPENSSL_malloc. However, if the secure heap is
 // initialized, SecureBuffer will automatically use it.
+//
+// RHEL 7 Note: secure_{malloc,clear_free} is not available in OpenSSL 1.0
+// As in this case the expected behaviour is to fall back to their
+// "regular" counterparts, the "secure" calls were replaced with them.
 void SecureBuffer(const FunctionCallbackInfo<Value>& args) {
   CHECK(args[0]->IsUint32());
   Environment* env = Environment::GetCurrent(args);
   uint32_t len = args[0].As<Uint32>()->Value();
-  char* data = static_cast<char*>(OPENSSL_secure_malloc(len));
+  char* data = static_cast<char*>(OPENSSL_malloc(len));
   if (data == nullptr) {
     // There's no memory available for the allocation.
     // Return nothing.
@@ -7135,7 +7139,7 @@ void SecureBuffer(const FunctionCallbackInfo<Value>& args) {
           data,
           len,
           [](void* data, size_t len, void* deleter_data) {
-            OPENSSL_secure_clear_free(data, len);
+            OPENSSL_clear_free(data, len);
           },
           data);
   Local<ArrayBuffer> buffer = ArrayBuffer::New(env->isolate(), store);
diff --git a/src/node_crypto_common.cc b/src/node_crypto_common.cc
index da1033fdef..89f01990f0 100644
--- a/src/node_crypto_common.cc
+++ b/src/node_crypto_common.cc
@@ -211,6 +211,7 @@ long VerifyPeerCertificate(  // NOLINT(runtime/int)
   if (X509* peer_cert = SSL_get_peer_certificate(ssl.get())) {
     X509_free(peer_cert);
     err = SSL_get_verify_result(ssl.get());
+#if !OPENSSL_IS_LEGACY
   } else {
     const SSL_CIPHER* curr_cipher = SSL_get_current_cipher(ssl.get());
     const SSL_SESSION* sess = SSL_get_session(ssl.get());
@@ -222,6 +223,7 @@ long VerifyPeerCertificate(  // NOLINT(runtime/int)
          SSL_session_reused(ssl.get()))) {
       return X509_V_OK;
     }
+#endif // !OPENSSL_IS_LEGACY
   }
   return err;
 }
@@ -239,6 +241,7 @@ int UseSNIContext(const SSLPointer& ssl, BaseObjectPtr<SecureContext> context) {
   return err;
 }
 
+#if !OPENSSL_IS_LEGACY
 const char* GetClientHelloALPN(const SSLPointer& ssl) {
   const unsigned char* buf;
   size_t len;
@@ -285,6 +288,7 @@ const char* GetClientHelloServerName(const SSLPointer& ssl) {
     return nullptr;
   return reinterpret_cast<const char*>(buf + 5);
 }
+#endif // !OPENSSL_IS_LEGACY
 
 const char* GetServerName(SSL* ssl) {
   return SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
@@ -398,11 +402,13 @@ MaybeLocal<Value> GetCipherName(Environment* env, const SSL_CIPHER* cipher) {
   return GetCipherValue(env, cipher, SSL_CIPHER_get_name);
 }
 
+#if !OPENSSL_IS_LEGACY
 MaybeLocal<Value> GetCipherStandardName(
     Environment* env,
     const SSL_CIPHER* cipher) {
   return GetCipherValue(env, cipher, SSL_CIPHER_standard_name);
 }
+#endif // !OPENSSL_IS_LEGACY
 
 MaybeLocal<Value> GetCipherVersion(Environment* env, const SSL_CIPHER* cipher) {
 #if OPENSSL_IS_LEGACY
@@ -762,16 +768,19 @@ MaybeLocal<Value> GetCipherName(Environment* env, const SSLPointer& ssl) {
   return GetCipherName(env, SSL_get_current_cipher(ssl.get()));
 }
 
+#if !OPENSSL_IS_LEGACY
 MaybeLocal<Value> GetCipherStandardName(
     Environment* env,
     const SSLPointer& ssl) {
   return GetCipherStandardName(env, SSL_get_current_cipher(ssl.get()));
 }
+#endif // !OPENSSL_IS_LEGACY
 
 MaybeLocal<Value> GetCipherVersion(Environment* env, const SSLPointer& ssl) {
   return GetCipherVersion(env, SSL_get_current_cipher(ssl.get()));
 }
 
+#if !OPENSSL_IS_LEGACY
 MaybeLocal<Array> GetClientHelloCiphers(
     Environment* env,
     const SSLPointer& ssl) {
@@ -804,6 +813,7 @@ MaybeLocal<Array> GetClientHelloCiphers(
   Local<Array> ret = Array::New(env->isolate(), ciphers.out(), count);
   return scope.Escape(ret);
 }
+#endif // !OPENSSL_IS_LEGACY
 
 
 MaybeLocal<Object> GetCipherInfo(Environment* env, const SSLPointer& ssl) {
@@ -814,10 +824,12 @@ MaybeLocal<Object> GetCipherInfo(Environment* env, const SSLPointer& ssl) {
                   info,
                   env->name_string(),
                   GetCipherName(env, ssl)) ||
+#if !OPENSSL_IS_LEGACY
       !Set<Value>(env->context(),
                   info,
                   env->standard_name_string(),
                   GetCipherStandardName(env, ssl)) ||
+#endif // !OPENSSL_IS_LEGACY
       !Set<Value>(env->context(),
                   info,
                   env->version_string(),
diff --git a/src/node_crypto_common.h b/src/node_crypto_common.h
index c373a97e47..220cb109bc 100644
--- a/src/node_crypto_common.h
+++ b/src/node_crypto_common.h
@@ -73,15 +73,19 @@ long VerifyPeerCertificate(  // NOLINT(runtime/int)
 
 int UseSNIContext(const SSLPointer& ssl, BaseObjectPtr<SecureContext> context);
 
+#if !OPENSSL_IS_LEGACY
 const char* GetClientHelloALPN(const SSLPointer& ssl);
 
 const char* GetClientHelloServerName(const SSLPointer& ssl);
+#endif // !OPENSSL_IS_LEGACY
 
 const char* GetServerName(SSL* ssl);
 
+#if !OPENSSL_IS_LEGACY
 v8::MaybeLocal<v8::Array> GetClientHelloCiphers(
     Environment* env,
     const SSLPointer& ssl);
+#endif // !OPENSSL_IS_LEGACY
 
 bool SetGroups(SecureContext* sc, const char* groups);
 
@@ -97,9 +101,11 @@ v8::MaybeLocal<v8::Value> GetCipherName(
     Environment* env,
     const SSLPointer& ssl);
 
+#if !OPENSSL_IS_LEGACY
 v8::MaybeLocal<v8::Value> GetCipherStandardName(
     Environment* env,
     const SSLPointer& ssl);
+#endif // !OPENSSL_IS_LEGACY
 
 v8::MaybeLocal<v8::Value> GetCipherVersion(
     Environment* env,
diff --git a/src/node_options.cc b/src/node_options.cc
index cee6fc1773..fbe408e12b 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -9,6 +9,8 @@
 #include <sstream>
 #include <cstdlib>  // strtoul, errno
 
+#include <node-ssl-shim/features.h>
+
 using v8::Boolean;
 using v8::Context;
 using v8::FunctionCallbackInfo;
@@ -113,10 +115,12 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
     errors->push_back("invalid value for --unhandled-rejections");
   }
 
+#if !OPENSSL_IS_LEGACY
   if (tls_min_v1_3 && tls_max_v1_2) {
     errors->push_back("either --tls-min-v1.3 or --tls-max-v1.2 can be "
                       "used, not both");
   }
+#endif // !OPENSSL_IS_LEGACY
 
   if (heap_snapshot_near_heap_limit < 0) {
     errors->push_back("--heap-snapshot-near-heap-limit must not be negative");
@@ -558,14 +562,17 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "set default TLS minimum to TLSv1.2 (default: TLSv1.2)",
             &EnvironmentOptions::tls_min_v1_2,
             kAllowedInEnvironment);
+#if !OPENSSL_IS_LEGACY
   AddOption("--tls-min-v1.3",
             "set default TLS minimum to TLSv1.3 (default: TLSv1.2)",
             &EnvironmentOptions::tls_min_v1_3,
             kAllowedInEnvironment);
+#endif // !OPENSSL_IS_LEGACY
   AddOption("--tls-max-v1.2",
-            "set default TLS maximum to TLSv1.2 (default: TLSv1.3)",
+            "set default TLS maximum to TLSv1.2 (default: TLSv1.2)",
             &EnvironmentOptions::tls_max_v1_2,
             kAllowedInEnvironment);
+#if !OPENSSL_IS_LEGACY
   // Current plan is:
   // - 11.x and below: TLS1.3 is opt-in with --tls-max-v1.3
   // - 12.x: TLS1.3 is opt-out with --tls-max-v1.2
@@ -574,6 +581,7 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "set default TLS maximum to TLSv1.3 (default: TLSv1.3)",
             &EnvironmentOptions::tls_max_v1_3,
             kAllowedInEnvironment);
+#endif // !OPENSSL_IS_LEGACY
 }
 
 PerIsolateOptionsParser::PerIsolateOptionsParser(
diff --git a/test/parallel/test-tls-cli-max-version-1.3.js b/test/known_issues/test-tls-cli-max-version-1.3.js
similarity index 100%
rename from test/parallel/test-tls-cli-max-version-1.3.js
rename to test/known_issues/test-tls-cli-max-version-1.3.js
diff --git a/test/parallel/test-tls-cli-min-max-conflict.js b/test/known_issues/test-tls-cli-min-max-conflict.js
similarity index 100%
rename from test/parallel/test-tls-cli-min-max-conflict.js
rename to test/known_issues/test-tls-cli-min-max-conflict.js
diff --git a/test/parallel/test-tls-cli-min-version-1.3.js b/test/known_issues/test-tls-cli-min-version-1.3.js
similarity index 100%
rename from test/parallel/test-tls-cli-min-version-1.3.js
rename to test/known_issues/test-tls-cli-min-version-1.3.js
-- 
2.33.1