From 8535302082fe4b04731562bb686ef2f8242d2465 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Mon, 9 Nov 2026 18:16:50 +0900 Subject: [PATCH 01/11] x509store: do not suppress exceptions in verify_callback Add ossl_cb_state_*() helper to store the temporary rb_protect() jump tag in Ractor-local storage. The caller checks this and re-raise it as necessary. The OpenSSL C API does not let us pass arbitrary data to some callbacks, including verify_callback, so we resort to passing this state out of band. --- ext/openssl/ossl.c | 67 ++++++++++++++++++++++++++++++++++ ext/openssl/ossl.h | 12 ++++++ ext/openssl/ossl_ocsp.c | 16 +++++++- ext/openssl/ossl_pkcs7.c | 7 ++++ ext/openssl/ossl_ssl.c | 17 ++++++--- ext/openssl/ossl_ts.c | 6 +++ ext/openssl/ossl_x509.h | 2 + ext/openssl/ossl_x509store.c | 50 ++++++++++++++----------- test/openssl/test_ocsp.rb | 14 +++++++ test/openssl/test_pkcs7.rb | 7 ++++ test/openssl/test_ssl.rb | 16 ++++---- test/openssl/test_ts.rb | 7 ++++ test/openssl/test_x509store.rb | 16 +++++--- 13 files changed, 196 insertions(+), 41 deletions(-) diff --git a/ext/openssl/ossl.c b/ext/openssl/ossl.c index 578d0051b..78233395e 100644 --- a/ext/openssl/ossl.c +++ b/ext/openssl/ossl.c @@ -145,6 +145,71 @@ ossl_bin2hex(const unsigned char *in, char *out, size_t inlen) } } +#ifdef HAVE_RB_EXT_RACTOR_SAFE +static rb_ractor_local_key_t cb_state_key; + +int +ossl_cb_state_get(void) +{ + int *cell = rb_ractor_local_storage_ptr(cb_state_key); + return cell ? *cell : 0; +} + +static VALUE +ensure_cell(VALUE unused) +{ + int *cell = rb_ractor_local_storage_ptr(cb_state_key); + if (!cell) { + cell = ZALLOC(int); + rb_ractor_local_storage_ptr_set(cb_state_key, cell); + } + return (VALUE)cell; +} + +void +ossl_cb_state_set(int state) +{ + int *cell = (int *)rb_protect(ensure_cell, Qnil, NULL); + if (cell == (void *)Qnil) { + /* As a last resort */ + rb_set_errinfo(Qnil); + return; + } + RUBY_ASSERT(!state || !*cell); + *cell = state; +} +#else +static int cb_state; + +int +ossl_cb_state_get(void) +{ + return cb_state; +} + +void +ossl_cb_state_set(int state) +{ + RUBY_ASSERT(!state || !cb_state); + cb_state = state; +} +#endif + +void +ossl_cb_state_begin(void) +{ + RUBY_ASSERT(!ossl_cb_state_get()); +} + +int +ossl_cb_state_end(void) +{ + int state = ossl_cb_state_get(); + if (state) + ossl_cb_state_set(0); + return state; +} + /* * our default PEM callback */ @@ -1023,6 +1088,8 @@ Init_openssl(void) { #ifdef HAVE_RB_EXT_RACTOR_SAFE rb_ext_ractor_safe(true); + cb_state_key = + rb_ractor_local_storage_ptr_newkey(RB_RACTOR_LOCAL_STORAGE_TYPE_FREE); #endif #undef rb_intern diff --git a/ext/openssl/ossl.h b/ext/openssl/ossl.h index 7762ad237..6ab35e881 100644 --- a/ext/openssl/ossl.h +++ b/ext/openssl/ossl.h @@ -155,6 +155,18 @@ do{\ */ void ossl_bin2hex(const unsigned char *in, char *out, size_t len); +/* + * Ractor-local storage for storing rb_protect() jump tag. Use this only if + * the OpenSSL API doesn't let you pass arbitrary data to the callback. + * + * _begin() expects that there's no currently pending jump. + * No context switch is expected between _set() and _end(). + */ +void ossl_cb_state_begin(void); +int ossl_cb_state_end(void); +int ossl_cb_state_get(void); +void ossl_cb_state_set(int state); + /* * Our default PEM callback */ diff --git a/ext/openssl/ossl_ocsp.c b/ext/openssl/ossl_ocsp.c index 2eaa3840f..b415c30c6 100644 --- a/ext/openssl/ossl_ocsp.c +++ b/ext/openssl/ossl_ocsp.c @@ -378,15 +378,21 @@ ossl_ocspreq_verify(int argc, VALUE *argv, VALUE self) OCSP_REQUEST *req; STACK_OF(X509) *x509s; X509_STORE *x509st; - int flg, result; + int flg, result, state; rb_scan_args(argc, argv, "21", &certs, &store, &flags); GetOCSPReq(self, req); x509st = GetX509StorePtr(store); flg = NIL_P(flags) ? 0 : NUM2INT(flags); x509s = ossl_x509_ary2sk(certs); + ossl_cb_state_begin(); result = OCSP_request_verify(req, x509s, x509st, flg); + state = ossl_cb_state_end(); sk_X509_pop_free(x509s, X509_free); + if (state) { + ossl_clear_error(); + rb_jump_tag(state); + } if (result <= 0) ossl_clear_error(); @@ -989,15 +995,21 @@ ossl_ocspbres_verify(int argc, VALUE *argv, VALUE self) OCSP_BASICRESP *bs; STACK_OF(X509) *x509s; X509_STORE *x509st; - int flg, result; + int flg, result, state; rb_scan_args(argc, argv, "21", &certs, &store, &flags); GetOCSPBasicRes(self, bs); x509st = GetX509StorePtr(store); flg = NIL_P(flags) ? 0 : NUM2INT(flags); x509s = ossl_x509_ary2sk(certs); + ossl_cb_state_begin(); result = OCSP_basic_verify(bs, x509s, x509st, flg); + state = ossl_cb_state_end(); sk_X509_pop_free(x509s, X509_free); + if (state) { + ossl_clear_error(); + rb_jump_tag(state); + } if (result <= 0) ossl_clear_error(); diff --git a/ext/openssl/ossl_pkcs7.c b/ext/openssl/ossl_pkcs7.c index e17a49410..b3c8baa5d 100644 --- a/ext/openssl/ossl_pkcs7.c +++ b/ext/openssl/ossl_pkcs7.c @@ -777,9 +777,16 @@ ossl_pkcs7_verify(int argc, VALUE *argv, VALUE self) sk_X509_pop_free(x509s, X509_free); ossl_raise(ePKCS7Error, NULL); } + ossl_cb_state_begin(); ok = PKCS7_verify(p7, x509s, x509st, in, out, flg); + status = ossl_cb_state_end(); BIO_free(in); sk_X509_pop_free(x509s, X509_free); + if (status) { + BIO_free(out); + ossl_clear_error(); + rb_jump_tag(status); + } data = ossl_membio2str(out); ossl_pkcs7_set_data(self, data); if (ok != 1) { diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index a3aff8009..8beac4908 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -215,7 +215,7 @@ ossl_ssl_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) { VALUE cb, ssl_obj, sslctx_obj, verify_hostname, ret; SSL *ssl; - int status; + int state, ok; ssl = X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()); ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); @@ -225,9 +225,9 @@ ossl_ssl_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) if (preverify_ok && RTEST(verify_hostname) && !SSL_is_server(ssl) && !X509_STORE_CTX_get_error_depth(ctx)) { - ret = rb_protect(call_verify_certificate_identity, (VALUE)ctx, &status); - if (status) { - rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(status)); + ret = rb_protect(call_verify_certificate_identity, (VALUE)ctx, &state); + if (state) { + rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(state)); return 0; } if (ret != Qtrue) { @@ -236,7 +236,14 @@ ossl_ssl_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) } } - return ossl_verify_cb_call(cb, preverify_ok, ctx); + ossl_cb_state_begin(); + ok = ossl_verify_cb_call(cb, preverify_ok, ctx); + state = ossl_cb_state_end(); + if (state) { + rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(state)); + return 0; + } + return ok; } static VALUE diff --git a/ext/openssl/ossl_ts.c b/ext/openssl/ossl_ts.c index 697d740f0..158a2424c 100644 --- a/ext/openssl/ossl_ts.c +++ b/ext/openssl/ossl_ts.c @@ -810,9 +810,15 @@ ossl_ts_resp_verify(int argc, VALUE *argv, VALUE self) #endif TS_VERIFY_CTX_add_flags(ctx, TS_VFY_SIGNATURE); + ossl_cb_state_begin(); ok = TS_RESP_verify_response(ctx, resp); + status = ossl_cb_state_end(); TS_VERIFY_CTX_free(ctx); + if (status) { + ossl_clear_error(); + rb_jump_tag(status); + } if (!ok) ossl_raise(eTimestampError, "TS_RESP_verify_response"); diff --git a/ext/openssl/ossl_x509.h b/ext/openssl/ossl_x509.h index 71932ef1a..3bb952c25 100644 --- a/ext/openssl/ossl_x509.h +++ b/ext/openssl/ossl_x509.h @@ -90,6 +90,8 @@ void Init_ossl_x509store(void); /* * Calls the verify callback Proc (the first parameter) with given pre-verify * result and the X509_STORE_CTX. + * + * Assumes ossl_cb_state_*() is active. */ int ossl_verify_cb_call(VALUE, int, X509_STORE_CTX *); diff --git a/ext/openssl/ossl_x509store.c b/ext/openssl/ossl_x509store.c index 037a03660..9266be8b8 100644 --- a/ext/openssl/ossl_x509store.c +++ b/ext/openssl/ossl_x509store.c @@ -56,37 +56,34 @@ ossl_verify_cb_call(VALUE proc, int ok, X509_STORE_CTX *ctx) struct ossl_verify_cb_args args; int state; + if (ossl_cb_state_get()) + goto err; if (NIL_P(proc)) return ok; - ret = Qfalse; rctx = rb_protect(ossl_x509stctx_new_i, (VALUE)ctx, &state); if (state) { - rb_set_errinfo(Qnil); - rb_warn("StoreContext initialization failure"); + ossl_cb_state_set(state); + goto err; } - else { - args.proc = proc; - args.preverify_ok = ok ? Qtrue : Qfalse; - args.store_ctx = rctx; - ret = rb_protect(call_verify_cb_proc, (VALUE)&args, &state); - RTYPEDDATA_DATA(rctx) = NULL; - if (state) { - rb_set_errinfo(Qnil); - rb_warn("exception in verify_callback is ignored"); - } + args.proc = proc; + args.preverify_ok = ok ? Qtrue : Qfalse; + args.store_ctx = rctx; + ret = rb_protect(call_verify_cb_proc, (VALUE)&args, &state); + RTYPEDDATA_DATA(rctx) = NULL; + if (state) { + ossl_cb_state_set(state); + goto err; } if (ret == Qtrue) { X509_STORE_CTX_set_error(ctx, X509_V_OK); - ok = 1; - } - else { - if (X509_STORE_CTX_get_error(ctx) == X509_V_OK) - X509_STORE_CTX_set_error(ctx, X509_V_ERR_CERT_REJECTED); - ok = 0; + return 1; } - return ok; + err: + if (X509_STORE_CTX_get_error(ctx) == X509_V_OK) + X509_STORE_CTX_set_error(ctx, X509_V_ERR_CERT_REJECTED); + return 0; } /* @@ -673,6 +670,7 @@ static VALUE ossl_x509stctx_verify(VALUE self) { X509_STORE_CTX *ctx; + int ret, state; GetX509StCtx(self, ctx); VALUE cb = rb_iv_get(self, "@verify_callback"); @@ -680,7 +678,14 @@ ossl_x509stctx_verify(VALUE self) ossl_raise(eX509StoreError, "X509_STORE_CTX_set_ex_data"); RB_OBJ_WRITTEN(self, Qundef, cb); - switch (X509_verify_cert(ctx)) { + ossl_cb_state_begin(); + ret = X509_verify_cert(ctx); + state = ossl_cb_state_end(); + if (state) { + ossl_clear_error(); + rb_jump_tag(state); + } + switch (ret) { case 1: return Qtrue; case 0: @@ -1026,7 +1031,8 @@ Init_ossl_x509store(void) * The callback can use StoreContext#error= to change the error code as * needed. The callback must return either true or false. * - * NOTE: any exception raised within the callback will be ignored. + * Exceptions raised within the callback propagate to the caller of the + * verification method. * * See also the man page X509_STORE_CTX_set_verify_cb(3). */ diff --git a/test/openssl/test_ocsp.rb b/test/openssl/test_ocsp.rb index c43ff5cb5..44dbe6c83 100644 --- a/test/openssl/test_ocsp.rb +++ b/test/openssl/test_ocsp.rb @@ -116,6 +116,13 @@ def test_request_sign_verify req.sign(@cert, @cert_key, []) assert_equal true, req.verify([], store) + # Exceptions in verify_callback are re-raised + s = OpenSSL::X509::Store.new.tap { |s| + s.add_cert(@ca_cert) + s.verify_callback = proc { raise "verify_callback" } + } + assert_raise(RuntimeError) { req.verify([], s) } + # without signer cert req = OpenSSL::OCSP::Request.new.add_certid(cid) req.sign(@cert, @cert_key, nil) @@ -195,6 +202,13 @@ def test_basic_response_sign_verify bres.sign(@ca_cert, @ca_key, [], 0, "SHA256") assert_equal true, bres.verify([], store) + # Exceptions in verify_callback are re-raised + s = OpenSSL::X509::Store.new.tap { |s| + s.add_cert(@ca_cert) + s.verify_callback = proc { raise "verify_callback" } + } + assert_raise(RuntimeError) { bres.verify([], s) } + # signed by OCSP signer bres = OpenSSL::OCSP::BasicResponse.new cid = OpenSSL::OCSP::CertificateId.new(@cert2, @cert) diff --git a/test/openssl/test_pkcs7.rb b/test/openssl/test_pkcs7.rb index c677d8223..cfbd7c36f 100644 --- a/test/openssl/test_pkcs7.rb +++ b/test/openssl/test_pkcs7.rb @@ -62,6 +62,13 @@ def test_signed assert_in_delta(Time.now, signers[0].signed_time, 10) unless aws_lc? assert_false(p7.verify([@ca_cert], OpenSSL::X509::Store.new)) + + # Exceptions raised in verify_callback are re-raised + store = OpenSSL::X509::Store.new + store.verify_callback = proc { raise "verify_callback" } + assert_raise_with_message(RuntimeError, "verify_callback") { + p7.verify([], store) + } end def test_signed_flags diff --git a/test/openssl/test_ssl.rb b/test/openssl/test_ssl.rb index c1ab7a07c..29d6eb7cd 100644 --- a/test/openssl/test_ssl.rb +++ b/test/openssl/test_ssl.rb @@ -695,22 +695,24 @@ def test_verify_result } end - def test_exception_in_verify_callback_is_ignored + def test_exception_in_verify_callback start_server(ignore_listener_error: true) { |port| sock = TCPSocket.new("127.0.0.1", port) ctx = OpenSSL::SSL::SSLContext.new - ctx.verify_mode = OpenSSL::SSL::VERIFY_PEER + ctx.cert_store = OpenSSL::X509::Store.new.add_cert(@ca_cert) + called = [] ctx.verify_callback = Proc.new do |preverify_ok, store_ctx| store_ctx.error = OpenSSL::X509::V_OK - raise RuntimeError + called << store_ctx.current_cert + raise "in verify_callback" end ssl = OpenSSL::SSL::SSLSocket.new(sock, ctx) ssl.sync_close = true begin - EnvUtil.suppress_warning do - # SSLError, not RuntimeError - assert_raise(OpenSSL::SSL::SSLError) { ssl.connect } - end + assert_raise_with_message(RuntimeError, "in verify_callback") { + ssl.connect + } + assert_equal([@ca_cert], called) assert_equal(OpenSSL::X509::V_ERR_CERT_REJECTED, ssl.verify_result) ensure ssl.close diff --git a/test/openssl/test_ts.rb b/test/openssl/test_ts.rb index 69780a657..4cc7e3d6d 100644 --- a/test/openssl/test_ts.rb +++ b/test/openssl/test_ts.rb @@ -407,6 +407,13 @@ def test_verify_ee_intermediate_type_error assert_raise(TypeError) { ts.verify(req, [ca_cert], 123) } end + def test_verify_with_verify_callback + ts, req = timestamp_ee + store = ca_store + store.verify_callback = proc { raise "verify callback" } + assert_raise(RuntimeError) { ts.verify(req, store) } + end + def test_verify_ee_def_policy req = OpenSSL::Timestamp::Request.new req.algorithm = "SHA1" diff --git a/test/openssl/test_x509store.rb b/test/openssl/test_x509store.rb index 7132af9f9..b2559b705 100644 --- a/test/openssl/test_x509store.rb +++ b/test/openssl/test_x509store.rb @@ -150,17 +150,23 @@ def test_verify_callback assert_equal(false, store.verify(ee1_cert, [ca2_cert])) assert_equal(OpenSSL::X509::V_ERR_APPLICATION_VERIFICATION, store.error) - # Exception raised by verify_callback is currently suppressed, and is - # treated as a non-truthy return (with warning) + # Exceptions raised in verify_callback are re-raised store = OpenSSL::X509::Store.new store.verify_callback = -> (preverify_ok, sctx) { - raise "suppressed" + raise "in verify_callback" } store.add_cert(ca1_cert) - assert_warning(/exception in verify_callback/) { - assert_equal(false, store.verify(ee1_cert, [ca2_cert])) + assert_raise_with_message(RuntimeError, "in verify_callback") { + store.verify(ee1_cert, [ca2_cert]) } + # verify_callback is nested + ret = store.verify(ca1_cert) do |preverify_ok, sctx| + assert_raise(RuntimeError) { store.verify(ca1_cert) } + preverify_ok + end + assert_true(ret) + # The block given to X509::Store#verify replaces it called = nil store = OpenSSL::X509::Store.new From 7fe484435181e71aeac5592cd196fad6ae5fe0d6 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Tue, 10 Nov 2026 03:19:17 +0900 Subject: [PATCH 02/11] ossl.c: fix possible exceptions in ossl_clear_error() ossl_clear_error() prints the contents of the OpenSSL error queue with rb_warn(), which can run Ruby code. Since this is used in contexts where longjmp cannot be allowed, suppress exceptions raised in it. --- ext/openssl/ossl.c | 92 ++++++++++++++++++++++++++++++++-------------- ext/openssl/ossl.h | 5 +++ 2 files changed, 69 insertions(+), 28 deletions(-) diff --git a/ext/openssl/ossl.c b/ext/openssl/ossl.c index 78233395e..c7cc8af97 100644 --- a/ext/openssl/ossl.c +++ b/ext/openssl/ossl.c @@ -210,6 +210,44 @@ ossl_cb_state_end(void) return state; } +struct suppress_args { + VALUE (*func)(VALUE args); + VALUE args; +}; + +static VALUE +do_nothing(VALUE unused) +{ + return Qnil; +} + +static VALUE +suppress_i(VALUE args_) +{ + struct suppress_args *p = (struct suppress_args *)args_; + int state; + rb_set_errinfo(Qnil); + rb_protect(p->func, p->args, &state); + if (state) { + fprintf(stderr, "ossl_suppress: exception suppressed"); + } + return Qnil; +} + +void +ossl_suppress(VALUE (*func)(VALUE args), VALUE args) +{ + struct suppress_args p = { func, args }; + if (ossl_cb_state_get()) { + int state = ossl_cb_state_end(); + rb_ensure(do_nothing, Qnil, suppress_i, (VALUE)&p); + ossl_cb_state_begin(); + ossl_cb_state_set(state); + return; + } + rb_ensure(do_nothing, Qnil, suppress_i, (VALUE)&p); +} + /* * our default PEM callback */ @@ -340,7 +378,7 @@ ossl_to_der_if_possible(VALUE obj) */ static ID id_i_errors; -static void collect_errors_into(VALUE ary); +static VALUE collect_errors_into(VALUE ary); VALUE ossl_make_error(VALUE exc, VALUE str) @@ -391,46 +429,44 @@ ossl_raise(VALUE exc, const char *fmt, ...) rb_exc_raise(ossl_make_error(exc, err)); } -static void +static VALUE collect_errors_into(VALUE ary) { - if (dOSSL == Qtrue || !NIL_P(ary)) { - unsigned long e; - const char *file, *data, *func, *lib, *reason; - int line, flags; + unsigned long e; + const char *file, *data, *func, *lib, *reason; + int line, flags; #ifdef HAVE_ERR_GET_ERROR_ALL - while ((e = ERR_get_error_all(&file, &line, &func, &data, &flags))) { + while ((e = ERR_get_error_all(&file, &line, &func, &data, &flags))) { #else - while ((e = ERR_get_error_line_data(&file, &line, &data, &flags))) { - func = ERR_func_error_string(e); + while ((e = ERR_get_error_line_data(&file, &line, &data, &flags))) { + func = ERR_func_error_string(e); #endif - lib = ERR_lib_error_string(e); - reason = ERR_reason_error_string(e); - - VALUE str = rb_sprintf("error:%08lX:%s:%s:%s", e, lib ? lib : "", - func ? func : "", reason ? reason : ""); - if (flags & ERR_TXT_STRING) { - if (!data) - data = "(null)"; - rb_str_catf(str, " (%s)", data); - } - - if (dOSSL == Qtrue) - rb_warn("error on stack: %"PRIsVALUE, str); - if (!NIL_P(ary)) - rb_ary_push(ary, str); + lib = ERR_lib_error_string(e); + reason = ERR_reason_error_string(e); + + VALUE str = rb_sprintf("error:%08lX:%s:%s:%s", e, lib ? lib : "", + func ? func : "", reason ? reason : ""); + if (flags & ERR_TXT_STRING) { + if (!data) + data = "(null)"; + rb_str_catf(str, " (%s)", data); } + + if (dOSSL == Qtrue) + rb_warn("error on stack: %"PRIsVALUE, str); + if (!NIL_P(ary)) + rb_ary_push(ary, str); } - else { - ERR_clear_error(); - } + return Qnil; } void ossl_clear_error(void) { - collect_errors_into(Qnil); + if (dOSSL == Qtrue) + ossl_suppress(collect_errors_into, Qnil); + ERR_clear_error(); } /* diff --git a/ext/openssl/ossl.h b/ext/openssl/ossl.h index 6ab35e881..d7679b7f3 100644 --- a/ext/openssl/ossl.h +++ b/ext/openssl/ossl.h @@ -166,6 +166,11 @@ void ossl_cb_state_begin(void); int ossl_cb_state_end(void); int ossl_cb_state_get(void); void ossl_cb_state_set(int state); +/* + * Equivalent to rb_protect(func, args, NULL) except that this avoids + * clobbering the existing errinfo on the ec. + */ +void ossl_suppress(VALUE (*func)(VALUE args), VALUE args); /* * Our default PEM callback From 07282eb0f8ae225f86539344c5c1bed85d355ba4 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Tue, 10 Nov 2026 02:33:12 +0900 Subject: [PATCH 03/11] ssl: use ossl_cb_state_*() for handling exceptions in callbacks Simplify handling of the temporary rb_protect() jump tags. As a side effect, this also avoids corruption of OpenSSL's internal state if the SSLSocket object is frozen in the middle of SSL_*() calls. --- ext/openssl/ossl_ssl.c | 91 ++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 57 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index 8beac4908..843698769 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -36,8 +36,7 @@ VALUE cSSLSocket; static VALUE eSSLErrorWaitReadable; static VALUE eSSLErrorWaitWritable; -static ID id_call, ID_callback_state, id_npn_protocols_encoded, id_each, - id_original_context; +static ID id_call, id_npn_protocols_encoded, id_each, id_original_context; static VALUE sym_wait_readable, sym_wait_writable; static ID id_i_cert_store, id_i_ca_file, id_i_ca_path, id_i_verify_mode, @@ -182,7 +181,7 @@ ossl_tmp_dh_callback(SSL *ssl, int is_export, int keylength) struct tmp_dh_callback_args args = {rb_ssl, is_export, keylength}; VALUE ret = rb_protect(ossl_call_tmp_dh_callback, (VALUE)&args, &state); if (state) { - rb_ivar_set(rb_ssl, ID_callback_state, INT2NUM(state)); + ossl_cb_state_set(state); return NULL; } return (DH *)ret; @@ -215,7 +214,7 @@ ossl_ssl_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) { VALUE cb, ssl_obj, sslctx_obj, verify_hostname, ret; SSL *ssl; - int state, ok; + int state; ssl = X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()); ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); @@ -227,7 +226,7 @@ ossl_ssl_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) !X509_STORE_CTX_get_error_depth(ctx)) { ret = rb_protect(call_verify_certificate_identity, (VALUE)ctx, &state); if (state) { - rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(state)); + ossl_cb_state_set(state); return 0; } if (ret != Qtrue) { @@ -236,14 +235,7 @@ ossl_ssl_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) } } - ossl_cb_state_begin(); - ok = ossl_verify_cb_call(cb, preverify_ok, ctx); - state = ossl_cb_state_end(); - if (state) { - rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(state)); - return 0; - } - return ok; + return ossl_verify_cb_call(cb, preverify_ok, ctx); } static VALUE @@ -275,7 +267,7 @@ ossl_sslctx_session_get_cb(SSL *ssl, const unsigned char *buf, int len, int *cop ret_obj = rb_protect(ossl_call_session_get_cb, ary, &state); if (state) { - rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(state)); + ossl_cb_state_set(state); return NULL; } if (!rb_obj_is_instance_of(ret_obj, cSSLSession)) @@ -321,7 +313,7 @@ ossl_sslctx_session_new_cb(SSL *ssl, SSL_SESSION *sess) rb_protect(ossl_call_session_new_cb, ary, &state); if (state) { - rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(state)); + ossl_cb_state_set(state); } /* @@ -377,7 +369,7 @@ ossl_sslctx_keylog_cb(const SSL *ssl, const char *line) rb_protect(ossl_call_keylog_cb, (VALUE)&args, &state); if (state) { - rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(state)); + ossl_cb_state_set(state); } } #endif @@ -484,8 +476,7 @@ ssl_servername_cb(SSL *ssl, int *ad, void *arg) rb_protect(ossl_call_servername_cb, (VALUE)ssl, &state); if (state) { - VALUE ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); - rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(state)); + ossl_cb_state_set(state); return SSL_TLSEXT_ERR_ALERT_FATAL; } @@ -566,18 +557,16 @@ ssl_npn_select_cb_common(SSL *ssl, VALUE cb, const unsigned char **out, unsigned int inlen) { VALUE selected; - int status; + int state; struct npn_select_cb_common_args args; args.cb = cb; args.in = in; args.inlen = inlen; - selected = rb_protect(npn_select_cb_common_i, (VALUE)&args, &status); - if (status) { - VALUE ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); - - rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(status)); + selected = rb_protect(npn_select_cb_common_i, (VALUE)&args, &state); + if (state) { + ossl_cb_state_set(state); return SSL_TLSEXT_ERR_ALERT_FATAL; } @@ -1827,9 +1816,22 @@ ossl_ssl_setup(VALUE self) return Qtrue; } +static void +ssl_call_begin(void) +{ + ossl_cb_state_begin(); +} + static int -errno_mapped(void) +ssl_call_end(void) { + int state = ossl_cb_state_end(); + if (state) { + /* must cleanup OpenSSL error stack before re-raising */ + ossl_clear_error(); + rb_jump_tag(state); + } + #ifdef _WIN32 return rb_w32_map_errno(WSAGetLastError()); #else @@ -1841,24 +1843,15 @@ static VALUE ossl_start_ssl(VALUE self, int (*func)(SSL *), const char *funcname) { SSL *ssl; - VALUE cb_state; - - rb_ivar_set(self, ID_callback_state, Qnil); GetSSL(self, ssl); #ifdef __APPLE__ retry:; #endif + ssl_call_begin(); int ret = func(ssl); - int saved_errno = errno_mapped(); - - cb_state = rb_attr_get(self, ID_callback_state); - if (!NIL_P(cb_state)) { - /* must cleanup OpenSSL error stack before re-raising */ - ossl_clear_error(); - rb_jump_tag(NUM2INT(cb_state)); - } + int saved_errno = ssl_call_end(); int code = SSL_get_error(ssl, ret); switch (code) { @@ -1952,7 +1945,6 @@ ossl_ssl_read(VALUE self, VALUE len, VALUE str) { SSL *ssl; int ilen; - VALUE cb_state; GetSSL(self, ssl); if (!ssl_started(ssl)) @@ -1972,17 +1964,11 @@ ossl_ssl_read(VALUE self, VALUE len, VALUE str) return str; } + ssl_call_begin(); rb_str_locktmp(str); int nread = SSL_read(ssl, RSTRING_PTR(str), ilen); rb_str_unlocktmp(str); - int saved_errno = errno_mapped(); - - cb_state = rb_attr_get(self, ID_callback_state); - if (!NIL_P(cb_state)) { - rb_ivar_set(self, ID_callback_state, Qnil); - ossl_clear_error(); - rb_jump_tag(NUM2INT(cb_state)); - } + int saved_errno = ssl_call_end(); switch (SSL_get_error(ssl, nread)) { case SSL_ERROR_NONE: @@ -2023,7 +2009,6 @@ static VALUE ossl_ssl_write(VALUE self, VALUE str) { SSL *ssl; - VALUE cb_state; GetSSL(self, ssl); if (!ssl_started(ssl)) @@ -2038,6 +2023,7 @@ ossl_ssl_write(VALUE self, VALUE str) #ifdef __APPLE__ retry:; #endif + ssl_call_begin(); int frozen = RB_OBJ_FROZEN(str); if (!frozen) { rb_str_locktmp(str); @@ -2046,14 +2032,7 @@ ossl_ssl_write(VALUE self, VALUE str) if (!frozen) { rb_str_unlocktmp(str); } - int saved_errno = errno_mapped(); - - cb_state = rb_attr_get(self, ID_callback_state); - if (!NIL_P(cb_state)) { - rb_ivar_set(self, ID_callback_state, Qnil); - ossl_clear_error(); - rb_jump_tag(NUM2INT(cb_state)); - } + int saved_errno = ssl_call_end(); switch (SSL_get_error(ssl, nwritten)) { case SSL_ERROR_NONE: @@ -2615,9 +2594,6 @@ Init_ossl_ssl(void) #endif #ifndef OPENSSL_NO_SOCK - id_call = rb_intern_const("call"); - ID_callback_state = rb_intern_const("callback_state"); - ossl_ssl_ex_ptr_idx = SSL_get_ex_new_index(0, (void *)"ossl_ssl_ex_ptr_idx", 0, 0, 0); if (ossl_ssl_ex_ptr_idx < 0) ossl_raise(rb_eRuntimeError, "SSL_get_ex_new_index"); @@ -3198,6 +3174,7 @@ Init_ossl_ssl(void) sym_wait_readable = ID2SYM(rb_intern_const("wait_readable")); sym_wait_writable = ID2SYM(rb_intern_const("wait_writable")); + id_call = rb_intern_const("call"); id_npn_protocols_encoded = rb_intern_const("npn_protocols_encoded"); id_each = rb_intern_const("each"); id_original_context = rb_intern_const("original_context"); From 765c46ae4e471f16f9002f279221f7181d17033d Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Fri, 6 Feb 2026 03:18:56 +0900 Subject: [PATCH 04/11] ssl: do not call another callback when an exception is pending Do not invoke another Ruby callback while we are waiting for a chance to do rb_jump_tag(). As far as I know, this cannot currently happen. Handle it explicitly in preparation for an upcoming patch that uses a custom BIO_METHOD backed by Ruby methods in SSLSocket. These callbacks may be called after Ruby code called from BIO_METHOD has failed, or vice versa. --- ext/openssl/ossl_ssl.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index 843698769..339d036a1 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -133,6 +133,8 @@ ossl_client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey) { VALUE obj, ret; + if (ossl_cb_state_get()) + return -1; obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); ret = rb_protect(ossl_call_client_cert_cb, obj, NULL); if (NIL_P(ret)) @@ -179,7 +181,11 @@ ossl_tmp_dh_callback(SSL *ssl, int is_export, int keylength) int state; VALUE rb_ssl = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); struct tmp_dh_callback_args args = {rb_ssl, is_export, keylength}; - VALUE ret = rb_protect(ossl_call_tmp_dh_callback, (VALUE)&args, &state); + VALUE ret; + + if (ossl_cb_state_get()) + return NULL; + ret = rb_protect(ossl_call_tmp_dh_callback, (VALUE)&args, &state); if (state) { ossl_cb_state_set(state); return NULL; @@ -216,6 +222,8 @@ ossl_ssl_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) SSL *ssl; int state; + if (ossl_cb_state_get()) + return 0; ssl = X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()); ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); sslctx_obj = rb_attr_get(ssl_obj, id_i_context); @@ -259,6 +267,8 @@ ossl_sslctx_session_get_cb(SSL *ssl, const unsigned char *buf, int len, int *cop SSL_SESSION *sess; int state = 0; + if (ossl_cb_state_get()) + return NULL; OSSL_Debug("SSL SESSION get callback entered"); ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); ary = rb_ary_new2(2); @@ -300,6 +310,8 @@ ossl_sslctx_session_new_cb(SSL *ssl, SSL_SESSION *sess) VALUE ary, ssl_obj, sess_obj; int state = 0; + if (ossl_cb_state_get()) + return 0; OSSL_Debug("SSL SESSION new callback entered"); ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); @@ -361,6 +373,8 @@ ossl_sslctx_keylog_cb(const SSL *ssl, const char *line) struct ossl_call_keylog_cb_args args; int state = 0; + if (ossl_cb_state_get()) + return; OSSL_Debug("SSL keylog callback entered"); ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); @@ -474,6 +488,8 @@ ssl_servername_cb(SSL *ssl, int *ad, void *arg) { int state; + if (ossl_cb_state_get()) + return SSL_TLSEXT_ERR_ALERT_FATAL; rb_protect(ossl_call_servername_cb, (VALUE)ssl, &state); if (state) { ossl_cb_state_set(state); @@ -560,6 +576,9 @@ ssl_npn_select_cb_common(SSL *ssl, VALUE cb, const unsigned char **out, int state; struct npn_select_cb_common_args args; + if (ossl_cb_state_get()) + return SSL_TLSEXT_ERR_ALERT_FATAL; + args.cb = cb; args.in = in; args.inlen = inlen; From bc98c7ad6ac0f8462e07f4016daa4be62ea3607d Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Thu, 27 Nov 2025 18:09:22 +0900 Subject: [PATCH 05/11] ssl: harden info_callback Use rb_protect() to prevent exiting from SSL_accept() prematurely. Also set the callback to SSL_CTX instead of SSL, similarly to what we do for other callbacks configurable on SSLContext. --- ext/openssl/ossl_ssl.c | 47 ++++++++++++++++++++++------------------ test/openssl/test_ssl.rb | 20 +++++++++++++++++ 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index 339d036a1..9e67e48a8 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -499,17 +499,31 @@ ssl_servername_cb(SSL *ssl, int *ad, void *arg) return SSL_TLSEXT_ERR_OK; } -static void -ssl_renegotiation_cb(const SSL *ssl) +static VALUE +call_renegotiation_cb(VALUE args_) { - VALUE ssl_obj, sslctx_obj, cb; - - ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); - sslctx_obj = rb_attr_get(ssl_obj, id_i_context); - cb = rb_attr_get(sslctx_obj, id_i_renegotiation_cb); - if (NIL_P(cb)) return; + VALUE *args = (VALUE *)args_; + return rb_funcall(args[0], id_call, 1, args[1]); +} - rb_funcallv(cb, id_call, 1, &ssl_obj); +/* This function may serve as the entry point to support further callbacks. */ +static void +ssl_info_cb(const SSL *ssl, int where, int val) +{ + if (ossl_cb_state_get()) + return; + if (where & SSL_CB_HANDSHAKE_START && SSL_is_server(ssl)) { + VALUE ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); + VALUE sslctx_obj = rb_attr_get(ssl_obj, id_i_context); + VALUE cb = rb_attr_get(sslctx_obj, id_i_renegotiation_cb); + if (!NIL_P(cb)) { + VALUE args[] = { cb, ssl_obj }; + int state; + rb_protect(call_renegotiation_cb, (VALUE)args, &state); + if (state) + ossl_cb_state_set(state); + } + } } static VALUE @@ -643,17 +657,6 @@ ssl_alpn_select_cb(SSL *ssl, const unsigned char **out, unsigned char *outlen, return ssl_npn_select_cb_common(ssl, cb, out, outlen, in, inlen); } -/* This function may serve as the entry point to support further callbacks. */ -static void -ssl_info_cb(const SSL *ssl, int where, int val) -{ - int is_server = SSL_is_server((SSL *)ssl); - - if (is_server && where & SSL_CB_HANDSHAKE_START) { - ssl_renegotiation_cb(ssl); - } -} - /* * call-seq: * ctx.options -> integer @@ -827,6 +830,9 @@ ossl_sslctx_setup(VALUE self) val = rb_attr_get(self, id_i_verify_depth); if(!NIL_P(val)) SSL_CTX_set_verify_depth(ctx, NUM2INT(val)); + if (!NIL_P(rb_attr_get(self, id_i_renegotiation_cb))) + SSL_CTX_set_info_callback(ctx, ssl_info_cb); + #ifdef OSSL_USE_NEXTPROTONEG val = rb_attr_get(self, id_i_npn_protocols); if (!NIL_P(val)) { @@ -1795,7 +1801,6 @@ ossl_ssl_initialize(int argc, VALUE *argv, VALUE self) if (!SSL_set_ex_data(ssl, ossl_ssl_ex_ptr_idx, (void *)self)) ossl_raise(eSSLError, "SSL_set_ex_data"); - SSL_set_info_callback(ssl, ssl_info_cb); rb_call_super(0, NULL); diff --git a/test/openssl/test_ssl.rb b/test/openssl/test_ssl.rb index 29d6eb7cd..65fe23c70 100644 --- a/test/openssl/test_ssl.rb +++ b/test/openssl/test_ssl.rb @@ -1727,6 +1727,26 @@ def test_renegotiation_cb ssl.puts "abc"; assert_equal "abc\n", ssl.gets } } + + client_can_close = Thread::Queue.new + server_proc = proc do |sock| + sctx = make_server_context + sctx.renegotiation_cb = -> ssl { raise "in renegotiation_cb" } + ssl = OpenSSL::SSL::SSLSocket.new(sock, sctx) + assert_raise_with_message(RuntimeError, "in renegotiation_cb") { + ssl.accept + } + ensure + client_can_close.close + end + start_server_proc(server_proc) do |port| + sock = TCPSocket.new("127.0.0.1", port) + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.connect_nonblock(exception: false) + client_can_close.pop + ensure + sock.close + end end def test_alpn_protocol_selection_ary From c4add568960e745e899e796eaaa8ac4ff00bb9d9 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Thu, 27 Nov 2025 02:55:20 +0900 Subject: [PATCH 06/11] ssl: harden client_cert_cb Currently, if the callback raises an exception or returns an invalid value, the handshake is resumed as if the callback has not been called. This is contrary to the documentation. Propagate exceptions raised by the callback to the originating method. Raise an exception if the callback returns an invalid value. It is immediately caught by rb_protect() and will similarly be re-raised in the originating method. --- ext/openssl/ossl_ssl.c | 70 ++++++++++++++++++++++++++++------------ test/openssl/test_ssl.rb | 28 ++++++++++++---- 2 files changed, 70 insertions(+), 28 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index 9e67e48a8..b0a57a4b5 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -110,40 +110,68 @@ ossl_sslctx_s_alloc(VALUE klass) return obj; } +struct client_cert_cb_args { + VALUE ssl_obj; + X509 **x509; + EVP_PKEY **pkey; +}; + static VALUE -ossl_call_client_cert_cb(VALUE obj) +ossl_call_client_cert_cb(VALUE args_) { + struct client_cert_cb_args *args = (struct client_cert_cb_args *)args_; VALUE ctx_obj, cb, ary, cert, key; + X509 *x509; + EVP_PKEY *pkey; - ctx_obj = rb_attr_get(obj, id_i_context); + ctx_obj = rb_attr_get(args->ssl_obj, id_i_context); cb = rb_attr_get(ctx_obj, id_i_client_cert_cb); if (NIL_P(cb)) return Qnil; - ary = rb_funcallv(cb, id_call, 1, &obj); - Check_Type(ary, T_ARRAY); - GetX509CertPtr(cert = rb_ary_entry(ary, 0)); - GetPrivPKeyPtr(key = rb_ary_entry(ary, 1)); + ary = rb_funcall(cb, id_call, 1, args->ssl_obj); + if (NIL_P(ary)) + return Qnil; + ary = rb_check_array_type(ary); + if (NIL_P(ary) || RARRAY_LEN(ary) != 2) { + rb_raise(rb_eTypeError, + "client_cert_cb must return either nil or [cert, key]"); + } + + x509 = GetX509CertPtr(cert = rb_ary_entry(ary, 0)); + pkey = GetPrivPKeyPtr(key = rb_ary_entry(ary, 1)); + if (!X509_check_private_key(x509, pkey)) + ossl_raise(eSSLError, "X509_check_private_key"); + if (!X509_up_ref(x509)) + ossl_raise(eSSLError, "X509_up_ref"); + if (!EVP_PKEY_up_ref(pkey)) { + X509_free(x509); + ossl_raise(eSSLError, "EVP_PKEY_up_ref"); + } + RB_GC_GUARD(cert); + RB_GC_GUARD(key); - return rb_ary_new3(2, cert, key); + *args->x509 = x509; + *args->pkey = pkey; + return Qtrue; } static int ossl_client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey) { - VALUE obj, ret; + VALUE obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); + struct client_cert_cb_args args = { obj, x509, pkey }; + VALUE ret; + int state; if (ossl_cb_state_get()) return -1; - obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); - ret = rb_protect(ossl_call_client_cert_cb, obj, NULL); - if (NIL_P(ret)) - return 0; - - *x509 = DupX509CertPtr(RARRAY_AREF(ret, 0)); - *pkey = DupPKeyPtr(RARRAY_AREF(ret, 1)); - - return 1; + ret = rb_protect(ossl_call_client_cert_cb, (VALUE)&args, &state); + if (state) { + ossl_cb_state_set(state); + return -1; + } + return RTEST(ret); } #if !defined(OPENSSL_NO_DH) @@ -2753,11 +2781,11 @@ Init_ossl_ssl(void) /* * A callback invoked when a client certificate is requested by a server - * and no certificate has been set. + * and no certificate has been set (see #add_certificate). * - * The callback is invoked with a Session and must return an Array - * containing an OpenSSL::X509::Certificate and an OpenSSL::PKey. If any - * other value is returned the handshake is suspended. + * The callback is invoked with the SSLSocket, and must return a + * two-element Array of an OpenSSL::X509::Certificate and the corresponding + * private key OpenSSL::PKey::PKey, or nil to send no certificate. */ rb_attr(cSSLContext, rb_intern_const("client_cert_cb"), 1, 1, Qfalse); diff --git a/test/openssl/test_ssl.rb b/test/openssl/test_ssl.rb index 65fe23c70..02fcb2416 100644 --- a/test/openssl/test_ssl.rb +++ b/test/openssl/test_ssl.rb @@ -565,20 +565,34 @@ def test_client_auth_success } end - def test_client_cert_cb_ignore_error + def test_client_cert_cb_error + omit "LibreSSL doesn't support client_cert_cb in TLS 1.3" if libressl? + sctx = make_server_context sctx.verify_mode = OpenSSL::SSL::VERIFY_PEER|OpenSSL::SSL::VERIFY_FAIL_IF_NO_PEER_CERT - start_server(sctx, ignore_listener_error: true) do |port| + # Bad return value ctx = OpenSSL::SSL::SSLContext.new ctx.client_cert_cb = -> ssl { - raise "exception in client_cert_cb must be suppressed" + assert_kind_of(OpenSSL::SSL::SSLSocket, ssl) + [@cli_cert, OpenSSL::PKey.read(@cli_key.public_to_der)] } - # 1. Exception in client_cert_cb is suppressed - # 2. No client certificate will be sent to the server - # 3. SSL_VERIFY_FAIL_IF_NO_PEER_CERT causes the handshake to fail - assert_raise(OpenSSL::SSL::SSLError) { + assert_raise_with_message(ArgumentError, /private key/) { + server_connect(port, ctx) { raise "unreachable" } + } + + # Exception in callback + ctx = OpenSSL::SSL::SSLContext.new + ctx.client_cert_cb = -> ssl { raise "in client_cert_cb" } + assert_raise_with_message(RuntimeError, /in client_cert_cb/) { + server_connect(port, ctx) { raise "unreachable" } + } + + # nil return, rejected due to OpenSSL::SSL::VERIFY_FAIL_IF_NO_PEER_CERT + ctx = OpenSSL::SSL::SSLContext.new + ctx.client_cert_cb = -> ssl { nil } + assert_raise_with_message(OpenSSL::SSL::SSLError, /alert number 116/) { server_connect(port, ctx) { |ssl| ssl.puts("abc"); ssl.gets } } end From 2d94755c86f20cda270b26b31b87d00e86ee690b Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Tue, 13 Oct 2026 02:06:46 +0900 Subject: [PATCH 07/11] ssl: harden session_{get,new,remove}_cb Prepare the Array objects for the argument inside rb_protect(). This object allocation may potentially raise an exception and corrupt OpenSSL's internal state. Also look up the callback Proc objects directly from SSLContext's instance variables to simplify. --- ext/openssl/ossl_ssl.c | 138 ++++++++++++++++++++--------------------- lib/openssl/ssl.rb | 12 ---- 2 files changed, 68 insertions(+), 82 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index b0a57a4b5..b84821c04 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -274,84 +274,90 @@ ossl_ssl_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) return ossl_verify_cb_call(cb, preverify_ok, ctx); } +struct sess_get_cb_args { + SSL *ssl; + const unsigned char *data; + int len; + SSL_SESSION *ret; +}; + static VALUE -ossl_call_session_get_cb(VALUE ary) +ossl_call_session_get_cb(VALUE args_) { - VALUE ssl_obj, cb; - - Check_Type(ary, T_ARRAY); - ssl_obj = rb_ary_entry(ary, 0); + struct sess_get_cb_args *args = (struct sess_get_cb_args *)args_; + VALUE ssl_obj, sslctx_obj, cb, session_id, ret; - cb = rb_funcall(ssl_obj, rb_intern("session_get_cb"), 0); + ssl_obj = (VALUE)SSL_get_ex_data(args->ssl, ossl_ssl_ex_ptr_idx); + sslctx_obj = rb_attr_get(ssl_obj, id_i_context); + cb = rb_attr_get(sslctx_obj, id_i_session_get_cb); if (NIL_P(cb)) return Qnil; - return rb_funcallv(cb, id_call, 1, &ary); + session_id = rb_str_new((const char *)args->data, args->len); + ret = rb_funcall(cb, id_call, 1, rb_assoc_new(ssl_obj, session_id)); + if (NIL_P(ret)) + return Qnil; + // XXX: Should we raise if ret is not a Session? + if (!rb_obj_is_instance_of(ret, cSSLSession)) + return Qnil; + + GetSSLSession(ret, args->ret); + return Qnil; } static SSL_SESSION * ossl_sslctx_session_get_cb(SSL *ssl, const unsigned char *buf, int len, int *copy) { - VALUE ary, ssl_obj, ret_obj; - SSL_SESSION *sess; - int state = 0; + struct sess_get_cb_args args = { ssl, buf, len, NULL }; + int state; if (ossl_cb_state_get()) return NULL; OSSL_Debug("SSL SESSION get callback entered"); - ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); - ary = rb_ary_new2(2); - rb_ary_push(ary, ssl_obj); - rb_ary_push(ary, rb_str_new((const char *)buf, len)); - - ret_obj = rb_protect(ossl_call_session_get_cb, ary, &state); + rb_protect(ossl_call_session_get_cb, (VALUE)&args, &state); if (state) { ossl_cb_state_set(state); return NULL; } - if (!rb_obj_is_instance_of(ret_obj, cSSLSession)) - return NULL; - - GetSSLSession(ret_obj, sess); *copy = 1; - - return sess; + return args.ret; } +struct sess_new_cb_args { + SSL *ssl; + SSL_SESSION *sess; +}; + static VALUE -ossl_call_session_new_cb(VALUE ary) +ossl_call_session_new_cb(VALUE args_) { - VALUE ssl_obj, cb; - - Check_Type(ary, T_ARRAY); - ssl_obj = rb_ary_entry(ary, 0); + struct sess_new_cb_args *args = (struct sess_new_cb_args *)args_; + VALUE ssl_obj, sslctx_obj, cb, sess_obj; - cb = rb_funcall(ssl_obj, rb_intern("session_new_cb"), 0); + ssl_obj = (VALUE)SSL_get_ex_data(args->ssl, ossl_ssl_ex_ptr_idx); + sslctx_obj = rb_attr_get(ssl_obj, id_i_context); + cb = rb_attr_get(sslctx_obj, id_i_session_new_cb); if (NIL_P(cb)) return Qnil; - return rb_funcallv(cb, id_call, 1, &ary); + sess_obj = rb_obj_alloc(cSSLSession); + if (!SSL_SESSION_up_ref(args->sess)) + ossl_raise(eSSLError, "SSL_SESSION_up_ref"); + RTYPEDDATA_DATA(sess_obj) = args->sess; + + rb_funcall(cb, id_call, 1, rb_assoc_new(ssl_obj, sess_obj)); + return Qnil; } /* return 1 normal. return 0 removes the session */ static int ossl_sslctx_session_new_cb(SSL *ssl, SSL_SESSION *sess) { - VALUE ary, ssl_obj, sess_obj; - int state = 0; + struct sess_new_cb_args args = { ssl, sess }; + int state; if (ossl_cb_state_get()) return 0; OSSL_Debug("SSL SESSION new callback entered"); - - ssl_obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); - sess_obj = rb_obj_alloc(cSSLSession); - SSL_SESSION_up_ref(sess); - DATA_PTR(sess_obj) = sess; - - ary = rb_ary_new2(2); - rb_ary_push(ary, ssl_obj); - rb_ary_push(ary, sess_obj); - - rb_protect(ossl_call_session_new_cb, ary, &state); + rb_protect(ossl_call_session_new_cb, (VALUE)&args, &state); if (state) { ossl_cb_state_set(state); } @@ -359,8 +365,8 @@ ossl_sslctx_session_new_cb(SSL *ssl, SSL_SESSION *sess) /* * return 0 which means to OpenSSL that the session is still * valid (since we created Ruby Session object) and was not freed by us - * with SSL_SESSION_free(). Call SSLContext#remove_session(sess) in - * session_get_cb block if you don't want OpenSSL to cache the session + * with SSL_SESSION_free(). Call SSLContext#session_remove(sess) in + * session_new_cb block if you don't want OpenSSL to cache the session * internally. */ return 0; @@ -416,25 +422,34 @@ ossl_sslctx_keylog_cb(const SSL *ssl, const char *line) } #endif +struct sess_remove_cb_args { + SSL_CTX *ctx; + SSL_SESSION *sess; +}; + static VALUE -ossl_call_session_remove_cb(VALUE ary) +ossl_call_session_remove_cb(VALUE args_) { - VALUE sslctx_obj, cb; - - Check_Type(ary, T_ARRAY); - sslctx_obj = rb_ary_entry(ary, 0); + struct sess_remove_cb_args *args = (struct sess_remove_cb_args *)args_; + VALUE sslctx_obj, cb, sess_obj; + sslctx_obj = (VALUE)SSL_CTX_get_ex_data(args->ctx, ossl_sslctx_ex_ptr_idx); cb = rb_attr_get(sslctx_obj, id_i_session_remove_cb); if (NIL_P(cb)) return Qnil; - return rb_funcallv(cb, id_call, 1, &ary); + sess_obj = rb_obj_alloc(cSSLSession); + if (!SSL_SESSION_up_ref(args->sess)) + ossl_raise(eSSLError, "SSL_SESSION_up_ref"); + RTYPEDDATA_DATA(sess_obj) = args->sess; + + rb_funcall(cb, id_call, 1, rb_assoc_new(sslctx_obj, sess_obj)); + return Qnil; } static void ossl_sslctx_session_remove_cb(SSL_CTX *ctx, SSL_SESSION *sess) { - VALUE ary, sslctx_obj, sess_obj; - int state = 0; + struct sess_remove_cb_args args = { ctx, sess }; /* * This callback is also called for all sessions in the internal store @@ -444,24 +459,7 @@ ossl_sslctx_session_remove_cb(SSL_CTX *ctx, SSL_SESSION *sess) return; OSSL_Debug("SSL SESSION remove callback entered"); - - sslctx_obj = (VALUE)SSL_CTX_get_ex_data(ctx, ossl_sslctx_ex_ptr_idx); - sess_obj = rb_obj_alloc(cSSLSession); - SSL_SESSION_up_ref(sess); - DATA_PTR(sess_obj) = sess; - - ary = rb_ary_new2(2); - rb_ary_push(ary, sslctx_obj); - rb_ary_push(ary, sess_obj); - - rb_protect(ossl_call_session_remove_cb, ary, &state); - if (state) { -/* - the SSL_CTX is frozen, nowhere to save state. - there is no common accessor method to check it either. - rb_ivar_set(sslctx_obj, ID_callback_state, INT2NUM(state)); -*/ - } + ossl_suppress(ossl_call_session_remove_cb, (VALUE)&args); } static VALUE diff --git a/lib/openssl/ssl.rb b/lib/openssl/ssl.rb index e50be00ff..e68c8c14b 100644 --- a/lib/openssl/ssl.rb +++ b/lib/openssl/ssl.rb @@ -609,18 +609,6 @@ def using_anon_cipher? ctx.ciphers.include?(cipher) end - def client_cert_cb - @context.client_cert_cb - end - - def session_new_cb - @context.session_new_cb - end - - def session_get_cb - @context.session_get_cb - end - class << self # call-seq: From 79b6a352da1418a7a2351b39988b4d7f9a786c7a Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Thu, 12 Nov 2026 22:28:24 +0900 Subject: [PATCH 08/11] squash! x509store: do not suppress exceptions in verify_callback USe Fiber-local storage instead because this may be used concurrently on TruffleRuby. --- ext/openssl/ossl.c | 44 +++++++++++--------------------------------- 1 file changed, 11 insertions(+), 33 deletions(-) diff --git a/ext/openssl/ossl.c b/ext/openssl/ossl.c index c7cc8af97..8bb2e7dd5 100644 --- a/ext/openssl/ossl.c +++ b/ext/openssl/ossl.c @@ -145,55 +145,34 @@ ossl_bin2hex(const unsigned char *in, char *out, size_t inlen) } } -#ifdef HAVE_RB_EXT_RACTOR_SAFE -static rb_ractor_local_key_t cb_state_key; +static ID id_cb_state; int ossl_cb_state_get(void) { - int *cell = rb_ractor_local_storage_ptr(cb_state_key); - return cell ? *cell : 0; + VALUE val = rb_thread_local_aref(rb_thread_current(), id_cb_state); + return FIXNUM_P(val) ? FIX2INT(val) : 0; } static VALUE -ensure_cell(VALUE unused) +cb_state_set_i(VALUE vstate) { - int *cell = rb_ractor_local_storage_ptr(cb_state_key); - if (!cell) { - cell = ZALLOC(int); - rb_ractor_local_storage_ptr_set(cb_state_key, cell); - } - return (VALUE)cell; + rb_thread_local_aset(rb_thread_current(), id_cb_state, vstate); + return Qnil; } void ossl_cb_state_set(int state) { - int *cell = (int *)rb_protect(ensure_cell, Qnil, NULL); - if (cell == (void *)Qnil) { + int s; + RUBY_ASSERT(!state || !ossl_cb_state_get()); + rb_protect(cb_state_set_i, state ? INT2FIX(state) : Qnil, &s); + if (s) { /* As a last resort */ rb_set_errinfo(Qnil); return; } - RUBY_ASSERT(!state || !*cell); - *cell = state; } -#else -static int cb_state; - -int -ossl_cb_state_get(void) -{ - return cb_state; -} - -void -ossl_cb_state_set(int state) -{ - RUBY_ASSERT(!state || !cb_state); - cb_state = state; -} -#endif void ossl_cb_state_begin(void) @@ -1124,8 +1103,6 @@ Init_openssl(void) { #ifdef HAVE_RB_EXT_RACTOR_SAFE rb_ext_ractor_safe(true); - cb_state_key = - rb_ractor_local_storage_ptr_newkey(RB_RACTOR_LOCAL_STORAGE_TYPE_FREE); #endif #undef rb_intern @@ -1258,6 +1235,7 @@ Init_openssl(void) */ ossl_s_to_der = rb_intern("to_der"); id_i_errors = rb_intern("@errors"); + id_cb_state = rb_intern("__openssl_cb_state__"); /* * Init components From 1b218079eec479e69b3ec635f79d1e5317e0d562 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Thu, 12 Nov 2026 22:42:11 +0900 Subject: [PATCH 09/11] fixup! ssl: use ossl_cb_state_*() for handling exceptions in callbacks --- ext/openssl/ossl_ssl.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index b84821c04..82548a4c6 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -1875,6 +1875,12 @@ ssl_call_begin(void) static int ssl_call_end(void) { +#ifdef _WIN32 + int saved_errno = rb_w32_map_errno(WSAGetLastError()); +#else + int saved_errno = errno; +#endif + int state = ossl_cb_state_end(); if (state) { /* must cleanup OpenSSL error stack before re-raising */ From debb2ecc56d304098dfbf82d10d618218f5227d1 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Thu, 12 Nov 2026 22:42:11 +0900 Subject: [PATCH 10/11] fixup! ssl: use ossl_cb_state_*() for handling exceptions in callbacks --- ext/openssl/ossl_ssl.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index 82548a4c6..3f8c447d5 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -1888,11 +1888,7 @@ ssl_call_end(void) rb_jump_tag(state); } -#ifdef _WIN32 - return rb_w32_map_errno(WSAGetLastError()); -#else - return errno; -#endif + return saved_errno; } static VALUE From 49451a27e270c22b09a3e8376e0b16367b90e319 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Thu, 12 Nov 2026 22:49:51 +0900 Subject: [PATCH 11/11] fixup! x509store: do not suppress exceptions in verify_callback --- test/openssl/test_pkcs7.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/openssl/test_pkcs7.rb b/test/openssl/test_pkcs7.rb index cfbd7c36f..8beae9662 100644 --- a/test/openssl/test_pkcs7.rb +++ b/test/openssl/test_pkcs7.rb @@ -62,8 +62,13 @@ def test_signed assert_in_delta(Time.now, signers[0].signed_time, 10) unless aws_lc? assert_false(p7.verify([@ca_cert], OpenSSL::X509::Store.new)) + end + + def test_signed_verify_callback + data = "aaaaa\r\nbbbbb\r\nccccc\r\n" + tmp = OpenSSL::PKCS7.sign(@ee1_cert, @ee1_key, data) - # Exceptions raised in verify_callback are re-raised + p7 = OpenSSL::PKCS7.new(tmp) store = OpenSSL::X509::Store.new store.verify_callback = proc { raise "verify_callback" } assert_raise_with_message(RuntimeError, "verify_callback") {