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.

Sign up for GitHub

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

Jump to bottom

TLS module: Support multiple ecdhCurve's #15054

Closed
Hativ opened this issue Aug 27, 2017 · 2 comments
Closed

TLS module: Support multiple ecdhCurve's #15054

Hativ opened this issue Aug 27, 2017 · 2 comments
Labels
feature request Issues that request new features to be added to Node.js. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. tls Issues and PRs related to the tls subsystem.

Comments

@Hativ
Copy link
Contributor

Hativ commented Aug 27, 2017 •
edited
Loading

The tls module respectively tls.createSecureContext should support multiple echdCurve's like nginx does.

Example:

const options = {
   ecdhCurves: 'x25519:secp521r1:secp384r1',
 };

The order should be honored (perhaps configurable like honorChiperOrder).
@mscdex mscdex added tls Issues and PRs related to the tls subsystem. feature request Issues that request new features to be added to Node.js. labels Aug 27, 2017
@bnoordhuis bnoordhuis added the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Aug 28, 2017
@bnoordhuis
Copy link
Member

Pull requests welcome. The way to do it is to call SSL_CTX_set_ecdh_auto() + SSL_CTX_set1_curves() or SSL_CTX_set1_curves_list(). We currently call SSL_CTX_set_tmp_ecdh(), that may have to change.

An array of strings is arguably a little more idiomatic in node.js but it's probably not a deal breaker.

honorChiperOrder should already affect who gets to select the curve, client or server.

@rogaps rogaps mentioned this issue Sep 5, 2017
Closed
4 tasks
rogaps added a commit to rogaps/node that referenced this issue Sep 18, 2017
@rogaps
crypto: support multiple ECDH curves and auto
31846f5 
Using SSL_CTX_set1_curves_list() (OpenSSL 1.0.2+), this allows to set
colon separated ECDH curve names in SecureContext's ecdhCurve option.
The option can also be set to "auto" to select the curve automatically
from list built in OpenSSL by enabling SSL_CTX_set_ecdh_auto()
(OpenSSL 1.0.2+).

Refs: nodejs#15054
jasnell pushed a commit that referenced this issue Sep 20, 2017
@rogaps @jasnell
crypto: support multiple ECDH curves and auto
873e5bd 
Using SSL_CTX_set1_curves_list() (OpenSSL 1.0.2+), this allows to set
colon separated ECDH curve names in SecureContext's ecdhCurve option.
The option can also be set to "auto" to select the curve automatically
from list built in OpenSSL by enabling SSL_CTX_set_ecdh_auto()
(OpenSSL 1.0.2+).

PR-URL: #15206
Ref: #15054
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
jasnell pushed a commit that referenced this issue Sep 20, 2017
@rogaps @jasnell
crypto: support multiple ECDH curves and auto
bd07574 
Using SSL_CTX_set1_curves_list() (OpenSSL 1.0.2+), this allows to set
colon separated ECDH curve names in SecureContext's ecdhCurve option.
The option can also be set to "auto" to select the curve automatically
from list built in OpenSSL by enabling SSL_CTX_set_ecdh_auto()
(OpenSSL 1.0.2+).

PR-URL: #15206
Ref: #15054
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@bnoordhuis
Copy link
Member

Closing, #15206 was merged and is scheduled to be released in v8.6.0.

Qard pushed a commit to Qard/ayo that referenced this issue Sep 21, 2017
@rogaps
crypto: support multiple ECDH curves and auto
ce0f18f 
Using SSL_CTX_set1_curves_list() (OpenSSL 1.0.2+), this allows to set
colon separated ECDH curve names in SecureContext's ecdhCurve option.
The option can also be set to "auto" to select the curve automatically
from list built in OpenSSL by enabling SSL_CTX_set_ecdh_auto()
(OpenSSL 1.0.2+).

PR-URL: nodejs/node#15206
Ref: nodejs/node#15054
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Qard pushed a commit to Qard/ayo that referenced this issue Sep 21, 2017
@rogaps
crypto: support multiple ECDH curves and auto
12ad010 
Using SSL_CTX_set1_curves_list() (OpenSSL 1.0.2+), this allows to set
colon separated ECDH curve names in SecureContext's ecdhCurve option.
The option can also be set to "auto" to select the curve automatically
from list built in OpenSSL by enabling SSL_CTX_set_ecdh_auto()
(OpenSSL 1.0.2+).

PR-URL: nodejs/node#15206
Ref: nodejs/node#15054
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@joshxyzhimself joshxyzhimself mentioned this issue Dec 13, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request Issues that request new features to be added to Node.js. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mscdex @bnoordhuis @Hativ