From 3b305263799d1aa50930f3218b0698d163cd521f Mon Sep 17 00:00:00 2001 From: Wade Simmons Date: Tue, 9 Jun 2026 13:24:59 -0400 Subject: [PATCH] boringcrypto cleanup --- Makefile | 7 ---- noiseutil/boring.go | 69 ++-------------------------------- noiseutil/boring_test.go | 32 ---------------- noiseutil/cipher_state_test.go | 2 +- noiseutil/fips140.go | 5 +-- noiseutil/fips140_test.go | 11 ++++-- noiseutil/notboring.go | 2 + 7 files changed, 17 insertions(+), 111 deletions(-) diff --git a/Makefile b/Makefile index 0c506e87..ae721a72 100644 --- a/Makefile +++ b/Makefile @@ -183,8 +183,6 @@ build/linux-mips-softfloat/%: LDFLAGS += -s -w # boringcrypto build/linux-amd64-boringcrypto/%: GOENV += GOEXPERIMENT=boringcrypto CGO_ENABLED=1 build/linux-arm64-boringcrypto/%: GOENV += GOEXPERIMENT=boringcrypto CGO_ENABLED=1 -build/linux-amd64-boringcrypto/%: LDFLAGS += -checklinkname=0 -build/linux-arm64-boringcrypto/%: LDFLAGS += -checklinkname=0 # fips140 build/linux-amd64-fips140/%: GOENV += GOFIPS140=v1.0.0 @@ -234,9 +232,6 @@ vet: test: $(TEST_ENV) go test $(TEST_FLAGS) -v ./... -test-boringcrypto: - GOEXPERIMENT=boringcrypto CGO_ENABLED=1 go test -ldflags "-checklinkname=0" -v ./... - test-pkcs11: CGO_ENABLED=1 go test -v -tags pkcs11 ./... @@ -309,8 +304,6 @@ fips140-latest: fips140 boringcrypto: @echo > $(NULL_FILE) $(eval GOENV += GOEXPERIMENT=boringcrypto CGO_ENABLED=1) - $(eval LDFLAGS += -checklinkname=0) - $(eval TEST_FLAGS += -ldflags -checklinkname=0) $(eval TEST_ENV += $(GOENV)) $(eval CURVE = P256) ifeq ($(words $(MAKECMDGOALS)),1) diff --git a/noiseutil/boring.go b/noiseutil/boring.go index 2129af71..c77fb573 100644 --- a/noiseutil/boring.go +++ b/noiseutil/boring.go @@ -4,77 +4,16 @@ package noiseutil import ( - "crypto/aes" - "crypto/cipher" - "encoding/binary" - - // unsafe needed for go:linkname - _ "unsafe" + "crypto/boring" "github.com/flynn/noise" ) +var CipherAESGCM noise.CipherFunc = CipherAESGCMFIPS140 + // EncryptLockNeeded indicates if calls to Encrypt need a lock // This is true for boringcrypto because the Seal function verifies that the // nonce is strictly increasing. const EncryptLockNeeded = true -// NewGCMTLS is no longer exposed in go1.19+, so we need to link it in -// See: https://github.com/golang/go/issues/56326 -// -// NewGCMTLS is the internal method used with boringcrypto that provides a -// validated mode of AES-GCM which enforces the nonce is strictly -// monotonically increasing. This is the TLS 1.2 specification for nonce -// generation (which also matches the method used by the Noise Protocol) -// -// - https://github.com/golang/go/blob/go1.19/src/crypto/tls/cipher_suites.go#L520-L522 -// - https://github.com/golang/go/blob/go1.19/src/crypto/internal/boring/aes.go#L235-L237 -// - https://github.com/golang/go/blob/go1.19/src/crypto/internal/boring/aes.go#L250 -// - https://github.com/google/boringssl/blob/ae223d6138807a13006342edfeef32e813246b39/include/openssl/aead.h#L379-L381 -// - https://github.com/google/boringssl/blob/ae223d6138807a13006342edfeef32e813246b39/crypto/fipsmodule/cipher/e_aes.c#L1082-L1093 -// -//go:linkname newGCMTLS crypto/internal/boring.NewGCMTLS -func newGCMTLS(c cipher.Block) (cipher.AEAD, error) - -type cipherFn struct { - fn func([32]byte) noise.Cipher - name string -} - -func (c cipherFn) Cipher(k [32]byte) noise.Cipher { return c.fn(k) } -func (c cipherFn) CipherName() string { return c.name } - -// CipherAESGCM is the AES256-GCM AEAD cipher (using NewGCMTLS when GoBoring is present) -var CipherAESGCM noise.CipherFunc = cipherFn{cipherAESGCMBoring, "AESGCM"} - -func cipherAESGCMBoring(k [32]byte) noise.Cipher { - c, err := aes.NewCipher(k[:]) - if err != nil { - panic(err) - } - gcm, err := newGCMTLS(c) - if err != nil { - panic(err) - } - return aeadCipher{ - gcm, - func(n uint64) []byte { - var nonce [12]byte - binary.BigEndian.PutUint64(nonce[4:], n) - return nonce[:] - }, - } -} - -type aeadCipher struct { - cipher.AEAD - nonce func(uint64) []byte -} - -func (c aeadCipher) Encrypt(out []byte, n uint64, ad, plaintext []byte) []byte { - return c.Seal(out, c.nonce(n), plaintext, ad) -} - -func (c aeadCipher) Decrypt(out []byte, n uint64, ad, ciphertext []byte) ([]byte, error) { - return c.Open(out, c.nonce(n), ciphertext, ad) -} +var boringEnabled = boring.Enabled() diff --git a/noiseutil/boring_test.go b/noiseutil/boring_test.go index 8c884392..c15d431a 100644 --- a/noiseutil/boring_test.go +++ b/noiseutil/boring_test.go @@ -4,8 +4,6 @@ package noiseutil import ( - "crypto/boring" - "encoding/hex" "testing" "github.com/stretchr/testify/assert" @@ -14,33 +12,3 @@ import ( func TestEncryptLockNeeded(t *testing.T) { assert.True(t, EncryptLockNeeded) } - -// Ensure NewGCMTLS validates the nonce is non-repeating -func TestNewGCMTLS(t *testing.T) { - assert.True(t, boring.Enabled()) - - // Test Case 16 from GCM Spec: - // - (now dead link): http://csrc.nist.gov/groups/ST/toolkit/BCM/documents/proposedmodes/gcm/gcm-spec.pdf - // - as listed in boringssl tests: https://github.com/google/boringssl/blob/fips-20220613/crypto/cipher_extra/test/cipher_tests.txt#L412-L418 - key, _ := hex.DecodeString("feffe9928665731c6d6a8f9467308308feffe9928665731c6d6a8f9467308308") - iv, _ := hex.DecodeString("cafebabefacedbaddecaf888") - plaintext, _ := hex.DecodeString("d9313225f88406e5a55909c5aff5269a86a7a9531534f7da2e4c303d8a318a721c3c0c95956809532fcf0e2449a6b525b16aedf5aa0de657ba637b39") - aad, _ := hex.DecodeString("feedfacedeadbeeffeedfacedeadbeefabaddad2") - expected, _ := hex.DecodeString("522dc1f099567d07f47f37a32a84427d643a8cdcbfe5c0c97598a2bd2555d1aa8cb08e48590dbb3da7b08b1056828838c5f61e6393ba7a0abcc9f662") - expectedTag, _ := hex.DecodeString("76fc6ece0f4e1768cddf8853bb2d551b") - - expected = append(expected, expectedTag...) - - var keyArray [32]byte - copy(keyArray[:], key) - c := CipherAESGCM.Cipher(keyArray) - aead := c.(aeadCipher).AEAD - - dst := aead.Seal([]byte{}, iv, plaintext, aad) - assert.Equal(t, expected, dst) - - // We expect this to fail since we are re-encrypting with a repeat IV - assert.PanicsWithError(t, "boringcrypto: EVP_AEAD_CTX_seal failed", func() { - dst = aead.Seal([]byte{}, iv, plaintext, aad) - }) -} diff --git a/noiseutil/cipher_state_test.go b/noiseutil/cipher_state_test.go index 8d844a25..12eeb1a2 100644 --- a/noiseutil/cipher_state_test.go +++ b/noiseutil/cipher_state_test.go @@ -23,7 +23,7 @@ func TestNewCipherStateDispatch(t *testing.T) { encA, _ := buildCipherStates(t, CipherAESGCM) encC, _ := buildCipherStates(t, noise.CipherChaChaPoly) - if !fips140.Enabled() { + if !boringEnabled && !fips140.Enabled() { assert.IsType(t, &CipherStateAESGCM{}, NewCipherState(encA, CipherAESGCM)) } else { // fips140 diff --git a/noiseutil/fips140.go b/noiseutil/fips140.go index 6251444b..a2b4589b 100644 --- a/noiseutil/fips140.go +++ b/noiseutil/fips140.go @@ -1,5 +1,3 @@ -//go:build !boringcrypto - package noiseutil import ( @@ -17,7 +15,8 @@ import ( // - https://github.com/golang/go/issues/73110 // - https://github.com/golang/go/issues/79219 // Using tls.aeadAESGCMTLS13 gives us the TLS 1.3 GCM, which also verifies -// that the nonce is strictly increasing. +// that the nonce is strictly increasing. This works for both boringcrypto +// and fips140. // //go:linkname aeadAESGCMTLS13 crypto/tls.aeadAESGCMTLS13 func aeadAESGCMTLS13(key, noncePrefix []byte) cipher.AEAD diff --git a/noiseutil/fips140_test.go b/noiseutil/fips140_test.go index 0ded06db..012cd2ed 100644 --- a/noiseutil/fips140_test.go +++ b/noiseutil/fips140_test.go @@ -11,7 +11,7 @@ import ( // Ensure NewAESGCM validates the nonce is non-repeating func TestNewAESGCM(t *testing.T) { - if !fips140.Enabled() { + if !boringEnabled && !fips140.Enabled() { t.Skip() return } @@ -32,11 +32,16 @@ func TestNewAESGCM(t *testing.T) { assert.Equal(t, expected, dst) // We expect this to fail since we are re-encrypting with a repeat IV - if fips140.Version() == "v1.0.0" { + switch { + case boringEnabled: + assert.PanicsWithError(t, "boringcrypto: EVP_AEAD_CTX_seal failed", func() { + dst = aead.Seal([]byte{}, iv, plaintext, aad) + }) + case fips140.Version() == "v1.0.0": assert.PanicsWithValue(t, "crypto/cipher: counter decreased", func() { dst = aead.Seal([]byte{}, iv, plaintext, aad) }) - } else { + default: assert.PanicsWithValue(t, "crypto/cipher: counter decreased or remained the same", func() { dst = aead.Seal([]byte{}, iv, plaintext, aad) }) diff --git a/noiseutil/notboring.go b/noiseutil/notboring.go index f288bf88..e8419d74 100644 --- a/noiseutil/notboring.go +++ b/noiseutil/notboring.go @@ -21,3 +21,5 @@ func initAESGCM() noise.CipherFunc { } } + +var boringEnabled = false