secp256k1: Add overflow check to field val set.

This modifies the SetBytes and SetByteSlice methods of the FieldVal type
to return whether or not the provided value was greater than or equal to
the field prime and adds tests to ensure  it works as expected.

This allows callers to easily check if a given uint256 value will fit
within the field element without modular reduction and makes it more
consistent with the ModNScalar type.

Finally, it updates all of the callers in the repository accordingly.

Author: davecgh

dcrec/secp256k1/curve.go

@@ -29,7 +29,11 @@ func hexToFieldVal(s string) *FieldVal {
 	if err != nil {
 		panic("invalid hex in source file: " + s)
 	}
-	return new(FieldVal).SetByteSlice(b)
+	var f FieldVal
+	if overflow := f.SetByteSlice(b); overflow {
+		panic("hex in source file overflows mod P: " + s)
+	}
+	return &f
 }
 
 var (

dcrec/secp256k1/ecdsa/signature.go

@@ -28,13 +28,14 @@ var (
 	// defined here to avoid extra allocations.
 	zero32 = [32]byte{}
 
-	// orderBytes is the raw bytes for the order of the secp256k1 curve group.
-	orderBytes = secp256k1.Params().N.Bytes()
-
 	// orderAsFieldVal is the order of the secp256k1 curve group stored as a
 	// field value.  It is provided here to avoid the need to create it multiple
 	// times.
-	orderAsFieldVal = new(secp256k1.FieldVal).SetByteSlice(orderBytes)
+	orderAsFieldVal = func() *secp256k1.FieldVal {
+		var f secp256k1.FieldVal
+		f.SetByteSlice(secp256k1.Params().N.Bytes())
+		return &f
+	}()
 )
 
 const (

dcrec/secp256k1/field.go

@@ -86,13 +86,19 @@ const (
 	// needed to represent the value.
 	fieldMSBMask = (1 << fieldMSBBits) - 1
 
-	// fieldPrimeWordZero is word zero of the secp256k1 prime in the
-	// internal field representation.  It is used during negation.
-	fieldPrimeWordZero = 0x3fffc2f
-
-	// fieldPrimeWordOne is word one of the secp256k1 prime in the
-	// internal field representation.  It is used during negation.
-	fieldPrimeWordOne = 0x3ffffbf
+	// These fields provide convenient access to each of the words of the
+	// secp256k1 prime in the internal field representation to improve code
+	// readability.
+	fieldPrimeWordZero  = 0x03fffc2f
+	fieldPrimeWordOne   = 0x03ffffbf
+	fieldPrimeWordTwo   = 0x03ffffff
+	fieldPrimeWordThree = 0x03ffffff
+	fieldPrimeWordFour  = 0x03ffffff
+	fieldPrimeWordFive  = 0x03ffffff
+	fieldPrimeWordSix   = 0x03ffffff
+	fieldPrimeWordSeven = 0x03ffffff
+	fieldPrimeWordEight = 0x03ffffff
+	fieldPrimeWordNine  = 0x003fffff
 )
 
 // FieldVal implements optimized fixed-precision arithmetic over the
@@ -227,15 +233,19 @@ func (f *FieldVal) SetInt(ui uint16) *FieldVal {
 }
 
 // SetBytes packs the passed 32-byte big-endian value into the internal field
-// value representation in constant time.
+// value representation in constant time.  SetBytes interprets the provided
+// array as a 256-bit big-endian unsigned integer, packs it into the internal
+// field value representation, and returns either 1 if it is greater than or
+// equal to the field prime (aka it overflowed) or 0 otherwise in constant time.
 //
-// The field value is returned to support chaining.  This enables syntax like:
-// f := new(FieldVal).SetBytes(byteArray).Mul(f2) so that f = ba * f2.
+// Note that a bool is not used here because it is not possible in Go to convert
+// from a bool to numeric value in constant time and many constant-time
+// operations require a numeric value.
 //
 // Preconditions: None
-// Output Normalized: Yes
+// Output Normalized: Yes if no overflow, no otherwise
 // Output Max Magnitude: 1
-func (f *FieldVal) SetBytes(b *[32]byte) *FieldVal {
+func (f *FieldVal) SetBytes(b *[32]byte) uint32 {
 	// Pack the 256 total bits across the 10 uint32 words with a max of
 	// 26-bits per word.  This could be done with a couple of for loops,
 	// but this unrolled version is significantly faster.  Benchmarks show
@@ -259,28 +269,59 @@ func (f *FieldVal) SetBytes(b *[32]byte) *FieldVal {
 	f.n[8] = uint32(b[5]) | uint32(b[4])<<8 | uint32(b[3])<<16 |
 		(uint32(b[2])&twoBitsMask)<<24
 	f.n[9] = uint32(b[2])>>2 | uint32(b[1])<<6 | uint32(b[0])<<14
-	return f
+
+	// The intuition here is that the field value is greater than the prime if
+	// one of the higher individual words is greater than corresponding word of
+	// the prime and all higher words in the field value are equal to their
+	// corresponding word of the prime.  Since this type is modulo the prime,
+	// being equal is also an overflow back to 0.
+	//
+	// Note that because the input is 32 bytes and it was just packed into the
+	// field representation, the only words that can possibly be greater are
+	// zero and one, because ceil(log_2(2^256 - 1 - P)) = 33 bits max and the
+	// internal field representation encodes 26 bits with each word.
+	//
+	// Thus, there is no need to test if the upper words of the field value
+	// exceeds them, hence, only equality is checked for them.
+	highWordsEq := constantTimeEq(f.n[9], fieldPrimeWordNine)
+	highWordsEq &= constantTimeEq(f.n[8], fieldPrimeWordEight)
+	highWordsEq &= constantTimeEq(f.n[7], fieldPrimeWordSeven)
+	highWordsEq &= constantTimeEq(f.n[6], fieldPrimeWordSix)
+	highWordsEq &= constantTimeEq(f.n[5], fieldPrimeWordFive)
+	highWordsEq &= constantTimeEq(f.n[4], fieldPrimeWordFour)
+	highWordsEq &= constantTimeEq(f.n[3], fieldPrimeWordThree)
+	highWordsEq &= constantTimeEq(f.n[2], fieldPrimeWordTwo)
+	overflow := highWordsEq & constantTimeGreater(f.n[1], fieldPrimeWordOne)
+	highWordsEq &= constantTimeEq(f.n[1], fieldPrimeWordOne)
+	overflow |= highWordsEq & constantTimeGreaterOrEq(f.n[0], fieldPrimeWordZero)
+
+	return overflow
 }
 
-// SetByteSlice packs the passed big-endian value into the internal field value
-// representation in constant time.  Only the first 32-bytes are used.  As a
-// result, it is up to the caller to ensure numbers of the appropriate size are
-// used or the value will be truncated.
+// SetByteSlice interprets the provided slice as a 256-bit big-endian unsigned
+// integer (meaning it is truncated to the first 32 bytes), packs it into the
+// internal field value representation, and returns whether or not the resulting
+// truncated 256-bit integer is greater than or equal to the field prime (aka it
+// overflowed) in constant time.
 //
-// The field value is returned to support chaining.  This enables syntax like:
-// f := new(FieldVal).SetByteSlice(byteSlice)
+// Note that since passing a slice with more than 32 bytes is truncated, it is
+// possible that the truncated value is less than the field prime and hence it
+// will not be reported as having overflowed in that case.  It is up to the
+// caller to decide whether it needs to provide numbers of the appropriate size
+// or it if is acceptable to use this function with the described truncation and
+// overflow behavior.
 //
 // Preconditions: None
-// Output Normalized: Yes
+// Output Normalized: Yes if no overflow, no otherwise
 // Output Max Magnitude: 1
-func (f *FieldVal) SetByteSlice(b []byte) *FieldVal {
+func (f *FieldVal) SetByteSlice(b []byte) bool {
 	var b32 [32]byte
 	b = b[:constantTimeMin(uint32(len(b)), 32)]
 	copy(b32[:], b32[:32-len(b)])
 	copy(b32[32-len(b):], b)
-	f.SetBytes(&b32)
+	result := f.SetBytes(&b32)
 	zeroArray32(&b32)
-	return f
+	return result != 0
 }
 
 // Normalize normalizes the internal field words into the desired range and
@@ -1590,7 +1631,8 @@ func (f *FieldVal) IsGtOrEqPrimeMinusOrder() bool {
 	//
 	// This can be verified with the following test code:
 	//   pMinusN := new(big.Int).Sub(curveParams.P, curveParams.N)
-	//   fv := new(FieldVal).SetByteSlice(pMinusN.Bytes())
+	//   var fv FieldVal
+	//   fv.SetByteSlice(pMinusN.Bytes())
 	//   t.Logf("%x", fv.n)
 	//
 	//   Outputs: [3c9baee 3685c8b 1fc4402 6542dd 1455123 0 0 0 0 0]

dcrec/secp256k1/field_test.go

@@ -28,7 +28,8 @@ func (f *FieldVal) SetHex(hexString string) *FieldVal {
 		hexString = "0" + hexString
 	}
 	bytes, _ := hex.DecodeString(hexString)
-	return f.SetByteSlice(bytes)
+	f.SetByteSlice(bytes)
+	return f
 }
 
 // randFieldVal returns a field value created from a random value generated by
@@ -96,6 +97,179 @@ func TestFieldSetInt(t *testing.T) {
 	}
 }
 
+// TestFieldSetBytes ensures that setting a field value to a 256-bit big-endian
+// unsigned integer via both the slice and array methods works as expected for
+// edge cases.  Random cases are tested via the various other tests.
+func TestFieldSetBytes(t *testing.T) {
+	tests := []struct {
+		name     string     // test description
+		in       string     // hex encoded test value
+		expected [10]uint32 // expected raw ints
+		overflow bool       // expected overflow result
+	}{{
+		name:     "zero",
+		in:       "00",
+		expected: [10]uint32{0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
+		overflow: false,
+	}, {
+		name: "field prime",
+		in:   "fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f",
+		expected: [10]uint32{
+			0x03fffc2f, 0x03ffffbf, 0x03ffffff, 0x03ffffff, 0x03ffffff,
+			0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x003fffff,
+		},
+		overflow: true,
+	}, {
+		name: "field prime - 1",
+		in:   "fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2e",
+		expected: [10]uint32{
+			0x03fffc2e, 0x03ffffbf, 0x03ffffff, 0x03ffffff, 0x03ffffff,
+			0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x003fffff,
+		},
+		overflow: false,
+	}, {
+		name: "field prime + 1 (overflow in word zero)",
+		in:   "fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc30",
+		expected: [10]uint32{
+			0x03fffc30, 0x03ffffbf, 0x03ffffff, 0x03ffffff, 0x03ffffff,
+			0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x003fffff,
+		},
+		overflow: true,
+	}, {
+		name: "field prime first 32 bits",
+		in:   "fffffc2f",
+		expected: [10]uint32{
+			0x03fffc2f, 0x00000003f, 0x00000000, 0x00000000, 0x00000000,
+			0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000,
+		},
+		overflow: false,
+	}, {
+		name: "field prime word zero",
+		in:   "03fffc2f",
+		expected: [10]uint32{
+			0x03fffc2f, 0x00000000, 0x00000000, 0x00000000, 0x00000000,
+			0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000,
+		},
+		overflow: false,
+	}, {
+		name: "field prime first 64 bits",
+		in:   "fffffffefffffc2f",
+		expected: [10]uint32{
+			0x03fffc2f, 0x03ffffbf, 0x00000fff, 0x00000000, 0x00000000,
+			0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000,
+		},
+		overflow: false,
+	}, {
+		name: "field prime word zero and one",
+		in:   "0ffffefffffc2f",
+		expected: [10]uint32{
+			0x03fffc2f, 0x03ffffbf, 0x00000000, 0x00000000, 0x00000000,
+			0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000,
+		},
+		overflow: false,
+	}, {
+		name: "field prime first 96 bits",
+		in:   "fffffffffffffffefffffc2f",
+		expected: [10]uint32{
+			0x03fffc2f, 0x03ffffbf, 0x03ffffff, 0x0003ffff, 0x00000000,
+			0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000,
+		},
+		overflow: false,
+	}, {
+		name: "field prime word zero, one, and two",
+		in:   "3ffffffffffefffffc2f",
+		expected: [10]uint32{
+			0x03fffc2f, 0x03ffffbf, 0x03ffffff, 0x00000000, 0x00000000,
+			0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000,
+		},
+		overflow: false,
+	}, {
+		name: "overflow in word one (prime + 1<<26)",
+		in:   "ffffffffffffffffffffffffffffffffffffffffffffffffffffffff03fffc2f",
+		expected: [10]uint32{
+			0x03fffc2f, 0x03ffffc0, 0x03ffffff, 0x03ffffff, 0x03ffffff,
+			0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x003fffff,
+		},
+		overflow: true,
+	}, {
+		name: "(field prime - 1) * 2 NOT mod P, truncated >32 bytes",
+		in:   "01fffffffffffffffffffffffffffffffffffffffffffffffffffffffdfffff85c",
+		expected: [10]uint32{
+			0x01fffff8, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff,
+			0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x00007fff,
+		},
+		overflow: false,
+	}, {
+		name: "2^256 - 1",
+		in:   "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+		expected: [10]uint32{
+			0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff,
+			0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x003fffff,
+		},
+		overflow: true,
+	}, {
+		name: "alternating bits",
+		in:   "a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5",
+		expected: [10]uint32{
+			0x01a5a5a5, 0x01696969, 0x025a5a5a, 0x02969696, 0x01a5a5a5,
+			0x01696969, 0x025a5a5a, 0x02969696, 0x01a5a5a5, 0x00296969,
+		},
+		overflow: false,
+	}, {
+		name: "alternating bits 2",
+		in:   "5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a",
+		expected: [10]uint32{
+			0x025a5a5a, 0x02969696, 0x01a5a5a5, 0x01696969, 0x025a5a5a,
+			0x02969696, 0x01a5a5a5, 0x01696969, 0x025a5a5a, 0x00169696,
+		},
+		overflow: false,
+	}}
+
+	for _, test := range tests {
+		inBytes := hexToBytes(test.in)
+
+		// Ensure setting the bytes via the slice method works as expected.
+		var f FieldVal
+		overflow := f.SetByteSlice(inBytes)
+		if !reflect.DeepEqual(f.n, test.expected) {
+			t.Errorf("%s: unexpected result\ngot: %x\nwant: %x", test.name, f.n,
+				test.expected)
+			continue
+		}
+
+		// Ensure the setting the bytes via the slice method produces the
+		// expected overflow result.
+		if overflow != test.overflow {
+			t.Errorf("%s: unexpected overflow -- got: %v, want: %v", test.name,
+				overflow, test.overflow)
+			continue
+		}
+
+		// Ensure setting the bytes via the array method works as expected.
+		var f2 FieldVal
+		var b32 [32]byte
+		truncatedInBytes := inBytes
+		if len(truncatedInBytes) > 32 {
+			truncatedInBytes = truncatedInBytes[:32]
+		}
+		copy(b32[32-len(truncatedInBytes):], truncatedInBytes)
+		overflow = f2.SetBytes(&b32) != 0
+		if !reflect.DeepEqual(f2.n, test.expected) {
+			t.Errorf("%s: unexpected result\ngot: %x\nwant: %x", test.name,
+				f2.n, test.expected)
+			continue
+		}
+
+		// Ensure the setting the bytes via the array method produces the
+		// expected overflow result.
+		if overflow != test.overflow {
+			t.Errorf("%s: unexpected overflow -- got: %v, want: %v", test.name,
+				overflow, test.overflow)
+			continue
+		}
+	}
+}
+
 // TestFieldZero ensures that zeroing a field value works as expected.
 func TestFieldZero(t *testing.T) {
 	var f FieldVal

dcrec/secp256k1/pubkey.go

@@ -51,9 +51,11 @@ 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) {
-	var fy FieldVal
-	fx := new(FieldVal).SetByteSlice(x.Bytes())
-	if !DecompressY(fx, ybit, &fy) {
+	var fx, fy FieldVal
+	if overflow := fx.SetByteSlice(x.Bytes()); overflow {
+		return nil, fmt.Errorf("invalid public key x coordinate")
+	}
+	if !DecompressY(&fx, ybit, &fy) {
 		return nil, fmt.Errorf("invalid public key x coordinate")
 	}
 	fy.Normalize()