Skip to content

Commit 24db78c

Browse files
authored
gh-146080: fix a crash in SNI callbacks when the SSL object is gone (#146573)
1 parent aa66807 commit 24db78c

File tree

3 files changed

+63
-2
lines changed

3 files changed

+63
-2
lines changed

Lib/test/test_ssl.py

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# Test the support for SSL and sockets
22

3+
import contextlib
34
import sys
45
import unittest
56
import unittest.mock
@@ -47,12 +48,13 @@
4748

4849
PROTOCOLS = sorted(ssl._PROTOCOL_NAMES)
4950
HOST = socket_helper.HOST
51+
IS_AWS_LC = "AWS-LC" in ssl.OPENSSL_VERSION
5052
IS_OPENSSL_3_0_0 = ssl.OPENSSL_VERSION_INFO >= (3, 0, 0)
5153
CAN_GET_SELECTED_OPENSSL_GROUP = ssl.OPENSSL_VERSION_INFO >= (3, 2)
5254
CAN_IGNORE_UNKNOWN_OPENSSL_GROUPS = ssl.OPENSSL_VERSION_INFO >= (3, 3)
5355
CAN_GET_AVAILABLE_OPENSSL_GROUPS = ssl.OPENSSL_VERSION_INFO >= (3, 5)
5456
CAN_GET_AVAILABLE_OPENSSL_SIGALGS = ssl.OPENSSL_VERSION_INFO >= (3, 4)
55-
CAN_SET_CLIENT_SIGALGS = "AWS-LC" not in ssl.OPENSSL_VERSION
57+
CAN_SET_CLIENT_SIGALGS = not IS_AWS_LC
5658
CAN_IGNORE_UNKNOWN_OPENSSL_SIGALGS = ssl.OPENSSL_VERSION_INFO >= (3, 3)
5759
CAN_GET_SELECTED_OPENSSL_SIGALG = ssl.OPENSSL_VERSION_INFO >= (3, 5)
5860
PY_SSL_DEFAULT_CIPHERS = sysconfig.get_config_var('PY_SSL_DEFAULT_CIPHERS')
@@ -383,6 +385,20 @@ def testing_context(server_cert=SIGNED_CERTFILE, *, server_chain=True,
383385
return client_context, server_context, hostname
384386

385387

388+
def do_ssl_object_handshake(sslobject, outgoing, max_retry=25):
389+
"""Call do_handshake() on the sslobject and return the sent data.
390+
391+
If do_handshake() fails more than *max_retry* times, return None.
392+
"""
393+
data, attempt = None, 0
394+
while not data and attempt < max_retry:
395+
with contextlib.suppress(ssl.SSLWantReadError):
396+
sslobject.do_handshake()
397+
data = outgoing.read()
398+
attempt += 1
399+
return data
400+
401+
386402
class BasicSocketTests(unittest.TestCase):
387403

388404
def test_constants(self):
@@ -1535,6 +1551,49 @@ def dummycallback(sock, servername, ctx):
15351551
ctx.set_servername_callback(None)
15361552
ctx.set_servername_callback(dummycallback)
15371553

1554+
def test_sni_callback_on_dead_references(self):
1555+
# See https://github.com/python/cpython/issues/146080.
1556+
c_ctx = make_test_context()
1557+
c_inc, c_out = ssl.MemoryBIO(), ssl.MemoryBIO()
1558+
client = c_ctx.wrap_bio(c_inc, c_out, server_hostname=SIGNED_CERTFILE_HOSTNAME)
1559+
1560+
def sni_callback(sock, servername, ctx): pass
1561+
sni_callback = unittest.mock.Mock(wraps=sni_callback)
1562+
s_ctx = make_test_context(server_side=True, certfile=SIGNED_CERTFILE)
1563+
s_ctx.set_servername_callback(sni_callback)
1564+
1565+
s_inc, s_out = ssl.MemoryBIO(), ssl.MemoryBIO()
1566+
server = s_ctx.wrap_bio(s_inc, s_out, server_side=True)
1567+
server_impl = server._sslobj
1568+
1569+
# Perform the handshake on the client side first.
1570+
data = do_ssl_object_handshake(client, c_out)
1571+
sni_callback.assert_not_called()
1572+
if data is None:
1573+
self.skipTest("cannot establish a handshake from the client")
1574+
s_inc.write(data)
1575+
sni_callback.assert_not_called()
1576+
# Delete the server object before it starts doing its handshake
1577+
# and ensure that we did not call the SNI callback yet.
1578+
del server
1579+
gc.collect()
1580+
# Try to continue the server's handshake by directly using
1581+
# the internal SSL object. The latter is a weak reference
1582+
# stored in the server context and has now a dead owner.
1583+
with self.assertRaises(ssl.SSLError) as cm:
1584+
server_impl.do_handshake()
1585+
# The SNI C callback raised an exception before calling our callback.
1586+
sni_callback.assert_not_called()
1587+
1588+
# In AWS-LC, any handshake failures reports SSL_R_PARSE_TLSEXT,
1589+
# while OpenSSL uses SSL_R_CALLBACK_FAILED on SNI callback failures.
1590+
if IS_AWS_LC:
1591+
libssl_error_reason = "PARSE_TLSEXT"
1592+
else:
1593+
libssl_error_reason = "callback failed"
1594+
self.assertIn(libssl_error_reason, str(cm.exception))
1595+
self.assertEqual(cm.exception.errno, ssl.SSL_ERROR_SSL)
1596+
15381597
def test_sni_callback_refcycle(self):
15391598
# Reference cycles through the servername callback are detected
15401599
# and cleared.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
:mod:`ssl`: fix a crash when an SNI callback tries to use an SSL object that
2+
has already been garbage-collected. Patch by Bénédikt Tran.

Modules/_ssl.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5205,7 +5205,7 @@ _servername_callback(SSL *s, int *al, void *args)
52055205
return ret;
52065206

52075207
error:
5208-
Py_DECREF(ssl_socket);
5208+
Py_XDECREF(ssl_socket);
52095209
*al = SSL_AD_INTERNAL_ERROR;
52105210
ret = SSL_TLSEXT_ERR_ALERT_FATAL;
52115211
PyGILState_Release(gstate);

0 commit comments

Comments
 (0)