diff --git a/dcrec/secp256k1/bench_test.go b/dcrec/secp256k1/bench_test.go index a8f39e10..beda64be 100644 --- a/dcrec/secp256k1/bench_test.go +++ b/dcrec/secp256k1/bench_test.go @@ -191,14 +191,12 @@ func BenchmarkNonceRFC6979(b *testing.B) { func BenchmarkPubKeyDecompress(b *testing.B) { // Randomly generated keypair. // Private key: 9e0699c91ca1e3b7e3c9ba71eb71c89890872be97576010fe593fbf3fd57e66d - pubKeyX := fromHex("d2e670a19c6d753d1a6d8b20bd045df8a08fb162cf508956c31268c6d81ffdab") + pubKeyX := new(fieldVal).SetHex("d2e670a19c6d753d1a6d8b20bd045df8a08fb162cf508956c31268c6d81ffdab") b.ReportAllocs() b.ResetTimer() + var y fieldVal for i := 0; i < b.N; i++ { - _, err := decompressPoint(pubKeyX, false) - if err != nil { - b.Fatalf("unexpected err: %v", err) - } + _ = decompressY(pubKeyX, false, &y) } } diff --git a/dcrec/secp256k1/field_test.go b/dcrec/secp256k1/field_test.go index ea2901d9..5b482664 100644 --- a/dcrec/secp256k1/field_test.go +++ b/dcrec/secp256k1/field_test.go @@ -15,9 +15,27 @@ import ( "time" ) +// randFieldVal returns a fieldVal created from a random value generated by the +// passed rng. +func randFieldVal(t *testing.T, rng *rand.Rand) *fieldVal { + t.Helper() + + var buf [32]byte + if _, err := rng.Read(buf[:]); err != nil { + t.Fatalf("failed to read random: %v", err) + } + + // Create and return both a big integer and a field value. + var fv fieldVal + fv.SetBytes(&buf) + return &fv +} + // randIntAndFieldVal returns a big integer and fieldVal both created from the // same random value generated by the passed rng. func randIntAndFieldVal(t *testing.T, rng *rand.Rand) (*big.Int, *fieldVal) { + t.Helper() + var buf [32]byte if _, err := rng.Read(buf[:]); err != nil { t.Fatalf("failed to read random: %v", err) diff --git a/dcrec/secp256k1/pubkey.go b/dcrec/secp256k1/pubkey.go index 1a866a55..762fc4e2 100644 --- a/dcrec/secp256k1/pubkey.go +++ b/dcrec/secp256k1/pubkey.go @@ -17,6 +17,30 @@ const ( PubKeyBytesLenUncompressed = 65 ) +// decompressY attempts to calculate the Y coordinate for the given X coordinate +// such that the result pair is a point on the secp256k1 curve. It adjusts Y +// based on the desired oddness and returns whether or not it was successful +// since not all X coordinates are valid. +// +// The magnitude of the provided X coordinate field val must be a max of 8 for a +// correct result. The resulting Y field val will have a max magnitude of 2. +func decompressY(x *fieldVal, odd bool, resultY *fieldVal) bool { + // The curve equation for secp256k1 is: y^2 = x^3 + 7. Thus + // y = +-sqrt(x^3 + 7). + // + // The x coordinate must be invalid if there is no square root for the + // calculated rhs because it means the X coordinate is not for a point on + // the curve. + x3PlusB := new(fieldVal).SquareVal(x).Mul(x).AddInt(7) + if hasSqrt := resultY.SquareRootVal(x3PlusB); !hasSqrt { + return false + } + if resultY.Normalize().IsOdd() != odd { + resultY.Negate(1) + } + return true +} + func isOdd(a *big.Int) bool { return a.Bit(0) == 1 } @@ -24,25 +48,14 @@ func isOdd(a *big.Int) bool { // decompressPoint decompresses a point on the given curve given the X point and // the solution to use. func decompressPoint(x *big.Int, ybit bool) (*big.Int, error) { - curve := S256() - // Y = +-sqrt(x^3 + B) - x3 := new(big.Int).Mul(x, x) - x3.Mul(x3, x) - x3.Add(x3, curve.Params().B) - - // now calculate sqrt mod p of x2 + B - // This code used to do a full sqrt based on tonelli/shanks, - // but this was replaced by the algorithms referenced in - // https://bitcointalk.org/index.php?topic=162805.msg1712294#msg1712294 - y := new(big.Int).Exp(x3, curve.q, curve.P) - - if ybit != isOdd(y) { - y.Sub(curve.P, y) + var fy fieldVal + fx := new(fieldVal).SetByteSlice(x.Bytes()) + if !decompressY(fx, ybit, &fy) { + return nil, fmt.Errorf("invalid public key x coordinate") } - if ybit != isOdd(y) { - return nil, fmt.Errorf("ybit doesn't match oddness") - } - return y, nil + fy.Normalize() + + return new(big.Int).SetBytes(fy.Bytes()[:]), nil } const ( diff --git a/dcrec/secp256k1/pubkey_test.go b/dcrec/secp256k1/pubkey_test.go index 7eb784ac..e6546983 100644 --- a/dcrec/secp256k1/pubkey_test.go +++ b/dcrec/secp256k1/pubkey_test.go @@ -7,7 +7,9 @@ package secp256k1 import ( "bytes" + "math/rand" "testing" + "time" "github.com/davecgh/go-spew/spew" ) @@ -277,3 +279,168 @@ func TestPublicKeyIsEqual(t *testing.T) { "equal to %v", pubKey1, pubKey2) } } + +// TestDecompressY ensures that decompressY works as expected for some edge +// cases. +func TestDecompressY(t *testing.T) { + tests := []struct { + name string // test description + x string // hex encoded x coordinate + valid bool // expected decompress result + wantOddY string // hex encoded expected odd y coordinate + wantEvenY string // hex encoded expected even y coordinate + }{{ + name: "x = 0 -- not a point on the curve", + x: "0", + valid: false, + wantOddY: "", + wantEvenY: "", + }, { + name: "x = 1", + x: "1", + valid: true, + wantOddY: "bde70df51939b94c9c24979fa7dd04ebd9b3572da7802290438af2a681895441", + wantEvenY: "4218f20ae6c646b363db68605822fb14264ca8d2587fdd6fbc750d587e76a7ee", + }, { + name: "x = secp256k1 prime (aka 0) -- not a point on the curve", + x: "fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f", + valid: false, + wantOddY: "", + wantEvenY: "", + }, { + name: "x = secp256k1 prime - 1 -- not a point on the curve", + x: "fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2e", + valid: false, + wantOddY: "", + wantEvenY: "", + }, { + name: "x = secp256k1 group order", + x: "fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141", + valid: true, + wantOddY: "670999be34f51e8894b9c14211c28801d9a70fde24b71d3753854b35d07c9a11", + wantEvenY: "98f66641cb0ae1776b463ebdee3d77fe2658f021db48e2c8ac7ab4c92f83621e", + }, { + name: "x = secp256k1 group order - 1 -- not a point on the curve", + x: "fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364140", + valid: false, + wantOddY: "", + wantEvenY: "", + }} + + for _, test := range tests { + // Decompress the test odd y coordinate for the given test x coordinate + // and ensure the returned validity flag matches the expected result. + var oddY fieldVal + fx := new(fieldVal).SetHex(test.x) + valid := decompressY(fx, true, &oddY) + if valid != test.valid { + t.Errorf("%s: unexpected valid flag -- got: %v, want: %v", + test.name, valid, test.valid) + continue + } + + // Decompress the test even y coordinate for the given test x coordinate + // and ensure the returned validity flag matches the expected result. + var evenY fieldVal + valid = decompressY(fx, false, &evenY) + if valid != test.valid { + t.Errorf("%s: unexpected valid flag -- got: %v, want: %v", + test.name, valid, test.valid) + continue + } + + // Skip checks related to the y coordinate when there isn't one. + if !valid { + continue + } + + // Ensure the decompressed odd Y coordinate is the expected value. + oddY.Normalize() + wantOddY := new(fieldVal).SetHex(test.wantOddY) + if !wantOddY.Equals(&oddY) { + t.Errorf("%s: mismatched odd y\ngot: %v, want: %v", test.name, + oddY, wantOddY) + continue + } + + // Ensure the decompressed even Y coordinate is the expected value. + evenY.Normalize() + wantEvenY := new(fieldVal).SetHex(test.wantEvenY) + if !wantEvenY.Equals(&evenY) { + t.Errorf("%s: mismatched even y\ngot: %v, want: %v", test.name, + evenY, wantEvenY) + continue + } + + // Ensure the decompressed odd y coordinate is actually odd. + if !oddY.IsOdd() { + t.Errorf("%s: odd y coordinate is even", test.name) + continue + } + + // Ensure the decompressed even y coordinate is actually even. + if evenY.IsOdd() { + t.Errorf("%s: even y coordinate is odd", test.name) + continue + } + } +} + +// TestDecompressYRandom ensures that decompressY works as expected with +// randomly-generated x coordinates. +func TestDecompressYRandom(t *testing.T) { + // Use a unique random seed each test instance and log it if the tests fail. + seed := time.Now().Unix() + rng := rand.New(rand.NewSource(seed)) + defer func(t *testing.T, seed int64) { + if t.Failed() { + t.Logf("random seed: %d", seed) + } + }(t, seed) + + for i := 0; i < 100; i++ { + origX := randFieldVal(t, rng) + + // Calculate both corresponding y coordinates for the random x when it + // is a valid coordinate. + var oddY, evenY fieldVal + x := new(fieldVal).Set(origX) + oddSuccess := decompressY(x, true, &oddY) + evenSuccess := decompressY(x, false, &evenY) + + // Ensure that the decompression success matches for both the even and + // odd cases depending on whether or not x is a valid coordinate. + if oddSuccess != evenSuccess { + t.Fatalf("mismatched decompress success for x = %v -- odd: %v, "+ + "even: %v", x, oddSuccess, evenSuccess) + } + if !oddSuccess { + continue + } + + // Ensure the x coordinate was not changed. + if !x.Equals(origX) { + t.Fatalf("x coordinate changed -- orig: %v, changed: %v", origX, x) + } + + // Ensure that the resulting y coordinates match their respective + // expected oddness. + oddY.Normalize() + evenY.Normalize() + if !oddY.IsOdd() { + t.Fatalf("requested odd y is even for x = %v", x) + } + if evenY.IsOdd() { + t.Fatalf("requested even y is odd for x = %v", x) + } + + // Ensure that the resulting x and y coordinates are actually on the + // curve for both cases. + if !isOnCurve(x, &oddY) { + t.Fatalf("(%v, %v) is not a valid point", x, oddY) + } + if !isOnCurve(x, &evenY) { + t.Fatalf("(%v, %v) is not a valid point", x, evenY) + } + } +} diff --git a/hdkeychain/extendedkey_test.go b/hdkeychain/extendedkey_test.go index ac081775..fccc76d5 100644 --- a/hdkeychain/extendedkey_test.go +++ b/hdkeychain/extendedkey_test.go @@ -726,7 +726,7 @@ func TestErrors(t *testing.T) { { name: "pubkey not on curve", key: "dpubZ9169KDAEUnyoTzA7pDGtXbxpji5LuUk8johUPVGY2CDsz6S7hahGNL6QkeYrUeAPnaJD1MBmrsUnErXScGZdjL6b2gjCRX1Z1GNhLdVCjv", - err: errors.New("pubkey [0,50963827496501355358210603252497135226159332537351223778668747140855667399507] isn't on secp256k1 curve"), + err: errors.New("invalid public key x coordinate"), }, { name: "unsupported version",