fix: unsafe pointer passing

Use github.com/mattn/go-pointer to save/restore "pointers" across FFI
bounderies. Go reserves the right to move pointers, so using
`unsafe.Pointer` for this is not safe.
This commit is contained in:
Steven Allen 2021-08-27 10:46:42 -07:00
parent cfeec8e4f5
commit ecfa88cc5b
8 changed files with 36 additions and 8 deletions

19
alloc.go Normal file
View File

@ -0,0 +1,19 @@
package openssl
// #include "shim.h"
import "C"
import (
"unsafe"
"github.com/mattn/go-pointer"
)
//export go_ssl_crypto_ex_free
func go_ssl_crypto_ex_free(
parent *C.void, ptr unsafe.Pointer,
cryptoData *C.CRYPTO_EX_DATA, idx C.int,
argl C.long, argp *C.void,
) {
pointer.Unref(ptr)
}

View File

@ -28,6 +28,7 @@ import (
"unsafe"
"github.com/libp2p/go-openssl/utils"
"github.com/mattn/go-pointer"
)
var (
@ -137,7 +138,7 @@ func newConn(conn net.Conn, ctx *Ctx) (*Conn, error) {
C.SSL_set_bio(ssl, into_ssl_cbio, from_ssl_cbio)
s := &SSL{ssl: ssl}
C.SSL_set_ex_data(s.ssl, get_ssl_idx(), unsafe.Pointer(s))
C.SSL_set_ex_data(s.ssl, get_ssl_idx(), pointer.Save(s))
c := &Conn{
SSL: s,

5
ctx.go
View File

@ -27,6 +27,7 @@ import (
"time"
"unsafe"
"github.com/mattn/go-pointer"
"github.com/spacemonkeygo/spacelog"
)
@ -61,7 +62,7 @@ func newCtx(method *C.SSL_METHOD) (*Ctx, error) {
return nil, errorFromErrorQueue()
}
c := &Ctx{ctx: ctx}
C.SSL_CTX_set_ex_data(ctx, get_ssl_ctx_idx(), unsafe.Pointer(c))
C.SSL_CTX_set_ex_data(ctx, get_ssl_ctx_idx(), pointer.Save(c))
runtime.SetFinalizer(c, func(c *Ctx) {
C.SSL_CTX_free(c.ctx)
})
@ -430,7 +431,7 @@ func go_ssl_ctx_verify_cb_thunk(p unsafe.Pointer, ok C.int, ctx *C.X509_STORE_CT
os.Exit(1)
}
}()
verify_cb := (*Ctx)(p).verify_cb
verify_cb := pointer.Restore(p).(*Ctx).verify_cb
// set up defaults just in case verify_cb is nil
if verify_cb != nil {
store := &CertificateStoreCtx{ctx: ctx}

1
go.mod
View File

@ -1,6 +1,7 @@
module github.com/libp2p/go-openssl
require (
github.com/mattn/go-pointer v0.0.1
github.com/spacemonkeygo/spacelog v0.0.0-20180420211403-2296661a0572
golang.org/x/sys v0.0.0-20190626221950-04f50cda93cb // indirect
)

2
go.sum
View File

@ -1,3 +1,5 @@
github.com/mattn/go-pointer v0.0.1 h1:n+XhsuGeVO6MEAp7xyEukFINEa+Quek5psIR/ylA6o0=
github.com/mattn/go-pointer v0.0.1/go.mod h1:2zXcozF6qYGgmsG+SeTZz3oAbFLdD3OWqnUbNvJZAlc=
github.com/spacemonkeygo/spacelog v0.0.0-20180420211403-2296661a0572 h1:RC6RW7j+1+HkWaX/Yh71Ee5ZHaHYt7ZP4sQgUrm6cDU=
github.com/spacemonkeygo/spacelog v0.0.0-20180420211403-2296661a0572/go.mod h1:w0SWMsp6j9O/dk4/ZpIhL+3CkG8ofA2vuv7k+ltqUMc=
golang.org/x/sys v0.0.0-20190626221950-04f50cda93cb h1:fgwFCsaw9buMuxNd6+DQfAuSFqbNiQZpcgJQAgJsK6k=

2
shim.c
View File

@ -428,7 +428,7 @@ int X_SSL_session_reused(SSL *ssl) {
}
int X_SSL_new_index() {
return SSL_get_ex_new_index(0, NULL, NULL, NULL, NULL);
return SSL_get_ex_new_index(0, NULL, NULL, NULL, go_ssl_crypto_ex_free);
}
int X_SSL_verify_cb(int ok, X509_STORE_CTX* store) {

8
ssl.go
View File

@ -20,6 +20,8 @@ import "C"
import (
"os"
"unsafe"
"github.com/mattn/go-pointer"
)
type SSLTLSExtErr int
@ -53,7 +55,7 @@ func go_ssl_verify_cb_thunk(p unsafe.Pointer, ok C.int, ctx *C.X509_STORE_CTX) C
os.Exit(1)
}
}()
verify_cb := (*SSL)(p).verify_cb
verify_cb := pointer.Restore(p).(*SSL).verify_cb
// set up defaults just in case verify_cb is nil
if verify_cb != nil {
store := &CertificateStoreCtx{ctx: ctx}
@ -159,11 +161,11 @@ func sni_cb_thunk(p unsafe.Pointer, con *C.SSL, ad unsafe.Pointer, arg unsafe.Po
}
}()
sni_cb := (*Ctx)(p).sni_cb
sni_cb := pointer.Restore(p).(*Ctx).sni_cb
s := &SSL{ssl: con}
// This attaches a pointer to our SSL struct into the SNI callback.
C.SSL_set_ex_data(s.ssl, get_ssl_idx(), unsafe.Pointer(s))
C.SSL_set_ex_data(s.ssl, get_ssl_idx(), pointer.Save(s))
// Note: this is ctx.sni_cb, not C.sni_cb
return C.int(sni_cb(s))

View File

@ -20,6 +20,8 @@ import "C"
import (
"os"
"unsafe"
"github.com/mattn/go-pointer"
)
const (
@ -127,7 +129,7 @@ func go_ticket_key_cb_thunk(p unsafe.Pointer, s *C.SSL, key_name *C.uchar,
}
}()
ctx := (*Ctx)(p)
ctx := pointer.Restore(p).(*Ctx)
store := ctx.ticket_store
if store == nil {
// TODO(jeff): should this be an error condition? it doesn't make sense