Skip to content

Commit 402015d

Browse files
sethmlarsongpshead
andcommitted
gh-158446: Use-after-free for server-side SSLContext with sni_callback
Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
1 parent 069c74a commit 402015d

4 files changed

Lines changed: 120 additions & 6 deletions

File tree

‎Doc/library/ssl.rst‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1859,6 +1859,12 @@ to speed up repeated connections from the same clients.
18591859
:class:`SSLContext` representing a certificate chain that matches the server
18601860
name.
18611861

1862+
If the callback assigns a new context to :attr:`SSLSocket.context`, any
1863+
further ClientHello message on the same connection (for example after a
1864+
TLS 1.3 HelloRetryRequest) is dispatched to the new context's
1865+
*sni_callback*, if it has one; the original callback is not called again
1866+
for that connection.
1867+
18621868
Due to the early negotiation phase of the TLS connection, only limited
18631869
methods and attributes are usable like
18641870
:meth:`SSLSocket.selected_alpn_protocol` and :attr:`SSLSocket.context`.
@@ -1883,6 +1889,11 @@ to speed up repeated connections from the same clients.
18831889

18841890
.. versionadded:: 3.7
18851891

1892+
.. versionchanged:: next
1893+
After the callback assigns a new :attr:`SSLSocket.context`, later
1894+
ClientHello messages on the connection are dispatched to the new
1895+
context's *sni_callback*.
1896+
18861897
.. method:: SSLContext.set_servername_callback(server_name_callback)
18871898

18881899
This is a legacy API retained for backwards compatibility. When possible,

‎Lib/test/test_ssl.py‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2179,6 +2179,86 @@ def test_unwrap(self):
21792179
c_in.write(s_out.read())
21802180
client.unwrap()
21812181

2182+
def test_sni_callback_context_released_and_callback_raises(self):
2183+
# Variant of the test below without a HelloRetryRequest: the callback
2184+
# switches the connection to another context, drops the last
2185+
# references to the context that carries it, and raises. The C
2186+
# callback must not touch that context after the Python callback
2187+
# returned.
2188+
client_ctx, server_ctx, hostname = testing_context()
2189+
leaf_ctx = server_ctx
2190+
2191+
def sni_cb(sslobj, server_name, ctx):
2192+
sslobj.context = leaf_ctx
2193+
del ctx
2194+
raise LookupError("no certificate for " + repr(server_name))
2195+
2196+
def make_server():
2197+
dispatch_ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
2198+
dispatch_ctx.load_cert_chain(SIGNED_CERTFILE)
2199+
dispatch_ctx.sni_callback = sni_cb
2200+
s_in, s_out = ssl.MemoryBIO(), ssl.MemoryBIO()
2201+
server = dispatch_ctx.wrap_bio(s_in, s_out, server_side=True)
2202+
return server, s_in, s_out
2203+
2204+
server, s_in, s_out = make_server()
2205+
c_in, c_out = ssl.MemoryBIO(), ssl.MemoryBIO()
2206+
client = client_ctx.wrap_bio(c_in, c_out, server_hostname=hostname)
2207+
with self.assertRaises(ssl.SSLWantReadError):
2208+
client.do_handshake()
2209+
s_in.write(c_out.read())
2210+
with support.catch_unraisable_exception() as cm:
2211+
with self.assertRaises(ssl.SSLError):
2212+
server.do_handshake()
2213+
self.assertIsInstance(cm.unraisable.exc_value, LookupError)
2214+
self.assertIs(server.context, leaf_ctx)
2215+
2216+
def test_sni_callback_context_released_before_second_client_hello(self):
2217+
# The SSLContext carrying sni_callback may be released by the
2218+
# application once the callback has switched the connection over to
2219+
# another context. If the server then sends a HelloRetryRequest, the
2220+
# second ClientHello makes OpenSSL consult the original SSL_CTX's
2221+
# servername callback again; that must not use the deallocated
2222+
# SSLContext object.
2223+
client_ctx, leaf_ctx, hostname = testing_context()
2224+
calls = []
2225+
2226+
def make_server():
2227+
dispatch_ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
2228+
dispatch_ctx.load_cert_chain(SIGNED_CERTFILE)
2229+
# Force a HelloRetryRequest: the client offers an X25519 key
2230+
# share first, the server only accepts P-384.
2231+
dispatch_ctx.set_ecdh_curve("secp384r1")
2232+
def sni_cb(sslobj, server_name, ctx):
2233+
calls.append(server_name)
2234+
sslobj.context = leaf_ctx
2235+
dispatch_ctx.sni_callback = sni_cb
2236+
s_in, s_out = ssl.MemoryBIO(), ssl.MemoryBIO()
2237+
server = dispatch_ctx.wrap_bio(s_in, s_out, server_side=True)
2238+
return server, s_in, s_out, weakref.ref(dispatch_ctx)
2239+
2240+
# After this only the C-level SSL object references dispatch_ctx.
2241+
server, s_in, s_out, dispatch_ref = make_server()
2242+
c_in, c_out = ssl.MemoryBIO(), ssl.MemoryBIO()
2243+
client = client_ctx.wrap_bio(c_in, c_out, server_hostname=hostname)
2244+
for _ in range(10):
2245+
for obj, out, peer_in in ((client, c_out, s_in),
2246+
(server, s_out, c_in)):
2247+
try:
2248+
obj.do_handshake()
2249+
except ssl.SSLWantReadError:
2250+
pass
2251+
if out.pending:
2252+
peer_in.write(out.read())
2253+
client.do_handshake()
2254+
server.do_handshake()
2255+
support.gc_collect()
2256+
self.assertIsNone(dispatch_ref())
2257+
self.assertGreaterEqual(len(calls), 1)
2258+
self.assertEqual(calls[0], hostname)
2259+
self.assertIs(server.context, leaf_ctx)
2260+
self.assertIsNotNone(client.cipher())
2261+
21822262
class SimpleBackgroundTests(unittest.TestCase):
21832263
"""Tests that connect to a simple server running in the background"""
21842264

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Fix a crash in :mod:`ssl` when an :attr:`~ssl.SSLContext.sni_callback`
2+
switches a connection to another :class:`~ssl.SSLContext` and the context
3+
that carries the callback is no longer referenced by the application.
4+
Servers that keep their ``sni_callback`` context alive (the usual case when
5+
it wraps the listening socket or is stored on the server object) were not
6+
affected.
7+
This addresses :cve:`2026-19445`.

‎Modules/_ssl.c‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3647,6 +3647,9 @@ context_dealloc(PyObject *op)
36473647
/* bpo-31095: UnTrack is needed before calling any callbacks */
36483648
PyObject_GC_UnTrack(self);
36493649
(void)context_clear(op);
3650+
/* The SSL_CTX may outlive this object as the session_ctx of sockets that
3651+
were switched to another context; leave no Python callback behind. */
3652+
SSL_CTX_set_tlsext_servername_callback(self->ctx, NULL);
36503653
SSL_CTX_free(self->ctx);
36513654
PyMem_FREE(self->alpn_protocols);
36523655
tp->tp_free(self);
@@ -5119,10 +5122,10 @@ _ssl__SSLContext_set_ecdh_curve_impl(PySSLContext *self, PyObject *name)
51195122
}
51205123

51215124
static int
5122-
_servername_callback(SSL *s, int *al, void *args)
5125+
_servername_callback(SSL *s, int *al, void *Py_UNUSED(args))
51235126
{
51245127
int ret;
5125-
PySSLContext *sslctx = (PySSLContext *) args;
5128+
PySSLContext *sslctx;
51265129
PySSLSocket *ssl;
51275130
PyObject *result;
51285131
/* The high-level ssl.SSLSocket object */
@@ -5131,18 +5134,30 @@ _servername_callback(SSL *s, int *al, void *args)
51315134
const char *servername = SSL_get_servername(s, TLSEXT_NAMETYPE_host_name);
51325135
PyGILState_STATE gstate = PyGILState_Ensure();
51335136

5137+
/* Do not use the SSL_CTX's servername arg to find the context: it is a
5138+
borrowed pointer to whichever _SSLContext installed the callback, and
5139+
that object may already be gone while OpenSSL still reaches this
5140+
callback through the connection's session_ctx (e.g. on the second
5141+
ClientHello after a HelloRetryRequest, once sni_callback has switched
5142+
the socket to another context). The socket's current context is
5143+
always alive. */
5144+
ssl = SSL_get_app_data(s);
5145+
assert(ssl != NULL);
5146+
Py_BEGIN_CRITICAL_SECTION(ssl);
5147+
sslctx = (PySSLContext *)Py_NewRef(ssl->ctx);
5148+
Py_END_CRITICAL_SECTION();
5149+
assert(Py_IS_TYPE(ssl, get_state_ctx(sslctx)->PySSLSocket_Type));
5150+
51345151
Py_BEGIN_CRITICAL_SECTION(sslctx);
51355152
sni_cb = Py_XNewRef(sslctx->set_sni_cb);
51365153
Py_END_CRITICAL_SECTION();
51375154

51385155
if (sni_cb == NULL) {
5156+
Py_DECREF(sslctx);
51395157
PyGILState_Release(gstate);
51405158
return SSL_TLSEXT_ERR_OK;
51415159
}
51425160

5143-
ssl = SSL_get_app_data(s);
5144-
assert(Py_IS_TYPE(ssl, get_state_ctx(sslctx)->PySSLSocket_Type));
5145-
51465161
/* The servername callback expects an argument that represents the current
51475162
* SSL connection and that has a .context attribute that can be changed to
51485163
* identify the requested hostname. Since the official API is the Python
@@ -5225,12 +5240,14 @@ _servername_callback(SSL *s, int *al, void *args)
52255240
}
52265241

52275242
Py_DECREF(sni_cb);
5243+
Py_DECREF(sslctx);
52285244
PyGILState_Release(gstate);
52295245
return ret;
52305246

52315247
error:
52325248
Py_XDECREF(ssl_socket);
52335249
Py_XDECREF(sni_cb);
5250+
Py_DECREF(sslctx);
52345251
*al = SSL_AD_INTERNAL_ERROR;
52355252
ret = SSL_TLSEXT_ERR_ALERT_FATAL;
52365253
PyGILState_Release(gstate);
@@ -5289,7 +5306,6 @@ _ssl__SSLContext_sni_callback_set_impl(PySSLContext *self, PyObject *value)
52895306
}
52905307
else {
52915308
Py_XSETREF(self->set_sni_cb, Py_NewRef(value));
5292-
SSL_CTX_set_tlsext_servername_arg(self->ctx, self);
52935309
SSL_CTX_set_tlsext_servername_callback(self->ctx, _servername_callback);
52945310
}
52955311
return 0;

0 commit comments

Comments
 (0)