crypto: add Crypto and CryptoFactory

This commit adds an abstract base class for Crypto to enable different crypto implementations/version to be supported. This work is very much a proof of concept to find out if this approach will work and have code to discuss around. The goal here is to decouple node.cc from OpenSSL and move that code into a concrete implementation of Crypto. Currently the only member functions that exist in Crypto are those that I found in node.cc and allowed the project to be built and the test to run successfully.
danbev · May 30, 2017 · May 29, 2017 · Jun 7, 2017 · Jun 8, 2017 · Mar 10, 2017
commit d3e9cc08cb909ed05cc4c7b6586c4ce779de437d
diff --git a/configure b/configure
@@ -159,6 +159,11 @@ parser.add_option('--openssl-use-def-ca-store',
     dest='use_openssl_ca_store',
     help='Use OpenSSL supplied CA store instead of compiled-in Mozilla CA copy.')
 
+parser.add_option("--crypto-version",
+    action="store",
+    dest="crypto_version",
+    help="The version of the crypto implementation.")
+
 shared_optgroup.add_option('--shared-http-parser',
     action='store_true',
     dest='shared_http_parser',
@@ -985,6 +990,14 @@ def configure_openssl(o):
   else:
     o['variables']['openssl_fips'] = ''
 
+  if options.crypto_version is None:
+      options.crypto_version = "openssl_1_0_2e"
+      print('Using default crypto_version = %s Please specify a specific '
+            'version if this does not match the version of the crypto library.'
+	    % options.crypto_version)
+
+  o['variables']['node_crypto_version'] = options.crypto_version
+  o['defines'] += ['NODE_CRYPTO_VERSION=%s' % options.crypto_version]
 
   if options.without_ssl:
     def without_ssl_error(option):

diff --git a/node.gyp b/node.gyp
@@ -652,7 +652,9 @@
         'test/cctest/test_base64.cc',
         'test/cctest/test_environment.cc',
         'test/cctest/test_util.cc',
-        'test/cctest/test_url.cc'
+        'test/cctest/test_url.cc',
+        'test/cctest/test_crypto_factory.cc',
+        'test/cctest/test_crypto_openssl_1_0_2e.cc',
       ],
 
       'sources!': [

diff --git a/node.gypi b/node.gypi
@@ -98,6 +98,11 @@
     [ 'node_use_openssl=="true"', {
       'defines': [ 'HAVE_OPENSSL=1' ],
       'sources': [
+        'src/node_crypto.h',
+        'src/node_crypto_factory.h',
+        'src/node_crypto_factory.cc',
+        'src/crypto_impl/openssl.h',
+        'src/crypto_impl/openssl_1_0_2e.cc',
         'src/crypto_impl/node_crypto.cc',
         'src/crypto_impl/node_crypto_bio.cc',
         'src/crypto_impl/node_crypto_clienthello.cc',

diff --git a/src/crypto_impl/openssl.h b/src/crypto_impl/openssl.h
@@ -0,0 +1,26 @@
+#ifndef SRC_CRYPTO_IMPL_OPENSSL_H_
+#define SRC_CRYPTO_IMPL_OPENSSL_H_
+
+#include "../node_crypto.h"
+
+namespace node {
+namespace crypto {
+
+class OpenSSL : public Crypto {
+ public:
+  OpenSSL();
+  ~OpenSSL();
+  const std::string Version();
+  const std::string Name() { return "openssl"; }
+  void UseExtraCaCerts(const std::string& file);
+  v8::EntropySource GetEntropySource();
+  bool HasSNI();
+  bool HasNPN();
+  bool HasALPN();
+  bool HasOCSP();
+};
+
+}  //  namespace crypto
+}  //  namespace node
+
+#endif  //  SRC_CRYPTO_IMPL_OPENSSL_H_
diff --git a/src/crypto_impl/openssl_1_0_2e.cc b/src/crypto_impl/openssl_1_0_2e.cc
@@ -0,0 +1,71 @@
+#include "openssl.h"
+#include "../node_crypto_factory.h"
+#include "node_crypto.h"
+
+namespace node {
+namespace crypto {
+
+constexpr char version_[] = "openssl_1_0_2e";
+
+const std::string OpenSSL::Version() {
+  return version_;
+}
+
+void Crypto::RegisterCrypto() {
+  CryptoFactory::Register(version_, []() -> Crypto* {
+     return new OpenSSL();
+  });
+}
+
+void Crypto::UnregisterCrypto() {
+  CryptoFactory::Unregister(version_);
+}
+
+bool OpenSSL::HasSNI() {
+#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
+    return true;
+#else
+    return false;
+#endif
+}
+
+bool OpenSSL::HasNPN() {
+#ifndef OPENSSL_NO_NEXTPROTONEG
+    return true;
+#else
+    return false;
+#endif
+}
+
+bool OpenSSL::HasALPN() {
+#ifndef TLSEXT_TYPE_application_layer_protocol_negotiation
+    return true;
+#else
+    return false;
+#endif
+}
+
+bool OpenSSL::HasOCSP() {
+#if !defined(OPENSSL_NO_TLSEXT) && defined(SSL_CTX_set_tlsext_status_cb)
+    return true;
+#else
+    return false;
+#endif
+}
+
+OpenSSL::OpenSSL() {
+}
+
+OpenSSL::~OpenSSL() {
+}
+
+void OpenSSL::UseExtraCaCerts(const std::string& file) {
+  crypto::UseExtraCaCerts(file);
+}
+
+v8::EntropySource OpenSSL::GetEntropySource() {
+  return crypto::EntropySource;
+}
+
+}  //  namespace crypto
+}  //  namespace node
diff --git a/src/node.cc b/src/node.cc
@@ -27,13 +27,14 @@
 #include "node_internals.h"
 #include "node_revert.h"
 #include "node_debug_options.h"
+#include <iostream>
 
 #if defined HAVE_PERFCTR
 #include "node_counters.h"
 #endif
 
 #if HAVE_OPENSSL
-#include "crypto_impl/node_crypto.h"
+#include "node_crypto_factory.h"
 #endif
 
 #if defined(NODE_HAVE_I18N_SUPPORT)
@@ -208,6 +209,12 @@ bool enable_fips_crypto = false;
 bool force_fips_crypto = false;
 # endif  // NODE_FIPS_MODE
 std::string openssl_config;  // NOLINT(runtime/string)
+
+#define MACRO_VALUE(s) TO_STRING(s)
+#define TO_STRING(s) #s
+
+//  NOLINTNEXTLINE(runtime/string)
+std::string crypto_version = MACRO_VALUE(NODE_CRYPTO_VERSION);
 #endif  // HAVE_OPENSSL
 
 // true if process warnings should be suppressed
@@ -3017,33 +3024,19 @@ static Local<Object> GetFeatures(Environment* env) {
   // TODO(bnoordhuis) ping libuv
   obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "ipv6"), True(env->isolate()));
 
-#ifndef OPENSSL_NO_NEXTPROTONEG
-  Local<Boolean> tls_npn = True(env->isolate());
-#else
-  Local<Boolean> tls_npn = False(env->isolate());
-#endif
-  obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_npn"), tls_npn);
+  auto crypto = crypto::CryptoFactory::Get(crypto_version, env);
 
-#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
-  Local<Boolean> tls_alpn = True(env->isolate());
-#else
-  Local<Boolean> tls_alpn = False(env->isolate());
-#endif
-  obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_alpn"), tls_alpn);
+  obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_npn"),
+      Boolean::New(env->isolate(), crypto->HasNPN()));
 
-#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
-  Local<Boolean> tls_sni = True(env->isolate());
-#else
-  Local<Boolean> tls_sni = False(env->isolate());
-#endif
-  obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_sni"), tls_sni);
+  obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_alpn"),
+      Boolean::New(env->isolate(), crypto->HasALPN()));
 
-#if !defined(OPENSSL_NO_TLSEXT) && defined(SSL_CTX_set_tlsext_status_cb)
-  Local<Boolean> tls_ocsp = True(env->isolate());
-#else
-  Local<Boolean> tls_ocsp = False(env->isolate());
-#endif  // !defined(OPENSSL_NO_TLSEXT) && defined(SSL_CTX_set_tlsext_status_cb)
-  obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_ocsp"), tls_ocsp);
+  obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_sni"),
+      Boolean::New(env->isolate(), crypto->HasSNI()));
+
+  obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls_ocsp"),
+      Boolean::New(env->isolate(), crypto->HasOCSP()));
 
   obj->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "tls"),
            Boolean::New(env->isolate(),
@@ -3225,25 +3218,12 @@ void SetupProcessObject(Environment* env,
                                  v8::kPromiseHandlerAddedAfterReject));
 
 #if HAVE_OPENSSL
-  // Stupid code to slice out the version string.
   {  // NOLINT(whitespace/braces)
-    size_t i, j, k;
-    int c;
-    for (i = j = 0, k = sizeof(OPENSSL_VERSION_TEXT) - 1; i < k; ++i) {
-      c = OPENSSL_VERSION_TEXT[i];
-      if ('0' <= c && c <= '9') {
-        for (j = i + 1; j < k; ++j) {
-          c = OPENSSL_VERSION_TEXT[j];
-          if (c == ' ')
-            break;
-        }
-        break;
-      }
-    }
+    auto crypto = crypto::CryptoFactory::Get(crypto_version, env);
     READONLY_PROPERTY(
         versions,
-        "openssl",
-        OneByteString(env->isolate(), &OPENSSL_VERSION_TEXT[i], j - i));
+        crypto->Name().c_str(),
+        OneByteString(env->isolate(), crypto->Version().c_str()));
   }
 #endif
 
@@ -4619,19 +4599,21 @@ int Start(int argc, char** argv) {
   Init(&argc, const_cast<const char**>(argv), &exec_argc, &exec_argv);
 
 #if HAVE_OPENSSL
+  auto crypto = crypto::CryptoFactory::Get(crypto_version);
   {
     std::string extra_ca_certs;
-    if (SafeGetenv("NODE_EXTRA_CA_CERTS", &extra_ca_certs))
-      crypto::UseExtraCaCerts(extra_ca_certs);
+    if (SafeGetenv("NODE_EXTRA_CA_CERTS", &extra_ca_certs)) {
+      crypto->UseExtraCaCerts(extra_ca_certs);
+    }
   }
 #ifdef NODE_FIPS_MODE
   // In the case of FIPS builds we should make sure
   // the random source is properly initialized first.
-  OPENSSL_init();
+  crypto->init();
 #endif  // NODE_FIPS_MODE
   // V8 on Windows doesn't have a good source of entropy. Seed it from
   // OpenSSL's pool.
-  V8::SetEntropySource(crypto::EntropySource);
+  V8::SetEntropySource(crypto->GetEntropySource());
 #endif  // HAVE_OPENSSL
 
   v8_platform.Initialize(v8_thread_pool_size);

diff --git a/src/node_crypto.h b/src/node_crypto.h
@@ -0,0 +1,56 @@
+#ifndef SRC_NODE_CRYPTO_H_
+#define SRC_NODE_CRYPTO_H_
+#include <string>
+#include "v8.h"  // NOLINT(build/include_order)
+
+namespace node {
+namespace crypto {
+
+class Crypto {
+ public:
+  virtual ~Crypto() {}
+  __attribute__((constructor)) virtual void RegisterCrypto();
+  __attribute__((destructor)) virtual void UnregisterCrypto();
+  /**
+   * Allows a crypto implementation to do any additional initializations
+   * required.
+   */
+  virtual void Init() {}
+  /**
+   */
+  virtual void UseExtraCaCerts(const std::string& file) = 0;
+  /**
+   */
+  virtual v8::EntropySource GetEntropySource() = 0;
+  /**
+   * The version of the underlying Crypto library
+   */
+  virtual const std::string Version() = 0;
+  /**
+   * The name of this crypto implemenetion.
+   * This is currently used as the attribute name on the process object.
+   */
+  virtual const std::string Name() = 0;
+  /**
+   * Returns true if TLS Name Server Indication (SNI) is supported.
+   */
+  virtual bool HasSNI() = 0;
+  /**
+   * Returns true if the TLS Next Protocol Negotiation extension is supprted.
+   */
+  virtual bool HasNPN() = 0;
+  /**
+   * Returns true if the TLS Application-Layer Protocol Negotiation extension
+   * is supprted.
+   */
+  virtual bool HasALPN() = 0;
+  /**
+   * Returns true if Online Certificate Status Protocol is supported.
+   */
+  virtual bool HasOCSP() = 0;
+};
+
+}  //  namespace crypto
+}  //  namespace node
+
+#endif  //  SRC_NODE_CRYPTO_H_
diff --git a/src/node_crypto_factory.cc b/src/node_crypto_factory.cc
@@ -0,0 +1,46 @@
+#include "node_crypto_factory.h"
+#include "node_crypto.h"
+#include "env.h"
+#include "env-inl.h"
+#include <string>
+#include <iostream>
+
+namespace node {
+namespace crypto {
+
+CryptoFactory::CallbackMap CryptoFactory::crypto_libs;
+
+typedef std::pair<const std::string, std::shared_ptr<Crypto>> CryptoPair;
+
+void CryptoFactory::Register(const std::string& version,
+                                   CreateCallback cb) {
+  if (!crypto_libs.count(version)) {
+    std::shared_ptr<Crypto> crypto{cb()};
+    crypto_libs.insert(CryptoPair(version, crypto));
+  }
+}
+
+void CryptoFactory::Unregister(const std::string& version) {
+  crypto_libs.erase(version);
+}
+
+std::shared_ptr<Crypto> CryptoFactory::Get(const std::string& version,
+                                           Environment* env) {
+  CallbackMap::iterator it = crypto_libs.find(version);
+  if (it != crypto_libs.end()) {
+    return it->second;
+  }
+
+  if (env == nullptr) {
+    fprintf(stderr,
+            "node: could not find a crypto implementation for %s",
+            version.c_str());
+    exit(9);
+  }
+  env->ThrowError(
+      ("Could not find a crypto implementation for " + version).c_str());
+  return nullptr;
+}
+
+}  //  namespace crypto
+}  //  namespace node