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.

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: panics on invalid curve instead of returning error [1.19 backport] #54295

Closed
gopherbot opened this issue Aug 5, 2022 · 2 comments
Closed

crypto/x509: panics on invalid curve instead of returning error [1.19 backport] #54295

gopherbot opened this issue Aug 5, 2022 · 2 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.19.1

Comments

@gopherbot
Copy link

@FiloSottile requested issue #54288 to be considered for backport to the next 1.19 minor release.

Ah, yeah, functions with an error return value should definitely return an error, not panic. I'll do a pass of all the marshal-side paths, and see if there are other issues like this.

@gopherbot please open a backport issue to Go 1.19. I don't think this is a security issue because the attacker can't control the curve of a certificate being marshaled, but panic'ing where we were returning an error is a regression and we should quash it.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Aug 5, 2022
@gopherbot gopherbot mentioned this issue Aug 5, 2022
Closed
@gopherbot gopherbot added this to the Go1.19.1 milestone Aug 5, 2022
@joedian joedian added the CherryPickApproved Used during the release process for point releases label Aug 10, 2022
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Aug 10, 2022
@gopherbot
Copy link
Author

Change https://go.dev/cl/425634 mentions this issue: [release-branch.go1.19] crypto/x509: don't panic marshaling invalid ECDSA keys

@gopherbot
Copy link
Author

Closed by merging 2553a09 to release-branch.go1.19.

gopherbot pushed a commit that referenced this issue Aug 29, 2022
@FiloSottile @heschi
[release-branch.go1.19] crypto/x509: don't panic marshaling invalid E…
2553a09 
…CDSA keys

MarshalPKIXPublicKey, CreateCertificate, CreateCertificateRequest,
MarshalECPrivateKey, and MarshalPKCS8PrivateKey started raising a panic
when encoding an invalid ECDSA key in Go 1.19. Since they have an error
return value, they should return an error instead.

Updates #54288
Fixes #54295

Change-Id: Iba132cd2f890ece36bb7d0396eb9a9a77bdb81df
Reviewed-on: https://go-review.googlesource.com/c/go/+/422298
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit f64f12f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/425634
bradfitz pushed a commit to tailscale/go that referenced this issue Sep 8, 2022
@FiloSottile @bradfitz
[release-branch.go1.19] crypto/x509: don't panic marshaling invalid E…
505e77f 
…CDSA keys

MarshalPKIXPublicKey, CreateCertificate, CreateCertificateRequest,
MarshalECPrivateKey, and MarshalPKCS8PrivateKey started raising a panic
when encoding an invalid ECDSA key in Go 1.19. Since they have an error
return value, they should return an error instead.

Updates golang#54288
Fixes golang#54295

Change-Id: Iba132cd2f890ece36bb7d0396eb9a9a77bdb81df
Reviewed-on: https://go-review.googlesource.com/c/go/+/422298
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit f64f12f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/425634
@golang golang locked and limited conversation to collaborators Aug 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Milestone
Go1.19.1
Development

No branches or pull requests
2 participants
@gopherbot @joedian