Uh oh!
There was an error while loading. Please reload this page.
doc: adding tls.createServer secureOptions section - #9340
Conversation
There was a problem hiding this comment.
A bitmask affecting the protocol....
Please describe the default value/behaviour when not specified.
There was a problem hiding this comment.
I'm having trouble determining how to specify the default behavior because I'm unsure whether we want to disclose the underlying implementation to the user. The setting itself doesn't really default to anything; however, SSL_OP_NO_SSLv2 and SSL_OP_NO_SSLv3 are always set on the SSL context; and _tls_common.createSecureContext is adding SSL_OP_CIPHER_SERVER_PREFERENCE if the honorCipherOrder options is true. Is this something that we wish to describe to the user in this section?
There was a problem hiding this comment.
I don't see SSL_OP_NO_SSLv2 and SSL_OP_NO_SSLv3 always set on the SSL context, can you reference where?
Please review https://github.com/nodejs/node/pull/9800/files, I think I document this.
There was a problem hiding this comment.
The SecureContext itself appears to always be setting these here: https://github.com/nodejs/node/blob/master/src/node_crypto.cc#L398
I have no issues with your PR, and think the documentation you did is sufficient. Feel free to close this one.
There was a problem hiding this comment.
There are no more details in the Crypto Constants section! There are actually three sections, and I can't tell which of the options from which of the sections are actually allowed to be specified as a secureOption. Probably there should be a specific section for the options that are allowed in secureOptions, unless they all are? Or can the options that are valid for secureOptions be used in other places?
sam-github
commented
Oct 28, 2016
Definitely needs docs, two thumbs up for that, but these docs aren't quite complete enough to describe how to use the |
silverwind
commented
Nov 23, 2016
ping @kobelb |
kobelb
commented
Nov 23, 2016
@silverwind I'll try and get something together to address the comments early next week |
43f4705 to
59538ebComparesilverwind
commented
Nov 30, 2016
There's now another PR that adds this option: 32c15c9 @sam-github what shall we do with this one? |
sam-github
commented
Dec 13, 2016
Author agrees this can be closed #9340 (comment) |
Checklist
Affected core subsystem(s)
doc
Description of change
Adding documentation for the tls.createServer secureOptions.
Resolves: #9025