crypto.NewGCMWithRandomNonce error for go >= 1.24 in FIPS mode

[simitt]

Problem Description

Calling crypto.NewGCMWithRandomNonce in FIPS mode returns an error.

When building a go program with the Microsoft build of Go and GOEXPERIMENT=systemcrypto and running it with GOFIPS=1, the following code

cipher, err := aes.NewCipher(key)
if err != nil {
  return  err
}
aesgcm, err := cipher.NewGCMWithRandomNonce(cipher)
if err != nil {
  return  err
}

leads to this error:

cipher: NewGCMWithRandomNonce requires aes.Block

The error seems to come from a concrete type check. When using microsoft/go in FIPS mode, the cipher is not of this concrete type.

Expectations

With the introduction of crypto.NewGCMWithRandomNonce as part of the go std-lib crypto package, microsoft/go 1.24 should also support this functionality.

Related Issues

Similar issue to what was reported for usage with boringcrypto in golang/go#72016.

[qmuntal]

Thanks for reporting @simitt. We should at least fall back to native Go if the crypto backend doesn't implement a given function, so this issue looks like an oversights from our end. Will investigate a bit more and report back.

[simitt]

Thanks for looking into it @qmuntal .
This is as far as I investigated yet: With the patch in https://github.com/microsoft/go/blob/microsoft/release-branch.go1.24/patches/0004-Use-crypto-backends.patch#L320-L328 the internal/backend logic is applied. If a call to boring.NewAESCipher(key) is made by go, it really calls into crypto/internal/backend.NewAESCipher(key).

diff --git a/src/crypto/aes/aes.go b/src/crypto/aes/aes.go
index 5bc2d13d673e0a..b803c77be62a66 100644
--- a/src/crypto/aes/aes.go
+++ b/src/crypto/aes/aes.go
@@ -15,7 +15,7 @@ package aes
 
 import (
 	"crypto/cipher"
-	"crypto/internal/boring"
+	boring "crypto/internal/backend"
 	"crypto/internal/fips140/aes"
 	"strconv"
 )

Looking into crypto/internal/backend, for linux with openssl, it makes use of the patch

func NewAESCipher(key []byte) (cipher.Block, error)   { return openssl.NewAESCipher(key) }

and the actual call at runtime is using https://github.com/golang-fips/openssl/blob/v2/aes.go#L11-L41:

func NewAESCipher(key []byte) (cipher.Block, error) {
	var kind cipherKind
	switch len(key) * 8 {
	case 128:
		kind = cipherAES128
	case 192:
		kind = cipherAES192
	case 256:
		kind = cipherAES256
	default:
		return nil, errors.New("crypto/aes: invalid key size")
	}
	c, err := newEVPCipher(key, kind)
	if err != nil {
		return nil, err
	}
	return newAESBlock(c, kind), nil
}

With newAESBlock calling into https://github.com/golang-fips/openssl/blob/cc9339b0ab5c3bba7bd1e0d3a0fb796f0288178e/zaes.go#L46-L85 and then not passing the cipher type check from the std lib.

[mauri870]

I think the Go check in NewGCMWithRandomNonce is too strict. It does a type assertion for a concrete type (*aes.Block) besides receiving an interface (cipher.Block). The custom backends (openssl/boringcrypto) have their own concrete types for the aes block cipher. While this could be worked around here, it's ultimately a bug in the Go implementation. See: golang/go#72016 (comment).

[mauri870]

Unfortunately I think this would require a patch here as well. It is up to the Go team to fix NewGCMWithRandomNonce to work with boringssl, but given that it is used internally at Google and under GOEXPERIMENT it is not covered with the compatibility policy. Even if they fix it, this wouldn't change the fact that this fork uses a custom implementation that will not be handled by whatever fix the team comes up with. We need a NewGCMWithRandomNonce patched to have a custom path for the openssl backend aesCipher.

[qmuntal]

Agree that we will have to patch this. I also think the fix will have to be backported to Go 1.24, as our promise is that functions not implemented by a backend will fall back to the native Go implementation.

[mauri870]

I noticed you assigned this task. I was looking into it, and my initial thought was to copy gcmAble and perform a type assertion against it:

diff --git a/src/crypto/cipher/gcm.go b/src/crypto/cipher/gcm.go
index 5580f96d55..01baf3a589 100644
--- a/src/crypto/cipher/gcm.go
+++ b/src/crypto/cipher/gcm.go
@@ -80,6 +80,10 @@ func newGCM(cipher Block, nonceSize, tagSize int) (AEAD, error) {
        return g, nil
 }

+type gcmAble interface {
+       NewGCM(nonceSize, tagSize int) (cipher.AEAD, error)
+}
+
 // NewGCMWithRandomNonce returns the given cipher wrapped in Galois Counter
 // Mode, with randomly-generated nonces. The cipher must have been created by
 // [aes.NewCipher].
@@ -91,6 +95,11 @@ func newGCM(cipher Block, nonceSize, tagSize int) (AEAD, error) {
 // A given key MUST NOT be used to encrypt more than 2^32 messages, to limit the
 // risk of a random nonce collision to negligible levels.
 func NewGCMWithRandomNonce(cipher Block) (AEAD, error) {
+       ci, ok := cipher.(gcmAble)
+       if ok {
+               return ci.NewGCM(gcmStandardNonceSize, gcmTagSize)
+       }
+
        c, ok := cipher.(*aes.Block)
        if !ok {
                return nil, errors.New("cipher: NewGCMWithRandomNonce requires aes.Block")

This could be a good starting point.

EDIT: Now that I think about it, Go exposing boring.extramodes could be a proposal on its own to support custom crypto backends.

[spalatnik]

Is the long term plan to continue using OpenSSL, or switching to the Go FIPS implementation in 1.24+?

[mertakman]

Is the long term plan to continue using OpenSSL, or switching to the Go FIPS implementation in 1.24+?

We are working on making Microsoft release of Go FIPS fully compatible. Soon we will release new patch. Thanks for following up on this topic.

[dagood]

[@spalatnik] Is the long term plan to continue using OpenSSL, or switching to the Go FIPS implementation in 1.24+?

About the Go FIPS implementation in particular, we have a blog post explaining this: https://devblogs.microsoft.com/go/go-1-24-fips-update/

We evaluated changing the Microsoft build of Go to use the new Go FIPS module rather than system libraries. However, we ultimately determined that this approach doesn’t align with Microsoft internal cryptography strategy and policies. We recommend, though, that Go developers requiring FIPS 140-3 compliance use the official Go FIPS module, once it is certified, if it fits their needs.

Please feel free to open a new issue with any further questions--this issue just tracks a bug with a specific API in our build.

[qmuntal]

Fixing this issue (in #1673) is giving us more grieve than expected. We will need to add a new API to our backends that implements the GCM Seal with random nonce function. The current GCM Seal API doesn't work well for this use case because the parameter semantics doesn't match.

We should do the following:

• Add a new method to the GCM implementation in each of the supported backends (namely cng.aesGCM, openssl.cipherGCM , and xcrypto.aesGCM) with the following signature:

// SealWithRandomNonce encrypts plaintext to out, and writes a random nonce to
// nonce. nonce must be 12 bytes, and out must be 16 bytes longer than plaintext.
// out and plaintext may overlap exactly or not at all. additionalData and out
// must not overlap.
func (*aesGCM) SealWithRandomNonce(out, nonce, plaintext, additionalData []byte)

• Implement SealWithRandomNonce similarly to crypto/internal/fips140/aes/gcm.SealWithRandomNonce. Some additional details:
  • Don't call fips140.RecordApproved (that is specific to the fips140 crypto module).
  • Use the RandReader of each backend instead of drbg.Read.
  • Use the raw seal function provided by the system (e.g. bcrypt.NewAUTHENTICATED_CIPHER_MODE_INFO + bcrypt.Encrypt on Windows).
  • This new method is a trimmed version of Seal: it just does parameter validation and calls the system Seal function without touching or reslicing any parameter. Think about it as a "raw" seal that don't need to conform with the cipher.AEAD interface.
  • The method can't be used for TLS, so it should panic if the GCM has been created with NewGCMTLS (and friends).
• Implementation as pseudocode:

func (g *aesGCM) SealWithRandomNonce(out, nonce, plaintext, additionalData []byte) {
  // validate parameters
  RandReader.Read(nonce)
  systemcrypto.Seal(g.handle, out, nonce, plaintext, additionalData) // replace this function call with whatever makes sense on each backend.
}

• Replace the *gcm.GCM field in crypto/ciper.gcmWithRandomNonce for AEAD.
• In crypto/ciper.NewGCMWithRandomNonce, if the cipher parameter is not an *aes.Block, try to type switch it to a gcmAble as we do in newGCMFallback.
• In crypto/ciper.gcmWithRandomNonce.Seal, do a type switch to detect if the gcm implements the SealWithRandomNonce method, and use it if it does.
• Before submitting a backend PR, validate that the new function works well when called from cyrpto/ciper.gcmWithRandomNonce.Seal.

[qmuntal]

We discussed this missing feature internally and decided that backporting is too risky given the amount of changes it requires and the lack of proper upstream tests. It is preferable that NewGCMWithRandomNonce returns an error than it causing a security issue due to us rushing it into a patch release.