From 022b7ceadaf44b33ad36bba78bce3632d364d8d9 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Thu, 14 Oct 2021 15:50:02 +0900 Subject: [PATCH 1/3] ssl: explicitly call rb_gc_mark() against SSLContext/SSLSocket objects We store the reverse reference to the Ruby object in the OpenSSL struct for use from OpenSSL callback functions. To prevent the Ruby object from being relocated by GC.compact, we must "pin" it by calling rb_gc_mark(). --- ext/openssl/ossl_ssl.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index 0156d4754..70dff91e0 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -59,6 +59,13 @@ static int ossl_sslctx_ex_ptr_idx; static int ossl_sslctx_ex_store_p; #endif +static void +ossl_sslctx_mark(void *ptr) +{ + SSL_CTX *ctx = ptr; + rb_gc_mark((VALUE)SSL_CTX_get_ex_data(ctx, ossl_sslctx_ex_ptr_idx)); +} + static void ossl_sslctx_free(void *ptr) { @@ -73,7 +80,7 @@ ossl_sslctx_free(void *ptr) static const rb_data_type_t ossl_sslctx_type = { "OpenSSL/SSL/CTX", { - 0, ossl_sslctx_free, + ossl_sslctx_mark, ossl_sslctx_free, }, 0, 0, RUBY_TYPED_FREE_IMMEDIATELY, }; @@ -1528,6 +1535,14 @@ ssl_started(SSL *ssl) return SSL_get_fd(ssl) >= 0; } +static void +ossl_ssl_mark(void *ptr) +{ + SSL *ssl = ptr; + rb_gc_mark((VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx)); + rb_gc_mark((VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_vcb_idx)); +} + static void ossl_ssl_free(void *ssl) { @@ -1537,7 +1552,7 @@ ossl_ssl_free(void *ssl) const rb_data_type_t ossl_ssl_type = { "OpenSSL/SSL", { - 0, ossl_ssl_free, + ossl_ssl_mark, ossl_ssl_free, }, 0, 0, RUBY_TYPED_FREE_IMMEDIATELY, }; From a6ba9f894f70e80ad2cc263e088d0b14af3a70ec Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Thu, 14 Oct 2021 15:52:39 +0900 Subject: [PATCH 2/3] x509store: explicitly call rb_gc_mark() against Store/StoreContext We store the reverse reference to the Ruby object in the OpenSSL struct for use from OpenSSL callback functions. To prevent the Ruby object from being relocated by GC.compact, we must "pin" it by calling rb_gc_mark(). --- ext/openssl/ossl_x509store.c | 38 ++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/ext/openssl/ossl_x509store.c b/ext/openssl/ossl_x509store.c index 6c5f1c794..9035a70aa 100644 --- a/ext/openssl/ossl_x509store.c +++ b/ext/openssl/ossl_x509store.c @@ -105,6 +105,13 @@ VALUE cX509Store; VALUE cX509StoreContext; VALUE eX509StoreError; +static void +ossl_x509store_mark(void *ptr) +{ + X509_STORE *store = ptr; + rb_gc_mark((VALUE)X509_STORE_get_ex_data(store, store_ex_verify_cb_idx)); +} + static void ossl_x509store_free(void *ptr) { @@ -114,7 +121,7 @@ ossl_x509store_free(void *ptr) static const rb_data_type_t ossl_x509store_type = { "OpenSSL/X509/STORE", { - 0, ossl_x509store_free, + ossl_x509store_mark, ossl_x509store_free, }, 0, 0, RUBY_TYPED_FREE_IMMEDIATELY, }; @@ -456,23 +463,16 @@ ossl_x509store_verify(int argc, VALUE *argv, VALUE self) return result; } -/* - * Public Functions - */ -static void ossl_x509stctx_free(void*); - - -static const rb_data_type_t ossl_x509stctx_type = { - "OpenSSL/X509/STORE_CTX", - { - 0, ossl_x509stctx_free, - }, - 0, 0, RUBY_TYPED_FREE_IMMEDIATELY, -}; - /* * Private functions */ +static void +ossl_x509stctx_mark(void *ptr) +{ + X509_STORE_CTX *ctx = ptr; + rb_gc_mark((VALUE)X509_STORE_CTX_get_ex_data(ctx, stctx_ex_verify_cb_idx)); +} + static void ossl_x509stctx_free(void *ptr) { @@ -484,6 +484,14 @@ ossl_x509stctx_free(void *ptr) X509_STORE_CTX_free(ctx); } +static const rb_data_type_t ossl_x509stctx_type = { + "OpenSSL/X509/STORE_CTX", + { + ossl_x509stctx_mark, ossl_x509stctx_free, + }, + 0, 0, RUBY_TYPED_FREE_IMMEDIATELY, +}; + static VALUE ossl_x509stctx_alloc(VALUE klass) { From 5eb68ba77855cdf82b6c2ecf884ed183629407b9 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Thu, 14 Oct 2021 15:53:00 +0900 Subject: [PATCH 3/3] ssl: avoid directly storing String object in NPN callback On the server side, the serialized list of protocols is stored in SSL_CTX as a String object reference. We utilize a hidden instance variable to prevent it from being GC'ed, but this is not enough because it can also be relocated by GC.compact. --- ext/openssl/ossl_ssl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index 70dff91e0..bfd80126c 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -699,7 +699,7 @@ static int ssl_npn_advertise_cb(SSL *ssl, const unsigned char **out, unsigned int *outlen, void *arg) { - VALUE protocols = (VALUE)arg; + VALUE protocols = rb_attr_get((VALUE)arg, id_npn_protocols_encoded); *out = (const unsigned char *) RSTRING_PTR(protocols); *outlen = RSTRING_LENINT(protocols); @@ -917,7 +917,7 @@ ossl_sslctx_setup(VALUE self) if (!NIL_P(val)) { VALUE encoded = ssl_encode_npn_protocols(val); rb_ivar_set(self, id_npn_protocols_encoded, encoded); - SSL_CTX_set_next_protos_advertised_cb(ctx, ssl_npn_advertise_cb, (void *)encoded); + SSL_CTX_set_next_protos_advertised_cb(ctx, ssl_npn_advertise_cb, (void *)self); OSSL_Debug("SSL NPN advertise callback added"); } if (RTEST(rb_attr_get(self, id_i_npn_select_cb))) {