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: specific unreduced P-256 scalars produce incorrect results [1.20 backport] #58720

Closed
gopherbot opened this issue Feb 24, 2023 · 2 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge Security
Milestone

Comments

@gopherbot
Copy link
Contributor

@FiloSottile requested issue #58647 to be considered for backport to the next 1.20 minor release.

@gopherbot please backport. CL 471255 is a security fix.

/cc @golang/security and @golang/release

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Feb 24, 2023
@gopherbot gopherbot added this to the Go1.20.2 milestone Feb 24, 2023
@rolandshoemaker rolandshoemaker added the CherryPickApproved Used during the release process for point releases label Feb 24, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Feb 24, 2023
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/471695 mentions this issue: [release-branch.go1.20] crypto/internal/nistec: reduce P-256 scalar

@gopherbot
Copy link
Contributor Author

Closed by merging 602eeaa to release-branch.go1.20.

gopherbot pushed a commit that referenced this issue Feb 27, 2023
Unlike the rest of nistec, the P-256 assembly doesn't use complete
addition formulas, meaning that p256PointAdd[Affine]Asm won't return the
correct value if the two inputs are equal.

This was (undocumentedly) ignored in the scalar multiplication loops
because as long as the input point is not the identity and the scalar is
lower than the order of the group, the addition inputs can't be the same.

As part of the math/big rewrite, we went however from always reducing
the scalar to only checking its length, under the incorrect assumption
that the scalar multiplication loop didn't require reduction.

Added a reduction, and while at it added it in P256OrdInverse, too, to
enforce a universal reduction invariant on p256OrdElement values.

Note that if the input point is the infinity, the code currently still
relies on undefined behavior, but that's easily tested to behave
acceptably, and will be addressed in a future CL.

Updates #58647
Fixes #58720
Fixes CVE-2023-24532

(Filed with the "safe APIs like complete addition formulas are good" dept.)

Change-Id: I7b2c75238440e6852be2710fad66ff1fdc4e2b24
Reviewed-on: https://go-review.googlesource.com/c/go/+/471255
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit 203e59a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/471695
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Mar 3, 2023
Unlike the rest of nistec, the P-256 assembly doesn't use complete
addition formulas, meaning that p256PointAdd[Affine]Asm won't return the
correct value if the two inputs are equal.

This was (undocumentedly) ignored in the scalar multiplication loops
because as long as the input point is not the identity and the scalar is
lower than the order of the group, the addition inputs can't be the same.

As part of the math/big rewrite, we went however from always reducing
the scalar to only checking its length, under the incorrect assumption
that the scalar multiplication loop didn't require reduction.

Added a reduction, and while at it added it in P256OrdInverse, too, to
enforce a universal reduction invariant on p256OrdElement values.

Note that if the input point is the infinity, the code currently still
relies on undefined behavior, but that's easily tested to behave
acceptably, and will be addressed in a future CL.

Updates golang#58647
Fixes golang#58720
Fixes CVE-2023-24532

(Filed with the "safe APIs like complete addition formulas are good" dept.)

Change-Id: I7b2c75238440e6852be2710fad66ff1fdc4e2b24
Reviewed-on: https://go-review.googlesource.com/c/go/+/471255
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit 203e59a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/471695
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
@golang golang locked and limited conversation to collaborators Feb 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge Security
Projects
None yet
Development

No branches or pull requests

3 participants