Openssl clientcertengine support2

doc/api/errors.md

@@ -653,6 +653,12 @@ Used when `Console` is instantiated without `stdout` stream or when `stdout` or
 
 Used when the native call from `process.cpuUsage` cannot be processed properly.
 
+<a id="ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED"></a>
+### ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED
+
+Used when a client certificate engine is requested that is not supported by the
+version of OpenSSL being used.
+
 <a id="ERR_CRYPTO_ECDH_INVALID_FORMAT"></a>
 ### ERR_CRYPTO_ECDH_INVALID_FORMAT
 

doc/api/https.md

@@ -150,6 +150,9 @@ Global instance of [`https.Agent`][] for all HTTPS client requests.
 <!-- YAML
 added: v0.3.6
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/6569
+    description: The `options` parameter can now include `clientCertEngine`.
   - version: v7.5.0
     pr-url: https://github.com/nodejs/node/pull/10638
     description: The `options` parameter can be a WHATWG `URL` object.
@@ -164,9 +167,9 @@ changes:
 
 Makes a request to a secure web server.
 
-The following additional `options` from [`tls.connect()`][] are also accepted when using a
-  custom [`Agent`][]:
-  `pfx`, `key`, `passphrase`, `cert`, `ca`, `ciphers`, `rejectUnauthorized`, `secureProtocol`, `servername`
+The following additional `options` from [`tls.connect()`][] are also accepted
+when using a custom [`Agent`][]: `ca`, `cert`, `ciphers`, `clientCertEngine`,
+`key`, `passphrase`, `pfx`, `rejectUnauthorized`, `secureProtocol`, `servername`
 
 `options` can be an object, a string, or a [`URL`][] object. If `options` is a
 string, it is automatically parsed with [`url.parse()`][]. If it is a [`URL`][]

doc/api/tls.md

@@ -905,6 +905,9 @@ port or host argument.
 <!-- YAML
 added: v0.11.13
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/6569
+    description: The `options` parameter can now include `clientCertEngine`.
   - version: v7.3.0
     pr-url: https://github.com/nodejs/node/pull/10294
     description: If the `key` option is an array, individual entries do not
@@ -959,8 +962,6 @@ changes:
     certificate can match or chain to.
     For self-signed certificates, the certificate is its own CA, and must be
     provided.
-  * `crl` {string|string[]|Buffer|Buffer[]} Optional PEM formatted
-    CRLs (Certificate Revocation Lists).
   * `ciphers` {string} Optional cipher suite specification, replacing the
     default.  For more information, see [modifying the default cipher suite][].
   * `honorCipherOrder` {boolean} Attempt to use the server's cipher suite
@@ -974,20 +975,24 @@ changes:
     [`crypto.getCurves()`][] to obtain a list of available curve names. On
     recent releases, `openssl ecparam -list_curves` will also display the name
     and description of each available elliptic curve.
+  * `clientCertEngine` {string} Optional name of an OpenSSL engine which can
+    provide the client certificate.
+  * `crl` {string|string[]|Buffer|Buffer[]} Optional PEM formatted
+    CRLs (Certificate Revocation Lists).
   * `dhparam` {string|Buffer} Diffie Hellman parameters, required for
     [Perfect Forward Secrecy][]. Use `openssl dhparam` to create the parameters.
     The key length must be greater than or equal to 1024 bits, otherwise an
     error will be thrown. It is strongly recommended to use 2048 bits or larger
     for stronger security. If omitted or invalid, the parameters are silently
     discarded and DHE ciphers will not be available.
-  * `secureProtocol` {string} Optional SSL method to use, default is
-    `"SSLv23_method"`. The possible values are listed as [SSL_METHODS][], use
-    the function names as strings. For example, `"SSLv3_method"` to force SSL
-    version 3.
   * `secureOptions` {number} Optionally affect the OpenSSL protocol behavior,
     which is not usually necessary. This should be used carefully if at all!
     Value is a numeric bitmask of the `SSL_OP_*` options from
     [OpenSSL Options][].
+  * `secureProtocol` {string} Optional SSL method to use, default is
+    `"SSLv23_method"`. The possible values are listed as [SSL_METHODS][], use
+    the function names as strings. For example, `"SSLv3_method"` to force SSL
+    version 3.
   * `sessionIdContext` {string} Optional opaque identifier used by servers to
     ensure session state is not shared between applications. Unused by clients.
 
@@ -1015,6 +1020,9 @@ publicly trusted list of CAs as given in
 <!-- YAML
 added: v0.3.2
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/6569
+    description: The `options` parameter can now include `clientCertEngine`.
   - version: v8.0.0
     pr-url: https://github.com/nodejs/node/pull/11984
     description: The `ALPNProtocols` and `NPNProtocols` options can
@@ -1025,6 +1033,8 @@ changes:
 -->
 
 * `options` {Object}
+  * `clientCertEngine` {string} Optional name of an OpenSSL engine which can
+    provide the client certificate.
   * `handshakeTimeout` {number} Abort the connection if the SSL/TLS handshake
     does not finish in the specified number of milliseconds. Defaults to `120`
     seconds. A `'tlsClientError'` is emitted on the `tls.Server` object whenever

lib/_tls_common.js

@@ -196,6 +196,18 @@ exports.createSecureContext = function createSecureContext(options, context) {
     c.context.setFreeListLength(0);
   }
 
+  if (typeof options.clientCertEngine === 'string') {
+    if (c.context.setClientCertEngine)
+      c.context.setClientCertEngine(options.clientCertEngine);
+    else
+      throw new errors.Error('ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED');
+  } else if (options.clientCertEngine != null) {
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
+                               'options.clientCertEngine',
+                               ['string', 'null', 'undefined'],
+                               options.clientCertEngine);
+  }
+
   return c;
 };
 

lib/_tls_wrap.js

@@ -816,6 +816,7 @@ function tlsConnectionListener(rawSocket) {
 // - rejectUnauthorized. Boolean, default to true.
 // - key. string.
 // - cert: string.
+// - clientCertEngine: string.
 // - ca: string or array of strings.
 // - sessionTimeout: integer.
 //
@@ -859,6 +860,7 @@ function Server(options, listener) {
     key: this.key,
     passphrase: this.passphrase,
     cert: this.cert,
+    clientCertEngine: this.clientCertEngine,
     ca: this.ca,
     ciphers: this.ciphers,
     ecdhCurve: this.ecdhCurve,
@@ -931,6 +933,8 @@ Server.prototype.setOptions = function(options) {
   if (options.key) this.key = options.key;
   if (options.passphrase) this.passphrase = options.passphrase;
   if (options.cert) this.cert = options.cert;
+  if (options.clientCertEngine)
+    this.clientCertEngine = options.clientCertEngine;
   if (options.ca) this.ca = options.ca;
   if (options.secureProtocol) this.secureProtocol = options.secureProtocol;
   if (options.crl) this.crl = options.crl;

lib/https.js

@@ -160,6 +160,10 @@ Agent.prototype.getName = function getName(options) {
   if (options.cert)
     name += options.cert;
 
+  name += ':';
+  if (options.clientCertEngine)
+    name += options.clientCertEngine;
+
   name += ':';
   if (options.ciphers)
     name += options.ciphers;

lib/internal/errors.js

@@ -232,6 +232,8 @@ E('ERR_CHILD_CLOSED_BEFORE_REPLY', 'Child closed before reply received');
 E('ERR_CONSOLE_WRITABLE_STREAM',
   'Console expects a writable stream instance for %s');
 E('ERR_CPU_USAGE', 'Unable to obtain cpu usage %s');
+E('ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED',
+  'Custom engines not supported by this OpenSSL');
 E('ERR_CRYPTO_ECDH_INVALID_FORMAT', 'Invalid ECDH format: %s');
 E('ERR_CRYPTO_ENGINE_UNKNOWN', 'Engine "%s" was not found');
 E('ERR_CRYPTO_FIPS_FORCED',

src/node_crypto.cc

@@ -230,6 +230,41 @@ static int PasswordCallback(char *buf, int size, int rwflag, void *u) {
   return 0;
 }
 
+// Loads OpenSSL engine by engine id and returns it. The loaded engine
+// gets a reference so remember the corresponding call to ENGINE_free.
+// In case of error the appropriate js exception is scheduled
+// and nullptr is returned.
+#ifndef OPENSSL_NO_ENGINE
+static ENGINE* LoadEngineById(const char* engine_id, char (*errmsg)[1024]) {
+  MarkPopErrorOnReturn mark_pop_error_on_return;
+
+  ENGINE* engine = ENGINE_by_id(engine_id);
+
+  if (engine == nullptr) {
+    // Engine not found, try loading dynamically.
+    engine = ENGINE_by_id("dynamic");
+    if (engine != nullptr) {
+      if (!ENGINE_ctrl_cmd_string(engine, "SO_PATH", engine_id, 0) ||
+          !ENGINE_ctrl_cmd_string(engine, "LOAD", nullptr, 0)) {
+        ENGINE_free(engine);
+        engine = nullptr;
+      }
+    }
+  }
+
+  if (engine == nullptr) {
+    int err = ERR_get_error();
+    if (err != 0) {
+      ERR_error_string_n(err, *errmsg, sizeof(*errmsg));
+    } else {
+      snprintf(*errmsg, sizeof(*errmsg),
+               "Engine \"%s\" was not found", engine_id);
+    }
+  }
+
+  return engine;
+}
+#endif  // !OPENSSL_NO_ENGINE
 
 // This callback is used to avoid the default passphrase callback in OpenSSL
 // which will typically prompt for the passphrase. The prompting is designed
@@ -374,6 +409,10 @@ void SecureContext::Initialize(Environment* env, Local<Object> target) {
                       SecureContext::SetSessionTimeout);
   env->SetProtoMethod(t, "close", SecureContext::Close);
   env->SetProtoMethod(t, "loadPKCS12", SecureContext::LoadPKCS12);
+#ifndef OPENSSL_NO_ENGINE
+  env->SetProtoMethod(t, "setClientCertEngine",
+                      SecureContext::SetClientCertEngine);
+#endif  // !OPENSSL_NO_ENGINE
   env->SetProtoMethod(t, "getTicketKeys", SecureContext::GetTicketKeys);
   env->SetProtoMethod(t, "setTicketKeys", SecureContext::SetTicketKeys);
   env->SetProtoMethod(t, "setFreeListLength", SecureContext::SetFreeListLength);
@@ -1176,6 +1215,46 @@ void SecureContext::LoadPKCS12(const FunctionCallbackInfo<Value>& args) {
 }
 
 
+#ifndef OPENSSL_NO_ENGINE
+void SecureContext::SetClientCertEngine(
+    const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  CHECK_EQ(args.Length(), 1);
+  CHECK(args[0]->IsString());
+
+  SecureContext* sc = Unwrap<SecureContext>(args.This());
+
+  MarkPopErrorOnReturn mark_pop_error_on_return;
+
+  // SSL_CTX_set_client_cert_engine does not itself support multiple
+  // calls by cleaning up before overwriting the client_cert_engine
+  // internal context variable.
+  // Instead of trying to fix up this problem we in turn also do not
+  // support multiple calls to SetClientCertEngine.
+  if (sc->client_cert_engine_provided_) {
+    return env->ThrowError(
+        "Multiple calls to SetClientCertEngine are not allowed");
+  }
+
+  const node::Utf8Value engine_id(env->isolate(), args[0]);
+  char errmsg[1024];
+  ENGINE* engine = LoadEngineById(*engine_id, &errmsg);
+
+  if (engine == nullptr) {
+    return env->ThrowError(errmsg);
+  }
+
+  int r = SSL_CTX_set_client_cert_engine(sc->ctx_, engine);
+  // Free reference (SSL_CTX_set_client_cert_engine took it via ENGINE_init).
+  ENGINE_free(engine);
+  if (r == 0) {
+    return ThrowCryptoError(env, ERR_get_error());
+  }
+  sc->client_cert_engine_provided_ = true;
+}
+#endif  // !OPENSSL_NO_ENGINE
+
+
 void SecureContext::GetTicketKeys(const FunctionCallbackInfo<Value>& args) {
 #if !defined(OPENSSL_NO_TLSEXT) && defined(SSL_CTX_get_tlsext_ticket_keys)
 
@@ -5881,20 +5960,10 @@ void SetEngine(const FunctionCallbackInfo<Value>& args) {
 
   ClearErrorOnReturn clear_error_on_return;
 
+  // Load engine.
   const node::Utf8Value engine_id(env->isolate(), args[0]);
-  ENGINE* engine = ENGINE_by_id(*engine_id);
-
-  // Engine not found, try loading dynamically
-  if (engine == nullptr) {
-    engine = ENGINE_by_id("dynamic");
-    if (engine != nullptr) {
-      if (!ENGINE_ctrl_cmd_string(engine, "SO_PATH", *engine_id, 0) ||
-          !ENGINE_ctrl_cmd_string(engine, "LOAD", nullptr, 0)) {
-        ENGINE_free(engine);
-        engine = nullptr;
-      }
-    }
-  }
+  char errmsg[1024];
+  ENGINE* engine = LoadEngineById(*engine_id, &errmsg);
 
   if (engine == nullptr) {
     int err = ERR_get_error();

src/node_crypto.h

@@ -95,6 +95,9 @@ class SecureContext : public BaseObject {
   SSL_CTX* ctx_;
   X509* cert_;
   X509* issuer_;
+#ifndef OPENSSL_NO_ENGINE
+  bool client_cert_engine_provided_ = false;
+#endif  // !OPENSSL_NO_ENGINE
 
   static const int kMaxSessionSize = 10 * 1024;
 
@@ -125,6 +128,10 @@ class SecureContext : public BaseObject {
       const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Close(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void LoadPKCS12(const v8::FunctionCallbackInfo<v8::Value>& args);
+#ifndef OPENSSL_NO_ENGINE
+  static void SetClientCertEngine(
+      const v8::FunctionCallbackInfo<v8::Value>& args);
+#endif  // !OPENSSL_NO_ENGINE
   static void GetTicketKeys(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void SetTicketKeys(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void SetFreeListLength(

test/addons/openssl-client-cert-engine/binding.gyp

@@ -0,0 +1,24 @@
+{
+  'targets': [
+    {
+      'target_name': 'testengine',
+      'type': 'none',
+      'conditions': [
+        ['OS=="mac" and '
+         'node_use_openssl=="true" and '
+         'node_shared=="false" and '
+         'node_shared_openssl=="false"', {
+          'type': 'shared_library',
+          'sources': [ 'testengine.cc' ],
+          'product_extension': 'engine',
+          'include_dirs': ['../../../deps/openssl/openssl/include'],
+          'link_settings': {
+            'libraries': [
+              '../../../../out/<(PRODUCT_DIR)/<(OPENSSL_PRODUCT)'
+            ]
+          },
+        }]
+      ]
+    }
+  ]
+}

test/addons/openssl-client-cert-engine/test.js

@@ -0,0 +1,62 @@
+'use strict';
+const common = require('../../common');
+const fixture = require('../../common/fixtures');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const fs = require('fs');
+const path = require('path');
+
+const engine = path.join(__dirname,
+                         `/build/${common.buildType}/testengine.engine`);
+
+if (!fs.existsSync(engine))
+  common.skip('no client cert engine');
+
+const assert = require('assert');
+const https = require('https');
+
+const agentKey = fs.readFileSync(fixture.path('/keys/agent1-key.pem'));
+const agentCert = fs.readFileSync(fixture.path('/keys/agent1-cert.pem'));
+const agentCa = fs.readFileSync(fixture.path('/keys/ca1-cert.pem'));
+
+const port = common.PORT;
+
+const serverOptions = {
+  key: agentKey,
+  cert: agentCert,
+  ca: agentCa,
+  requestCert: true,
+  rejectUnauthorized: true
+};
+
+const server = https.createServer(serverOptions, (req, res) => {
+  res.writeHead(200);
+  res.end('hello world');
+}).listen(port, common.localhostIPv4, () => {
+  const clientOptions = {
+    method: 'GET',
+    host: common.localhostIPv4,
+    port: port,
+    path: '/test',
+    clientCertEngine: engine,  // engine will provide key+cert
+    rejectUnauthorized: false, // prevent failing on self-signed certificates
+    headers: {}
+  };
+
+  const req = https.request(clientOptions, common.mustCall(function(response) {
+    let body = '';
+    response.setEncoding('utf8');
+    response.on('data', function(chunk) {
+      body += chunk;
+    });
+
+    response.on('end', common.mustCall(function() {
+      assert.strictEqual(body, 'hello world');
+      server.close();
+    }));
+  }));
+
+  req.end();
+});