Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Document ciphers as part of tls.connect() options #25325

Closed
wants to merge 1 commit into from
Closed

Document ciphers as part of tls.connect() options #25325

wants to merge 1 commit into from

Conversation

iamthechad
Copy link

Refs #25270,#25271

@dnakamura
Copy link

Might also be worth mentioning mention crypto.getCiphers() and/or http://www.openssl.org/docs/apps/ciphers.html#CIPHER-LIST-FORMAT.
EDIT:
also honorCipherOrder is accepted, see tls.createSecureContext. (All the options are forwarded on to createSecureContext)

@iamthechad
Copy link
Author

I added a link to the getCiphers function. There was already a link to the cipher list format.

I don't think it's worth adding anything about honorCipherOrder because this setting doesn't make sense when connecting as a client.

@mhdawson
Copy link
Member

Thanks for taking the time to submit this PR as adding the doc will help people trying to figure out how you can pass ciphers.

I still want to take one last look after generating the doc with the change applied which I plan to do next week (its a long weekend here) but a few comments you can either fix up now or wait until I've done that:

  1. node.js should be Node.js.

  2. The first line of the commit comment should start with "doc: " and the total length should be under 50 chars.

  3. The commits will need to be squashed into a single commit. You can do this with a git rebase -i HEAD~2 and selecting squash for one of the two, followed by a push with a --force

@mhdawson
Copy link
Member

Looks good except that as a minor nit there is extra white space on 2 of the blank lines (I'm guessing an extra tab) above:

 - `ciphers`: A string describing the ciphers to use or exclude.
and
The full list of available ciphers can be obtained via [tls.getCiphers][].

Can you remove the extra white space, squash again to one commit and then re-push and then I"ll pull in

I just as general info I validated that passing the ciphers in the options for tls.connect is used in at least one of the existing test cases (simple/test-tls-honorcipherorder.js)

mhdawson pushed a commit that referenced this pull request May 21, 2015
@mhdawson
Copy link
Member

Landed as a294aee

Please close the associated issues if you believe they are now resolved

@mhdawson mhdawson closed this May 21, 2015
@iamthechad iamthechad deleted the tlsConnectDoc branch May 21, 2015 15:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants