From 59e9a8002392fa9ba6b16fb954bc90f297caf4e1 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Fri, 1 Aug 2025 18:39:41 +0900 Subject: [PATCH 1/5] provider: load "default" provider in test_openssl_legacy_provider Explicitly load both the "default" and "legacy" providers in the test case. The "legacy" provider is intended to be used with the "default" provider. The "default" provider is typically loaded automatically, but that only happens when no other provider has been loaded yet. --- test/openssl/test_provider.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/openssl/test_provider.rb b/test/openssl/test_provider.rb index 6f85c00c9..90c7771db 100644 --- a/test/openssl/test_provider.rb +++ b/test/openssl/test_provider.rb @@ -46,9 +46,10 @@ def test_openssl_legacy_provider with_openssl(<<-'end;') begin + OpenSSL::Provider.load("default") OpenSSL::Provider.load("legacy") rescue OpenSSL::Provider::ProviderError - omit "Only for OpenSSL with legacy provider" + omit "Providers 'default' and 'legacy' are required for this test" end algo = "RC4" From 6e8d3d9e8c51ef308626ce65a4b6078bb189a37a Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sun, 20 Apr 2025 19:24:27 +0900 Subject: [PATCH 2/5] ssl: fix extconf.rb check for SSL_CTX_set0_tmp_dh_pkey() Check for the function we actually use. Both SSL_set0_tmp_dh_pkey() and SSL_CTX_set0_tmp_dh_pkey() were added in OpenSSL 3.0. --- ext/openssl/extconf.rb | 2 +- ext/openssl/ossl_ssl.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/openssl/extconf.rb b/ext/openssl/extconf.rb index 8aac52ef4..6c178c12f 100644 --- a/ext/openssl/extconf.rb +++ b/ext/openssl/extconf.rb @@ -147,7 +147,7 @@ def find_openssl_library have_func("EVP_PKEY_check(NULL)", evp_h) # added in 3.0.0 -have_func("SSL_set0_tmp_dh_pkey(NULL, NULL)", ssl_h) +have_func("SSL_CTX_set0_tmp_dh_pkey(NULL, NULL)", ssl_h) have_func("ERR_get_error_all(NULL, NULL, NULL, NULL, NULL)", "openssl/err.h") have_func("SSL_CTX_load_verify_file(NULL, \"\")", ssl_h) have_func("BN_check_prime(NULL, NULL, NULL)", "openssl/bn.h") diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index 29564a813..9e34bd252 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -1148,7 +1148,7 @@ ossl_sslctx_set_client_sigalgs(VALUE self, VALUE v) * contained in the key object, if any, are ignored. The server will always * generate a new key pair for each handshake. * - * Added in version 3.0. See also the man page SSL_set0_tmp_dh_pkey(3). + * Added in version 3.0. See also the man page SSL_CTX_set0_tmp_dh_pkey(3). * * Example: * ctx = OpenSSL::SSL::SSLContext.new @@ -1169,7 +1169,7 @@ ossl_sslctx_set_tmp_dh(VALUE self, VALUE arg) if (EVP_PKEY_base_id(pkey) != EVP_PKEY_DH) rb_raise(eSSLError, "invalid pkey type %s (expected DH)", OBJ_nid2sn(EVP_PKEY_base_id(pkey))); -#ifdef HAVE_SSL_SET0_TMP_DH_PKEY +#ifdef HAVE_SSL_CTX_SET0_TMP_DH_PKEY if (!SSL_CTX_set0_tmp_dh_pkey(ctx, pkey)) ossl_raise(eSSLError, "SSL_CTX_set0_tmp_dh_pkey"); EVP_PKEY_up_ref(pkey); From a6cf6fe8e19f681c87b927c656c8d78e993f42ac Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sun, 20 Apr 2025 20:26:00 +0900 Subject: [PATCH 3/5] ssl: use SSL_CTX_set_dh_auto() by default Enable automatic DH parameters for TLS 1.2 or earlier when neither SSLSocket#tmp_dh nor SSLSocket#tmp_dh_callback is set. This is supported since OpenSSL 1.1.0. This allows us to remove the default tmp_dh_callback proc defined in lib/openssl/ssl.rb, which uses hard-coded parameters and is not Ractor-shareable. --- ext/openssl/ossl_ssl.c | 12 ++++++++++-- lib/openssl/ssl.rb | 21 +-------------------- test/openssl/test_ssl.rb | 6 +----- 3 files changed, 12 insertions(+), 27 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index 9e34bd252..c7b9cb74a 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -47,7 +47,7 @@ static ID id_i_cert_store, id_i_ca_file, id_i_ca_path, id_i_verify_mode, id_i_session_id_context, id_i_session_get_cb, id_i_session_new_cb, id_i_session_remove_cb, id_i_npn_select_cb, id_i_npn_protocols, id_i_alpn_select_cb, id_i_alpn_protocols, id_i_servername_cb, - id_i_verify_hostname, id_i_keylog_cb; + id_i_verify_hostname, id_i_keylog_cb, id_i_tmp_dh_callback; static ID id_i_io, id_i_context, id_i_hostname; static int ossl_ssl_ex_ptr_idx; @@ -90,6 +90,7 @@ ossl_sslctx_s_alloc(VALUE klass) ossl_raise(eSSLError, "SSL_CTX_new"); } SSL_CTX_set_mode(ctx, mode); + SSL_CTX_set_dh_auto(ctx, 1); RTYPEDDATA_DATA(obj) = ctx; SSL_CTX_set_ex_data(ctx, ossl_sslctx_ex_ptr_idx, (void *)obj); @@ -703,7 +704,10 @@ ossl_sslctx_setup(VALUE self) GetSSLCTX(self, ctx); #if !defined(OPENSSL_NO_DH) - SSL_CTX_set_tmp_dh_callback(ctx, ossl_tmp_dh_callback); + if (!NIL_P(rb_attr_get(self, id_i_tmp_dh_callback))) { + SSL_CTX_set_tmp_dh_callback(ctx, ossl_tmp_dh_callback); + SSL_CTX_set_dh_auto(ctx, 0); + } #endif #if !defined(OPENSSL_IS_AWSLC) /* AWS-LC has no support for TLS 1.3 PHA. */ @@ -1178,6 +1182,9 @@ ossl_sslctx_set_tmp_dh(VALUE self, VALUE arg) ossl_raise(eSSLError, "SSL_CTX_set_tmp_dh"); #endif + // Turn off the "auto" DH parameters set by ossl_sslctx_s_alloc() + SSL_CTX_set_dh_auto(ctx, 0); + return arg; } #endif @@ -3289,6 +3296,7 @@ Init_ossl_ssl(void) DefIVarID(servername_cb); DefIVarID(verify_hostname); DefIVarID(keylog_cb); + DefIVarID(tmp_dh_callback); DefIVarID(io); DefIVarID(context); diff --git a/lib/openssl/ssl.rb b/lib/openssl/ssl.rb index a0ad5dc3a..bb6c4bcf1 100644 --- a/lib/openssl/ssl.rb +++ b/lib/openssl/ssl.rb @@ -32,25 +32,6 @@ class SSLContext }.call } - if defined?(OpenSSL::PKey::DH) - DH_ffdhe2048 = OpenSSL::PKey::DH.new <<-_end_of_pem_ ------BEGIN DH PARAMETERS----- -MIIBCAKCAQEA//////////+t+FRYortKmq/cViAnPTzx2LnFg84tNpWp4TZBFGQz -+8yTnc4kmz75fS/jY2MMddj2gbICrsRhetPfHtXV/WVhJDP1H18GbtCFY2VVPe0a -87VXE15/V8k1mE8McODmi3fipona8+/och3xWKE2rec1MKzKT0g6eXq8CrGCsyT7 -YdEIqUuyyOP7uWrat2DX9GgdT0Kj3jlN9K5W7edjcrsZCwenyO4KbXCeAvzhzffi -7MA0BM0oNC9hkXL+nOmFg/+OTxIy7vKBg8P+OxtMb61zO7X8vC7CIAXFjvGDfRaD -ssbzSibBsu/6iGtCOGEoXJf//////////wIBAg== ------END DH PARAMETERS----- - _end_of_pem_ - private_constant :DH_ffdhe2048 - - DEFAULT_TMP_DH_CALLBACK = lambda { |ctx, is_export, keylen| # :nodoc: - warn "using default DH parameters." if $VERBOSE - DH_ffdhe2048 - } - end - if !OpenSSL::OPENSSL_VERSION.start_with?("OpenSSL") DEFAULT_PARAMS.merge!( min_version: OpenSSL::SSL::TLS1_VERSION, @@ -458,7 +439,7 @@ def client_cert_cb end def tmp_dh_callback - @context.tmp_dh_callback || OpenSSL::SSL::SSLContext::DEFAULT_TMP_DH_CALLBACK + @context.tmp_dh_callback end def session_new_cb diff --git a/test/openssl/test_ssl.rb b/test/openssl/test_ssl.rb index fc19f4d94..86a4d4aaa 100644 --- a/test/openssl/test_ssl.rb +++ b/test/openssl/test_ssl.rb @@ -2122,11 +2122,7 @@ def test_connect_works_when_setting_dh_callback_to_nil ctx.tmp_dh_callback = nil } start_server(ctx_proc: ctx_proc) do |port| - EnvUtil.suppress_warning { # uses default callback - assert_nothing_raised { - server_connect(port) { } - } - } + assert_nothing_raised { server_connect(port) } end end From db341736fc20b0285ad725245fdedcb215ab7a75 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sun, 20 Apr 2025 22:28:04 +0900 Subject: [PATCH 4/5] ssl: refactor tmp_dh_callback handling tmp_dh_callback no longer has a default value. It also no longer has to share code with tmp_ecdh_callback, which was removed in v3.0.0. --- ext/openssl/ossl_ssl.c | 68 +++++++++++++++++++++--------------------- lib/openssl/ssl.rb | 17 ----------- 2 files changed, 34 insertions(+), 51 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index c7b9cb74a..7304cee7c 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_tmp_dh_callback, - id_npn_protocols_encoded, id_each; +static ID id_call, ID_callback_state, id_npn_protocols_encoded, id_each; static VALUE sym_exception, sym_wait_readable, sym_wait_writable; static ID id_i_cert_store, id_i_ca_file, id_i_ca_path, id_i_verify_mode, @@ -134,8 +133,6 @@ ossl_client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey) #if !defined(OPENSSL_NO_DH) struct tmp_dh_callback_args { VALUE ssl_obj; - ID id; - int type; int is_export; int keylength; }; @@ -144,48 +141,35 @@ static VALUE ossl_call_tmp_dh_callback(VALUE arg) { struct tmp_dh_callback_args *args = (struct tmp_dh_callback_args *)arg; - VALUE cb, dh; - EVP_PKEY *pkey; + VALUE ctx_obj, cb, obj; + const DH *dh; - cb = rb_funcall(args->ssl_obj, args->id, 0); + ctx_obj = rb_attr_get(args->ssl_obj, id_i_context); + cb = rb_attr_get(ctx_obj, id_i_tmp_dh_callback); if (NIL_P(cb)) - return (VALUE)NULL; - dh = rb_funcall(cb, id_call, 3, args->ssl_obj, INT2NUM(args->is_export), - INT2NUM(args->keylength)); - pkey = GetPKeyPtr(dh); - if (EVP_PKEY_base_id(pkey) != args->type) - return (VALUE)NULL; + return (VALUE)NULL; + + obj = rb_funcall(cb, id_call, 3, args->ssl_obj, INT2NUM(args->is_export), + INT2NUM(args->keylength)); + dh = EVP_PKEY_get0_DH(GetPKeyPtr(obj)); + if (!dh) + ossl_clear_error(); - return (VALUE)pkey; + return (VALUE)dh; } -#endif -#if !defined(OPENSSL_NO_DH) static DH * ossl_tmp_dh_callback(SSL *ssl, int is_export, int keylength) { - VALUE rb_ssl; - EVP_PKEY *pkey; - struct tmp_dh_callback_args args; int state; - - rb_ssl = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx); - args.ssl_obj = rb_ssl; - args.id = id_tmp_dh_callback; - args.is_export = is_export; - args.keylength = keylength; - args.type = EVP_PKEY_DH; - - pkey = (EVP_PKEY *)rb_protect(ossl_call_tmp_dh_callback, - (VALUE)&args, &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); if (state) { rb_ivar_set(rb_ssl, ID_callback_state, INT2NUM(state)); return NULL; } - if (!pkey) - return NULL; - - return (DH *)EVP_PKEY_get0_DH(pkey); + return (DH *)ret; } #endif /* OPENSSL_NO_DH */ @@ -2872,6 +2856,23 @@ Init_ossl_ssl(void) */ rb_attr(cSSLContext, rb_intern_const("client_cert_cb"), 1, 1, Qfalse); +#ifndef OPENSSL_NO_DH + /* + * A callback invoked when DH parameters are required for ephemeral DH key + * exchange. + * + * The callback is invoked with the SSLSocket, a + * flag indicating the use of an export cipher and the keylength + * required. + * + * The callback must return an OpenSSL::PKey::DH instance of the correct + * key length. + * + * Deprecated in version 3.0. Use #tmp_dh= instead. + */ + rb_attr(cSSLContext, rb_intern_const("tmp_dh_callback"), 1, 1, Qfalse); +#endif + /* * Sets the context in which a session can be reused. This allows * sessions for multiple applications to be distinguished, for example, by @@ -3265,7 +3266,6 @@ Init_ossl_ssl(void) sym_wait_readable = ID2SYM(rb_intern_const("wait_readable")); sym_wait_writable = ID2SYM(rb_intern_const("wait_writable")); - id_tmp_dh_callback = rb_intern_const("tmp_dh_callback"); id_npn_protocols_encoded = rb_intern_const("npn_protocols_encoded"); id_each = rb_intern_const("each"); diff --git a/lib/openssl/ssl.rb b/lib/openssl/ssl.rb index bb6c4bcf1..f13613fd7 100644 --- a/lib/openssl/ssl.rb +++ b/lib/openssl/ssl.rb @@ -74,19 +74,6 @@ class SSLContext DEFAULT_CERT_STORE.set_default_paths DEFAULT_CERT_STORE.flags = OpenSSL::X509::V_FLAG_CRL_CHECK_ALL - # A callback invoked when DH parameters are required for ephemeral DH key - # exchange. - # - # The callback is invoked with the SSLSocket, a - # flag indicating the use of an export cipher and the keylength - # required. - # - # The callback must return an OpenSSL::PKey::DH instance of the correct - # key length. - # - # Deprecated in version 3.0. Use #tmp_dh= instead. - attr_accessor :tmp_dh_callback - # A callback invoked at connect time to distinguish between multiple # server names. # @@ -438,10 +425,6 @@ def client_cert_cb @context.client_cert_cb end - def tmp_dh_callback - @context.tmp_dh_callback - end - def session_new_cb @context.session_new_cb end From c05b300c6e8c6f8538c3bd6047a29670284050de Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sat, 2 Aug 2025 00:48:38 +0900 Subject: [PATCH 5/5] ssl: allow SSLContext#set_params to be used from non-main Ractors Freeze OpenSSL::SSL::SSLContext::DEFAULT_PARAMS to make it Ractor-shareable. When called from a non-main Ractor, SSLContext#set_params now prepares a new OpenSSL::X509::Store in Ractor-local storage. --- lib/openssl/ssl.rb | 11 ++++++++++- test/openssl/test_ssl.rb | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/lib/openssl/ssl.rb b/lib/openssl/ssl.rb index f13613fd7..c2ecd0c5c 100644 --- a/lib/openssl/ssl.rb +++ b/lib/openssl/ssl.rb @@ -69,6 +69,7 @@ class SSLContext }.join(":"), ) end + DEFAULT_PARAMS.freeze DEFAULT_CERT_STORE = OpenSSL::X509::Store.new # :nodoc: DEFAULT_CERT_STORE.set_default_paths @@ -115,7 +116,15 @@ def set_params(params={}) params.each{|name, value| self.__send__("#{name}=", value) } if self.verify_mode != OpenSSL::SSL::VERIFY_NONE unless self.ca_file or self.ca_path or self.cert_store - self.cert_store = DEFAULT_CERT_STORE + if not defined?(Ractor) or Ractor.current == Ractor.main + self.cert_store = DEFAULT_CERT_STORE + else + self.cert_store = Ractor.current[:__openssl_ssl_default_cert_store__] ||= + OpenSSL::X509::Store.new.tap { |store| + store.set_default_paths + store.flags = OpenSSL::X509::V_FLAG_CRL_CHECK_ALL + } + end end end return params diff --git a/test/openssl/test_ssl.rb b/test/openssl/test_ssl.rb index 86a4d4aaa..ee1033285 100644 --- a/test/openssl/test_ssl.rb +++ b/test/openssl/test_ssl.rb @@ -2308,6 +2308,40 @@ def test_export_keying_material end end + # $/ must be accessible from non-main Ractors + # https://bugs.ruby-lang.org/issues/21109 + if respond_to?(:ractor) && RUBY_VERSION >= "3.5" + ractor + def test_ractor_client + start_server { |port| + s = Ractor.new(port) { |port| + sock = TCPSocket.new("127.0.0.1", port) + begin + ssl = OpenSSL::SSL::SSLSocket.new(sock) + ssl.connect + ssl.puts("abc") + ssl.gets + ensure + ssl.close + sock.close + end + }.value + assert_equal("abc\n", s) + } + end + + ractor + def test_ractor_set_params + # Cannot test the default store here, but it should not raise an exception + ok = Ractor.new { + ctx = OpenSSL::SSL::SSLContext.new + ctx.set_params + ctx.cert_store.kind_of?(OpenSSL::X509::Store) + }.value + assert(ok, "ctx.cert_store is an instance of OpenSSL::X509::Store") + end + end + private def server_connect(port, ctx = nil)