Skip to content

Fix callback exception handling to be per-Connection instead of per-Context #1535

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.

Sign up for GitHub

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

Merged
alex merged 3 commits into from
Sep 6, 2026
Merged

Fix callback exception handling to be per-Connection instead of per-Context #1535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
32f8a93
Track callback exceptions per Connection instead of per helper
claude Sep 6, 2026
26ab7eb
Clear the OpenSSL error queue directly before raising a callback exce…
claude Sep 6, 2026
b89f8b3
Credit the reporter in the changelog
claude Sep 6, 2026
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
16 changes: 16 additions & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,22 @@ Changelog
Versions are year-based with a strict backward-compatibility policy.
The third digit is only for regressions.

26.4.1 (UNRELEASED)
-------------------

Backward-incompatible changes:
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Deprecations:
^^^^^^^^^^^^^

Changes:
^^^^^^^^

- Fixed a race in which an exception raised by a verify, ALPN selection, OCSP, or DTLS cookie callback for one ``Connection`` could be raised on an unrelated ``Connection`` created from the same ``Context`` and used concurrently from another thread. Exceptions from these callbacks are now tracked per ``Connection``. Discovered and reported by SecDim Security Research.
- Fixed exceptions raised by a verify callback registered with ``Connection.set_verify`` being swallowed instead of being propagated to the caller.


26.4.0 (2026-08-01)
-------------------

Expand Down
117 changes: 47 additions & 70 deletions src/OpenSSL/SSL.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,43 +466,13 @@ class SysCallError(Error):
pass


class _CallbackExceptionHelper:
"""
A base class for wrapper classes that allow for intelligent exception
handling in OpenSSL callbacks.

:ivar list _problems: Any exceptions that occurred while executing in a
context where they could not be raised in the normal way. Typically
this is because OpenSSL has called into some Python code and requires a
return value. The exceptions are saved to be raised later when it is
possible to do so.
"""

def __init__(self) -> None:
self._problems: list[Exception] = []

def raise_if_problem(self) -> None:
"""
Raise an exception from the OpenSSL error queue or that was previously
captured whe running a callback.
"""
if self._problems:
try:
_raise_current_error()
except Error:
pass
raise self._problems.pop(0)


class _VerifyHelper(_CallbackExceptionHelper):
class _VerifyHelper:
"""
Wrap a callback such that it can be used as a certificate verification
callback.
"""

def __init__(self, callback: _VerifyCallback) -> None:
_CallbackExceptionHelper.__init__(self)

@wraps(callback)
def wrapper(ok, store_ctx): # type: ignore[no-untyped-def]
x509 = _lib.X509_STORE_CTX_get_current_cert(store_ctx)
Expand All @@ -520,7 +490,7 @@ def wrapper(ok, store_ctx): # type: ignore[no-untyped-def]
connection, cert, error_number, error_depth, ok
)
except Exception as e:
self._problems.append(e)
connection._callback_problems.append(e)
return 0
else:
if result:
Expand All @@ -534,19 +504,16 @@ def wrapper(ok, store_ctx): # type: ignore[no-untyped-def]
)


class _ALPNSelectHelper(_CallbackExceptionHelper):
class _ALPNSelectHelper:
"""
Wrap a callback such that it can be used as an ALPN selection callback.
"""

def __init__(self, callback: _ALPNSelectCallback) -> None:
_CallbackExceptionHelper.__init__(self)

@wraps(callback)
def wrapper(ssl, out, outlen, in_, inlen, arg): # type: ignore[no-untyped-def]
conn = Connection._reverse_mapping[ssl]
try:
conn = Connection._reverse_mapping[ssl]

# The string passed to us is made up of multiple
# length-prefixed bytestrings. We need to split that into a
# list.
Expand Down Expand Up @@ -583,19 +550,26 @@ def wrapper(ssl, out, outlen, in_, inlen, arg): # type: ignore[no-untyped-def]
return _lib.SSL_TLSEXT_ERR_NOACK
return _lib.SSL_TLSEXT_ERR_OK
except Exception as e:
self._problems.append(e)
conn._callback_problems.append(e)
return _lib.SSL_TLSEXT_ERR_ALERT_FATAL

# If an exception ever escapes ``wrapper`` (it should not: the only
# statement outside its ``try`` is the connection lookup, which cannot
# fail for an SSL object owned by a Connection), cffi prints it and
# returns this value instead of the default 0, which would be
# SSL_TLSEXT_ERR_OK and make OpenSSL read the never-assigned ``out``
# and ``outlen`` parameters.
self.callback = _ffi.callback(
(
"int (*)(SSL *, unsigned char **, unsigned char *, "
"const unsigned char *, unsigned int, void *)"
),
wrapper,
error=_lib.SSL_TLSEXT_ERR_ALERT_FATAL,
)


class _OCSPServerCallbackHelper(_CallbackExceptionHelper):
class _OCSPServerCallbackHelper:
"""
Wrap a callback such that it can be used as an OCSP callback for the server
side.
Expand All @@ -618,13 +592,10 @@ class _OCSPServerCallbackHelper(_CallbackExceptionHelper):
"""

def __init__(self, callback: _OCSPServerCallback[Any]) -> None:
_CallbackExceptionHelper.__init__(self)

@wraps(callback)
def wrapper(ssl, cdata): # type: ignore[no-untyped-def]
conn = Connection._reverse_mapping[ssl]
try:
conn = Connection._reverse_mapping[ssl]

# Extract the data if any was provided.
if cdata != _ffi.NULL:
data = _ffi.from_handle(cdata)
Expand Down Expand Up @@ -656,13 +627,13 @@ def wrapper(ssl, cdata): # type: ignore[no-untyped-def]

return 0
except Exception as e:
self._problems.append(e)
conn._callback_problems.append(e)
return 2 # SSL_TLSEXT_ERR_ALERT_FATAL

self.callback = _ffi.callback("int (*)(SSL *, void *)", wrapper)


class _OCSPClientCallbackHelper(_CallbackExceptionHelper):
class _OCSPClientCallbackHelper:
"""
Wrap a callback such that it can be used as an OCSP callback for the client
side.
Expand All @@ -685,13 +656,10 @@ class _OCSPClientCallbackHelper(_CallbackExceptionHelper):
"""

def __init__(self, callback: _OCSPClientCallback[Any]) -> None:
_CallbackExceptionHelper.__init__(self)

@wraps(callback)
def wrapper(ssl, cdata): # type: ignore[no-untyped-def]
conn = Connection._reverse_mapping[ssl]
try:
conn = Connection._reverse_mapping[ssl]

# Extract the data if any was provided.
if cdata != _ffi.NULL:
data = _ffi.from_handle(cdata)
Expand All @@ -714,23 +682,21 @@ def wrapper(ssl, cdata): # type: ignore[no-untyped-def]
return int(bool(valid))

except Exception as e:
self._problems.append(e)
conn._callback_problems.append(e)
# Return negative value if an exception is hit.
return -1

self.callback = _ffi.callback("int (*)(SSL *, void *)", wrapper)


class _CookieGenerateCallbackHelper(_CallbackExceptionHelper):
class _CookieGenerateCallbackHelper:
def __init__(self, callback: _CookieGenerateCallback) -> None:
_CallbackExceptionHelper.__init__(self)

max_cookie_len = getattr(_lib, "DTLS1_COOKIE_LENGTH", 255)

@wraps(callback)
def wrapper(ssl, out, outlen): # type: ignore[no-untyped-def]
conn = Connection._reverse_mapping[ssl]
try:
conn = Connection._reverse_mapping[ssl]
cookie = callback(conn)
if len(cookie) > max_cookie_len:
raise ValueError(
Expand All @@ -741,7 +707,7 @@ def wrapper(ssl, out, outlen): # type: ignore[no-untyped-def]
outlen[0] = len(cookie)
return 1
except Exception as e:
self._problems.append(e)
conn._callback_problems.append(e)
# "a zero return value can be used to abort the handshake"
return 0

Expand All @@ -751,17 +717,15 @@ def wrapper(ssl, out, outlen): # type: ignore[no-untyped-def]
)


class _CookieVerifyCallbackHelper(_CallbackExceptionHelper):
class _CookieVerifyCallbackHelper:
def __init__(self, callback: _CookieVerifyCallback) -> None:
_CallbackExceptionHelper.__init__(self)

@wraps(callback)
def wrapper(ssl, c_cookie, cookie_len): # type: ignore[no-untyped-def]
conn = Connection._reverse_mapping[ssl]
try:
conn = Connection._reverse_mapping[ssl]
return callback(conn, bytes(c_cookie[0:cookie_len]))
except Exception as e:
self._problems.append(e)
conn._callback_problems.append(e)
return 0

self.callback = _ffi.callback(
Expand Down Expand Up @@ -2003,6 +1967,16 @@ def __init__(
# avoid them getting freed.
self._alpn_select_callback_args: Any = None

# Exceptions raised by callbacks (verify, ALPN selection, OCSP, DTLS
# cookies) while OpenSSL was calling into Python on behalf of this
# connection. They cannot be raised from inside the callback, so they
# are recorded here and raised by the next method that OpenSSL
# returns control to. They are kept on the connection rather than on
# the (context-wide, shared) callback wrapper so that a connection
# driven concurrently from another thread can never be handed an
# exception that belongs to a different connection.
self._callback_problems: list[Exception] = []

# Reference the verify_callback of the Context. This ensures that if
# set_verify is called again after the SSL object has been created we
# do not point to a dangling reference
Expand Down Expand Up @@ -2046,13 +2020,19 @@ def __getattr__(self, name: str) -> Any:
else:
return getattr(self._socket, name)

def _raise_callback_problem(self) -> None:
"""
Raise an exception that was captured while running a callback on
behalf of this connection, if there is one, after discarding whatever
OpenSSL left on its error queue as a consequence of the callback
failing.
"""
if self._callback_problems:
_lib.ERR_clear_error()
raise self._callback_problems.pop(0)

def _raise_ssl_error(self, ssl: Any, result: int) -> None:
if self._context._verify_helper is not None:
self._context._verify_helper.raise_if_problem()
if self._context._alpn_select_helper is not None:
self._context._alpn_select_helper.raise_if_problem()
if self._context._ocsp_helper is not None:
self._context._ocsp_helper.raise_if_problem()
self._raise_callback_problem()

error = _lib.SSL_get_error(ssl, result)
if error == _lib.SSL_ERROR_WANT_READ:
Expand Down Expand Up @@ -2561,10 +2541,7 @@ def DTLSv1_listen(self) -> None:
# ClientHello with valid cookie, but keep trying'. So basically
# WantReadError. But it doesn't work correctly with _raise_ssl_error.
# So we raise it manually instead.
if self._cookie_generate_helper is not None:
self._cookie_generate_helper.raise_if_problem()
if self._cookie_verify_helper is not None:
self._cookie_verify_helper.raise_if_problem()
self._raise_callback_problem()
if result == 0:
raise WantReadError()
if result < 0:
Expand Down
66 changes: 66 additions & 0 deletions tests/test_ssl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2399,6 +2399,46 @@ def select(conn: Connection, options: list[bytes]) -> bytes:
interact_in_memory(server, client)
assert select_args == [(server, [b"http/1.1", b"spdy/2"])]

def test_alpn_callback_exception_stays_with_its_connection(self) -> None:
"""
An exception raised by the ALPN select callback while serving one
connection is only ever raised on that connection, even though every
connection created from the same `Context` shares the callback
wrapper. Otherwise, with connections driven from several threads, an
unrelated connection could be handed another connection's exception.
"""

def select(conn: Connection, options: list[bytes]) -> bytes:
return options[0]

server_context = Context(SSLv23_METHOD)
server_context.set_alpn_select_callback(select)
server_context.use_privatekey(
load_privatekey(FILETYPE_PEM, server_key_pem)
)
server_context.use_certificate(
load_certificate(FILETYPE_PEM, server_cert_pem)
)

client_context = Context(SSLv23_METHOD)
client_context.set_alpn_protos([b"http/1.1"])

failing = Connection(server_context, None)
other = Connection(server_context, None)
client = Connection(client_context, None)

# Simulate the callback having raised on ``failing`` in another
# thread that has not yet had the chance to surface the exception.
failing._callback_problems.append(ValueError("belongs to failing"))

# The unrelated connection is unaffected...
handshake_in_memory(client, other)
assert other.get_alpn_proto_negotiated() == b"http/1.1"

# ...and the exception is still delivered to the right connection.
with pytest.raises(ValueError, match="belongs to failing"):
failing.do_handshake()


class TestSession:
"""
Expand Down Expand Up @@ -3012,6 +3052,32 @@ def test_set_verify_overrides_context(self) -> None:
with pytest.raises(TypeError):
conn.set_verify(VERIFY_PEER, "not a callable") # type: ignore[arg-type]

def test_set_verify_callback_exception(self) -> None:
"""
If the verify callback passed to `Connection.set_verify` raises an
exception, verification fails and the exception is propagated to the
caller of `Connection.do_handshake`.
"""
server_context = Context(SSLv23_METHOD)
server_context.use_privatekey(
load_privatekey(FILETYPE_PEM, root_key_pem)
)
server_context.use_certificate(
load_certificate(FILETYPE_PEM, root_cert_pem)
)
server = Connection(server_context, None)

def verify_callback(
conn: Connection, cert: X509, err: int, depth: int, ok: int
) -> bool:
raise KeyError("silly verify failure")

client = Connection(Context(SSLv23_METHOD), None)
client.set_verify(VERIFY_PEER, verify_callback)

with pytest.raises(KeyError, match="silly verify failure"):
handshake_in_memory(client, server)

def test_set_verify_callback_reference(self) -> None:
"""
The callback for certificate verification should only be forgotten if
Expand Down