Skip to content

Commit

Permalink
tls: add option for private keys for OpenSSL engines
Browse files Browse the repository at this point in the history
Add `privateKeyIdentifier` and `privateKeyEngine` options
to get private key from an OpenSSL engine in tls.createSecureContext().

PR-URL: #28973
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
  • Loading branch information
OYTIS authored and BridgeAR committed Oct 9, 2019
1 parent 54ef0fd commit cf7b405
Show file tree
Hide file tree
Showing 9 changed files with 314 additions and 3 deletions.
10 changes: 10 additions & 0 deletions doc/api/tls.md
Original file line number Diff line number Diff line change
Expand Up @@ -1358,6 +1358,10 @@ argument.
<!-- YAML
added: v0.11.13
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/28973
description: Added `privateKeyIdentifier` and `privateKeyEngine` options
to get private key from an OpenSSL engine.
- version: v12.11.0
pr-url: https://github.com/nodejs/node/pull/29598
description: Added `sigalgs` option to override supported signature
Expand Down Expand Up @@ -1462,6 +1466,12 @@ changes:
occur in an array. `object.passphrase` is optional. Encrypted keys will be
decrypted with `object.passphrase` if provided, or `options.passphrase` if
it is not.
* `privateKeyEngine` {string} Name of an OpenSSL engine to get private key
from. Should be used together with `privateKeyIdentifier`.
* `privateKeyIdentifier` {string} Identifier of a private key managed by
an OpenSSL engine. Should be used together with `privateKeyEngine`.
Should not be set together with `key`, because both options define a
private key in different ways.
* `maxVersion` {string} Optionally set the maximum TLS version to allow. One
of `TLSv1.3`, `TLSv1.2'`, `'TLSv1.1'`, or `'TLSv1'`. Cannot be specified
along with the `secureProtocol` option, use one or the other.
Expand Down
30 changes: 30 additions & 0 deletions lib/_tls_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,36 @@ exports.createSecureContext = function createSecureContext(options) {
c.context.setSigalgs(sigalgs);
}

const { privateKeyIdentifier, privateKeyEngine } = options;
if (privateKeyIdentifier !== undefined) {
if (privateKeyEngine === undefined) {
// Engine is required when privateKeyIdentifier is present
throw new ERR_INVALID_OPT_VALUE('privateKeyEngine',
privateKeyEngine);
}
if (key) {
// Both data key and engine key can't be set at the same time
throw new ERR_INVALID_OPT_VALUE('privateKeyIdentifier',
privateKeyIdentifier);
}

if (typeof privateKeyIdentifier === 'string' &&
typeof privateKeyEngine === 'string') {
if (c.context.setEngineKey)
c.context.setEngineKey(privateKeyIdentifier, privateKeyEngine);
else
throw new ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED();
} else if (typeof privateKeyIdentifier !== 'string') {
throw new ERR_INVALID_ARG_TYPE('options.privateKeyIdentifier',
['string', 'undefined'],
privateKeyIdentifier);
} else {
throw new ERR_INVALID_ARG_TYPE('options.privateKeyEngine',
['string', 'undefined'],
privateKeyEngine);
}
}

if (options.ciphers && typeof options.ciphers !== 'string') {
throw new ERR_INVALID_ARG_TYPE(
'options.ciphers', 'string', options.ciphers);
Expand Down
56 changes: 53 additions & 3 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,9 @@ void SecureContext::Initialize(Environment* env, Local<Object> target) {

env->SetProtoMethod(t, "init", Init);
env->SetProtoMethod(t, "setKey", SetKey);
#ifndef OPENSSL_NO_ENGINE
env->SetProtoMethod(t, "setEngineKey", SetEngineKey);
#endif // !OPENSSL_NO_ENGINE
env->SetProtoMethod(t, "setCert", SetCert);
env->SetProtoMethod(t, "addCACert", AddCACert);
env->SetProtoMethod(t, "addCRL", AddCRL);
Expand Down Expand Up @@ -764,6 +767,56 @@ void SecureContext::SetSigalgs(const FunctionCallbackInfo<Value>& args) {
}
}

#ifndef OPENSSL_NO_ENGINE
// Helpers for the smart pointer.
void ENGINE_free_fn(ENGINE* engine) { ENGINE_free(engine); }

void ENGINE_finish_and_free_fn(ENGINE* engine) {
ENGINE_finish(engine);
ENGINE_free(engine);
}

void SecureContext::SetEngineKey(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

SecureContext* sc;
ASSIGN_OR_RETURN_UNWRAP(&sc, args.Holder());

CHECK_EQ(args.Length(), 2);

char errmsg[1024];
const node::Utf8Value engine_id(env->isolate(), args[1]);
std::unique_ptr<ENGINE, std::function<void(ENGINE*)>> e =
{ LoadEngineById(*engine_id, &errmsg),
ENGINE_free_fn };
if (e.get() == nullptr) {
return env->ThrowError(errmsg);
}

if (!ENGINE_init(e.get())) {
return env->ThrowError("ENGINE_init");
}

e.get_deleter() = ENGINE_finish_and_free_fn;

const node::Utf8Value key_name(env->isolate(), args[0]);
EVPKeyPointer key(ENGINE_load_private_key(e.get(), *key_name,
nullptr, nullptr));

if (!key) {
return ThrowCryptoError(env, ERR_get_error(), "ENGINE_load_private_key");
}

int rv = SSL_CTX_use_PrivateKey(sc->ctx_.get(), key.get());

if (rv == 0) {
return ThrowCryptoError(env, ERR_get_error(), "SSL_CTX_use_PrivateKey");
}

sc->private_key_engine_ = std::move(e);
}
#endif // !OPENSSL_NO_ENGINE

int SSL_CTX_get_issuer(SSL_CTX* ctx, X509* cert, X509** issuer) {
X509_STORE* store = SSL_CTX_get_cert_store(ctx);
DeleteFnPtr<X509_STORE_CTX, X509_STORE_CTX_free> store_ctx(
Expand Down Expand Up @@ -1438,9 +1491,6 @@ void SecureContext::LoadPKCS12(const FunctionCallbackInfo<Value>& args) {


#ifndef OPENSSL_NO_ENGINE
// Helper for the smart pointer.
void ENGINE_free_fn(ENGINE* engine) { ENGINE_free(engine); }

void SecureContext::SetClientCertEngine(
const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Expand Down
4 changes: 4 additions & 0 deletions src/node_crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ class SecureContext : public BaseObject {
X509Pointer issuer_;
#ifndef OPENSSL_NO_ENGINE
bool client_cert_engine_provided_ = false;
std::unique_ptr<ENGINE, std::function<void(ENGINE*)>> private_key_engine_;
#endif // !OPENSSL_NO_ENGINE

static const int kMaxSessionSize = 10 * 1024;
Expand All @@ -119,6 +120,9 @@ class SecureContext : public BaseObject {
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Init(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetKey(const v8::FunctionCallbackInfo<v8::Value>& args);
#ifndef OPENSSL_NO_ENGINE
static void SetEngineKey(const v8::FunctionCallbackInfo<v8::Value>& args);
#endif // !OPENSSL_NO_ENGINE
static void SetCert(const v8::FunctionCallbackInfo<v8::Value>& args);
static void AddCACert(const v8::FunctionCallbackInfo<v8::Value>& args);
static void AddCRL(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down
25 changes: 25 additions & 0 deletions test/addons/openssl-key-engine/binding.gyp
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
'targets': [
{
'target_name': 'testkeyengine',
'type': 'none',
'includes': ['../common.gypi'],
'conditions': [
['OS=="mac" and '
'node_use_openssl=="true" and '
'node_shared=="false" and '
'node_shared_openssl=="false"', {
'type': 'shared_library',
'sources': [ 'testkeyengine.cc' ],
'product_extension': 'engine',
'include_dirs': ['../../../deps/openssl/openssl/include'],
'link_settings': {
'libraries': [
'../../../../out/<(PRODUCT_DIR)/<(openssl_product)'
]
},
}],
]
}
]
}
62 changes: 62 additions & 0 deletions test/addons/openssl-key-engine/test.js
Original file line number Diff line number Diff line change
@@ -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}/testkeyengine.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 serverOptions = {
key: agentKey,
cert: agentCert,
ca: agentCa,
requestCert: true,
rejectUnauthorized: true
};

const server = https.createServer(serverOptions, common.mustCall((req, res) => {
res.writeHead(200);
res.end('hello world');
})).listen(0, common.localhostIPv4, common.mustCall(() => {
const clientOptions = {
method: 'GET',
host: common.localhostIPv4,
port: server.address().port,
path: '/test',
privateKeyEngine: engine,
privateKeyIdentifier: 'dummykey',
cert: agentCert,
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();
}));
73 changes: 73 additions & 0 deletions test/addons/openssl-key-engine/testkeyengine.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
#include <assert.h>
#include <string.h>
#include <stdlib.h>

#include <openssl/engine.h>
#include <openssl/pem.h>

#include <fstream>
#include <iterator>
#include <string>

#ifndef ENGINE_CMD_BASE
# error did not get engine.h
#endif

#define TEST_ENGINE_ID "testkeyengine"
#define TEST_ENGINE_NAME "dummy test key engine"

#define PRIVATE_KEY "test/fixtures/keys/agent1-key.pem"

namespace {

int EngineInit(ENGINE* engine) {
return 1;
}

int EngineFinish(ENGINE* engine) {
return 1;
}

int EngineDestroy(ENGINE* engine) {
return 1;
}

std::string LoadFile(const char* filename) {
std::ifstream file(filename);
return std::string(std::istreambuf_iterator<char>(file),
std::istreambuf_iterator<char>());
}

static EVP_PKEY* EngineLoadPrivkey(ENGINE* engine, const char* name,
UI_METHOD* ui_method, void* callback_data) {
if (strcmp(name, "dummykey") == 0) {
std::string key = LoadFile(PRIVATE_KEY);
BIO* bio = BIO_new_mem_buf(key.data(), key.size());
EVP_PKEY* ret = PEM_read_bio_PrivateKey(bio, nullptr, nullptr, nullptr);

BIO_vfree(bio);
if (ret != nullptr) {
return ret;
}
}

return nullptr;
}

int bind_fn(ENGINE* engine, const char* 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_privkey_function(engine, EngineLoadPrivkey);

return 1;
}

extern "C" {
IMPLEMENT_DYNAMIC_CHECK_FN();
IMPLEMENT_DYNAMIC_BIND_FN(bind_fn);
}

} // anonymous namespace
23 changes: 23 additions & 0 deletions test/parallel/test-tls-keyengine-invalid-arg-type.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict';
const common = require('../common');

if (!common.hasCrypto)
common.skip('missing crypto');

const tls = require('tls');

common.expectsError(
() => {
tls.createSecureContext({ privateKeyEngine: 0,
privateKeyIdentifier: 'key' });
},
{ code: 'ERR_INVALID_ARG_TYPE',
message: / Received type number$/ });

common.expectsError(
() => {
tls.createSecureContext({ privateKeyEngine: 'engine',
privateKeyIdentifier: 0 });
},
{ code: 'ERR_INVALID_ARG_TYPE',
message: / Received type number$/ });
34 changes: 34 additions & 0 deletions test/parallel/test-tls-keyengine-unsupported.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');

if (!common.hasCrypto)
common.skip('missing crypto');

// Monkey-patch SecureContext
const { internalBinding } = require('internal/test/binding');
const binding = internalBinding('crypto');
const NativeSecureContext = binding.SecureContext;

binding.SecureContext = function() {
const rv = new NativeSecureContext();
rv.setEngineKey = undefined;
return rv;
};

const tls = require('tls');

{
common.expectsError(
() => {
tls.createSecureContext({
privateKeyEngine: 'engine',
privateKeyIdentifier: 'key'
});
},
{
code: 'ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED',
message: 'Custom engines not supported by this OpenSSL'
}
);
}

0 comments on commit cf7b405

Please sign in to comment.