crypto/ecdsa: reject negative inputs.

The fact that crypto/ecdsa.Verify didn't reject negative inputs was a
mistake on my part: I had unsigned numbers on the brain. However, it
doesn't generally cause problems. (ModInverse results in zero, which
results in x being zero, which is rejected.)

The amd64 P-256 code will crash when given a large, negative input.

This fixes both crypto/ecdsa to reject these values and also the P-256
code to ignore the sign of inputs.

Change-Id: I6370ed7ca8125e53225866f55b616a4022b818f8
Reviewed-on: https://go-review.googlesource.com/22093
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/crypto/ecdsa/ecdsa.go b/src/crypto/ecdsa/ecdsa.go
index e63bd8669ef443087afd1f4099e7cea36af197b2..288e366a882de237da907f6a4025ad910346eb95 100644 (file)
--- a/src/crypto/ecdsa/ecdsa.go
+++ b/src/crypto/ecdsa/ecdsa.go
@@ -228,7 +228,7 @@ func Verify(pub *PublicKey, hash []byte, r, s *big.Int) bool {
        c := pub.Curve
        N := c.Params().N
 
-       if r.Sign() == 0 || s.Sign() == 0 {
+       if r.Sign() <= 0 || s.Sign() <= 0 {
                return false
        }
        if r.Cmp(N) >= 0 || s.Cmp(N) >= 0 {

diff --git a/src/crypto/ecdsa/ecdsa_test.go b/src/crypto/ecdsa/ecdsa_test.go
index 5e588b92582bbc18e1cfd27b461491c6270955e6..fc25fd74a78ce7285bcdf96972fcdbdd577e7888 100644 (file)
--- a/src/crypto/ecdsa/ecdsa_test.go
+++ b/src/crypto/ecdsa/ecdsa_test.go
@@ -296,3 +296,26 @@ func TestVectors(t *testing.T) {
                }
        }
 }
+
+func testNegativeInputs(t *testing.T, curve elliptic.Curve, tag string) {
+       key, err := GenerateKey(curve, rand.Reader)
+       if err != nil {
+               t.Errorf("failed to generate key for %q", tag)
+       }
+
+       var hash [32]byte
+       r := new(big.Int).SetInt64(1)
+       r.Lsh(r, 550 /* larger than any supported curve */)
+       r.Neg(r)
+
+       if Verify(&key.PublicKey, hash[:], r, r) {
+               t.Errorf("bogus signature accepted for %q", tag)
+       }
+}
+
+func TestNegativeInputs(t *testing.T) {
+       testNegativeInputs(t, elliptic.P224(), "p224")
+       testNegativeInputs(t, elliptic.P256(), "p256")
+       testNegativeInputs(t, elliptic.P384(), "p384")
+       testNegativeInputs(t, elliptic.P521(), "p521")
+}

diff --git a/src/crypto/elliptic/p256_amd64.go b/src/crypto/elliptic/p256_amd64.go
index e96933e0c5566723238bee11ce4672d6b877948f..66b7cf8dc512a4948ab68a1bce75834377e93573 100644 (file)
--- a/src/crypto/elliptic/p256_amd64.go
+++ b/src/crypto/elliptic/p256_amd64.go
@@ -93,10 +93,14 @@ func p256PointAddAsm(res, in1, in2 []uint64)
 func p256PointDoubleAsm(res, in []uint64)
 
 func (curve p256Curve) Inverse(k *big.Int) *big.Int {
+       if k.Sign() < 0 {
+               // This should never happen.
+               k = new(big.Int).Neg(k)
+       }
+
        if k.Cmp(p256.N) >= 0 {
                // This should never happen.
-               reducedK := new(big.Int).Mod(k, p256.N)
-               k = reducedK
+               k = new(big.Int).Mod(k, p256.N)
        }
 
        // table will store precomputed powers of x. The four words at index