crypto/internal/fips140/aes/gcm: don't panic on bad nonces out of FIPS 140-3 mode

The enforcement is good beyond compliance if it is correct, but I am
more nervous about accidental DoS due to mismatches between how the
caller calculates a nonce and how the enforcement expects it to be
calculated.

We need to have this enforcement in FIPS 140-3 mode, but no need to blow
ourselves up when it's off.

If all goes well, this code is unreachable anyway.

Change-Id: If73ec59ebbd283b0e5506354961a87a06a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/728504
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: David Chase <drchase@google.com>

3 files changed, 80 insertions, 43 deletions

diff --git a/src/crypto/cipher/gcm_fips140v2.0_test.go b/src/crypto/cipher/gcm_fips140v2.0_test.go
index c44497d4d8..4983822cc6 100644
--- a/src/crypto/cipher/gcm_fips140v2.0_test.go
+++ b/src/crypto/cipher/gcm_fips140v2.0_test.go
@@ -8,15 +8,32 @@ package cipher_test
 
 import (
 	"crypto/cipher"
+	"crypto/internal/cryptotest"
 	"crypto/internal/fips140"
 	fipsaes "crypto/internal/fips140/aes"
 	"crypto/internal/fips140/aes/gcm"
 	"encoding/binary"
+	"internal/testenv"
 	"math"
 	"testing"
 )
 
 func TestGCMNoncesFIPSV2(t *testing.T) {
+	cryptotest.MustSupportFIPS140(t)
+	if !fips140.Enabled {
+		cmd := testenv.Command(t, testenv.Executable(t), "-test.run=^TestGCMNoncesFIPSV2$", "-test.v")
+		cmd = testenv.CleanCmdEnv(cmd)
+		cmd.Env = append(cmd.Env, "GODEBUG=fips140=on")
+		out, err := cmd.CombinedOutput()
+		if len(out) != 0 {
+			t.Logf("\n%s", out)
+		}
+		if err != nil {
+			t.Errorf("fips140=on subprocess failed: %v", err)
+		}
+		return
+	}
+
 	tryNonce := func(aead cipher.AEAD, nonce []byte) bool {
 		fips140.ResetServiceIndicator()
 		aead.Seal(nil, nonce, []byte("x"), nil)
diff --git a/src/crypto/cipher/gcm_test.go b/src/crypto/cipher/gcm_test.go
index fe6bf5506e..7bcfe9d7dc 100644
--- a/src/crypto/cipher/gcm_test.go
+++ b/src/crypto/cipher/gcm_test.go
@@ -17,6 +17,7 @@ import (
 	"encoding/hex"
 	"errors"
 	"fmt"
+	"internal/testenv"
 	"io"
 	"reflect"
 	"testing"
@@ -761,7 +762,22 @@ func TestGCMExtraMethods(t *testing.T) {
 	})
 }
 
-func TestGCMNonces(t *testing.T) {
+func TestGCMNoncesFIPSV1(t *testing.T) {
+	cryptotest.MustSupportFIPS140(t)
+	if !fips140.Enabled {
+		cmd := testenv.Command(t, testenv.Executable(t), "-test.run=^TestGCMNoncesFIPSV1$", "-test.v")
+		cmd = testenv.CleanCmdEnv(cmd)
+		cmd.Env = append(cmd.Env, "GODEBUG=fips140=on")
+		out, err := cmd.CombinedOutput()
+		if len(out) != 0 {
+			t.Logf("\n%s", out)
+		}
+		if err != nil {
+			t.Errorf("fips140=on subprocess failed: %v", err)
+		}
+		return
+	}
+
 	tryNonce := func(aead cipher.AEAD, nonce []byte) bool {
 		fips140.ResetServiceIndicator()
 		aead.Seal(nil, nonce, []byte("x"), nil)
diff --git a/src/crypto/internal/fips140/aes/gcm/gcm_nonces.go b/src/crypto/internal/fips140/aes/gcm/gcm_nonces.go
index 5686380376..4ae570d9a5 100644
--- a/src/crypto/internal/fips140/aes/gcm/gcm_nonces.go
+++ b/src/crypto/internal/fips140/aes/gcm/gcm_nonces.go
@@ -107,31 +107,33 @@ func (g *GCMWithCounterNonce) Seal(dst, nonce, plaintext, data []byte) []byte {
 		panic("crypto/cipher: incorrect nonce length given to GCM")
 	}
 
-	if !g.prefixReady {
-		// The first invocation sets the fixed prefix.
-		g.prefixReady = true
-		g.prefix = byteorder.BEUint32(nonce[:4])
-	}
-	if g.prefix != byteorder.BEUint32(nonce[:4]) {
-		panic("crypto/cipher: GCM nonce prefix changed")
-	}
+	if fips140.Enabled {
+		if !g.prefixReady {
+			// The first invocation sets the fixed prefix.
+			g.prefixReady = true
+			g.prefix = byteorder.BEUint32(nonce[:4])
+		}
+		if g.prefix != byteorder.BEUint32(nonce[:4]) {
+			panic("crypto/cipher: GCM nonce prefix changed")
+		}
 
-	counter := byteorder.BEUint64(nonce[len(nonce)-8:])
-	if !g.startReady {
-		// The first invocation sets the starting counter, if not fixed.
-		g.startReady = true
-		g.start = counter
-	}
-	counter -= g.start
+		counter := byteorder.BEUint64(nonce[len(nonce)-8:])
+		if !g.startReady {
+			// The first invocation sets the starting counter, if not fixed.
+			g.startReady = true
+			g.start = counter
+		}
+		counter -= g.start
 
-	// Ensure the counter is strictly increasing.
-	if counter == math.MaxUint64 {
-		panic("crypto/cipher: counter exhausted")
-	}
-	if counter < g.next {
-		panic("crypto/cipher: counter decreased or remained the same")
+		// Ensure the counter is strictly increasing.
+		if counter == math.MaxUint64 {
+			panic("crypto/cipher: counter exhausted")
+		}
+		if counter < g.next {
+			panic("crypto/cipher: counter decreased or remained the same")
+		}
+		g.next = counter + 1
 	}
-	g.next = counter + 1
 
 	fips140.RecordApproved()
 	return g.g.sealAfterIndicator(dst, nonce, plaintext, data)
@@ -250,28 +252,30 @@ func (g *GCMWithXORCounterNonce) Seal(dst, nonce, plaintext, data []byte) []byte
 		panic("crypto/cipher: incorrect nonce length given to GCM")
 	}
 
-	counter := byteorder.BEUint64(nonce[len(nonce)-8:])
-	if !g.ready {
-		// In the first call, if [GCMWithXORCounterNonce.SetNoncePrefixAndMask]
-		// wasn't used, we assume the counter is zero to learn the XOR mask and
-		// fixed prefix.
-		g.ready = true
-		g.mask = counter
-		g.prefix = byteorder.BEUint32(nonce[:4])
-	}
-	if g.prefix != byteorder.BEUint32(nonce[:4]) {
-		panic("crypto/cipher: GCM nonce prefix changed")
-	}
-	counter ^= g.mask
+	if fips140.Enabled {
+		counter := byteorder.BEUint64(nonce[len(nonce)-8:])
+		if !g.ready {
+			// In the first call, if [GCMWithXORCounterNonce.SetNoncePrefixAndMask]
+			// wasn't used, we assume the counter is zero to learn the XOR mask and
+			// fixed prefix.
+			g.ready = true
+			g.mask = counter
+			g.prefix = byteorder.BEUint32(nonce[:4])
+		}
+		if g.prefix != byteorder.BEUint32(nonce[:4]) {
+			panic("crypto/cipher: GCM nonce prefix changed")
+		}
+		counter ^= g.mask
 
-	// Ensure the counter is strictly increasing.
-	if counter == math.MaxUint64 {
-		panic("crypto/cipher: counter exhausted")
-	}
-	if counter < g.next {
-		panic("crypto/cipher: counter decreased or remained the same")
+		// Ensure the counter is strictly increasing.
+		if counter == math.MaxUint64 {
+			panic("crypto/cipher: counter exhausted")
+		}
+		if counter < g.next {
+			panic("crypto/cipher: counter decreased or remained the same")
+		}
+		g.next = counter + 1
 	}
-	g.next = counter + 1
 
 	fips140.RecordApproved()
 	return g.g.sealAfterIndicator(dst, nonce, plaintext, data)