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

crypto/x509: Incorrect documentation for ParsePKCS8PrivateKey #58789

Closed
babolivier opened this issue Feb 28, 2023 · 5 comments
Closed

crypto/x509: Incorrect documentation for ParsePKCS8PrivateKey #58789

babolivier opened this issue Feb 28, 2023 · 5 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.21

Comments

@babolivier
Copy link

Apologies for not following the template, it did not seem relevant since my issue is with the documentation of crypto/x509 as it currently shows on https://pkg.go.dev/crypto/x509#ParsePKCS8PrivateKey. I also did not open this issue as a pkg.go.dev bug since it's an issue with the package's documentation, not the documentation website itself. If either decision was incorrect please let me know and I'll correct them.

The documentation for x509.ParsePKCS8PrivateKey mentions the possible return types of the function as:

It returns a *rsa.PrivateKey, a *ecdsa.PrivateKey, a ed25519.PrivateKey (not a pointer), or a *ecdh.PublicKey (for X25519).

However, in the case of X25519, the actual implementation returns a *ecdh.PrivateKey (ref), which makes more sense, and also fits with what types x509.MarshalPKCS8PrivateKey is documented to accept for the key parameter.

Therefore it looks to me like the documentation is incorrect, and should instead be:

It returns a *rsa.PrivateKey, a *ecdsa.PrivateKey, a ed25519.PrivateKey (not a pointer), or a *ecdh.PrivateKey (for X25519).
@dmitshur
Copy link
Contributor

CC @golang/security.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 28, 2023
@dmitshur dmitshur added this to the Backlog milestone Feb 28, 2023
@gopherbot
Copy link

Change https://go.dev/cl/472155 mentions this issue: crypto/x509: fix ParsePKCS8PrivateKey comment

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 28, 2023
@dmitshur dmitshur modified the milestones: Backlog, Go1.21 Feb 28, 2023
@FiloSottile
Copy link
Contributor

Good catch, thank you! @gopherbot please open a backport issue to Go 1.20, this is a confusing documentation bug and it would be nice to get the fix live on pkg.go.dev before the next major release.

@gopherbot gopherbot mentioned this issue Feb 28, 2023
Closed
@gopherbot
Copy link

Backport issue(s) opened: #58793 (for 1.20).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@gopherbot
Copy link

Change https://go.dev/cl/472415 mentions this issue: [release-branch.go1.20] crypto/x509: fix ParsePKCS8PrivateKey comment

gopherbot pushed a commit that referenced this issue Mar 1, 2023
@rolandshoemaker @gopherbot
[release-branch.go1.20] crypto/x509: fix ParsePKCS8PrivateKey comment
bdd86bd 
Updates #58789.
Fixes #58793.

Change-Id: I91cdd20c6d4f05baaacd6a38717aa7bed6682573
Reviewed-on: https://go-review.googlesource.com/c/go/+/472155
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit ec26277)
Reviewed-on: https://go-review.googlesource.com/c/go/+/472415
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Mar 3, 2023
@rolandshoemaker @romaindoumenc
[release-branch.go1.20] crypto/x509: fix ParsePKCS8PrivateKey comment
e414da4 
Updates golang#58789.
Fixes golang#58793.

Change-Id: I91cdd20c6d4f05baaacd6a38717aa7bed6682573
Reviewed-on: https://go-review.googlesource.com/c/go/+/472155
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit ec26277)
Reviewed-on: https://go-review.googlesource.com/c/go/+/472415
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
@golang golang locked and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
4 participants
@FiloSottile @dmitshur @babolivier @gopherbot