Skip to content

Commit 63fab14

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

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

1850+
If the callback assigns a new context to :attr:`SSLSocket.context`, any
1851+
further ClientHello message on the same connection (for example after a
1852+
TLS 1.3 HelloRetryRequest) is dispatched to the new context's
1853+
*sni_callback*, if it has one; the original callback is not called again
1854+
for that connection.
1855+
18501856
Due to the early negotiation phase of the TLS connection, only limited
18511857
methods and attributes are usable like
18521858
:meth:`SSLSocket.selected_alpn_protocol` and :attr:`SSLSocket.context`.
@@ -1871,6 +1877,11 @@ to speed up repeated connections from the same clients.
18711877

18721878
.. versionadded:: 3.7
18731879

1880+
.. versionchanged:: next
1881+
After the callback assigns a new :attr:`SSLSocket.context`, later
1882+
ClientHello messages on the connection are dispatched to the new
1883+
context's *sni_callback*.
1884+
18741885
.. method:: SSLContext.set_servername_callback(server_name_callback)
18751886

18761887
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
@@ -2116,6 +2116,86 @@ def test_unwrap(self):
21162116
c_in.write(s_out.read())
21172117
client.unwrap()
21182118

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

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
@@ -3674,6 +3674,9 @@ context_dealloc(PyObject *op)
36743674
/* bpo-31095: UnTrack is needed before calling any callbacks */
36753675
PyObject_GC_UnTrack(self);
36763676
(void)context_clear(op);
3677+
/* The SSL_CTX may outlive this object as the session_ctx of sockets that
3678+
were switched to another context; leave no Python callback behind. */
3679+
SSL_CTX_set_tlsext_servername_callback(self->ctx, NULL);
36773680
SSL_CTX_free(self->ctx);
36783681
PyMem_FREE(self->alpn_protocols);
36793682
tp->tp_free(self);
@@ -5154,10 +5157,10 @@ _ssl__SSLContext_set_ecdh_curve_impl(PySSLContext *self, PyObject *name)
51545157
}
51555158

51565159
static int
5157-
_servername_callback(SSL *s, int *al, void *args)
5160+
_servername_callback(SSL *s, int *al, void *Py_UNUSED(args))
51585161
{
51595162
int ret;
5160-
PySSLContext *sslctx = (PySSLContext *) args;
5163+
PySSLContext *sslctx;
51615164
PySSLSocket *ssl;
51625165
PyObject *result;
51635166
/* The high-level ssl.SSLSocket object */
@@ -5166,18 +5169,30 @@ _servername_callback(SSL *s, int *al, void *args)
51665169
const char *servername = SSL_get_servername(s, TLSEXT_NAMETYPE_host_name);
51675170
PyGILState_STATE gstate = PyGILState_Ensure();
51685171

5172+
/* Do not use the SSL_CTX's servername arg to find the context: it is a
5173+
borrowed pointer to whichever _SSLContext installed the callback, and
5174+
that object may already be gone while OpenSSL still reaches this
5175+
callback through the connection's session_ctx (e.g. on the second
5176+
ClientHello after a HelloRetryRequest, once sni_callback has switched
5177+
the socket to another context). The socket's current context is
5178+
always alive. */
5179+
ssl = SSL_get_app_data(s);
5180+
assert(ssl != NULL);
5181+
Py_BEGIN_CRITICAL_SECTION(ssl);
5182+
sslctx = (PySSLContext *)Py_NewRef(ssl->ctx);
5183+
Py_END_CRITICAL_SECTION();
5184+
assert(Py_IS_TYPE(ssl, get_state_ctx(sslctx)->PySSLSocket_Type));
5185+
51695186
Py_BEGIN_CRITICAL_SECTION(sslctx);
51705187
sni_cb = Py_XNewRef(sslctx->set_sni_cb);
51715188
Py_END_CRITICAL_SECTION();
51725189

51735190
if (sni_cb == NULL) {
5191+
Py_DECREF(sslctx);
51745192
PyGILState_Release(gstate);
51755193
return SSL_TLSEXT_ERR_OK;
51765194
}
51775195

5178-
ssl = SSL_get_app_data(s);
5179-
assert(Py_IS_TYPE(ssl, get_state_ctx(sslctx)->PySSLSocket_Type));
5180-
51815196
/* The servername callback expects an argument that represents the current
51825197
* SSL connection and that has a .context attribute that can be changed to
51835198
* identify the requested hostname. Since the official API is the Python
@@ -5260,12 +5275,14 @@ _servername_callback(SSL *s, int *al, void *args)
52605275
}
52615276

52625277
Py_DECREF(sni_cb);
5278+
Py_DECREF(sslctx);
52635279
PyGILState_Release(gstate);
52645280
return ret;
52655281

52665282
error:
52675283
Py_XDECREF(ssl_socket);
52685284
Py_XDECREF(sni_cb);
5285+
Py_DECREF(sslctx);
52695286
*al = SSL_AD_INTERNAL_ERROR;
52705287
ret = SSL_TLSEXT_ERR_ALERT_FATAL;
52715288
PyGILState_Release(gstate);
@@ -5324,7 +5341,6 @@ _ssl__SSLContext_sni_callback_set_impl(PySSLContext *self, PyObject *value)
53245341
}
53255342
else {
53265343
Py_XSETREF(self->set_sni_cb, Py_NewRef(value));
5327-
SSL_CTX_set_tlsext_servername_arg(self->ctx, self);
53285344
SSL_CTX_set_tlsext_servername_callback(self->ctx, _servername_callback);
53295345
}
53305346
return 0;

0 commit comments

Comments
 (0)