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

crypto/elliptic: normalize behavior of invalid curve points #50975

Closed
Tracked by #52182
FiloSottile opened this issue Feb 2, 2022 · 1 comment
Closed
Tracked by #52182

crypto/elliptic: normalize behavior of invalid curve points #50975

FiloSottile opened this issue Feb 2, 2022 · 1 comment
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Security
Milestone

Comments

@FiloSottile
Copy link
Contributor

See #50974 for background. tl;dr is that Marshal, Add, Double, and ScalarMult take a pair of big.Ints as inputs, which might not represent a valid point on the curve, and don't return an error value. The behavior is documented to be undefined.

I am fond of the idea of returning random points, like P-224, P-384, and P-521 do in Go 1.18, but it feels like it would be a pain to debug, and doesn't feel like the right answer for Marshal. Returning nil is definitely not the answer for Marshal, as that will get encoded as the empty string, which would be catastrophic for e.g. an ECDH shared secret, and anyway is likely to cause a panic. A panic is a DoS risk, but it would only occur where before there was a key leak risk.

The @golang/security team consensus is to move to triggering an explicit panic in Go 1.19. (Hopefully, we'll soon provide a better and safer API, too.)

@FiloSottile FiloSottile added Security NeedsFix The path to resolution is known, but the work has not been done. labels Feb 2, 2022
@FiloSottile FiloSottile added this to the Go1.19 milestone Feb 2, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/382995 mentions this issue: crypto/elliptic: panic when operating on invalid points

fritterhoff pushed a commit to hm-edu/pki-acme-service that referenced this issue Jun 16, 2022
The next Go release call panic on elliptic.Marshal [1][2], which
affect the test case fail_ec_marshal on createPublicKey.

This changes fix this by initializing the P and B in test case
PublicKey CurveParams to prevent panic.

[1] golang/go#50975
[2] golang/go@a218b35
@golang golang locked and limited conversation to collaborators May 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Security
Projects
None yet
Development

No branches or pull requests

2 participants