Skip to content

Refactor callbacks in OpenSSL::SSL - #1080

Open
rhenium wants to merge 5 commits into
ruby:masterfrom
rhenium:ky/ssl-callback-errors
Open

rhenium wants to merge 5 commits into
ruby:masterfrom
rhenium:ky/ssl-callback-errors

Conversation

@rhenium

@rhenium rhenium commented Jul 5, 2026

Copy link
Copy Markdown
Member

Extracted from #736


ssl: add struct ossl_ssl_data

We will need to store additional internal states in SSLSocket in the upcoming patches. Prepare a new struct for such purposes and store it in SSL's ex_data.

Refactor existing internal states that are local to C to use this new struct:

  • The back reference to the Ruby object, currently in another ex_data
  • The callback jump tag, currently in an instance variable

ssl: do not call another callback when an exception is pending

Do not attempt to execute Ruby callback when another callback has already raised an exception and we are waiting for a chance to call rb_jump_tag().

As far as I know, this currently cannot happen. However, an upcoming patch to use custom BIO_METHOD in SSLSocket may make it possible. Let's handle it explicitly.


ssl: refactor info_callback

Set the callback to SSL_CTX instead of SSL so that we do not have to configure every SSL object, similar to all other existing callbacks.

Also, wrap rb_funcall() in rb_protect() to prevent exiting from SSL_accept() prematurely.


ssl: re-raise exceptions in client_cert_cb

Re-raise exceptions raised inside client_cert_cb from the originating Ruby method, which can be any of #connect, #sysread*, or #syswrite*.


ssl: simplify looking up callback procs for session_{get,new}_cb

Look up the callback procs by checking the corresponding instance variables on SSLContext directly. The removed methods SSLSocket#session_new_cb and #session_get_cb are private and have not been part of the public interface of ruby/openssl.

Also remove the unused method SSLSocket#client_cert_cb.


ssl: refactor session_{get,new,remove}_cb

Prepare the argument array object inside rb_protect(). The callback proc is already called in it, but the array allocation can also potentially fail and needs to be done there.

@rhenium
rhenium force-pushed the ky/ssl-callback-errors branch 2 times, most recently from a05a89f to 5ae433b Compare August 31, 2026 12:32
Simplify handling of the temporary jump tag. 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.
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.
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.
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.

If the callback returns an invalid value, raise TypeError. It is
immediately caught by rb_protect() and will be re-raised in the
originating method in the same way.
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.
@rhenium
rhenium force-pushed the ky/ssl-callback-errors branch from 5ae433b to 11bf938 Compare October 13, 2026 09:58

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant