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

docs: fix: correctly use public key instead of private key #16038

Closed
wants to merge 1 commit into from
Closed

docs: fix: correctly use public key instead of private key #16038

wants to merge 1 commit into from

Conversation

pomerantsev
Copy link
Contributor

Fixes #13633

Although, as docs mention, private keys can be used instead of
public keys, I presume that these parameter explanations
should be corrected.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@pomerantsev
docs: fix: correctly use public key instead of private key
efb5a0a 
Fixes nodejs#13633

Although, as docs mention, private keys can be used instead of
public keys, I presume that these parameter explanations
should be corrected.
@nodejs-github-bot nodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. labels Oct 6, 2017
@Trott Trott added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Oct 6, 2017
@addaleax
Copy link
Member

addaleax commented Oct 7, 2017

Landed in 1b358f1

@addaleax addaleax closed this Oct 7, 2017
addaleax pushed a commit that referenced this pull request Oct 7, 2017
@pomerantsev @addaleax
doc: fix: correctly use public key instead of private key
1b358f1 
Although, as docs mention, private keys can be used instead of
public keys, I presume that these parameter explanations
should be corrected.

Fixes: #13633
PR-URL: #16038
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@tniessen
Copy link
Member

tniessen commented Oct 7, 2017 •
edited
Loading

I believe I fixed this in a backport to v6.x (#14376), assuming it will land at some point, so this does not need to be backported to v6.x.

@seishun
Copy link
Contributor

seishun commented Oct 7, 2017

This is actually incorrect. Public keys don't have passphrases - this property is only needed if you provide an encrypted private key instead of a public key.

@addaleax
Copy link
Member

addaleax commented Oct 7, 2017

@seishun If I understand you correctly, this patch is correct, it’s just that there were more mistakes in the documentation than what was reported in #13633. Do you want to open a PR for that?

@tniessen
Copy link
Member

tniessen commented Oct 7, 2017 •
edited
Loading

No, I think @seishun is right, this patch is not entirely correct. The change is partially correct, but the passphrase is only needed if a private key is passed instead of a public key.

@addaleax
Copy link
Member

addaleax commented Oct 7, 2017

Okay – that doesn’t really change the point, does either of you (@seishun @tniessen) want to open a PR for that?

MylesBorins pushed a commit that referenced this pull request Oct 7, 2017
@pomerantsev @MylesBorins
doc: fix: correctly use public key instead of private key
6d975ea 
Although, as docs mention, private keys can be used instead of
public keys, I presume that these parameter explanations
should be corrected.

Fixes: #13633
PR-URL: #16038
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Oct 7, 2017
Merged
@tniessen
Copy link
Member

tniessen commented Oct 7, 2017

Oops, my bad, this is unrelated to the mentioned backport PR, it was just a very similar fix (but that one was hopefully correct).
I won't be able to open a PR until the 13th.

@bnoordhuis bnoordhuis mentioned this pull request Oct 8, 2017
Closed
@bnoordhuis
Copy link
Member

#16087

bnoordhuis added a commit to bnoordhuis/io.js that referenced this pull request Oct 8, 2017
@bnoordhuis
doc: public keys don't accept passphrases
3cdd070 
Since `crypto.publicDecrypt()` and `crypto.publicEncrypt()` accept both
public and private keys, make it clear that the `passphrase` option only
applies to private keys.

Refs: nodejs#16038
MylesBorins pushed a commit that referenced this pull request Oct 11, 2017
@pomerantsev @MylesBorins
doc: fix: correctly use public key instead of private key
7475541 
Although, as docs mention, private keys can be used instead of
public keys, I presume that these parameter explanations
should be corrected.

Fixes: #13633
PR-URL: #16038
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
addaleax pushed a commit to addaleax/ayo that referenced this pull request Oct 12, 2017
@pomerantsev @addaleax
doc: fix: correctly use public key instead of private key
73450b3 
Although, as docs mention, private keys can be used instead of
public keys, I presume that these parameter explanations
should be corrected.

Fixes: nodejs/node#13633
PR-URL: nodejs/node#16038
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@MylesBorins
Copy link
Contributor

This does not land cleanly in LTS. Please feel free to manually backport by following the guide. Please also feel free to replace do-not-land if it is being backported

jasnell pushed a commit that referenced this pull request Oct 18, 2017
@bnoordhuis @jasnell
doc: public keys don't accept passphrases
3469eb9 
Since `crypto.publicDecrypt()` and `crypto.publicEncrypt()` accept both
public and private keys, make it clear that the `passphrase` option only
applies to private keys.

PR-URL: #16087
Ref: #16038
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
targos pushed a commit that referenced this pull request Oct 18, 2017
@bnoordhuis @targos
doc: public keys don't accept passphrases
e34509e 
Since `crypto.publicDecrypt()` and `crypto.publicEncrypt()` accept both
public and private keys, make it clear that the `passphrase` option only
applies to private keys.

PR-URL: #16087
Ref: #16038
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 26, 2017
@bnoordhuis @addaleax
doc: public keys don't accept passphrases
76e0ecb 
Since `crypto.publicDecrypt()` and `crypto.publicEncrypt()` accept both
public and private keys, make it clear that the `passphrase` option only
applies to private keys.

PR-URL: nodejs/node#16087
Ref: nodejs/node#16038
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
@bnoordhuis @addaleax
doc: public keys don't accept passphrases
dca3ef6 
Since `crypto.publicDecrypt()` and `crypto.publicEncrypt()` accept both
public and private keys, make it clear that the `passphrase` option only
applies to private keys.

PR-URL: nodejs/node#16087
Ref: nodejs/node#16038
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BridgeAR BridgeAR BridgeAR approved these changes

@addaleax addaleax addaleax approved these changes

Assignees
No one assigned
Labels
code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Misleading documentation in crypto.publicEncrypt and publicDecrypt
9 participants
@pomerantsev @addaleax @tniessen @seishun @bnoordhuis @MylesBorins @BridgeAR @Trott @nodejs-github-bot