From: Filippo Valsorda Date: Mon, 8 Dec 2025 23:47:26 +0000 (+0100) Subject: crypto/internal/fips140/aes/gcm: don't panic on bad nonces out of FIPS 140-3 mode X-Git-Url: https://git.feebdaed.xyz/?a=commitdiff_plain;h=cd873cf7e98c3fd3e8138e9d97d6a974a1ee0b53;p=0xmirror%2Fgo.git 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 LUCI-TryBot-Result: Go LUCI Reviewed-by: Roland Shoemaker Reviewed-by: David Chase --- 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)