From 067f28ad73e096a7ca1e892ba89280dfa4fa419d Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Wed, 25 Oct 2023 15:04:45 +0000 Subject: [PATCH] Revert "crypto/internal/boring: use noescape and nocallback cgo directives" This reverts CL 525035. Reason for revert: breaks many Google-internal tests (#63739), suspected miscompilation Change-Id: I8cbebca0a187d12e16c405b2373c754e4a397ef4 Reviewed-on: https://go-review.googlesource.com/c/go/+/537598 Reviewed-by: Bryan Mills LUCI-TryBot-Result: Go LUCI Reviewed-by: Roland Shoemaker Auto-Submit: Bryan Mills --- src/crypto/cipher/gcm_test.go | 36 -------------------- src/crypto/internal/boring/aes.go | 55 +++++++++++++++++++++++-------- 2 files changed, 41 insertions(+), 50 deletions(-) diff --git a/src/crypto/cipher/gcm_test.go b/src/crypto/cipher/gcm_test.go index 7b9d1852d7..3556146ea6 100644 --- a/src/crypto/cipher/gcm_test.go +++ b/src/crypto/cipher/gcm_test.go @@ -654,39 +654,3 @@ func TestGCMAsm(t *testing.T) { } } } - -func BenchmarkGCMSeal(b *testing.B) { - key, _ := hex.DecodeString("ab72c77b97cb5fe9a382d9fe81ffdbed") - nonce, _ := hex.DecodeString("54cc7dc2c37ec006bcc6d1db") - plaintext, _ := hex.DecodeString("f1cc3818e421876bb6b8bbd6c9") - - aes, _ := aes.NewCipher(key) - aesgcm, _ := cipher.NewGCM(aes) - - ciphertext := make([]byte, 32) - b.SetBytes(int64(len(plaintext))) - b.ResetTimer() - for i := 0; i < b.N; i++ { - _ = aesgcm.Seal(ciphertext[:0], nonce, plaintext, nil) - } -} - -func BenchmarkGCMOpen(b *testing.B) { - key, _ := hex.DecodeString("ab72c77b97cb5fe9a382d9fe81ffdbed") - nonce, _ := hex.DecodeString("54cc7dc2c37ec006bcc6d1db") - plaintext, _ := hex.DecodeString("f1cc3818e421876bb6b8bbd6c9") - - aes, _ := aes.NewCipher(key) - aesgcm, _ := cipher.NewGCM(aes) - - ciphertext := aesgcm.Seal(nil, nonce, plaintext, nil) - - b.SetBytes(int64(len(ciphertext))) - b.ResetTimer() - for i := 0; i < b.N; i++ { - _, err := aesgcm.Open(plaintext[:0], nonce, ciphertext, nil) - if err != nil { - b.Fatal(err) - } - } -} diff --git a/src/crypto/internal/boring/aes.go b/src/crypto/internal/boring/aes.go index 9520bb0c17..8819f576f4 100644 --- a/src/crypto/internal/boring/aes.go +++ b/src/crypto/internal/boring/aes.go @@ -7,11 +7,40 @@ package boring /* + #include "goboringcrypto.h" -#cgo noescape _goboringcrypto_EVP_AEAD_CTX_seal -#cgo nocallback _goboringcrypto_EVP_AEAD_CTX_seal -#cgo noescape _goboringcrypto_EVP_AEAD_CTX_open -#cgo nocallback _goboringcrypto_EVP_AEAD_CTX_open + +// These wrappers allocate out_len on the C stack, and check that it matches the expected +// value, to avoid having to pass a pointer from Go, which would escape to the heap. + +int EVP_AEAD_CTX_seal_wrapper(const GO_EVP_AEAD_CTX *ctx, uint8_t *out, + size_t exp_out_len, + const uint8_t *nonce, size_t nonce_len, + const uint8_t *in, size_t in_len, + const uint8_t *ad, size_t ad_len) { + size_t out_len; + int ok = _goboringcrypto_EVP_AEAD_CTX_seal(ctx, out, &out_len, exp_out_len, + nonce, nonce_len, in, in_len, ad, ad_len); + if (out_len != exp_out_len) { + return 0; + } + return ok; +}; + +int EVP_AEAD_CTX_open_wrapper(const GO_EVP_AEAD_CTX *ctx, uint8_t *out, + size_t exp_out_len, + const uint8_t *nonce, size_t nonce_len, + const uint8_t *in, size_t in_len, + const uint8_t *ad, size_t ad_len) { + size_t out_len; + int ok = _goboringcrypto_EVP_AEAD_CTX_open(ctx, out, &out_len, exp_out_len, + nonce, nonce_len, in, in_len, ad, ad_len); + if (out_len != exp_out_len) { + return 0; + } + return ok; +}; + */ import "C" import ( @@ -289,16 +318,15 @@ func (g *aesGCM) Seal(dst, nonce, plaintext, additionalData []byte) []byte { panic("cipher: invalid buffer overlap") } - var outLen C.size_t - expOutLen := C.size_t(len(plaintext) + gcmTagSize) - ok := C._goboringcrypto_EVP_AEAD_CTX_seal( + outLen := C.size_t(len(plaintext) + gcmTagSize) + ok := C.EVP_AEAD_CTX_seal_wrapper( &g.ctx, - (*C.uint8_t)(unsafe.Pointer(&dst[n])), &outLen, expOutLen, + (*C.uint8_t)(unsafe.Pointer(&dst[n])), outLen, base(nonce), C.size_t(len(nonce)), base(plaintext), C.size_t(len(plaintext)), base(additionalData), C.size_t(len(additionalData))) runtime.KeepAlive(g) - if ok == 0 || outLen != expOutLen { + if ok == 0 { panic(fail("EVP_AEAD_CTX_seal")) } return dst[:n+int(outLen)] @@ -329,16 +357,15 @@ func (g *aesGCM) Open(dst, nonce, ciphertext, additionalData []byte) ([]byte, er panic("cipher: invalid buffer overlap") } - var outLen C.size_t - expOutLen := C.size_t(len(ciphertext) - gcmTagSize) - ok := C._goboringcrypto_EVP_AEAD_CTX_open( + outLen := C.size_t(len(ciphertext) - gcmTagSize) + ok := C.EVP_AEAD_CTX_open_wrapper( &g.ctx, - base(dst[n:]), &outLen, expOutLen, + base(dst[n:]), outLen, base(nonce), C.size_t(len(nonce)), base(ciphertext), C.size_t(len(ciphertext)), base(additionalData), C.size_t(len(additionalData))) runtime.KeepAlive(g) - if ok == 0 || outLen != expOutLen { + if ok == 0 { return nil, errOpen } return dst[:n+int(outLen)], nil