From 498e189074f5185ad6c0f5f2feb873c439112634 Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Fri, 15 Apr 2016 16:49:36 +0200 Subject: [PATCH 01/17] implemented clientCertEngine option to allow using openssl client certificate engines with https.request --- lib/_tls_common.js | 4 + lib/_tls_wrap.js | 3 + lib/https.js | 4 + src/node_crypto.cc | 51 +++++ src/node_crypto.h | 3 + .../openssl-client-cert-engine/binding.cc | 43 ++++ .../openssl-client-cert-engine/binding.gyp | 14 ++ .../addons/openssl-client-cert-engine/test.js | 75 +++++++ .../openssl-client-cert-engine/testengine.cc | 205 ++++++++++++++++++ 9 files changed, 402 insertions(+) create mode 100644 test/addons/openssl-client-cert-engine/binding.cc create mode 100644 test/addons/openssl-client-cert-engine/binding.gyp create mode 100644 test/addons/openssl-client-cert-engine/test.js create mode 100644 test/addons/openssl-client-cert-engine/testengine.cc diff --git a/lib/_tls_common.js b/lib/_tls_common.js index d75b9ffe28188d..dc7f958ef87b1f 100644 --- a/lib/_tls_common.js +++ b/lib/_tls_common.js @@ -146,6 +146,10 @@ exports.createSecureContext = function createSecureContext(options, context) { c.context.setFreeListLength(0); } + if (options.clientCertEngine) { + c.context.setClientCertEngine(options.clientCertEngine); + } + return c; }; diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index c2ecd07a4b77bc..010201e503df76 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -707,6 +707,7 @@ TLSSocket.prototype.getProtocol = function() { // - rejectUnauthorized. Boolean, default to true. // - key. string. // - cert: string. +// - clientCertEngine: string. // - ca: string or array of strings. // - sessionTimeout: integer. // @@ -751,6 +752,7 @@ function Server(/* [options], listener */) { key: self.key, passphrase: self.passphrase, cert: self.cert, + clientCertEngine: self.clientCertEngine, ca: self.ca, ciphers: self.ciphers, ecdhCurve: self.ecdhCurve, @@ -882,6 +884,7 @@ 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; diff --git a/lib/https.js b/lib/https.js index b8969b68452451..c3c3893326075b 100644 --- a/lib/https.js +++ b/lib/https.js @@ -123,6 +123,10 @@ Agent.prototype.getName = function(options) { if (options.cert) name += options.cert; + name += ':'; + if (options.clientCertEngine) + name += options.clientCertEngine; + name += ':'; if (options.ciphers) name += options.ciphers; diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 9cf216f2d60440..2312b6b50e2453 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -289,6 +289,9 @@ void SecureContext::Initialize(Environment* env, Local 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); @@ -1024,6 +1027,54 @@ void SecureContext::LoadPKCS12(const FunctionCallbackInfo& args) { } +#ifndef OPENSSL_NO_ENGINE +void SecureContext::SetClientCertEngine(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + CHECK(args.Length() == 1 && args[0]->IsString()); + + SecureContext* sc = Unwrap(args.This()); + + // If an engine was previously set, free it. + if (sc->ctx_->client_cert_engine != NULL) { + ENGINE* e = sc->ctx_->client_cert_engine; + sc->ctx_->client_cert_engine = NULL; + ENGINE_free(e); + } + + 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; + } + } + } + + if (engine == nullptr) { + int err = ERR_get_error(); + if (err == 0) { + char tmp[1024]; + snprintf(tmp, sizeof(tmp), "Engine \"%s\" was not found", *engine_id); + return env->ThrowError(tmp); + } else { + return ThrowCryptoError(env, err); + } + } + + int r = SSL_CTX_set_client_cert_engine(sc->ctx_, engine); + ENGINE_free(engine); + if (r == 0) + return ThrowCryptoError(env, ERR_get_error()); +} +#endif // !OPENSSL_NO_ENGINE + + void SecureContext::GetTicketKeys(const FunctionCallbackInfo& args) { #if !defined(OPENSSL_NO_TLSEXT) && defined(SSL_CTX_get_tlsext_ticket_keys) diff --git a/src/node_crypto.h b/src/node_crypto.h index 24ac77365cf455..d62583812e5a85 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -110,6 +110,9 @@ class SecureContext : public BaseObject { const v8::FunctionCallbackInfo& args); static void Close(const v8::FunctionCallbackInfo& args); static void LoadPKCS12(const v8::FunctionCallbackInfo& args); +#ifndef OPENSSL_NO_ENGINE + static void SetClientCertEngine(const v8::FunctionCallbackInfo& args); +#endif // !OPENSSL_NO_ENGINE static void GetTicketKeys(const v8::FunctionCallbackInfo& args); static void SetTicketKeys(const v8::FunctionCallbackInfo& args); static void SetFreeListLength( diff --git a/test/addons/openssl-client-cert-engine/binding.cc b/test/addons/openssl-client-cert-engine/binding.cc new file mode 100644 index 00000000000000..5210de298b8b39 --- /dev/null +++ b/test/addons/openssl-client-cert-engine/binding.cc @@ -0,0 +1,43 @@ +#undef NDEBUG +#include +#include +#include +#include + +using node::AtExit; +using v8::HandleScope; +using v8::Isolate; +using v8::Local; +using v8::Object; + +static char cookie[] = "yum yum"; +static int at_exit_cb1_called = 0; +static int at_exit_cb2_called = 0; + +static void at_exit_cb1(void* arg) { + Isolate* isolate = static_cast(arg); + HandleScope handle_scope(isolate); + Local obj = Object::New(isolate); + assert(!obj.IsEmpty()); // Assert VM is still alive. + assert(obj->IsObject()); + at_exit_cb1_called++; +} + +static void at_exit_cb2(void* arg) { + assert(arg == static_cast(cookie)); + at_exit_cb2_called++; +} + +static void sanity_check(void) { + assert(at_exit_cb1_called == 1); + assert(at_exit_cb2_called == 2); +} + +void init(Local target) { + AtExit(at_exit_cb1, target->CreationContext()->GetIsolate()); + AtExit(at_exit_cb2, cookie); + AtExit(at_exit_cb2, cookie); + atexit(sanity_check); +} + +NODE_MODULE(binding, init); diff --git a/test/addons/openssl-client-cert-engine/binding.gyp b/test/addons/openssl-client-cert-engine/binding.gyp new file mode 100644 index 00000000000000..cf7d1fcff21672 --- /dev/null +++ b/test/addons/openssl-client-cert-engine/binding.gyp @@ -0,0 +1,14 @@ +{ + 'targets': [ + { + 'target_name': 'binding', + 'sources': [ 'binding.cc' ] + }, + { + 'target_name': 'testengine', + 'type': 'shared_library', + 'sources': [ 'testengine.cc' ], + 'include_dirs': [ '../../../deps/openssl/openssl/include' ] + } + ] +} diff --git a/test/addons/openssl-client-cert-engine/test.js b/test/addons/openssl-client-cert-engine/test.js new file mode 100644 index 00000000000000..4051d4b9d2721b --- /dev/null +++ b/test/addons/openssl-client-cert-engine/test.js @@ -0,0 +1,75 @@ +const assert = require('assert'); +const https = require('https'); +const fs = require('fs'); +const path = require('path'); + +var engine = path.dirname(module.filename) + "/build/Release/testengine.so"; +//process.env.OPENSSL_ENGINES = path.dirname(module.filename) + "/build/Release"; + +var agentKey = fs.readFileSync('test/fixtures/keys/agent1-key.pem'); +var agentCert = fs.readFileSync('test/fixtures/keys/agent1-cert.pem'); +var agentCa = fs.readFileSync('test/fixtures/keys/ca1-cert.pem'); + +var port = 18020; + +const serverOptions = { + key: agentKey, + cert: agentCert, + ca: agentCa, + requestCert: true, + rejectUnauthorized: true +}; + +var server = https.createServer(serverOptions, (req, res) => { + res.writeHead(200); + res.end('hello world'); +}).listen(port); + +function testFailed(message) +{ + server.close(); + assert(false, message); +} + +function testPassed() +{ + console.log("Test passed!"); + server.close(); +} + +var clientOptions = +{ + method: 'GET', + host: '127.0.0.1', + port: port, + path: '/test', +// key: agentKey, +// cert: agentCert, + clientCertEngine: engine, // engine will provide key+cert + rejectUnauthorized: false, // prevent failing on self-signed certificates + headers: {} +}; + +console.log("name:<" + https.globalAgent.getName(clientOptions)+">"); + +var req = https.request(clientOptions, (response) => { + var body = ''; + response.setEncoding('utf8'); + response.on('data', (chunk) => { + body += chunk; + }); + + response.on('end', () => { + if( body=='hello world' ) { + testPassed(); + } else { + testFailed("unexpected body: <"+body+">"); + } + }); +}); + +req.on('error', (e) => { + testFailed("request error: "+e.message); +}); + +req.end(); diff --git a/test/addons/openssl-client-cert-engine/testengine.cc b/test/addons/openssl-client-cert-engine/testengine.cc new file mode 100644 index 00000000000000..2f68830d95dc57 --- /dev/null +++ b/test/addons/openssl-client-cert-engine/testengine.cc @@ -0,0 +1,205 @@ +#undef NDEBUG +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#ifndef ENGINE_CMD_BASE +# error did not get engine.h +#endif + +#define TEST_ENGINE_ID "testengine" +#define TEST_ENGINE_NAME "dummy test engine" + +#define AGENT_KEY "test/fixtures/keys/agent1-key.pem" +#define AGENT_CERT "test/fixtures/keys/agent1-cert.pem" + +static int engineInit(ENGINE* engine) +{ + printf("engineInit\n"); + return 1; +} + +static int engineFinish(ENGINE* engine) +{ + printf("engineFinish"); + return 1; +} + +static int engineDestroy(ENGINE* engine) +{ + printf("engineDestroy"); + return 1; +} + +//static int engineCtrl(ENGINE* engine, int cmd, long i, void *p, void (*f) ()) +//{ +// return 1; +//} + +//static EVP_PKEY* engineLoadPrivateKey(ENGINE* engine, const char* s_key_id, UI_METHOD* ui_method, void* cb_data) +//{ +// return NULL; +//} + +//static EVP_PKEY* engineLoadPublicKey(ENGINE* engine, const char* s_key_id, UI_METHOD* ui_method, void* cb_data) +//{ +// return NULL; +//} + +static std::string loadFile(const char *filename) +{ + std::string ret; + FILE *fp = fopen(filename, "r"); + if( fp==NULL ) + { + printf("Could not open file '%s'\n", filename); + return ret; + } + + char buf[1024]; + while( true ) + { + size_t r = fread(buf, 1, 1024, fp); + if( r>0 ) + { + ret.append(buf, r); + } + if( r<1024 ) + { + break; + } + } + + fclose(fp); + return ret; +} + + +static int engineLoadSSLClientCert(ENGINE* engine, SSL *ssl, STACK_OF(X509_NAME) *ca_dn, X509 **ppcert, EVP_PKEY **ppkey, STACK_OF(X509) **pother, UI_METHOD *ui_method, void *callback_data) +{ + printf("engineLoadSSLClientCert\n"); + + if( ppcert ) + { + std::string cert = loadFile(AGENT_CERT); + if( cert.size()==0 ) + { + return 0; + } + + BIO *bio = BIO_new_mem_buf((void*)cert.data(), (int)cert.size()); + *ppcert = PEM_read_bio_X509(bio, NULL, NULL, NULL); + BIO_vfree(bio); + if( *ppcert==NULL ) + { + printf("Could not read certificate\n"); + return 0; + } + } + + if( ppkey ) + { + std::string key = loadFile(AGENT_KEY); + if( key.size()==0 ) + { + return 0; + } + + BIO *bio = BIO_new_mem_buf((void*)key.data(), (int)key.size()); + *ppkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL); + BIO_vfree(bio); + if( *ppkey==NULL ) + { + printf("Could not read private key\n"); + return 0; + } + } + + return 1; +} + +/* +static int rsaEncrypt(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, int padding) +{ + return 0; +} + +static int rsaDecrypt(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, int padding) +{ + return 0; +} + +static int rsaSign(int type, const unsigned char *m, unsigned int m_length, unsigned char *sigret, unsigned int *siglen, const RSA *rsa) +{ + return 0; +} + +static int rsaVerify(int type, const unsigned char *m, unsigned int m_length, const unsigned char *sigbuf, unsigned int siglen, const RSA *rsa) +{ + return 0; +} + +static void rsaExDataFree(void *parent, void *ptr, CRYPTO_EX_DATA *ad, int idx, long argl, void *argp) +{ +} + +static int rsaExDataDup(CRYPTO_EX_DATA *to, CRYPTO_EX_DATA *from, void *from_d, int idx, long argl, void *argp) +{ + return 0; +} + +static RSA_METHOD rsaMethods; +static RSA_METHOD* engineRSAMethods() +{ + rsaMethods.rsa_priv_enc = rsaEncrypt; + rsaMethods.rsa_priv_dec = rsaDecrypt; + rsaMethods.rsa_sign = rsaSign; + rsaMethods.rsa_verify = rsaVerify; +// rsaMethods.finish = rsaFinish; + rsaMethods.flags |= RSA_FLAG_SIGN_VER; + return &rsaMethods; +} +*/ + +static int bind_fn(ENGINE * engine, const char *id) +{ + printf("loaded engine: id=%s\n", id); + + if( !ENGINE_set_id(engine, TEST_ENGINE_ID) + || !ENGINE_set_name(engine, TEST_ENGINE_NAME) + || !ENGINE_set_init_function(engine, engineInit) + || !ENGINE_set_finish_function(engine, engineFinish) + || !ENGINE_set_destroy_function(engine, engineDestroy) + || !ENGINE_set_load_ssl_client_cert_function(engine, engineLoadSSLClientCert) +// || !ENGINE_set_load_privkey_function(engine, engineLoadPrivateKey) +// || !ENGINE_set_ctrl_function(engine, engineCtrl) +// || !ENGINE_set_cmd_defns(engine, mCommands) +#ifndef OPENSSL_NO_RSA +// !ENGINE_set_RSA(engine, engineRSAMethods()) +#endif + ) { + printf("failed to setup all engine functions"); + return 0; + } + + return 1; +} + +extern "C" +{ + IMPLEMENT_DYNAMIC_CHECK_FN(); + IMPLEMENT_DYNAMIC_BIND_FN(bind_fn); +} From 0c82a373d665809f423e798a42924cacc464ecde Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Fri, 15 Apr 2016 17:32:52 +0200 Subject: [PATCH 02/17] fixed linting --- lib/_tls_wrap.js | 3 +- src/node_crypto.cc | 6 +- src/node_crypto.h | 3 +- .../addons/openssl-client-cert-engine/test.js | 24 ++- .../openssl-client-cert-engine/testengine.cc | 144 +++++------------- 5 files changed, 53 insertions(+), 127 deletions(-) diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 010201e503df76..969e5179299275 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -884,7 +884,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.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; diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 2312b6b50e2453..23005ebf53f402 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -290,7 +290,8 @@ void SecureContext::Initialize(Environment* env, Local target) { env->SetProtoMethod(t, "close", SecureContext::Close); env->SetProtoMethod(t, "loadPKCS12", SecureContext::LoadPKCS12); #ifndef OPENSSL_NO_ENGINE - env->SetProtoMethod(t, "setClientCertEngine", SecureContext::SetClientCertEngine); + env->SetProtoMethod(t, "setClientCertEngine", + SecureContext::SetClientCertEngine); #endif // !OPENSSL_NO_ENGINE env->SetProtoMethod(t, "getTicketKeys", SecureContext::GetTicketKeys); env->SetProtoMethod(t, "setTicketKeys", SecureContext::SetTicketKeys); @@ -1028,7 +1029,8 @@ void SecureContext::LoadPKCS12(const FunctionCallbackInfo& args) { #ifndef OPENSSL_NO_ENGINE -void SecureContext::SetClientCertEngine(const FunctionCallbackInfo& args) { +void SecureContext::SetClientCertEngine( + const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK(args.Length() == 1 && args[0]->IsString()); diff --git a/src/node_crypto.h b/src/node_crypto.h index d62583812e5a85..34a06d8ea0637f 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -111,7 +111,8 @@ class SecureContext : public BaseObject { static void Close(const v8::FunctionCallbackInfo& args); static void LoadPKCS12(const v8::FunctionCallbackInfo& args); #ifndef OPENSSL_NO_ENGINE - static void SetClientCertEngine(const v8::FunctionCallbackInfo& args); + static void SetClientCertEngine( + const v8::FunctionCallbackInfo& args); #endif // !OPENSSL_NO_ENGINE static void GetTicketKeys(const v8::FunctionCallbackInfo& args); static void SetTicketKeys(const v8::FunctionCallbackInfo& args); diff --git a/test/addons/openssl-client-cert-engine/test.js b/test/addons/openssl-client-cert-engine/test.js index 4051d4b9d2721b..86033b58116068 100644 --- a/test/addons/openssl-client-cert-engine/test.js +++ b/test/addons/openssl-client-cert-engine/test.js @@ -1,10 +1,11 @@ +'use strict'; +require('../../common'); const assert = require('assert'); const https = require('https'); const fs = require('fs'); const path = require('path'); -var engine = path.dirname(module.filename) + "/build/Release/testengine.so"; -//process.env.OPENSSL_ENGINES = path.dirname(module.filename) + "/build/Release"; +var engine = path.dirname(module.filename) + '/build/Release/testengine.so'; var agentKey = fs.readFileSync('test/fixtures/keys/agent1-key.pem'); var agentCert = fs.readFileSync('test/fixtures/keys/agent1-cert.pem'); @@ -27,18 +28,17 @@ var server = https.createServer(serverOptions, (req, res) => { function testFailed(message) { - server.close(); - assert(false, message); + server.close(); + assert(false, message); } function testPassed() { - console.log("Test passed!"); - server.close(); + console.log('Test passed!'); + server.close(); } -var clientOptions = -{ +var clientOptions = { method: 'GET', host: '127.0.0.1', port: port, @@ -50,8 +50,6 @@ var clientOptions = headers: {} }; -console.log("name:<" + https.globalAgent.getName(clientOptions)+">"); - var req = https.request(clientOptions, (response) => { var body = ''; response.setEncoding('utf8'); @@ -60,16 +58,16 @@ var req = https.request(clientOptions, (response) => { }); response.on('end', () => { - if( body=='hello world' ) { + if (body == 'hello world') { testPassed(); } else { - testFailed("unexpected body: <"+body+">"); + testFailed('unexpected body: <' + body + '>'); } }); }); req.on('error', (e) => { - testFailed("request error: "+e.message); + testFailed('request error: ' + e.message); }); req.end(); diff --git a/test/addons/openssl-client-cert-engine/testengine.cc b/test/addons/openssl-client-cert-engine/testengine.cc index 2f68830d95dc57..834a5b8b9a4799 100644 --- a/test/addons/openssl-client-cert-engine/testengine.cc +++ b/test/addons/openssl-client-cert-engine/testengine.cc @@ -26,59 +26,36 @@ #define AGENT_KEY "test/fixtures/keys/agent1-key.pem" #define AGENT_CERT "test/fixtures/keys/agent1-cert.pem" -static int engineInit(ENGINE* engine) -{ +static int engineInit(ENGINE* engine) { printf("engineInit\n"); return 1; } -static int engineFinish(ENGINE* engine) -{ +static int engineFinish(ENGINE* engine) { printf("engineFinish"); return 1; } -static int engineDestroy(ENGINE* engine) -{ +static int engineDestroy(ENGINE* engine) { printf("engineDestroy"); return 1; } -//static int engineCtrl(ENGINE* engine, int cmd, long i, void *p, void (*f) ()) -//{ -// return 1; -//} - -//static EVP_PKEY* engineLoadPrivateKey(ENGINE* engine, const char* s_key_id, UI_METHOD* ui_method, void* cb_data) -//{ -// return NULL; -//} - -//static EVP_PKEY* engineLoadPublicKey(ENGINE* engine, const char* s_key_id, UI_METHOD* ui_method, void* cb_data) -//{ -// return NULL; -//} - -static std::string loadFile(const char *filename) -{ +static std::string loadFile(const char *filename) { std::string ret; FILE *fp = fopen(filename, "r"); - if( fp==NULL ) - { + if (fp == NULL) { printf("Could not open file '%s'\n", filename); return ret; } char buf[1024]; - while( true ) - { + while (true) { size_t r = fread(buf, 1, 1024, fp); - if( r>0 ) - { + if (r > 0) { ret.append(buf, r); } - if( r<1024 ) - { + if (r < 1024) { break; } } @@ -88,41 +65,43 @@ static std::string loadFile(const char *filename) } -static int engineLoadSSLClientCert(ENGINE* engine, SSL *ssl, STACK_OF(X509_NAME) *ca_dn, X509 **ppcert, EVP_PKEY **ppkey, STACK_OF(X509) **pother, UI_METHOD *ui_method, void *callback_data) -{ +static int engineLoadSSLClientCert(ENGINE* engine, + SSL *ssl, + STACK_OF(X509_NAME) *ca_dn, + X509 **ppcert, + EVP_PKEY **ppkey, + STACK_OF(X509) **pother, + UI_METHOD *ui_method, + void *callback_data) { printf("engineLoadSSLClientCert\n"); - if( ppcert ) - { + if (ppcert) { std::string cert = loadFile(AGENT_CERT); - if( cert.size()==0 ) - { + if (cert.size() == 0) { return 0; } - BIO *bio = BIO_new_mem_buf((void*)cert.data(), (int)cert.size()); + BIO *bio = BIO_new_mem_buf(reinterpret_cast(cert.data()), + static_cast(cert.size())); *ppcert = PEM_read_bio_X509(bio, NULL, NULL, NULL); BIO_vfree(bio); - if( *ppcert==NULL ) - { + if (*ppcert == NULL) { printf("Could not read certificate\n"); return 0; } } - if( ppkey ) - { + if (ppkey) { std::string key = loadFile(AGENT_KEY); - if( key.size()==0 ) - { + if (key.size() == 0) { return 0; } - BIO *bio = BIO_new_mem_buf((void*)key.data(), (int)key.size()); + BIO *bio = BIO_new_mem_buf(reinterpret_cast(key.data()), + static_cast(key.size())); *ppkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL); BIO_vfree(bio); - if( *ppkey==NULL ) - { + if (*ppkey == NULL) { printf("Could not read private key\n"); return 0; } @@ -131,75 +110,20 @@ static int engineLoadSSLClientCert(ENGINE* engine, SSL *ssl, STACK_OF(X509_NAME) return 1; } -/* -static int rsaEncrypt(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, int padding) -{ - return 0; -} - -static int rsaDecrypt(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, int padding) -{ - return 0; -} - -static int rsaSign(int type, const unsigned char *m, unsigned int m_length, unsigned char *sigret, unsigned int *siglen, const RSA *rsa) -{ - return 0; -} - -static int rsaVerify(int type, const unsigned char *m, unsigned int m_length, const unsigned char *sigbuf, unsigned int siglen, const RSA *rsa) -{ - return 0; -} - -static void rsaExDataFree(void *parent, void *ptr, CRYPTO_EX_DATA *ad, int idx, long argl, void *argp) -{ -} - -static int rsaExDataDup(CRYPTO_EX_DATA *to, CRYPTO_EX_DATA *from, void *from_d, int idx, long argl, void *argp) -{ - return 0; -} - -static RSA_METHOD rsaMethods; -static RSA_METHOD* engineRSAMethods() -{ - rsaMethods.rsa_priv_enc = rsaEncrypt; - rsaMethods.rsa_priv_dec = rsaDecrypt; - rsaMethods.rsa_sign = rsaSign; - rsaMethods.rsa_verify = rsaVerify; -// rsaMethods.finish = rsaFinish; - rsaMethods.flags |= RSA_FLAG_SIGN_VER; - return &rsaMethods; -} -*/ - -static int bind_fn(ENGINE * engine, const char *id) -{ +static int bind_fn(ENGINE * engine, const char *id) { printf("loaded engine: id=%s\n", id); - if( !ENGINE_set_id(engine, TEST_ENGINE_ID) - || !ENGINE_set_name(engine, TEST_ENGINE_NAME) - || !ENGINE_set_init_function(engine, engineInit) - || !ENGINE_set_finish_function(engine, engineFinish) - || !ENGINE_set_destroy_function(engine, engineDestroy) - || !ENGINE_set_load_ssl_client_cert_function(engine, engineLoadSSLClientCert) -// || !ENGINE_set_load_privkey_function(engine, engineLoadPrivateKey) -// || !ENGINE_set_ctrl_function(engine, engineCtrl) -// || !ENGINE_set_cmd_defns(engine, mCommands) -#ifndef OPENSSL_NO_RSA -// !ENGINE_set_RSA(engine, engineRSAMethods()) -#endif - ) { - printf("failed to setup all engine functions"); - return 0; - } + ENGINE_set_id(engine, TEST_ENGINE_ID); + ENGINE_set_name(engine, TEST_ENGINE_NAME); + ENGINE_set_init_function(engine, engineInit); + ENGINE_set_finish_function(engine, engineFinish); + ENGINE_set_destroy_function(engine, engineDestroy); + ENGINE_set_load_ssl_client_cert_function(engine, engineLoadSSLClientCert); return 1; } -extern "C" -{ +extern "C" { IMPLEMENT_DYNAMIC_CHECK_FN(); IMPLEMENT_DYNAMIC_BIND_FN(bind_fn); } From f2aab176af4602174ff76cc2c9fe61949a5902c9 Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Thu, 21 Apr 2016 10:49:33 +0200 Subject: [PATCH 03/17] minor cleanup --- .../openssl-client-cert-engine/binding.cc | 43 ------------------- .../openssl-client-cert-engine/binding.gyp | 4 -- .../addons/openssl-client-cert-engine/test.js | 2 +- 3 files changed, 1 insertion(+), 48 deletions(-) delete mode 100644 test/addons/openssl-client-cert-engine/binding.cc diff --git a/test/addons/openssl-client-cert-engine/binding.cc b/test/addons/openssl-client-cert-engine/binding.cc deleted file mode 100644 index 5210de298b8b39..00000000000000 --- a/test/addons/openssl-client-cert-engine/binding.cc +++ /dev/null @@ -1,43 +0,0 @@ -#undef NDEBUG -#include -#include -#include -#include - -using node::AtExit; -using v8::HandleScope; -using v8::Isolate; -using v8::Local; -using v8::Object; - -static char cookie[] = "yum yum"; -static int at_exit_cb1_called = 0; -static int at_exit_cb2_called = 0; - -static void at_exit_cb1(void* arg) { - Isolate* isolate = static_cast(arg); - HandleScope handle_scope(isolate); - Local obj = Object::New(isolate); - assert(!obj.IsEmpty()); // Assert VM is still alive. - assert(obj->IsObject()); - at_exit_cb1_called++; -} - -static void at_exit_cb2(void* arg) { - assert(arg == static_cast(cookie)); - at_exit_cb2_called++; -} - -static void sanity_check(void) { - assert(at_exit_cb1_called == 1); - assert(at_exit_cb2_called == 2); -} - -void init(Local target) { - AtExit(at_exit_cb1, target->CreationContext()->GetIsolate()); - AtExit(at_exit_cb2, cookie); - AtExit(at_exit_cb2, cookie); - atexit(sanity_check); -} - -NODE_MODULE(binding, init); diff --git a/test/addons/openssl-client-cert-engine/binding.gyp b/test/addons/openssl-client-cert-engine/binding.gyp index cf7d1fcff21672..1649a7273516a4 100644 --- a/test/addons/openssl-client-cert-engine/binding.gyp +++ b/test/addons/openssl-client-cert-engine/binding.gyp @@ -1,9 +1,5 @@ { 'targets': [ - { - 'target_name': 'binding', - 'sources': [ 'binding.cc' ] - }, { 'target_name': 'testengine', 'type': 'shared_library', diff --git a/test/addons/openssl-client-cert-engine/test.js b/test/addons/openssl-client-cert-engine/test.js index 86033b58116068..cdce05fdb56952 100644 --- a/test/addons/openssl-client-cert-engine/test.js +++ b/test/addons/openssl-client-cert-engine/test.js @@ -1,5 +1,5 @@ 'use strict'; -require('../../common'); +const common = require('../../common'); const assert = require('assert'); const https = require('https'); const fs = require('fs'); From dbf47ae2c4d66194e9903c1a322586de4eea64a0 Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Thu, 21 Apr 2016 12:23:21 +0200 Subject: [PATCH 04/17] doc: added description of clientCertEngine option --- doc/api/https.md | 2 ++ doc/api/tls.md | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/doc/api/https.md b/doc/api/https.md index bc0e4114c39761..efd7980fe6d444 100644 --- a/doc/api/https.md +++ b/doc/api/https.md @@ -199,6 +199,8 @@ The following options from [`tls.connect()`][] can also be specified. However, a - `key`: Private key to use for SSL. Default `null`. - `passphrase`: A string of passphrase for the private key or pfx. Default `null`. - `cert`: Public x509 certificate to use. Default `null`. +- `clientCertEngine`: A string containing the name of and OpenSSL engine for + providing the client certificate. - `ca`: A string, [`Buffer`][] or array of strings or [`Buffer`][]s of trusted certificates in PEM format. If this is omitted several well known "root" CAs will be used, like VeriSign. These are used to authorize connections. diff --git a/doc/api/tls.md b/doc/api/tls.md index 7feaff2acb2731..6e88609063c20f 100644 --- a/doc/api/tls.md +++ b/doc/api/tls.md @@ -910,6 +910,8 @@ socket.on('end', () => { server.close(); }); ``` + - `clientCertEngine`: A string with the name of an OpenSSL engine which + can provide the client certificate. ## tls.createSecureContext(options) @@ -928,6 +930,8 @@ added: v0.11.13 * `passphrase` {string} A string containing the passphrase for the private key or pfx. * `cert` {string} A string containing the PEM encoded certificate + * `clientCertEngine` {string} A string containing the name of and OpenSSL + engine for providing the client certificate. * `ca`{string|string[]|Buffer|Buffer[]} A string, `Buffer`, array of strings, or array of `Buffer`s of trusted certificates in PEM format. If omitted, several well known "root" CAs (like VeriSign) will be used. These are used @@ -971,6 +975,8 @@ added: v0.3.2 or array of `Buffer`s of trusted certificates in PEM format. If this is omitted several well known "root" CAs (like VeriSign) will be used. These are used to authorize connections. + * `clientCertEngine` {string} A string with the name of an OpenSSL engine + which can provide the client certificate. * `crl` {string|string[]} Either a string or array of strings of PEM encoded CRLs (Certificate Revocation List). * `ciphers` {string} A string describing the ciphers to use or exclude, From 9fc1a9ae4cd41e2cbeae5e9912dd5169ca17a983 Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Thu, 21 Apr 2016 12:23:39 +0200 Subject: [PATCH 05/17] fixed lint --- test/addons/openssl-client-cert-engine/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/addons/openssl-client-cert-engine/test.js b/test/addons/openssl-client-cert-engine/test.js index cdce05fdb56952..86033b58116068 100644 --- a/test/addons/openssl-client-cert-engine/test.js +++ b/test/addons/openssl-client-cert-engine/test.js @@ -1,5 +1,5 @@ 'use strict'; -const common = require('../../common'); +require('../../common'); const assert = require('assert'); const https = require('https'); const fs = require('fs'); From 7b27ab31cbe62f18d8d62bd4f4857a187a0b81d9 Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Mon, 25 Apr 2016 16:17:56 +0200 Subject: [PATCH 06/17] fixed c++ spacing --- .../openssl-client-cert-engine/testengine.cc | 130 +++++++++--------- 1 file changed, 65 insertions(+), 65 deletions(-) diff --git a/test/addons/openssl-client-cert-engine/testengine.cc b/test/addons/openssl-client-cert-engine/testengine.cc index 834a5b8b9a4799..33d1fbf56df7aa 100644 --- a/test/addons/openssl-client-cert-engine/testengine.cc +++ b/test/addons/openssl-client-cert-engine/testengine.cc @@ -17,7 +17,7 @@ #include #ifndef ENGINE_CMD_BASE -# error did not get engine.h +#error did not get engine.h #endif #define TEST_ENGINE_ID "testengine" @@ -27,41 +27,41 @@ #define AGENT_CERT "test/fixtures/keys/agent1-cert.pem" static int engineInit(ENGINE* engine) { - printf("engineInit\n"); - return 1; + printf("engineInit\n"); + return 1; } static int engineFinish(ENGINE* engine) { - printf("engineFinish"); - return 1; + printf("engineFinish"); + return 1; } static int engineDestroy(ENGINE* engine) { - printf("engineDestroy"); - return 1; + printf("engineDestroy"); + return 1; } static std::string loadFile(const char *filename) { - std::string ret; - FILE *fp = fopen(filename, "r"); - if (fp == NULL) { - printf("Could not open file '%s'\n", filename); - return ret; - } + std::string ret; + FILE *fp = fopen(filename, "r"); + if (fp == NULL) { + printf("Could not open file '%s'\n", filename); + return ret; + } - char buf[1024]; - while (true) { - size_t r = fread(buf, 1, 1024, fp); - if (r > 0) { - ret.append(buf, r); - } - if (r < 1024) { - break; - } + char buf[1024]; + while (true) { + size_t r = fread(buf, 1, 1024, fp); + if (r > 0) { + ret.append(buf, r); } + if (r < 1024) { + break; + } + } - fclose(fp); - return ret; + fclose(fp); + return ret; } @@ -73,57 +73,57 @@ static int engineLoadSSLClientCert(ENGINE* engine, STACK_OF(X509) **pother, UI_METHOD *ui_method, void *callback_data) { - printf("engineLoadSSLClientCert\n"); - - if (ppcert) { - std::string cert = loadFile(AGENT_CERT); - if (cert.size() == 0) { - return 0; - } - - BIO *bio = BIO_new_mem_buf(reinterpret_cast(cert.data()), - static_cast(cert.size())); - *ppcert = PEM_read_bio_X509(bio, NULL, NULL, NULL); - BIO_vfree(bio); - if (*ppcert == NULL) { - printf("Could not read certificate\n"); - return 0; - } + printf("engineLoadSSLClientCert\n"); + + if (ppcert) { + std::string cert = loadFile(AGENT_CERT); + if (cert.size() == 0) { + return 0; + } + + BIO *bio = BIO_new_mem_buf(reinterpret_cast(cert.data()), + static_cast(cert.size())); + *ppcert = PEM_read_bio_X509(bio, NULL, NULL, NULL); + BIO_vfree(bio); + if (*ppcert == NULL) { + printf("Could not read certificate\n"); + return 0; + } + } + + if (ppkey) { + std::string key = loadFile(AGENT_KEY); + if (key.size() == 0) { + return 0; } - if (ppkey) { - std::string key = loadFile(AGENT_KEY); - if (key.size() == 0) { - return 0; - } - - BIO *bio = BIO_new_mem_buf(reinterpret_cast(key.data()), - static_cast(key.size())); - *ppkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL); - BIO_vfree(bio); - if (*ppkey == NULL) { - printf("Could not read private key\n"); - return 0; - } + BIO *bio = BIO_new_mem_buf(reinterpret_cast(key.data()), + static_cast(key.size())); + *ppkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL); + BIO_vfree(bio); + if (*ppkey == NULL) { + printf("Could not read private key\n"); + return 0; } + } - return 1; + return 1; } static int bind_fn(ENGINE * engine, const char *id) { - printf("loaded engine: id=%s\n", id); + printf("loaded engine: id=%s\n", id); - ENGINE_set_id(engine, TEST_ENGINE_ID); - ENGINE_set_name(engine, TEST_ENGINE_NAME); - ENGINE_set_init_function(engine, engineInit); - ENGINE_set_finish_function(engine, engineFinish); - ENGINE_set_destroy_function(engine, engineDestroy); - ENGINE_set_load_ssl_client_cert_function(engine, engineLoadSSLClientCert); + ENGINE_set_id(engine, TEST_ENGINE_ID); + ENGINE_set_name(engine, TEST_ENGINE_NAME); + ENGINE_set_init_function(engine, engineInit); + ENGINE_set_finish_function(engine, engineFinish); + ENGINE_set_destroy_function(engine, engineDestroy); + ENGINE_set_load_ssl_client_cert_function(engine, engineLoadSSLClientCert); - return 1; + return 1; } extern "C" { - IMPLEMENT_DYNAMIC_CHECK_FN(); - IMPLEMENT_DYNAMIC_BIND_FN(bind_fn); + IMPLEMENT_DYNAMIC_CHECK_FN(); + IMPLEMENT_DYNAMIC_BIND_FN(bind_fn); } From efe5aca0da99e82863dfe35423c2084b84ea119e Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Mon, 25 Apr 2016 18:40:25 +0200 Subject: [PATCH 07/17] refactored common code into a function --- src/node_crypto.cc | 106 ++++++++++++++++++++++++--------------------- 1 file changed, 57 insertions(+), 49 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 23005ebf53f402..1c8f2a6b9e9fa8 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -209,6 +209,43 @@ static int CryptoPemCallback(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 schduled +// and nullptr is returned. +#ifndef OPENSSL_NO_ENGINE +static ENGINE* loadEngineById(Environment* env, const char *engine_id) { + 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) { + char errmsg[1024]; + 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); + } + env->ThrowError(errmsg); + // Forcibly clear OpenSSL's error stack + ERR_clear_error(); + } + + return engine; +} +#endif // !OPENSSL_NO_ENGINE void ThrowCryptoError(Environment* env, unsigned long err, // NOLINT(runtime/int) @@ -1037,42 +1074,31 @@ void SecureContext::SetClientCertEngine( SecureContext* sc = Unwrap(args.This()); // If an engine was previously set, free it. + // This is because SSL_CTX_set_client_cert_engine does not itself + // 'support' multiple calls by cleaning up before overwriting the + // client_cert_engine internal context variable if (sc->ctx_->client_cert_engine != NULL) { ENGINE* e = sc->ctx_->client_cert_engine; - sc->ctx_->client_cert_engine = NULL; - ENGINE_free(e); + sc->ctx_->client_cert_engine = nullptr; + ENGINE_finish(e); } + // Load engine const node::Utf8Value engine_id(env->isolate(), args[0]); - ENGINE* engine = ENGINE_by_id(*engine_id); + ENGINE* engine = loadEngineById(env, *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; - } - } - } - - if (engine == nullptr) { - int err = ERR_get_error(); - if (err == 0) { - char tmp[1024]; - snprintf(tmp, sizeof(tmp), "Engine \"%s\" was not found", *engine_id); - return env->ThrowError(tmp); - } else { - return ThrowCryptoError(env, err); - } + // Load failed... appropriate js exception has been scheduled + return; } 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) + if (r == 0) { + ERR_clear_error(); return ThrowCryptoError(env, ERR_get_error()); + } } #endif // !OPENSSL_NO_ENGINE @@ -5867,39 +5893,21 @@ void SetEngine(const FunctionCallbackInfo& args) { CHECK(args.Length() >= 2 && args[0]->IsString()); unsigned int flags = args[1]->Uint32Value(); - ClearErrorOnReturn clear_error_on_return; - (void) &clear_error_on_return; // Silence compiler warning. - + // 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; - } - } - } + ENGINE* engine = loadEngineById(env, *engine_id); if (engine == nullptr) { - int err = ERR_get_error(); - if (err == 0) { - char tmp[1024]; - snprintf(tmp, sizeof(tmp), "Engine \"%s\" was not found", *engine_id); - return env->ThrowError(tmp); - } else { - return ThrowCryptoError(env, err); - } + // Load failed... appropriate js exception has been scheduled + return; } int r = ENGINE_set_default(engine, flags); ENGINE_free(engine); - if (r == 0) + if (r == 0) { + ERR_clear_error(); return ThrowCryptoError(env, ERR_get_error()); + } } #endif // !OPENSSL_NO_ENGINE From 2faddc4abcb4ad197e3aed40ab4d58051e5d4ea2 Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Wed, 4 May 2016 17:39:54 +0200 Subject: [PATCH 08/17] changes made due to review comments --- src/node_crypto.cc | 11 +++-- .../openssl-client-cert-engine/binding.gyp | 1 + .../addons/openssl-client-cert-engine/test.js | 13 +++-- .../openssl-client-cert-engine/testengine.cc | 47 +++++++++---------- 4 files changed, 37 insertions(+), 35 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 1c8f2a6b9e9fa8..9ce831f98592e4 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -214,7 +214,7 @@ static int CryptoPemCallback(char *buf, int size, int rwflag, void *u) { // In case of error the appropriate js exception is schduled // and nullptr is returned. #ifndef OPENSSL_NO_ENGINE -static ENGINE* loadEngineById(Environment* env, const char *engine_id) { +static ENGINE* LoadEngineById(Environment* env, const char *engine_id) { ENGINE* engine = ENGINE_by_id(engine_id); if (engine == nullptr) { @@ -1073,6 +1073,8 @@ void SecureContext::SetClientCertEngine( SecureContext* sc = Unwrap(args.This()); + ClearErrorOnReturn clear_error_on_return; + // If an engine was previously set, free it. // This is because SSL_CTX_set_client_cert_engine does not itself // 'support' multiple calls by cleaning up before overwriting the @@ -1085,7 +1087,7 @@ void SecureContext::SetClientCertEngine( // Load engine const node::Utf8Value engine_id(env->isolate(), args[0]); - ENGINE* engine = loadEngineById(env, *engine_id); + ENGINE* engine = LoadEngineById(env, *engine_id); if (engine == nullptr) { // Load failed... appropriate js exception has been scheduled @@ -5893,9 +5895,11 @@ void SetEngine(const FunctionCallbackInfo& args) { CHECK(args.Length() >= 2 && args[0]->IsString()); unsigned int flags = args[1]->Uint32Value(); + ClearErrorOnReturn clear_error_on_return; + // Load engine const node::Utf8Value engine_id(env->isolate(), args[0]); - ENGINE* engine = loadEngineById(env, *engine_id); + ENGINE* engine = LoadEngineById(env, *engine_id); if (engine == nullptr) { // Load failed... appropriate js exception has been scheduled @@ -5905,7 +5909,6 @@ void SetEngine(const FunctionCallbackInfo& args) { int r = ENGINE_set_default(engine, flags); ENGINE_free(engine); if (r == 0) { - ERR_clear_error(); return ThrowCryptoError(env, ERR_get_error()); } } diff --git a/test/addons/openssl-client-cert-engine/binding.gyp b/test/addons/openssl-client-cert-engine/binding.gyp index 1649a7273516a4..7f8fa56b9920c3 100644 --- a/test/addons/openssl-client-cert-engine/binding.gyp +++ b/test/addons/openssl-client-cert-engine/binding.gyp @@ -3,6 +3,7 @@ { 'target_name': 'testengine', 'type': 'shared_library', + 'product_extension': 'engine', 'sources': [ 'testengine.cc' ], 'include_dirs': [ '../../../deps/openssl/openssl/include' ] } diff --git a/test/addons/openssl-client-cert-engine/test.js b/test/addons/openssl-client-cert-engine/test.js index 86033b58116068..f93315034d4e80 100644 --- a/test/addons/openssl-client-cert-engine/test.js +++ b/test/addons/openssl-client-cert-engine/test.js @@ -5,7 +5,10 @@ const https = require('https'); const fs = require('fs'); const path = require('path'); -var engine = path.dirname(module.filename) + '/build/Release/testengine.so'; +var engine = path.join(path.dirname(module.filename), + 'build', + 'Release', + 'testengine.engine'); var agentKey = fs.readFileSync('test/fixtures/keys/agent1-key.pem'); var agentCert = fs.readFileSync('test/fixtures/keys/agent1-cert.pem'); @@ -26,14 +29,12 @@ var server = https.createServer(serverOptions, (req, res) => { res.end('hello world'); }).listen(port); -function testFailed(message) -{ +function testFailed(message) { server.close(); assert(false, message); } -function testPassed() -{ +function testPassed() { console.log('Test passed!'); server.close(); } @@ -43,8 +44,6 @@ var clientOptions = { host: '127.0.0.1', port: port, path: '/test', -// key: agentKey, -// cert: agentCert, clientCertEngine: engine, // engine will provide key+cert rejectUnauthorized: false, // prevent failing on self-signed certificates headers: {} diff --git a/test/addons/openssl-client-cert-engine/testengine.cc b/test/addons/openssl-client-cert-engine/testengine.cc index 33d1fbf56df7aa..9fec46b0ac2047 100644 --- a/test/addons/openssl-client-cert-engine/testengine.cc +++ b/test/addons/openssl-client-cert-engine/testengine.cc @@ -1,4 +1,3 @@ -#undef NDEBUG #include #include #include @@ -17,7 +16,7 @@ #include #ifndef ENGINE_CMD_BASE -#error did not get engine.h +# error did not get engine.h #endif #define TEST_ENGINE_ID "testengine" @@ -26,18 +25,18 @@ #define AGENT_KEY "test/fixtures/keys/agent1-key.pem" #define AGENT_CERT "test/fixtures/keys/agent1-cert.pem" -static int engineInit(ENGINE* engine) { - printf("engineInit\n"); +static int EngineInit(ENGINE* engine) { + printf("EngineInit\n"); return 1; } -static int engineFinish(ENGINE* engine) { - printf("engineFinish"); +static int EngineFinish(ENGINE* engine) { + printf("EngineFinish"); return 1; } -static int engineDestroy(ENGINE* engine) { - printf("engineDestroy"); +static int EngineDestroy(ENGINE* engine) { + printf("EngineDestroy"); return 1; } @@ -65,15 +64,15 @@ static std::string loadFile(const char *filename) { } -static int engineLoadSSLClientCert(ENGINE* engine, - SSL *ssl, - STACK_OF(X509_NAME) *ca_dn, - X509 **ppcert, - EVP_PKEY **ppkey, - STACK_OF(X509) **pother, - UI_METHOD *ui_method, - void *callback_data) { - printf("engineLoadSSLClientCert\n"); +static int EngineLoadSSLClientCert(ENGINE* engine, + SSL* ssl, + STACK_OF(X509_NAME)* ca_dn, + X509** ppcert, + EVP_PKEY** ppkey, + STACK_OF(X509)** pother, + UI_METHOD* ui_method, + void* callback_data) { + printf("EngineLoadSSLClientCert\n"); if (ppcert) { std::string cert = loadFile(AGENT_CERT); @@ -81,7 +80,7 @@ static int engineLoadSSLClientCert(ENGINE* engine, return 0; } - BIO *bio = BIO_new_mem_buf(reinterpret_cast(cert.data()), + BIO* bio = BIO_new_mem_buf(reinterpret_cast(cert.data()), static_cast(cert.size())); *ppcert = PEM_read_bio_X509(bio, NULL, NULL, NULL); BIO_vfree(bio); @@ -97,7 +96,7 @@ static int engineLoadSSLClientCert(ENGINE* engine, return 0; } - BIO *bio = BIO_new_mem_buf(reinterpret_cast(key.data()), + BIO* bio = BIO_new_mem_buf(reinterpret_cast(key.data()), static_cast(key.size())); *ppkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL); BIO_vfree(bio); @@ -110,15 +109,15 @@ static int engineLoadSSLClientCert(ENGINE* engine, return 1; } -static int bind_fn(ENGINE * engine, const char *id) { +static int bind_fn(ENGINE* engine, const char* id) { printf("loaded engine: id=%s\n", id); ENGINE_set_id(engine, TEST_ENGINE_ID); ENGINE_set_name(engine, TEST_ENGINE_NAME); - ENGINE_set_init_function(engine, engineInit); - ENGINE_set_finish_function(engine, engineFinish); - ENGINE_set_destroy_function(engine, engineDestroy); - ENGINE_set_load_ssl_client_cert_function(engine, engineLoadSSLClientCert); + ENGINE_set_init_function(engine, EngineInit); + ENGINE_set_finish_function(engine, EngineFinish); + ENGINE_set_destroy_function(engine, EngineDestroy); + ENGINE_set_load_ssl_client_cert_function(engine, EngineLoadSSLClientCert); return 1; } From 9fea66a3f0ce93f50774056d4ee05036328579e5 Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Wed, 25 May 2016 16:53:01 +0200 Subject: [PATCH 09/17] fixes due to further feedback --- src/node_crypto.cc | 43 ++++++++++--------- .../addons/openssl-client-cert-engine/test.js | 34 ++++----------- .../openssl-client-cert-engine/testengine.cc | 12 +++--- 3 files changed, 36 insertions(+), 53 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 9ce831f98592e4..f057cea74a3d78 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -210,15 +210,17 @@ static int CryptoPemCallback(char *buf, int size, int rwflag, void *u) { } // 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 schduled +// 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(Environment* env, const char *engine_id) { +static ENGINE* LoadEngineById(Environment* env, const char* engine_id) { ENGINE* engine = ENGINE_by_id(engine_id); + MarkPopErrorOnReturn mark_pop_error_on_return; + if (engine == nullptr) { - // Engine not found, try loading dynamically + // 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) || @@ -239,8 +241,6 @@ static ENGINE* LoadEngineById(Environment* env, const char *engine_id) { "Engine \"%s\" was not found", engine_id); } env->ThrowError(errmsg); - // Forcibly clear OpenSSL's error stack - ERR_clear_error(); } return engine; @@ -328,7 +328,7 @@ void SecureContext::Initialize(Environment* env, Local target) { env->SetProtoMethod(t, "loadPKCS12", SecureContext::LoadPKCS12); #ifndef OPENSSL_NO_ENGINE env->SetProtoMethod(t, "setClientCertEngine", - SecureContext::SetClientCertEngine); + SecureContext::SetClientCertEngine); #endif // !OPENSSL_NO_ENGINE env->SetProtoMethod(t, "getTicketKeys", SecureContext::GetTicketKeys); env->SetProtoMethod(t, "setTicketKeys", SecureContext::SetTicketKeys); @@ -1067,38 +1067,39 @@ void SecureContext::LoadPKCS12(const FunctionCallbackInfo& args) { #ifndef OPENSSL_NO_ENGINE void SecureContext::SetClientCertEngine( - const FunctionCallbackInfo& args) { + const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - CHECK(args.Length() == 1 && args[0]->IsString()); + CHECK_EQ(args.Length(), 1); + CHECK_EQ(args[0]->IsString(), true); SecureContext* sc = Unwrap(args.This()); - ClearErrorOnReturn clear_error_on_return; + MarkPopErrorOnReturn mark_pop_error_on_return; - // If an engine was previously set, free it. - // This is because SSL_CTX_set_client_cert_engine does not itself - // 'support' multiple calls by cleaning up before overwriting the - // client_cert_engine internal context variable + // 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. As this may surprise + // a developer we throw him an exception. if (sc->ctx_->client_cert_engine != NULL) { - ENGINE* e = sc->ctx_->client_cert_engine; - sc->ctx_->client_cert_engine = nullptr; - ENGINE_finish(e); + return env->ThrowError( + "Multiple calls to SetClientCertEngine is not allowed"); } - // Load engine + // Load engine. const node::Utf8Value engine_id(env->isolate(), args[0]); ENGINE* engine = LoadEngineById(env, *engine_id); if (engine == nullptr) { - // Load failed... appropriate js exception has been scheduled + // Load failed... appropriate js exception has been scheduled. return; } int r = SSL_CTX_set_client_cert_engine(sc->ctx_, engine); - // Free reference (SSL_CTX_set_client_cert_engine took it via ENGINE_init) + // Free reference (SSL_CTX_set_client_cert_engine took it via ENGINE_init). ENGINE_free(engine); if (r == 0) { - ERR_clear_error(); return ThrowCryptoError(env, ERR_get_error()); } } diff --git a/test/addons/openssl-client-cert-engine/test.js b/test/addons/openssl-client-cert-engine/test.js index f93315034d4e80..b9850039d64b7b 100644 --- a/test/addons/openssl-client-cert-engine/test.js +++ b/test/addons/openssl-client-cert-engine/test.js @@ -1,5 +1,5 @@ 'use strict'; -require('../../common'); +const common = require('../../common'); const assert = require('assert'); const https = require('https'); const fs = require('fs'); @@ -14,7 +14,7 @@ var agentKey = fs.readFileSync('test/fixtures/keys/agent1-key.pem'); var agentCert = fs.readFileSync('test/fixtures/keys/agent1-cert.pem'); var agentCa = fs.readFileSync('test/fixtures/keys/ca1-cert.pem'); -var port = 18020; +var port = common.PORT; const serverOptions = { key: agentKey, @@ -27,21 +27,11 @@ const serverOptions = { var server = https.createServer(serverOptions, (req, res) => { res.writeHead(200); res.end('hello world'); -}).listen(port); - -function testFailed(message) { - server.close(); - assert(false, message); -} - -function testPassed() { - console.log('Test passed!'); - server.close(); -} +}).listen(port, common.localhostIPv4); var clientOptions = { method: 'GET', - host: '127.0.0.1', + host: common.localhostIPv4, port: port, path: '/test', clientCertEngine: engine, // engine will provide key+cert @@ -56,17 +46,11 @@ var req = https.request(clientOptions, (response) => { body += chunk; }); - response.on('end', () => { - if (body == 'hello world') { - testPassed(); - } else { - testFailed('unexpected body: <' + body + '>'); - } - }); -}); - -req.on('error', (e) => { - testFailed('request error: ' + e.message); + response.on('end', common.mustCall(function() { + assert.strictEqual(body, 'hello world'); + console.log('Test passed!'); + server.close(); + })); }); req.end(); diff --git a/test/addons/openssl-client-cert-engine/testengine.cc b/test/addons/openssl-client-cert-engine/testengine.cc index 9fec46b0ac2047..b7a796927d6568 100644 --- a/test/addons/openssl-client-cert-engine/testengine.cc +++ b/test/addons/openssl-client-cert-engine/testengine.cc @@ -40,9 +40,9 @@ static int EngineDestroy(ENGINE* engine) { return 1; } -static std::string loadFile(const char *filename) { +static std::string loadFile(const char* filename) { std::string ret; - FILE *fp = fopen(filename, "r"); + FILE* fp = fopen(filename, "r"); if (fp == NULL) { printf("Could not open file '%s'\n", filename); return ret; @@ -80,8 +80,7 @@ static int EngineLoadSSLClientCert(ENGINE* engine, return 0; } - BIO* bio = BIO_new_mem_buf(reinterpret_cast(cert.data()), - static_cast(cert.size())); + BIO* bio = BIO_new_mem_buf(cert.data(), cert.size()); *ppcert = PEM_read_bio_X509(bio, NULL, NULL, NULL); BIO_vfree(bio); if (*ppcert == NULL) { @@ -92,12 +91,11 @@ static int EngineLoadSSLClientCert(ENGINE* engine, if (ppkey) { std::string key = loadFile(AGENT_KEY); - if (key.size() == 0) { + if (key.empty()) { return 0; } - BIO* bio = BIO_new_mem_buf(reinterpret_cast(key.data()), - static_cast(key.size())); + BIO* bio = BIO_new_mem_buf(key.data(), key.size()); *ppkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL); BIO_vfree(bio); if (*ppkey == NULL) { From 7ce61871a9b8ab5bf5ceebacc09f3cdc36ddffc6 Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Wed, 25 May 2016 16:59:42 +0200 Subject: [PATCH 10/17] added a few more common.mustCall wrappings --- test/addons/openssl-client-cert-engine/test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/addons/openssl-client-cert-engine/test.js b/test/addons/openssl-client-cert-engine/test.js index b9850039d64b7b..36880ca01adf27 100644 --- a/test/addons/openssl-client-cert-engine/test.js +++ b/test/addons/openssl-client-cert-engine/test.js @@ -39,18 +39,18 @@ var clientOptions = { headers: {} }; -var req = https.request(clientOptions, (response) => { +var req = https.request(clientOptions, common.mustCall(function(response) { var body = ''; response.setEncoding('utf8'); - response.on('data', (chunk) => { + response.on('data', common.mustCall(function(chunk) { body += chunk; - }); + })); response.on('end', common.mustCall(function() { assert.strictEqual(body, 'hello world'); console.log('Test passed!'); server.close(); })); -}); +})); req.end(); From ddeb0249d1be3bf5d876eb9fb4f585da32c094bc Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Wed, 1 Jun 2016 12:51:45 +0200 Subject: [PATCH 11/17] minor style changes --- src/node_crypto.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index f057cea74a3d78..f9220c0fef4c0b 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -235,7 +235,7 @@ static ENGINE* LoadEngineById(Environment* env, const char* engine_id) { char errmsg[1024]; int err = ERR_get_error(); if (err != 0) { - ERR_error_string_n(err, errmsg, sizeof(errmsg)); + ERR_error_string_n(err, errmsg, sizeof(errmsg)); } else { snprintf(errmsg, sizeof(errmsg), "Engine \"%s\" was not found", engine_id); @@ -5909,9 +5909,8 @@ void SetEngine(const FunctionCallbackInfo& args) { int r = ENGINE_set_default(engine, flags); ENGINE_free(engine); - if (r == 0) { + if (r == 0) return ThrowCryptoError(env, ERR_get_error()); - } } #endif // !OPENSSL_NO_ENGINE From ad9139ff1fa916e3d616f6e6e4f61a2f6c0d9b2e Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Wed, 20 Jul 2016 13:42:44 +0200 Subject: [PATCH 12/17] fixed typo in documentation --- doc/api/https.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/https.md b/doc/api/https.md index efd7980fe6d444..c9d6ef631a45ef 100644 --- a/doc/api/https.md +++ b/doc/api/https.md @@ -199,7 +199,7 @@ The following options from [`tls.connect()`][] can also be specified. However, a - `key`: Private key to use for SSL. Default `null`. - `passphrase`: A string of passphrase for the private key or pfx. Default `null`. - `cert`: Public x509 certificate to use. Default `null`. -- `clientCertEngine`: A string containing the name of and OpenSSL engine for +- `clientCertEngine`: A string containing the name of an OpenSSL engine for providing the client certificate. - `ca`: A string, [`Buffer`][] or array of strings or [`Buffer`][]s of trusted certificates in PEM format. If this is omitted several well known "root" From a52d4052d07f2b7d90cee17df9a27f4a467759f7 Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Tue, 26 Jul 2016 15:40:42 +0200 Subject: [PATCH 13/17] removed spurious old text that appeared during a merge conflict --- doc/api/tls.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/doc/api/tls.md b/doc/api/tls.md index 6e88609063c20f..cc2fc468d93055 100644 --- a/doc/api/tls.md +++ b/doc/api/tls.md @@ -910,8 +910,6 @@ socket.on('end', () => { server.close(); }); ``` - - `clientCertEngine`: A string with the name of an OpenSSL engine which - can provide the client certificate. ## tls.createSecureContext(options) From e95d2046bb9871561a5cd5d108bbc3c6c2890ad4 Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Tue, 26 Jul 2016 15:42:12 +0200 Subject: [PATCH 14/17] updated test to include the extra field in agent name --- test/parallel/test-https-agent-getname.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-https-agent-getname.js b/test/parallel/test-https-agent-getname.js index 49c1dcef4fb5fb..7d87e670a55757 100644 --- a/test/parallel/test-https-agent-getname.js +++ b/test/parallel/test-https-agent-getname.js @@ -14,7 +14,7 @@ const agent = new https.Agent(); // empty options assert.strictEqual( agent.getName({}), - 'localhost:::::::::' + 'localhost::::::::::' ); // pass all options arguments @@ -33,5 +33,5 @@ const options = { assert.strictEqual( agent.getName(options), - '0.0.0.0:443:192.168.1.1:ca:cert:ciphers:key:pfx:false:localhost' + '0.0.0.0:443:192.168.1.1:ca:cert::ciphers:key:pfx:false:localhost' ); From 922e5da0ac03b0d36b6aa5beb4487c5094cdce51 Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Tue, 26 Jul 2016 15:42:47 +0200 Subject: [PATCH 15/17] small fixes after review --- lib/_tls_common.js | 5 +- src/node_crypto.cc | 7 +- .../addons/openssl-client-cert-engine/test.js | 66 +++++++++---------- .../openssl-client-cert-engine/testengine.cc | 6 +- 4 files changed, 43 insertions(+), 41 deletions(-) diff --git a/lib/_tls_common.js b/lib/_tls_common.js index dc7f958ef87b1f..3c9f6512840e44 100644 --- a/lib/_tls_common.js +++ b/lib/_tls_common.js @@ -147,7 +147,10 @@ exports.createSecureContext = function createSecureContext(options, context) { } if (options.clientCertEngine) { - c.context.setClientCertEngine(options.clientCertEngine); + if (c.context.setClientCertEngine) + c.context.setClientCertEngine(options.clientCertEngine); + else + throw new Error('Custom engines not supported by this OpenSSL'); } return c; diff --git a/src/node_crypto.cc b/src/node_crypto.cc index f9220c0fef4c0b..24b1ced42100dd 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -1070,7 +1070,7 @@ void SecureContext::SetClientCertEngine( const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK_EQ(args.Length(), 1); - CHECK_EQ(args[0]->IsString(), true); + CHECK(args[0]->IsString()); SecureContext* sc = Unwrap(args.This()); @@ -1082,12 +1082,11 @@ void SecureContext::SetClientCertEngine( // Instead of trying to fix up this problem we in turn also do not // support multiple calls to SetClientCertEngine. As this may surprise // a developer we throw him an exception. - if (sc->ctx_->client_cert_engine != NULL) { + if (sc->ctx_->client_cert_engine != nullptr) { return env->ThrowError( - "Multiple calls to SetClientCertEngine is not allowed"); + "Multiple calls to SetClientCertEngine are not allowed"); } - // Load engine. const node::Utf8Value engine_id(env->isolate(), args[0]); ENGINE* engine = LoadEngineById(env, *engine_id); diff --git a/test/addons/openssl-client-cert-engine/test.js b/test/addons/openssl-client-cert-engine/test.js index 36880ca01adf27..1508ecebb222e8 100644 --- a/test/addons/openssl-client-cert-engine/test.js +++ b/test/addons/openssl-client-cert-engine/test.js @@ -5,16 +5,16 @@ const https = require('https'); const fs = require('fs'); const path = require('path'); -var engine = path.join(path.dirname(module.filename), - 'build', - 'Release', - 'testengine.engine'); +const engine = path.join(__dirname, '/build/Release/testengine.engine'); -var agentKey = fs.readFileSync('test/fixtures/keys/agent1-key.pem'); -var agentCert = fs.readFileSync('test/fixtures/keys/agent1-cert.pem'); -var agentCa = fs.readFileSync('test/fixtures/keys/ca1-cert.pem'); +const agentKey = fs.readFileSync(path.join(common.fixturesDir, + '/keys/agent1-key.pem')); +const agentCert = fs.readFileSync(path.join(common.fixturesDir, + '/keys/agent1-cert.pem')); +const agentCa = fs.readFileSync(path.join(common.fixturesDir, + '/keys/ca1-cert.pem')); -var port = common.PORT; +const port = common.PORT; const serverOptions = { key: agentKey, @@ -27,30 +27,30 @@ const serverOptions = { var server = https.createServer(serverOptions, (req, res) => { res.writeHead(200); res.end('hello world'); -}).listen(port, common.localhostIPv4); - -var 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: {} -}; - -var req = https.request(clientOptions, common.mustCall(function(response) { - var body = ''; - response.setEncoding('utf8'); - response.on('data', common.mustCall(function(chunk) { - body += chunk; - })); - - response.on('end', common.mustCall(function() { - assert.strictEqual(body, 'hello world'); - console.log('Test passed!'); - server.close(); +}).listen(port, common.localhostIPv4, () => { + var 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: {} + }; + + var req = https.request(clientOptions, common.mustCall(function(response) { + var body = ''; + response.setEncoding('utf8'); + response.on('data', common.mustCall(function(chunk) { + body += chunk; + })); + + response.on('end', common.mustCall(function() { + assert.strictEqual(body, 'hello world'); + console.log('Test passed!'); + server.close(); + })); })); -})); -req.end(); + req.end(); +}); diff --git a/test/addons/openssl-client-cert-engine/testengine.cc b/test/addons/openssl-client-cert-engine/testengine.cc index b7a796927d6568..7d3156c08060f7 100644 --- a/test/addons/openssl-client-cert-engine/testengine.cc +++ b/test/addons/openssl-client-cert-engine/testengine.cc @@ -40,7 +40,7 @@ static int EngineDestroy(ENGINE* engine) { return 1; } -static std::string loadFile(const char* filename) { +static std::string LoadFile(const char* filename) { std::string ret; FILE* fp = fopen(filename, "r"); if (fp == NULL) { @@ -75,7 +75,7 @@ static int EngineLoadSSLClientCert(ENGINE* engine, printf("EngineLoadSSLClientCert\n"); if (ppcert) { - std::string cert = loadFile(AGENT_CERT); + std::string cert = LoadFile(AGENT_CERT); if (cert.size() == 0) { return 0; } @@ -90,7 +90,7 @@ static int EngineLoadSSLClientCert(ENGINE* engine, } if (ppkey) { - std::string key = loadFile(AGENT_KEY); + std::string key = LoadFile(AGENT_KEY); if (key.empty()) { return 0; } From 249a46b3ae9b6d958990aab044e029023a6e3d41 Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Thu, 1 Sep 2016 16:20:25 +0200 Subject: [PATCH 16/17] doc: aligned the description of clientCertEngine to be the same everywhere --- doc/api/tls.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/tls.md b/doc/api/tls.md index cc2fc468d93055..086b5ddb027b60 100644 --- a/doc/api/tls.md +++ b/doc/api/tls.md @@ -928,8 +928,8 @@ added: v0.11.13 * `passphrase` {string} A string containing the passphrase for the private key or pfx. * `cert` {string} A string containing the PEM encoded certificate - * `clientCertEngine` {string} A string containing the name of and OpenSSL - engine for providing the client certificate. + * `clientCertEngine` {string} A string with the name of an OpenSSL engine + which can provide the client certificate. * `ca`{string|string[]|Buffer|Buffer[]} A string, `Buffer`, array of strings, or array of `Buffer`s of trusted certificates in PEM format. If omitted, several well known "root" CAs (like VeriSign) will be used. These are used From e72b12b382f1e3bedc2c655ad0e848af04e71598 Mon Sep 17 00:00:00 2001 From: joelostrowski Date: Thu, 1 Sep 2016 16:24:20 +0200 Subject: [PATCH 17/17] minor fixes of style and silence succeeding test --- test/addons/openssl-client-cert-engine/test.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/addons/openssl-client-cert-engine/test.js b/test/addons/openssl-client-cert-engine/test.js index 1508ecebb222e8..a4819b069dd1f4 100644 --- a/test/addons/openssl-client-cert-engine/test.js +++ b/test/addons/openssl-client-cert-engine/test.js @@ -8,11 +8,11 @@ const path = require('path'); const engine = path.join(__dirname, '/build/Release/testengine.engine'); const agentKey = fs.readFileSync(path.join(common.fixturesDir, - '/keys/agent1-key.pem')); + '/keys/agent1-key.pem')); const agentCert = fs.readFileSync(path.join(common.fixturesDir, - '/keys/agent1-cert.pem')); + '/keys/agent1-cert.pem')); const agentCa = fs.readFileSync(path.join(common.fixturesDir, - '/keys/ca1-cert.pem')); + '/keys/ca1-cert.pem')); const port = common.PORT; @@ -47,7 +47,6 @@ var server = https.createServer(serverOptions, (req, res) => { response.on('end', common.mustCall(function() { assert.strictEqual(body, 'hello world'); - console.log('Test passed!'); server.close(); })); }));