Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TLS: keys from engines? #28921

Closed
OYTIS opened this issue Aug 1, 2019 · 6 comments
Closed

TLS: keys from engines? #28921

OYTIS opened this issue Aug 1, 2019 · 6 comments
Labels
crypto Issues and PRs related to the crypto subsystem. feature request Issues that request new features to be added to Node.js. tls Issues and PRs related to the tls subsystem.

Comments

@OYTIS
Copy link
Contributor

OYTIS commented Aug 1, 2019

As far as I can see NodeJS TLS does support OpenSSL engines. But it looks like it only supports them for certificates.

My use-case is a private key managed inside an engine. What I would do, e.g. with libcurl is

// load the engine
curl_easy_setopt(curl, CURLOPT_SSLENGINE, "mycoolengine");

// set it as a default engine
curl_easy_setopt(curl, CURLOPT_SSLENGINE_DEFAULT, 1L);

// tell libcurl that private key is managed by the engine
curl_easy_setopt(curl, CURLOPT_SSLKEYTYPE, "ENG");

// set key name (arbitrary string, interpreted by the engine)
curl_easy_setopt(curl, CURLOPT_SSLKEY, "myenginekeyname");

With nodejs it goes like:

> tls_ctx = tls.createSecureContext({"clientCertEngine":"mycoolengine", "key":"myenginekeyname"})

Error: error:0906D06C:PEM routines:PEM_read_bio:no start line
    at Object.createSecureContext (_tls_common.js:104:17)

So looks like it tries to interpret "myenginekeyname" as PEM. Am I doing something wrong, or is this use-case not supported at all?

Thanks!

@bnoordhuis bnoordhuis added crypto Issues and PRs related to the crypto subsystem. feature request Issues that request new features to be added to Node.js. tls Issues and PRs related to the tls subsystem. labels Aug 3, 2019
@bnoordhuis
Copy link
Member

Your custom engine needs to call ENGINE_set_load_privkey_function() (and implement the appropriate callback, of course.)

@OYTIS
Copy link
Contributor Author

OYTIS commented Aug 3, 2019

@bnoordhuis load_privkey callback is there, but I can't see how it would be used by tls package.

tls uses SecureContext and SecureContext::SetKey always interprets key as PEM: https://github.com/nodejs/node/blob/master/src/node_crypto.cc#L696

@OYTIS
Copy link
Contributor Author

OYTIS commented Aug 3, 2019

@bnoordhuis Thank you for getting to me by the way :)

@bnoordhuis
Copy link
Member

Ah, I think I misunderstood your example. 'key' is always interpreted as a PEM key right now and I don't think overloading it is a good idea (or could even work because of ambiguity.)

A new option that tells Node.js to load the key with ENGINE_ctrl_cmd("LOAD_CERT_CTRL") is probably acceptable. PR welcome. I might take a stab at it if you don't.

@OYTIS
Copy link
Contributor Author

OYTIS commented Aug 5, 2019

@bnoordhuis Cool, thank you for confirming. I've added a PR with a possible implementation, not sure how well it fits the existing code base.

@OYTIS
Copy link
Contributor Author

OYTIS commented Sep 28, 2019

Fixed by #28973

@OYTIS OYTIS closed this as completed Sep 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. feature request Issues that request new features to be added to Node.js. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

No branches or pull requests

2 participants