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 <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>

diff --git a/src/crypto/cipher/gcm_test.go b/src/crypto/cipher/gcm_test.go
index 7b9d1852d72f72d82702509f4bf74ee8f12502bb..3556146ea69d321dbdb66c0054c5fbf18cbaee06 100644 (file)
--- 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 9520bb0c176bf9baecda0ed5d513891038d80ed8..8819f576f4f4c5bdd3f26079fe54f9b6a8a62f1f 100644 (file)
--- 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