From 063dbb3dd3f72bc992cf4ead883fc0c4da5f841a Mon Sep 17 00:00:00 2001 From: qmuntal Date: Tue, 18 Feb 2025 13:55:34 +0100 Subject: [PATCH 1/3] use cgo noescape/nocallback instead of C wrappers --- cgo_go124.go | 20 ++++++++++++-- cipher.go | 12 ++++++--- ecdh.go | 12 ++++----- ed25519.go | 24 ++++++++--------- goopenssl.h | 76 ---------------------------------------------------- hkdf.go | 15 ++++++----- tls1prf.go | 2 +- 7 files changed, 53 insertions(+), 108 deletions(-) diff --git a/cgo_go124.go b/cgo_go124.go index 933a7518..7bfd3404 100644 --- a/cgo_go124.go +++ b/cgo_go124.go @@ -13,6 +13,22 @@ package openssl // observed to benefit from these directives, not every function that is merely // expected to meet the noescape/nocallback criteria. -// #cgo noescape go_openssl_RAND_bytes -// #cgo nocallback go_openssl_RAND_bytes +/* +#cgo noescape go_openssl_RAND_bytes +#cgo nocallback go_openssl_RAND_bytes +#cgo noescape go_openssl_EVP_EncryptUpdate +#cgo nocallback go_openssl_EVP_EncryptUpdate +#cgo noescape go_openssl_EVP_DecryptUpdate +#cgo nocallback go_openssl_EVP_DecryptUpdate +#cgo noescape go_openssl_EVP_CipherUpdate +#cgo nocallback go_openssl_EVP_CipherUpdate +#cgo noescape go_openssl_EVP_PKEY_derive +#cgo nocallback go_openssl_EVP_PKEY_derive +#cgo noescape go_openssl_EVP_PKEY_get_raw_public_key +#cgo nocallback go_openssl_EVP_PKEY_get_raw_public_key +#cgo noescape go_openssl_EVP_PKEY_get_raw_private_key +#cgo nocallback go_openssl_EVP_PKEY_get_raw_private_key +#cgo noescape go_openssl_EVP_DigestSign +#cgo nocallback go_openssl_EVP_DigestSign +*/ import "C" diff --git a/cipher.go b/cipher.go index 72f7aebf..a559fa50 100644 --- a/cipher.go +++ b/cipher.go @@ -179,7 +179,8 @@ func (c *evpCipher) encrypt(dst, src []byte) error { } defer C.go_openssl_EVP_CIPHER_CTX_free(enc_ctx) - if C.go_openssl_EVP_EncryptUpdate_wrapper(enc_ctx, base(dst), base(src), C.int(c.blockSize)) != 1 { + var outl C.int + if C.go_openssl_EVP_EncryptUpdate(enc_ctx, base(dst), &outl, base(src), C.int(c.blockSize)) != 1 { return errors.New("EncryptUpdate failed") } runtime.KeepAlive(c) @@ -208,7 +209,8 @@ func (c *evpCipher) decrypt(dst, src []byte) error { return errors.New("could not disable cipher padding") } - C.go_openssl_EVP_DecryptUpdate_wrapper(dec_ctx, base(dst), base(src), C.int(c.blockSize)) + var outl C.int + C.go_openssl_EVP_DecryptUpdate(dec_ctx, base(dst), &outl, base(src), C.int(c.blockSize)) runtime.KeepAlive(c) return nil } @@ -235,7 +237,8 @@ func (x *cipherCBC) CryptBlocks(dst, src []byte) { panic("crypto/cipher: output smaller than input") } if len(src) > 0 { - if C.go_openssl_EVP_CipherUpdate_wrapper(x.ctx, base(dst), base(src), C.int(len(src))) != 1 { + var outl C.int + if C.go_openssl_EVP_CipherUpdate(x.ctx, base(dst), &outl, base(src), C.int(len(src))) != 1 { panic("crypto/cipher: CipherUpdate failed") } runtime.KeepAlive(x) @@ -278,7 +281,8 @@ func (x *cipherCTR) XORKeyStream(dst, src []byte) { if len(src) == 0 { return } - if C.go_openssl_EVP_EncryptUpdate_wrapper(x.ctx, base(dst), base(src), C.int(len(src))) != 1 { + var outl C.int + if C.go_openssl_EVP_EncryptUpdate(x.ctx, base(dst), &outl, base(src), C.int(len(src))) != 1 { panic("crypto/cipher: EncryptUpdate failed") } runtime.KeepAlive(x) diff --git a/ecdh.go b/ecdh.go index ad392dca..0b76b847 100644 --- a/ecdh.go +++ b/ecdh.go @@ -249,13 +249,13 @@ func ECDH(priv *PrivateKeyECDH, pub *PublicKeyECDH) ([]byte, error) { if C.go_openssl_EVP_PKEY_derive_set_peer(ctx, pub._pkey) != 1 { return nil, newOpenSSLError("EVP_PKEY_derive_set_peer") } - r := C.go_openssl_EVP_PKEY_derive_wrapper(ctx, nil, 0) - if r.result != 1 { - return nil, newOpenSSLError("EVP_PKEY_derive_init") + var keylen C.size_t + if C.go_openssl_EVP_PKEY_derive(ctx, nil, &keylen) != 1 { + return nil, newOpenSSLError("EVP_PKEY_derive") } - out := make([]byte, r.keylen) - if C.go_openssl_EVP_PKEY_derive_wrapper(ctx, base(out), r.keylen).result != 1 { - return nil, newOpenSSLError("EVP_PKEY_derive_init") + out := make([]byte, keylen) + if C.go_openssl_EVP_PKEY_derive(ctx, base(out), &keylen) != 1 { + return nil, newOpenSSLError("EVP_PKEY_derive") } return out, nil } diff --git a/ed25519.go b/ed25519.go index f96db2cd..7474245d 100644 --- a/ed25519.go +++ b/ed25519.go @@ -155,12 +155,12 @@ func NewPrivateKeyEd25519FromSeed(seed []byte) (*PrivateKeyEd25519, error) { } func extractPKEYPubEd25519(pkey C.GO_EVP_PKEY_PTR, pub []byte) error { - r := C.go_openssl_EVP_PKEY_get_raw_public_key_wrapper(pkey, base(pub), C.size_t(publicKeySizeEd25519)) - if r.result != 1 { + keylen := C.size_t(publicKeySizeEd25519) + if C.go_openssl_EVP_PKEY_get_raw_public_key(pkey, base(pub), &keylen) != 1 { return newOpenSSLError("EVP_PKEY_get_raw_public_key") } - if r.len != publicKeySizeEd25519 { - return errors.New("ed25519: bad public key length: " + strconv.Itoa(int(r.len))) + if int(keylen) != publicKeySizeEd25519 { + return errors.New("ed25519: bad public key length: " + strconv.Itoa(int(keylen))) } return nil } @@ -169,12 +169,12 @@ func extractPKEYPrivEd25519(pkey C.GO_EVP_PKEY_PTR, priv []byte) error { if err := extractPKEYPubEd25519(pkey, priv[seedSizeEd25519:]); err != nil { return err } - r := C.go_openssl_EVP_PKEY_get_raw_private_key_wrapper(pkey, base(priv), C.size_t(seedSizeEd25519)) - if r.result != 1 { + keylen := C.size_t(seedSizeEd25519) + if C.go_openssl_EVP_PKEY_get_raw_private_key(pkey, base(priv), &keylen) != 1 { return newOpenSSLError("EVP_PKEY_get_raw_private_key") } - if r.len != seedSizeEd25519 { - return errors.New("ed25519: bad private key length: " + strconv.Itoa(int(r.len))) + if int(keylen) != seedSizeEd25519 { + return errors.New("ed25519: bad private key length: " + strconv.Itoa(int(keylen))) } return nil } @@ -200,12 +200,12 @@ func signEd25519(priv *PrivateKeyEd25519, sig, message []byte) error { if C.go_openssl_EVP_DigestSignInit(ctx, nil, nil, nil, priv._pkey) != 1 { return newOpenSSLError("EVP_DigestSignInit") } - r := C.go_openssl_EVP_DigestSign_wrapper(ctx, base(sig), C.size_t(signatureSizeEd25519), base(message), C.size_t(len(message))) - if r.result != 1 { + siglen := C.size_t(signatureSizeEd25519) + if C.go_openssl_EVP_DigestSign(ctx, base(sig), &siglen, base(message), C.size_t(len(message))) != 1 { return newOpenSSLError("EVP_DigestSign") } - if r.siglen != signatureSizeEd25519 { - return errors.New("ed25519: bad signature length: " + strconv.Itoa(int(r.siglen))) + if int(siglen) != signatureSizeEd25519 { + return errors.New("ed25519: bad signature length: " + strconv.Itoa(int(siglen))) } return nil } diff --git a/goopenssl.h b/goopenssl.h index 623fe375..6d1a2210 100644 --- a/goopenssl.h +++ b/goopenssl.h @@ -76,82 +76,6 @@ go_hash_sum(GO_EVP_MD_CTX_PTR ctx, GO_EVP_MD_CTX_PTR ctx2, unsigned char *out) return go_openssl_EVP_DigestFinal(ctx2, out, NULL); } -// These wrappers allocate out_len on the C stack to avoid having to pass a pointer from Go, which would escape to the heap. -// Use them only in situations where the output length can be safely discarded. -static inline int -go_openssl_EVP_EncryptUpdate_wrapper(GO_EVP_CIPHER_CTX_PTR ctx, unsigned char *out, const unsigned char *in, int in_len) -{ - int len; - return go_openssl_EVP_EncryptUpdate(ctx, out, &len, in, in_len); -} - -static inline int -go_openssl_EVP_DecryptUpdate_wrapper(GO_EVP_CIPHER_CTX_PTR ctx, unsigned char *out, const unsigned char *in, int in_len) -{ - int len; - return go_openssl_EVP_DecryptUpdate(ctx, out, &len, in, in_len); -} - -static inline int -go_openssl_EVP_CipherUpdate_wrapper(GO_EVP_CIPHER_CTX_PTR ctx, unsigned char *out, const unsigned char *in, int in_len) -{ - int len; - return go_openssl_EVP_CipherUpdate(ctx, out, &len, in, in_len); -} - -// These wrappers also allocate length variables on the C stack to avoid escape to the heap, but do return the result. -// A struct is returned that contains multiple return values instead of OpenSSL's approach of using pointers. - -typedef struct -{ - int result; - size_t keylen; -} go_openssl_EVP_PKEY_derive_wrapper_out; - -static inline go_openssl_EVP_PKEY_derive_wrapper_out -go_openssl_EVP_PKEY_derive_wrapper(GO_EVP_PKEY_CTX_PTR ctx, unsigned char *key, size_t keylen) -{ - go_openssl_EVP_PKEY_derive_wrapper_out r = {0, keylen}; - r.result = go_openssl_EVP_PKEY_derive(ctx, key, &r.keylen); - return r; -} - -typedef struct -{ - int result; - size_t len; -} go_openssl_EVP_PKEY_get_raw_key_out; - -static inline go_openssl_EVP_PKEY_get_raw_key_out -go_openssl_EVP_PKEY_get_raw_public_key_wrapper(const GO_EVP_PKEY_PTR pkey, unsigned char *pub, size_t len) -{ - go_openssl_EVP_PKEY_get_raw_key_out r = {0, len}; - r.result = go_openssl_EVP_PKEY_get_raw_public_key(pkey, pub, &r.len); - return r; -} - -static inline go_openssl_EVP_PKEY_get_raw_key_out -go_openssl_EVP_PKEY_get_raw_private_key_wrapper(const GO_EVP_PKEY_PTR pkey, unsigned char *priv, size_t len) -{ - go_openssl_EVP_PKEY_get_raw_key_out r = {0, len}; - r.result = go_openssl_EVP_PKEY_get_raw_private_key(pkey, priv, &r.len); - return r; -} - -typedef struct -{ - int result; - size_t siglen; -} go_openssl_EVP_DigestSign_wrapper_out; - -static inline go_openssl_EVP_DigestSign_wrapper_out -go_openssl_EVP_DigestSign_wrapper(GO_EVP_MD_CTX_PTR ctx, unsigned char *sigret, size_t siglen, const unsigned char *tbs, size_t tbslen) -{ - go_openssl_EVP_DigestSign_wrapper_out r = {0, siglen}; - r.result = go_openssl_EVP_DigestSign(ctx, sigret, &r.siglen, tbs, tbslen); - return r; -} - // 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. diff --git a/hkdf.go b/hkdf.go index d4f8aa6a..9ea51f95 100644 --- a/hkdf.go +++ b/hkdf.go @@ -102,7 +102,7 @@ func (c *hkdf1) Read(p []byte) (int, error) { } c.buf = append(c.buf, make([]byte, needLen)...) outLen := C.size_t(prevLen + needLen) - if C.go_openssl_EVP_PKEY_derive_wrapper(c.ctx, base(c.buf), outLen).result != 1 { + if C.go_openssl_EVP_PKEY_derive(c.ctx, base(c.buf), &outLen) != 1 { return 0, newOpenSSLError("EVP_PKEY_derive") } n := copy(p, c.buf[prevLen:outLen]) @@ -126,15 +126,15 @@ func ExtractHKDF(h func() hash.Hash, secret, salt []byte) ([]byte, error) { return nil, err } defer C.go_openssl_EVP_PKEY_CTX_free(ctx) - r := C.go_openssl_EVP_PKEY_derive_wrapper(ctx, nil, 0) - if r.result != 1 { + var keylen C.size_t + if C.go_openssl_EVP_PKEY_derive(ctx, nil, &keylen) != 1 { return nil, newOpenSSLError("EVP_PKEY_derive_init") } - out := make([]byte, r.keylen) - if C.go_openssl_EVP_PKEY_derive_wrapper(ctx, base(out), r.keylen).result != 1 { + out := make([]byte, keylen) + if C.go_openssl_EVP_PKEY_derive(ctx, base(out), &keylen) != 1 { return nil, newOpenSSLError("EVP_PKEY_derive") } - return out[:r.keylen], nil + return out[:keylen], nil case 3: ctx, err := newHKDFCtx3(md, C.GO_EVP_KDF_HKDF_MODE_EXTRACT_ONLY, secret, salt, nil, nil) if err != nil { @@ -170,7 +170,8 @@ func ExpandHKDFOneShot(h func() hash.Hash, pseudorandomKey, info []byte, keyLeng return nil, err } defer C.go_openssl_EVP_PKEY_CTX_free(ctx) - if C.go_openssl_EVP_PKEY_derive_wrapper(ctx, base(out), C.size_t(keyLength)).result != 1 { + keylen := C.size_t(keyLength) + if C.go_openssl_EVP_PKEY_derive(ctx, base(out), &keylen) != 1 { return nil, newOpenSSLError("EVP_PKEY_derive") } case 3: diff --git a/tls1prf.go b/tls1prf.go index 33134548..f4178db8 100644 --- a/tls1prf.go +++ b/tls1prf.go @@ -97,7 +97,7 @@ func tls1PRF1(result, secret, label, seed []byte, md C.GO_EVP_MD_PTR) error { return newOpenSSLError("EVP_PKEY_CTX_add1_tls1_prf_seed") } outLen := C.size_t(len(result)) - if C.go_openssl_EVP_PKEY_derive_wrapper(ctx, base(result), outLen).result != 1 { + if C.go_openssl_EVP_PKEY_derive(ctx, base(result), &outLen) != 1 { return newOpenSSLError("EVP_PKEY_derive") } // The Go standard library expects TLS1PRF to return the requested number of bytes, From 523a97e90a7dfc29a5d86d68c66c2a6f4a8d018d Mon Sep 17 00:00:00 2001 From: qmuntal Date: Tue, 18 Feb 2025 13:56:28 +0100 Subject: [PATCH 2/3] bump Go test versions --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8b0fbb45..14fce80b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -5,7 +5,7 @@ jobs: strategy: fail-fast: false matrix: - go-version: [1.22.x, 1.23.x] + go-version: [1.23.x, 1.24.x] openssl-version: [1.1.0, 1.1.1, 3.0.1, 3.0.13, 3.1.5, 3.2.1, 3.3.0, 3.3.1] runs-on: ubuntu-20.04 steps: From 41d59d2049af6e6394eee8eebcf7ffcce61ff7f2 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Tue, 18 Feb 2025 14:15:49 +0100 Subject: [PATCH 3/3] skip TestAllocationsif asan is enabled --- asan_disabled_test.go | 7 +++++++ asan_enabled_test.go | 7 +++++++ rand_test.go | 3 +++ 3 files changed, 17 insertions(+) create mode 100644 asan_disabled_test.go create mode 100644 asan_enabled_test.go diff --git a/asan_disabled_test.go b/asan_disabled_test.go new file mode 100644 index 00000000..8360cd48 --- /dev/null +++ b/asan_disabled_test.go @@ -0,0 +1,7 @@ +//go:build !asan + +package openssl_test + +func Asan() bool { + return false +} diff --git a/asan_enabled_test.go b/asan_enabled_test.go new file mode 100644 index 00000000..ffe1464e --- /dev/null +++ b/asan_enabled_test.go @@ -0,0 +1,7 @@ +//go:build asan + +package openssl_test + +func Asan() bool { + return true +} diff --git a/rand_test.go b/rand_test.go index 6dc956b8..56665781 100644 --- a/rand_test.go +++ b/rand_test.go @@ -14,6 +14,9 @@ func TestRand(t *testing.T) { } func TestAllocations(t *testing.T) { + if Asan() { + t.Skip("skipping allocations test with sanitizers") + } n := int(testing.AllocsPerRun(10, func() { buf := make([]byte, 32) openssl.RandReader.Read(buf)