go
go copied to clipboard
golang
crypto/x509: panics on invalid curve instead of returning error
What version of Go are you using (go version)?
$ go version 1.19
What did you do?
In go 1.19 crypto/elliptic will now panic when operating on invalid curve points. This is great as it avoids undefined behavior, but crypto/x509 will now panic in marshalPublicKey in the case *ecdsa.PublicKey: where it previosly would return errors.New("x509: unsupported elliptic curve") as it calls elliptic.Marshal(pub.Curve, pub.X, pub.Y) before it checks oidFromNamedCurve(pub.Curve).
So simply switching those two calls around should fix the issue.
Meanwhile clients have to manually check if their key satisfies IsOnCurve before calling something like x509.MarshalPKIXPublicKey where in previous versions of go you could rely on an error being returned instead of a panic.
Change https://go.dev/cl/421617 mentions this issue: crypto/x509: err instead of panic on invalid curve
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.
Backport issue(s) opened: #54295 (for 1.19).
Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.
Change https://go.dev/cl/422298 mentions this issue: crypto/x509: don't panic marshaling invalid ECDSA keys
Ok. I had already opened #54290 along with this issue, anything else i should do on my end?
@FnuGk no, we've got it, thank you for the report! https://go.dev/cl/422298 is a slightly broader fix that is now under review.
Change https://go.dev/cl/425634 mentions this issue: [release-branch.go1.19] crypto/x509: don't panic marshaling invalid ECDSA keys