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>

diff --git a/src/crypto/cipher/gcm_fips140v2.0_test.go b/src/crypto/cipher/gcm_fips140v2.0_test.go
index c44497d4d820569b875064949728c3094750af6f..4983822cc62d011a65594a16da1f34a9cca705e2 100644 (file)
--- 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 fe6bf5506e5a1028ca48a44b1c57869f7b15115d..7bcfe9d7dced8db67f3f3a97d3b148198286e8d8 100644 (file)
--- 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 56863803760466bcfa4fd62a0f2e436abb4766e4..4ae570d9a5e84c623a3fb258a1fca771a88ce330 100644 (file)
--- 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)