go icon indicating copy to clipboard operation
go copied to clipboard

crypto/cipher: unnecessary allocations when using boringcrypto's AES-GCM implementation

Open juergw opened this issue 11 months ago • 2 comments

Go version

go1.24-20241213-RC00

Output of go env in your module/workspace:

AR='ar'
CC='clang'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='clang++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/usr/local/google/home/juerg/.cache/go-build'
GODEBUG=''
GOENV='/usr/local/google/home/juerg/.config/go/env'
GOEXE=''
GOEXPERIMENT='fieldtrack,boringcrypto'
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build116742542=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/dev/null'
GOMODCACHE='/usr/local/google/home/juerg/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/usr/local/google/home/juerg/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/google-golang'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/usr/local/google/home/juerg/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/google-golang/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.24-20241213-RC00 cl/706019355 +e39e965e0e X:fieldtrack,boringcrypto'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

I ran the following benchmark tests for AES-GCM encryption and decryption:

package aead_test

import (
	"crypto/aes"
	"crypto/cipher"
	"math/rand"
	"testing"
)

const (
	plaintextSize      = 10 * 1024 * 1024
	associatedDataSize = 256
	nonceSize          = 12
)

func GetRandomBytes(n uint32) []byte {
	buf := make([]byte, n)
	_, err := rand.Read(buf)
	if err != nil {
		panic(err)
	}
	return buf
}

func BenchmarkAesGcmEncrypt(b *testing.B) {
	b.ReportAllocs()

	a, err := aes.NewCipher(GetRandomBytes(16))
	if err != nil {
		b.Fatal(err)
	}
	aesGCM, err := cipher.NewGCM(a)
	if err != nil {
		b.Fatal(err)
	}

	plaintext := GetRandomBytes(plaintextSize)
	associatedData := GetRandomBytes(associatedDataSize)
	nonce := GetRandomBytes(nonceSize)

	b.ResetTimer()

	for i := 0; i < b.N; i++ {
		_ = aesGCM.Seal(nil, nonce, plaintext, associatedData)
	}
}

func BenchmarkAesGcmDecrypt(b *testing.B) {
	b.ReportAllocs()

	a, err := aes.NewCipher(GetRandomBytes(16))
	if err != nil {
		b.Fatal(err)
	}
	aesGCM, err := cipher.NewGCM(a)
	if err != nil {
		b.Fatal(err)
	}

	plaintext := GetRandomBytes(plaintextSize)
	associatedData := GetRandomBytes(associatedDataSize)
	nonce := GetRandomBytes(nonceSize)
	ciphertext := aesGCM.Seal(nil, nonce, plaintext, associatedData)

	b.ResetTimer()

	for i := 0; i < b.N; i++ {
		if _, err = aesGCM.Open(nil, nonce, ciphertext, associatedData); err != nil {
			b.Error(err)
		}
	}
}

What did you see happen?

I built the test target and run the benchmark tests with

./aead_test --test.run=NONE --test.bench=. --test.count=1

the output was:

goos: linux
goarch: amd64
pkg: google3/third_party/tink/go/aead/aead_test
cpu: AMD EPYC 7B12
BenchmarkAesGcmEncrypt-8   	     232	   4958494 ns/op	10493980 B/op	       2 allocs/op
BenchmarkAesGcmDecrypt-8   	      58	  26919690 ns/op	52263828 B/op	      48 allocs/op
PASS

The decryption takes 5x longer than the encryption, and does 48 allocations, while the encryption only does 2 allocations.

What did you expect to see?

That the decryption is about as fast as the encryption.

I think the problem is here:

https://github.com/golang/go/blob/master/src/crypto/internal/boring/aes.go#L366

When I replace this loop by this:

		newDstLen := n + len(ciphertext) - gcmTagSize
		if cap(dst) < newDstLen {
			oldDst := dst
			dst = make([]byte, n, newDstLen)
			copy(dst, oldDst)
		}
		dst = dst[:newDstLen]

The issue goes away.

juergw avatar Jan 06 '25 14:01 juergw

CC @golang/security

ianlancetaylor avatar Jan 06 '25 18:01 ianlancetaylor