crypto/hmac: panic if reusing hash.Hash values

Also put Reset in the correct place for the other
benchmarks.

name           old time/op    new time/op    delta
NewWriteSum-8    1.01µs ± 0%    1.01µs ± 1%   ~     (p=0.945 n=9+9)

name           old speed      new speed      delta
NewWriteSum-8  31.7MB/s ± 0%  31.6MB/s ± 1%   ~     (p=0.948 n=9+9)

name           old alloc/op   new alloc/op   delta
NewWriteSum-8      544B ± 0%      544B ± 0%   ~     (all equal)

name           old allocs/op  new allocs/op  delta
NewWriteSum-8      7.00 ± 0%      7.00 ± 0%   ~     (all equal)

Fixes #41089

Change-Id: I3dae660adbe4993963130bf3c2636bd53899164b
Reviewed-on: https://go-review.googlesource.com/c/go/+/261960
Trust: Katie Hockman <[email protected]>
Trust: Roland Shoemaker <[email protected]>
Run-TryBot: Katie Hockman <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Filippo Valsorda <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>

doc/go1.16.html

@@ -181,6 +181,14 @@ <h2 id="library">Core library</h2>
   TODO
 </p>
 
+<h3 id="crypto/hmac"><a href="/pkg/crypto/hmac">crypto/hmac</a></h3>
+
+<p><!-- CL 261960 -->
+  <a href="/pkg/crypto/hmac/#New">New</a> will now panic if separate calls to
+  the hash generation function fail to return new values. Previously, the
+  behavior was undefined and invalid outputs were sometimes generated.
+</p>
+
 <h3 id="crypto/tls"><a href="/pkg/crypto/tls">crypto/tls</a></h3>
 
 <p><!-- CL 256897 -->

src/crypto/hmac/hmac.go

@@ -120,13 +120,28 @@ func (h *hmac) Reset() {
 }
 
 // New returns a new HMAC hash using the given hash.Hash type and key.
+// New functions like sha256.New from crypto/sha256 can be used as h.
+// h must return a new Hash every time it is called.
 // Note that unlike other hash implementations in the standard library,
 // the returned Hash does not implement encoding.BinaryMarshaler
 // or encoding.BinaryUnmarshaler.
 func New(h func() hash.Hash, key []byte) hash.Hash {
 	hm := new(hmac)
 	hm.outer = h()
 	hm.inner = h()
+	unique := true
+	func() {
+		defer func() {
+			// The comparison might panic if the underlying types are not comparable.
+			_ = recover()
+		}()
+		if hm.outer == hm.inner {
+			unique = false
+		}
+	}()
+	if !unique {
+		panic("crypto/hmac: hash generation function does not produce unique values")
+	}
 	blocksize := hm.inner.BlockSize()
 	hm.ipad = make([]byte, blocksize)
 	hm.opad = make([]byte, blocksize)

src/crypto/hmac/hmac_test.go

@@ -556,6 +556,17 @@ func TestHMAC(t *testing.T) {
 	}
 }
 
+func TestNonUniqueHash(t *testing.T) {
+	sha := sha256.New()
+	defer func() {
+		err := recover()
+		if err == nil {
+			t.Error("expected panic when calling New with a non-unique hash generation function")
+		}
+	}()
+	New(func() hash.Hash { return sha }, []byte("bytes"))
+}
+
 // justHash implements just the hash.Hash methods and nothing else
 type justHash struct {
 	hash.Hash
@@ -587,8 +598,8 @@ func BenchmarkHMACSHA256_1K(b *testing.B) {
 	b.SetBytes(int64(len(buf)))
 	for i := 0; i < b.N; i++ {
 		h.Write(buf)
-		h.Reset()
 		mac := h.Sum(nil)
+		h.Reset()
 		buf[0] = mac[0]
 	}
 }
@@ -600,7 +611,18 @@ func BenchmarkHMACSHA256_32(b *testing.B) {
 	b.SetBytes(int64(len(buf)))
 	for i := 0; i < b.N; i++ {
 		h.Write(buf)
+		mac := h.Sum(nil)
 		h.Reset()
+		buf[0] = mac[0]
+	}
+}
+
+func BenchmarkNewWriteSum(b *testing.B) {
+	buf := make([]byte, 32)
+	b.SetBytes(int64(len(buf)))
+	for i := 0; i < b.N; i++ {
+		h := New(sha256.New, make([]byte, 32))
+		h.Write(buf)
 		mac := h.Sum(nil)
 		buf[0] = mac[0]
 	}