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.createServer secureOptions #9025

Closed
kobelb opened this issue Oct 11, 2016 · 9 comments
Closed

tls.createServer secureOptions #9025

kobelb opened this issue Oct 11, 2016 · 9 comments
Labels
doc Issues and PRs related to the documentations. tls Issues and PRs related to the tls subsystem.

Comments

@kobelb
Copy link
Contributor

kobelb commented Oct 11, 2016

I was looking for a way to limit the versions of TLS that are supported, and I came across the secureOptions in the tls.createServer method that appears to do exactly that.

However, I noticed that the documentation doesn't reference this option. Is this something that a pull request to modify the docs would be appropriate, or is this something that you all intend to deprecate/remove in the near-future?

@silverwind silverwind added tls Issues and PRs related to the tls subsystem. doc Issues and PRs related to the documentations. labels Oct 11, 2016
@mscdex
Copy link
Contributor

mscdex commented Oct 11, 2016

I'm assuming you meant secureProtocol? If so, I'm not sure what is missing since there is a link to the OpenSSL documentation that shows the valid names. Unless you're asking to include that inline in the node documentation?

@kobelb
Copy link
Contributor Author

kobelb commented Oct 11, 2016

I'm sorry for being unclear earlier, I've tried to document below what I've traced through the code.

The constructor for the tls Server calls self.setOptions, which has the following line: https://github.com/nodejs/node/blob/master/lib/_tls_wrap.js#L901

that ends up getting passed again by: https://github.com/nodejs/node/blob/master/lib/_tls_wrap.js#L761

which is passed to the SecureContext constructor as flags:
https://github.com/nodejs/node/blob/master/lib/_tls_common.js#L45

which is then passed again by:
https://github.com/nodejs/node/blob/master/lib/_tls_common.js#L32

ending with it calling the SecureContext::SetOptions method that passes through the options to OpenSSL here: https://github.com/nodejs/node/blob/master/src/node_crypto.cc#L875

@kobelb
Copy link
Contributor Author

kobelb commented Oct 18, 2016

@mscdex I know you're very busy, any chance you've had a chance to to look at my clarifications?

@mscdex
Copy link
Contributor

mscdex commented Oct 18, 2016

Why not use secureProtocol?

@kobelb
Copy link
Contributor Author

kobelb commented Oct 18, 2016

For my specific use-case, we can get a greater degree of flexibility by allowing users to blacklist TLSv1 while continuing to support TLSv1.1 and TLSv1.2.

@mscdex
Copy link
Contributor

mscdex commented Oct 18, 2016

/cc @nodejs/crypto

@bnoordhuis
Copy link
Member

secureOptions is not deprecated and documenting it is acceptable. I think the reason we haven't done that before is that most options are fairly subtle and require some understanding from the reader on the inner workings of openssl and TLS. You could start with SSL_OP_NO_TLSv1 and friends, those are pretty straightforward.

@kobelb
Copy link
Contributor Author

kobelb commented Oct 19, 2016

Fantastic, thanks @bnoordhuis. I'll get a pull request together for this.

@sam-github
Copy link
Contributor

closed by #9800, see #9340 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants