Skip to content

Commit cd7e51e

Browse files
miss-islingtonsethmlarsongpshead
authored
[3.14] gh-156293: Use-after-free for server-side SSLContext with sni_callback (GH-158504) (#158515)
Co-authored-by: Seth Larson <seth@python.org> Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
1 parent 06f875c commit cd7e51e

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
@@ -1721,6 +1721,12 @@ to speed up repeated connections from the same clients.
17211721
:class:`SSLContext` representing a certificate chain that matches the server
17221722
name.
17231723

1724+
If the callback assigns a new context to :attr:`SSLSocket.context`, any
1725+
further ClientHello message on the same connection (for example after a
1726+
TLS 1.3 HelloRetryRequest) is dispatched to the new context's
1727+
*sni_callback*, if it has one; the original callback is not called again
1728+
for that connection.
1729+
17241730
Due to the early negotiation phase of the TLS connection, only limited
17251731
methods and attributes are usable like
17261732
:meth:`SSLSocket.selected_alpn_protocol` and :attr:`SSLSocket.context`.
@@ -1745,6 +1751,11 @@ to speed up repeated connections from the same clients.
17451751

17461752
.. versionadded:: 3.7
17471753

1754+
.. versionchanged:: next
1755+
After the callback assigns a new :attr:`SSLSocket.context`, later
1756+
ClientHello messages on the connection are dispatched to the new
1757+
context's *sni_callback*.
1758+
17481759
.. method:: SSLContext.set_servername_callback(server_name_callback)
17491760

17501761
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
@@ -2096,6 +2096,86 @@ def test_unwrap(self):
20962096
c_in.write(s_out.read())
20972097
client.unwrap()
20982098

2099+
def test_sni_callback_context_released_and_callback_raises(self):
2100+
# Variant of the test below without a HelloRetryRequest: the callback
2101+
# switches the connection to another context, drops the last
2102+
# references to the context that carries it, and raises. The C
2103+
# callback must not touch that context after the Python callback
2104+
# returned.
2105+
client_ctx, server_ctx, hostname = testing_context()
2106+
leaf_ctx = server_ctx
2107+
2108+
def sni_cb(sslobj, server_name, ctx):
2109+
sslobj.context = leaf_ctx
2110+
del ctx
2111+
raise LookupError("no certificate for " + repr(server_name))
2112+
2113+
def make_server():
2114+
dispatch_ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
2115+
dispatch_ctx.load_cert_chain(SIGNED_CERTFILE)
2116+
dispatch_ctx.sni_callback = sni_cb
2117+
s_in, s_out = ssl.MemoryBIO(), ssl.MemoryBIO()
2118+
server = dispatch_ctx.wrap_bio(s_in, s_out, server_side=True)
2119+
return server, s_in, s_out
2120+
2121+
server, s_in, s_out = make_server()
2122+
c_in, c_out = ssl.MemoryBIO(), ssl.MemoryBIO()
2123+
client = client_ctx.wrap_bio(c_in, c_out, server_hostname=hostname)
2124+
with self.assertRaises(ssl.SSLWantReadError):
2125+
client.do_handshake()
2126+
s_in.write(c_out.read())
2127+
with support.catch_unraisable_exception() as cm:
2128+
with self.assertRaises(ssl.SSLError):
2129+
server.do_handshake()
2130+
self.assertIsInstance(cm.unraisable.exc_value, LookupError)
2131+
self.assertIs(server.context, leaf_ctx)
2132+
2133+
def test_sni_callback_context_released_before_second_client_hello(self):
2134+
# The SSLContext carrying sni_callback may be released by the
2135+
# application once the callback has switched the connection over to
2136+
# another context. If the server then sends a HelloRetryRequest, the
2137+
# second ClientHello makes OpenSSL consult the original SSL_CTX's
2138+
# servername callback again; that must not use the deallocated
2139+
# SSLContext object.
2140+
client_ctx, leaf_ctx, hostname = testing_context()
2141+
calls = []
2142+
2143+
def make_server():
2144+
dispatch_ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
2145+
dispatch_ctx.load_cert_chain(SIGNED_CERTFILE)
2146+
# Force a HelloRetryRequest: the client offers an X25519 key
2147+
# share first, the server only accepts P-384.
2148+
dispatch_ctx.set_ecdh_curve("secp384r1")
2149+
def sni_cb(sslobj, server_name, ctx):
2150+
calls.append(server_name)
2151+
sslobj.context = leaf_ctx
2152+
dispatch_ctx.sni_callback = sni_cb
2153+
s_in, s_out = ssl.MemoryBIO(), ssl.MemoryBIO()
2154+
server = dispatch_ctx.wrap_bio(s_in, s_out, server_side=True)
2155+
return server, s_in, s_out, weakref.ref(dispatch_ctx)
2156+
2157+
# After this only the C-level SSL object references dispatch_ctx.
2158+
server, s_in, s_out, dispatch_ref = make_server()
2159+
c_in, c_out = ssl.MemoryBIO(), ssl.MemoryBIO()
2160+
client = client_ctx.wrap_bio(c_in, c_out, server_hostname=hostname)
2161+
for _ in range(10):
2162+
for obj, out, peer_in in ((client, c_out, s_in),
2163+
(server, s_out, c_in)):
2164+
try:
2165+
obj.do_handshake()
2166+
except ssl.SSLWantReadError:
2167+
pass
2168+
if out.pending:
2169+
peer_in.write(out.read())
2170+
client.do_handshake()
2171+
server.do_handshake()
2172+
support.gc_collect()
2173+
self.assertIsNone(dispatch_ref())
2174+
self.assertGreaterEqual(len(calls), 1)
2175+
self.assertEqual(calls[0], hostname)
2176+
self.assertIs(server.context, leaf_ctx)
2177+
self.assertIsNotNone(client.cipher())
2178+
20992179
class SimpleBackgroundTests(unittest.TestCase):
21002180
"""Tests that connect to a simple server running in the background"""
21012181

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
@@ -3351,6 +3351,9 @@ context_dealloc(PyObject *op)
33513351
/* bpo-31095: UnTrack is needed before calling any callbacks */
33523352
PyObject_GC_UnTrack(self);
33533353
(void)context_clear(op);
3354+
/* The SSL_CTX may outlive this object as the session_ctx of sockets that
3355+
were switched to another context; leave no Python callback behind. */
3356+
SSL_CTX_set_tlsext_servername_callback(self->ctx, NULL);
33543357
SSL_CTX_free(self->ctx);
33553358
PyMem_FREE(self->alpn_protocols);
33563359
tp->tp_free(self);
@@ -4664,10 +4667,10 @@ _ssl__SSLContext_set_ecdh_curve_impl(PySSLContext *self, PyObject *name)
46644667
}
46654668

46664669
static int
4667-
_servername_callback(SSL *s, int *al, void *args)
4670+
_servername_callback(SSL *s, int *al, void *Py_UNUSED(args))
46684671
{
46694672
int ret;
4670-
PySSLContext *sslctx = (PySSLContext *) args;
4673+
PySSLContext *sslctx;
46714674
PySSLSocket *ssl;
46724675
PyObject *result;
46734676
/* The high-level ssl.SSLSocket object */
@@ -4676,18 +4679,30 @@ _servername_callback(SSL *s, int *al, void *args)
46764679
const char *servername = SSL_get_servername(s, TLSEXT_NAMETYPE_host_name);
46774680
PyGILState_STATE gstate = PyGILState_Ensure();
46784681

4682+
/* Do not use the SSL_CTX's servername arg to find the context: it is a
4683+
borrowed pointer to whichever _SSLContext installed the callback, and
4684+
that object may already be gone while OpenSSL still reaches this
4685+
callback through the connection's session_ctx (e.g. on the second
4686+
ClientHello after a HelloRetryRequest, once sni_callback has switched
4687+
the socket to another context). The socket's current context is
4688+
always alive. */
4689+
ssl = SSL_get_app_data(s);
4690+
assert(ssl != NULL);
4691+
Py_BEGIN_CRITICAL_SECTION(ssl);
4692+
sslctx = (PySSLContext *)Py_NewRef(ssl->ctx);
4693+
Py_END_CRITICAL_SECTION();
4694+
assert(Py_IS_TYPE(ssl, get_state_ctx(sslctx)->PySSLSocket_Type));
4695+
46794696
Py_BEGIN_CRITICAL_SECTION(sslctx);
46804697
sni_cb = Py_XNewRef(sslctx->set_sni_cb);
46814698
Py_END_CRITICAL_SECTION();
46824699

46834700
if (sni_cb == NULL) {
4701+
Py_DECREF(sslctx);
46844702
PyGILState_Release(gstate);
46854703
return SSL_TLSEXT_ERR_OK;
46864704
}
46874705

4688-
ssl = SSL_get_app_data(s);
4689-
assert(Py_IS_TYPE(ssl, get_state_ctx(sslctx)->PySSLSocket_Type));
4690-
46914706
/* The servername callback expects an argument that represents the current
46924707
* SSL connection and that has a .context attribute that can be changed to
46934708
* identify the requested hostname. Since the official API is the Python
@@ -4770,12 +4785,14 @@ _servername_callback(SSL *s, int *al, void *args)
47704785
}
47714786

47724787
Py_DECREF(sni_cb);
4788+
Py_DECREF(sslctx);
47734789
PyGILState_Release(gstate);
47744790
return ret;
47754791

47764792
error:
47774793
Py_XDECREF(ssl_socket);
47784794
Py_XDECREF(sni_cb);
4795+
Py_DECREF(sslctx);
47794796
*al = SSL_AD_INTERNAL_ERROR;
47804797
ret = SSL_TLSEXT_ERR_ALERT_FATAL;
47814798
PyGILState_Release(gstate);
@@ -4832,7 +4849,6 @@ _ssl__SSLContext_sni_callback_set_impl(PySSLContext *self, PyObject *value)
48324849
}
48334850
else {
48344851
Py_XSETREF(self->set_sni_cb, Py_NewRef(value));
4835-
SSL_CTX_set_tlsext_servername_arg(self->ctx, self);
48364852
SSL_CTX_set_tlsext_servername_callback(self->ctx, _servername_callback);
48374853
}
48384854
return 0;

0 commit comments

Comments
 (0)