[Fix] bchec: Constant time field normalization

bchec/bench_test.go

@@ -121,3 +121,14 @@ func BenchmarkFieldNormalize(b *testing.B) {
 		f.Normalize()
 	}
 }
+
+// BenchmarkFieldNormalize2 requires the evaluation of more involved logic in
+// field.Normalize(). We should ensure the runtime is the same as in
+// BenchmarkFieldNormalize
+func BenchmarkFieldNormalize2(b *testing.B) {
+	f := new(fieldVal)
+	f.n = [10]uint32{0x148f6, 0x3ffffc0, 0x3ffffff, 0x3ffffff, 0x3ffffff, 0x3ffffff, 0x3ffffff, 0x3ffffff, 0x3ffffff, 0x000007}
+	for i := 0; i < b.N; i++ {
+		f.Normalize()
+	}
+}

bchec/constant_time.go

@@ -0,0 +1,41 @@
+// Copyright (c) 2017 The btcsuite developers
+// Copyright (c) 2017 Brent Perreault
+// Use of this source code is governed by an ISC
+// license that can be found in the LICENSE file.
+
+package bchec
+
+// constant_time.go provides constant time implementations of useful
+// mathematical operations. In addition, these functions return integers,
+// using 0 or 1 to represent false or true respectively, which is useful
+// for writing logic in terms of bitwise operators
+
+// References
+// These functions are based on the sample implementation in
+// golang.org/src/crypto/subtle/constant_time.go
+// Here we have refactored these functions for uint32 arithmetic and
+// to avoid extra shifts and casts
+
+// Note - these use the sign bit of int32 internally. For that reason all
+// of the inputs need to be less than 2^31 to avoid overflowing int32.
+// These are intended for use internal to btcec.
+
+// lessThanUint32 returns 1 if x < y and 0 otherwise.
+// It works by checking the most significant bit, and then testing the
+// rest of the bits by casting to int32
+func lessThanUint32(x, y uint32) uint32 {
+	diff := int32(x) - int32(y)
+	return uint32((diff >> 31) & 1)
+}
+
+// isZeroUint32 returns 1 if x == y and 0 otherwise.
+func isZeroUint32(x uint32) uint32 {
+	x32 := int32(x)
+	return uint32((((x32 - 1) ^ x32) >> 31) & 1)
+}
+
+// notZeroUint32 returns 1 if x != y and 0 otherwise.
+func notZeroUint32(x uint32) uint32 {
+	x32 := int32(x)
+	return uint32((((-x32) | x32) >> 31) & 1)
+}

bchec/constant_time_test.go

@@ -0,0 +1,74 @@
+// Copyright (c) 2017 The btcsuite developers
+// Copyright (c) 2017 Brent Perreault
+// Use of this source code is governed by an ISC
+// license that can be found in the LICENSE file.
+
+package bchec
+
+import "testing"
+
+func TestLessThanUint32(t *testing.T) {
+	tests := []struct {
+		x uint32
+		y uint32
+		a uint32
+	}{
+		{0, 1, 1},
+		{2, 2, 0},
+		{1 << 30, 1 << 30, 0},
+		{17, 1 << 30, 1},
+		{1 << 30, 0, 0},
+	}
+
+	t.Logf("Running %d tests", len(tests))
+	for i, test := range tests {
+		answer := lessThanUint32(test.x, test.y)
+		if test.a != answer {
+			t.Errorf("lessThanUint32 #%d wrong result\ngot: %v\n"+
+				"want: %v", i, answer, test.a)
+			continue
+		}
+	}
+}
+
+func TestIsZeroUint32(t *testing.T) {
+	tests := []struct {
+		x uint32
+		a uint32
+	}{
+		{1, 0},
+		{0, 1},
+		{1 << 30, 0},
+	}
+
+	t.Logf("Running %d tests", len(tests))
+	for i, test := range tests {
+		answer := isZeroUint32(test.x)
+		if test.a != answer {
+			t.Errorf("isZeroUint32 #%d wrong result\ngot: %v\n"+
+				"want: %v", i, answer, test.a)
+			continue
+		}
+	}
+}
+
+func TestNotZeroUint32(t *testing.T) {
+	tests := []struct {
+		x uint32
+		a uint32
+	}{
+		{1, 1},
+		{0, 0},
+		{1 << 30, 1},
+	}
+
+	t.Logf("Running %d tests", len(tests))
+	for i, test := range tests {
+		answer := notZeroUint32(test.x)
+		if test.a != answer {
+			t.Errorf("notZeroUint32 #%d wrong result\ngot: %v\n"+
+				"want: %v", i, answer, test.a)
+			continue
+		}
+	}
+}

bchec/field.go

@@ -305,32 +305,11 @@ func (f *fieldVal) Normalize() *fieldVal {
 	// following determines if either or these conditions are true and does
 	// the final reduction in constant time.
 	//
-	// Note that the if/else statements here intentionally do the bitwise
-	// operators even when it won't change the value to ensure constant time
-	// between the branches.  Also note that 'm' will be zero when neither
-	// of the aforementioned conditions are true and the value will not be
-	// changed when 'm' is zero.
-	m = 1
-	if t9 == fieldMSBMask {
-		m &= 1
-	} else {
-		m &= 0
-	}
-	if t2&t3&t4&t5&t6&t7&t8 == fieldBaseMask {
-		m &= 1
-	} else {
-		m &= 0
-	}
-	if ((t0+977)>>fieldBase + t1 + 64) > fieldBaseMask {
-		m &= 1
-	} else {
-		m &= 0
-	}
-	if t9>>fieldMSBBits != 0 {
-		m |= 1
-	} else {
-		m |= 0
-	}
+	// Note that 'm' will be zero when neither of the aforementioned conditions
+	// are true and the value will not be changed when 'm' is zero.
+	m = isZeroUint32((t9 - fieldMSBMask) | (t2&t3&t4&t5&t6&t7&t8 - fieldBaseMask))
+	m &= lessThanUint32(fieldBaseMask, (t0+977)>>fieldBase+t1+64)
+	m |= notZeroUint32(t9 >> fieldMSBBits)
 	t0 = t0 + m*977
 	t1 = (t0 >> fieldBase) + t1 + (m << 6)
 	t0 = t0 & fieldBaseMask

bchec/field_test.go

@@ -307,6 +307,16 @@ func TestNormalize(t *testing.T) {
 			[10]uint32{0x03fffc30, 0x03ffffc0, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x07ffffff, 0x003fffff},
 			[10]uint32{0x00000001, 0x00000001, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000001},
 		},
+		// A more difficult branch for the Normalize logic
+		{
+			[10]uint32{0x148f6, 0x3ffffc0, 0x3ffffff, 0x3ffffff, 0x3ffffff, 0x3ffffff, 0x3ffffff, 0x3ffffff, 0x3ffffff, 0x000007},
+			[10]uint32{0x148f6, 0x3ffffc0, 0x3ffffff, 0x3ffffff, 0x3ffffff, 0x3ffffff, 0x3ffffff, 0x3ffffff, 0x3ffffff, 0x000007},
+		},
+		// A test that would fail without the check t2&t3&t4&t5&t6&t7&t8 == fieldBaseMask before normalization
+		{
+			[10]uint32{0x03fffc30, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03fffff0, 0x003fffff},
+			[10]uint32{0x03fffc30, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03ffffff, 0x03fffff0, 0x003fffff},
+		},
 	}
 
 	t.Logf("Running %d tests", len(tests))