Skip to content

doc: add note about available ECC curves#1913

Closed
petschekr wants to merge 4 commits into
nodejs:masterfrom
petschekr:doc-list-curves
Closed

doc: add note about available ECC curves#1913
petschekr wants to merge 4 commits into
nodejs:masterfrom
petschekr:doc-list-curves

Conversation

@petschekr
Copy link
Copy Markdown
Contributor

@petschekr petschekr commented Jun 7, 2015

Added instructions on how to get the elliptic curves supported by the OpenSSL installation in the crypto.createECDH() constructor. Also made a few minor grammar fixes within the same paragraph.

@petschekr
doc: add note about available ECC curves
e6f7d93 
Added instructions on how to get the elliptic curves supported by the
OpenSSL installation in the crypto.createECDH() constructor. Also made
a few minor grammar fixes within the same paragraph.
@silverwind
Copy link
Copy Markdown
Contributor

silverwind commented Jun 7, 2015

Could you also add this to the ecdhCurve option of tls.createServer? If that RFC link is still relevant, keep it, otherwise remove it I'd say.

@mscdex mscdex added crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. labels Jun 7, 2015
@petschekr
Copy link
Copy Markdown
Contributor Author

petschekr commented Jun 7, 2015

Sure, I'll go do that right now

@mscdex
Copy link
Copy Markdown
Contributor

mscdex commented Jun 7, 2015

Maybe we should have a function that returns the supported curve names like we have with ciphers and hashes?

@petschekr
Copy link
Copy Markdown
Contributor Author

petschekr commented Jun 7, 2015

@silverwind I just extended the note about getting available curves to the TLS docs as well.

@silverwind
Copy link
Copy Markdown
Contributor

silverwind commented Jun 7, 2015

Let's remove the RFC note above (it's not really helpful at first glance) and then put your message on the same line.

@petschekr
Copy link
Copy Markdown
Contributor Author

petschekr commented Jun 7, 2015

Ok, done

@silverwind
Copy link
Copy Markdown
Contributor

silverwind commented Jun 7, 2015

LGTM

@mscdex
Copy link
Copy Markdown
Contributor

mscdex commented Jun 7, 2015

FWIW we can use crypto.getCurves() to get available curves if #1914 gets accepted.

@silverwind
Copy link
Copy Markdown
Contributor

silverwind commented Jun 7, 2015

Wow, that is pretty neat indeed.

@silverwind
Copy link
Copy Markdown
Contributor

silverwind commented Jun 7, 2015

Going to merge this now, we can update docs again in the other PR.

silverwind
silverwind reviewed Jun 7, 2015
View reviewed changes
Comment thread doc/api/crypto.markdown Outdated
Copy link
Copy Markdown
Contributor

@silverwind silverwind Jun 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petschekr trailing whitespace on this line, could you remove that?

Copy link
Copy Markdown
Contributor Author

@petschekr petschekr Jun 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@silverwind Just fixed it in b116c6f

silverwind pushed a commit that referenced this pull request Jun 7, 2015
@petschekr @silverwind
doc: add note about available ECC curves
deb8b87 
Added instructions on how to get the elliptic curves supported by the
OpenSSL installation in the crypto.createECDH() constructor. Also made
a few minor grammar fixes within the same paragraph.

PR-URL: #1913
Reviewed-By: Roman Reiss <me@silverwind.io>
@silverwind
Copy link
Copy Markdown
Contributor

silverwind commented Jun 7, 2015

Thanks! landed in deb8b87

@silverwind silverwind closed this Jun 7, 2015
@silverwind silverwind added the tls Issues and PRs related to the tls subsystem. label Jun 7, 2015
@rvagg rvagg mentioned this pull request Jun 11, 2015
Closed
@aliscco aliscco mentioned this pull request Mar 29, 2024
Open
@leonardoadame leonardoadame mentioned this pull request Apr 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. tls Issues and PRs related to the tls subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@petschekr @silverwind @mscdex