Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrap in/out length pointer APIs: avoid heap escape #130

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ecdh.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,12 +269,12 @@ 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")
}
var outLen C.size_t
if C.go_openssl_EVP_PKEY_derive(ctx, nil, &outLen) != 1 {
outLen := C.go_openssl_EVP_PKEY_derive_wrapper_get_len(ctx)
if outLen == 0 {
return nil, newOpenSSLError("EVP_PKEY_derive_init")
}
out := make([]byte, outLen)
if C.go_openssl_EVP_PKEY_derive(ctx, base(out), &outLen) != 1 {
if C.go_openssl_EVP_PKEY_derive_wrapper_with_len(ctx, base(out), outLen) != 1 {
return nil, newOpenSSLError("EVP_PKEY_derive_init")
}
return out, nil
Expand Down
7 changes: 4 additions & 3 deletions ed25519.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func NewPrivateKeyEd25519FromSeed(seed []byte) (*PrivateKeyEd25519, error) {

func extractPKEYPubEd25519(pkey C.GO_EVP_PKEY_PTR, pub []byte) error {
pubSize := C.size_t(publicKeySizeEd25519)
if C.go_openssl_EVP_PKEY_get_raw_public_key(pkey, base(pub), &pubSize) != 1 {
if C.go_openssl_EVP_PKEY_get_raw_public_key_wrapper_with_len(pkey, base(pub), pubSize) != 1 {
return newOpenSSLError("EVP_PKEY_get_raw_public_key")
}
if pubSize != publicKeySizeEd25519 {
Expand All @@ -160,7 +160,7 @@ func extractPKEYPrivEd25519(pkey C.GO_EVP_PKEY_PTR, priv []byte) error {
return err
}
privSize := C.size_t(seedSizeEd25519)
if C.go_openssl_EVP_PKEY_get_raw_private_key(pkey, base(priv), &privSize) != 1 {
if C.go_openssl_EVP_PKEY_get_raw_private_key_wrapper_with_len(pkey, base(priv), privSize) != 1 {
return newOpenSSLError("EVP_PKEY_get_raw_private_key")
}
if privSize != seedSizeEd25519 {
Expand Down Expand Up @@ -191,7 +191,8 @@ func signEd25519(priv *PrivateKeyEd25519, sig, message []byte) error {
return newOpenSSLError("EVP_DigestSignInit")
}
siglen := C.size_t(signatureSizeEd25519)
if C.go_openssl_EVP_DigestSign(ctx, base(sig), &siglen, base(message), C.size_t(len(message))) != 1 {
siglen = C.go_openssl_EVP_DigestSign_wrapper_modify_len(ctx, base(sig), siglen, base(message), C.size_t(len(message)))
if siglen == 0 {
return newOpenSSLError("EVP_DigestSign")
}
if siglen != signatureSizeEd25519 {
Expand Down
41 changes: 41 additions & 0 deletions goopenssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,47 @@ go_openssl_EVP_CipherUpdate_wrapper(GO_EVP_CIPHER_CTX_PTR ctx, unsigned char *ou
return go_openssl_EVP_CipherUpdate(ctx, out, &len, in, in_len);
}

// These wrappers allocate a length variable on the C stack to avoid having to pass a pointer from Go, which would escape to the heap.
// Some OpenSSL APIs use length as either "in" and "out" depending on the value of another parameter, so they accept a pointer.
// By splitting the "in" and "out" situations into distinct functions, a pointer is not necessary.
//
// Note: for simplicity, the "out" wrappers treat a 0 length the same as an error.
// A 0 is not an expected return value in this context, so this is expected to be ok.

static inline size_t
go_openssl_EVP_PKEY_derive_wrapper_get_len(GO_EVP_PKEY_CTX_PTR ctx)
{
size_t keylen;
if (go_openssl_EVP_PKEY_derive(ctx, NULL, &keylen) != 1)
return 0;
return keylen;
}

static inline int
go_openssl_EVP_PKEY_derive_wrapper_with_len(GO_EVP_PKEY_CTX_PTR ctx, unsigned char *key, size_t keylen)
{
return go_openssl_EVP_PKEY_derive(ctx, key, &keylen);
}

static inline int
go_openssl_EVP_PKEY_get_raw_public_key_wrapper_with_len(const GO_EVP_PKEY_PTR pkey, unsigned char *pub, size_t len)
{
return go_openssl_EVP_PKEY_get_raw_public_key(pkey, pub, &len);
}

static inline int
go_openssl_EVP_PKEY_get_raw_private_key_wrapper_with_len(const GO_EVP_PKEY_PTR pkey, unsigned char *priv, size_t len)
{
return go_openssl_EVP_PKEY_get_raw_private_key(pkey, priv, &len);
}

static inline size_t
go_openssl_EVP_DigestSign_wrapper_modify_len(GO_EVP_MD_CTX_PTR ctx, unsigned char *sigret, size_t siglen, const unsigned char *tbs, size_t tbslen)
{
if (go_openssl_EVP_DigestSign(ctx, sigret, &siglen, tbs, tbslen) != 1)
return 0;
return siglen;
}

// 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.
Expand Down
8 changes: 4 additions & 4 deletions hkdf.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (c *hkdf) 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(c.ctx, base(c.buf), &outLen) != 1 {
if C.go_openssl_EVP_PKEY_derive_wrapper_with_len(c.ctx, base(c.buf), outLen) != 1 {
return 0, newOpenSSLError("EVP_PKEY_derive")
}
n := copy(p, c.buf[prevLen:outLen])
Expand Down Expand Up @@ -132,12 +132,12 @@ func ExtractHKDF(h func() hash.Hash, secret, salt []byte) ([]byte, error) {
return nil, newOpenSSLError("EVP_PKEY_CTX_set1_hkdf_salt")
}
}
var outLen C.size_t
if C.go_openssl_EVP_PKEY_derive(c.ctx, nil, &outLen) != 1 {
outLen := C.go_openssl_EVP_PKEY_derive_wrapper_get_len(c.ctx)
if outLen == 0 {
return nil, newOpenSSLError("EVP_PKEY_derive_init")
}
out := make([]byte, outLen)
if C.go_openssl_EVP_PKEY_derive(c.ctx, base(out), &outLen) != 1 {
if C.go_openssl_EVP_PKEY_derive_wrapper_with_len(c.ctx, base(out), outLen) != 1 {
return nil, newOpenSSLError("EVP_PKEY_derive")
}
return out[:outLen], nil
Expand Down
2 changes: 1 addition & 1 deletion tls1prf.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func TLS1PRF(result, secret, label, seed []byte, h func() hash.Hash) error {
}
}
outLen := C.size_t(len(result))
if C.go_openssl_EVP_PKEY_derive(ctx, base(result), &outLen) != 1 {
if C.go_openssl_EVP_PKEY_derive_wrapper_with_len(ctx, base(result), outLen) != 1 {
return newOpenSSLError("EVP_PKEY_derive")
}
// The Go standard library expects TLS1PRF to return the requested number of bytes,
Expand Down