From b21feec2cdfd43dd441fa3e849762ebfa01852db Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Wed, 19 Feb 2020 04:24:11 -0600 Subject: [PATCH] secp256k1: Use field val for y coord decompression. This modifies the core logic for decompressing a y coordinate from an x coordinate, which is primarily used when parsing public keys, but can also be used when recovering public keys from compact signatures in the future, to make use of field vals and their recently-added optimized modular square root calculation capability and also adds comprehensive tests to ensure the decompression works properly independent of public key parsing. Since the public key type is still defined in terms of big ints, the parsing function still relies on them and so the existing decompressPoint function was updated to make use of the new function and perform the necessary conversion to and from big ints. Ultimately, the goal is make that unnecessary. This is work towards eventually using the new more efficient mod n scalar throughout. The following benchmark shows a before and after comparison of decompressing a public key: benchmark old ns/op new ns/op delta ------------------------------------------------------------- BenchmarkPubKeyDecompress 43574 10961 -74.85% benchmark old allocs new allocs delta ------------------------------------------------------------- BenchmarkPubKeyDecompress 28 0 -100.00% benchmark old bytes new bytes delta ------------------------------------------------------------- BenchmarkPubKeyDecompress 2586 0 -100.00% --- dcrec/secp256k1/bench_test.go | 8 +- dcrec/secp256k1/field_test.go | 18 ++++ dcrec/secp256k1/pubkey.go | 49 ++++++---- dcrec/secp256k1/pubkey_test.go | 167 +++++++++++++++++++++++++++++++++ hdkeychain/extendedkey_test.go | 2 +- 5 files changed, 220 insertions(+), 24 deletions(-) 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",