Skip to content

ssl: allow SSLContext#set_params to be used from non-main Ractors #925

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion ext/openssl/extconf.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
84 changes: 46 additions & 38 deletions ext/openssl/ossl_ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -47,7 +46,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;
Expand Down Expand Up @@ -90,6 +89,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);

Expand Down Expand Up @@ -133,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;
};
Expand All @@ -143,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 */

Expand Down Expand Up @@ -703,7 +688,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. */
Expand Down Expand Up @@ -1148,7 +1136,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
Expand All @@ -1169,7 +1157,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);
Expand All @@ -1178,6 +1166,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
Expand Down Expand Up @@ -2865,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.
*
* <b>Deprecated in version 3.0.</b> 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
Expand Down Expand Up @@ -3258,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");

Expand Down Expand Up @@ -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);
Expand Down
47 changes: 10 additions & 37 deletions lib/openssl/ssl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -88,24 +69,12 @@ class SSLContext
}.join(":"),
)
end
DEFAULT_PARAMS.freeze

DEFAULT_CERT_STORE = OpenSSL::X509::Store.new # :nodoc:
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.
#
# <b>Deprecated in version 3.0.</b> Use #tmp_dh= instead.
attr_accessor :tmp_dh_callback

# A callback invoked at connect time to distinguish between multiple
# server names.
#
Expand Down Expand Up @@ -147,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
Expand Down Expand Up @@ -457,10 +434,6 @@ def client_cert_cb
@context.client_cert_cb
end

def tmp_dh_callback
@context.tmp_dh_callback || OpenSSL::SSL::SSLContext::DEFAULT_TMP_DH_CALLBACK
end

def session_new_cb
@context.session_new_cb
end
Expand Down
3 changes: 2 additions & 1 deletion test/openssl/test_provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
40 changes: 35 additions & 5 deletions test/openssl/test_ssl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -2312,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)
Expand Down
Loading