diff options
author | Anton Gerasimov <agerasimov@twilio.com> | 2019-08-05 12:03:23 +0200 |
---|---|---|
committer | Rich Trott <rtrott@gmail.com> | 2019-09-27 15:50:56 -0700 |
commit | c2ce8d05474c38c503b6ac57e94366421c960762 (patch) | |
tree | def403dc2cec32e1e689023669b23a37f9c03b68 | |
parent | 3de5eae6dbe503485b95bdeb8bddbd67e4613d59 (diff) | |
download | android-node-v8-c2ce8d05474c38c503b6ac57e94366421c960762.tar.gz android-node-v8-c2ce8d05474c38c503b6ac57e94366421c960762.tar.bz2 android-node-v8-c2ce8d05474c38c503b6ac57e94366421c960762.zip |
tls: add option for private keys for OpenSSL engines
Add `privateKeyIdentifier` and `privateKeyEngine` options
to get private key from an OpenSSL engine in tls.createSecureContext().
PR-URL: https://github.com/nodejs/node/pull/28973
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
-rw-r--r-- | doc/api/tls.md | 10 | ||||
-rw-r--r-- | lib/_tls_common.js | 30 | ||||
-rw-r--r-- | src/node_crypto.cc | 56 | ||||
-rw-r--r-- | src/node_crypto.h | 4 | ||||
-rw-r--r-- | test/addons/openssl-key-engine/binding.gyp | 25 | ||||
-rw-r--r-- | test/addons/openssl-key-engine/test.js | 62 | ||||
-rw-r--r-- | test/addons/openssl-key-engine/testkeyengine.cc | 73 | ||||
-rw-r--r-- | test/parallel/test-tls-keyengine-invalid-arg-type.js | 23 | ||||
-rw-r--r-- | test/parallel/test-tls-keyengine-unsupported.js | 34 |
9 files changed, 314 insertions, 3 deletions
diff --git a/doc/api/tls.md b/doc/api/tls.md index 68cf5e36bc..297c1e7fd4 100644 --- a/doc/api/tls.md +++ b/doc/api/tls.md @@ -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 @@ -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. diff --git a/lib/_tls_common.js b/lib/_tls_common.js index f24cfcbca6..6cd93036c2 100644 --- a/lib/_tls_common.js +++ b/lib/_tls_common.js @@ -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); diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 5f2e744e58..5630657d25 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -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); @@ -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( @@ -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); diff --git a/src/node_crypto.h b/src/node_crypto.h index 91586563f4..206a19119a 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -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; @@ -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); diff --git a/test/addons/openssl-key-engine/binding.gyp b/test/addons/openssl-key-engine/binding.gyp new file mode 100644 index 0000000000..6f9a8c32c1 --- /dev/null +++ b/test/addons/openssl-key-engine/binding.gyp @@ -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)' + ] + }, + }], + ] + } + ] +} diff --git a/test/addons/openssl-key-engine/test.js b/test/addons/openssl-key-engine/test.js new file mode 100644 index 0000000000..5c93e62636 --- /dev/null +++ b/test/addons/openssl-key-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}/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(); +})); diff --git a/test/addons/openssl-key-engine/testkeyengine.cc b/test/addons/openssl-key-engine/testkeyengine.cc new file mode 100644 index 0000000000..78a4fc49a6 --- /dev/null +++ b/test/addons/openssl-key-engine/testkeyengine.cc @@ -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 diff --git a/test/parallel/test-tls-keyengine-invalid-arg-type.js b/test/parallel/test-tls-keyengine-invalid-arg-type.js new file mode 100644 index 0000000000..385841a654 --- /dev/null +++ b/test/parallel/test-tls-keyengine-invalid-arg-type.js @@ -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$/ }); diff --git a/test/parallel/test-tls-keyengine-unsupported.js b/test/parallel/test-tls-keyengine-unsupported.js new file mode 100644 index 0000000000..149fc4dc31 --- /dev/null +++ b/test/parallel/test-tls-keyengine-unsupported.js @@ -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' + } + ); +} |