Skip to content

Commit 3a2a686

Browse files
authored
gh-142516: fix reference leaks in ssl.SSLContext objects (#143685)
1 parent 7b0bd9e commit 3a2a686

File tree

3 files changed

+111
-22
lines changed

3 files changed

+111
-22
lines changed

Lib/test/test_ssl.py

Lines changed: 90 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,16 @@
5757
CAN_GET_SELECTED_OPENSSL_SIGALG = ssl.OPENSSL_VERSION_INFO >= (3, 5)
5858
PY_SSL_DEFAULT_CIPHERS = sysconfig.get_config_var('PY_SSL_DEFAULT_CIPHERS')
5959

60+
HAS_KEYLOG = hasattr(ssl.SSLContext, 'keylog_filename')
61+
requires_keylog = unittest.skipUnless(
62+
HAS_KEYLOG, 'test requires OpenSSL 1.1.1 with keylog callback')
63+
CAN_SET_KEYLOG = HAS_KEYLOG and os.name != "nt"
64+
requires_keylog_setter = unittest.skipUnless(
65+
CAN_SET_KEYLOG,
66+
"cannot set 'keylog_filename' on Windows"
67+
)
68+
69+
6070
PROTOCOL_TO_TLS_VERSION = {}
6171
for proto, ver in (
6272
("PROTOCOL_SSLv3", "SSLv3"),
@@ -266,34 +276,69 @@ def utc_offset(): #NOTE: ignore issues like #1647654
266276
)
267277

268278

269-
def test_wrap_socket(sock, *,
270-
cert_reqs=ssl.CERT_NONE, ca_certs=None,
271-
ciphers=None, ciphersuites=None,
272-
min_version=None, max_version=None,
273-
certfile=None, keyfile=None,
274-
**kwargs):
275-
if not kwargs.get("server_side"):
276-
kwargs["server_hostname"] = SIGNED_CERTFILE_HOSTNAME
277-
context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
278-
else:
279+
def make_test_context(
280+
*,
281+
server_side=False,
282+
check_hostname=None,
283+
cert_reqs=ssl.CERT_NONE,
284+
ca_certs=None, certfile=None, keyfile=None,
285+
ciphers=None, ciphersuites=None,
286+
min_version=None, max_version=None,
287+
):
288+
if server_side:
279289
context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
280-
if cert_reqs is not None:
290+
else:
291+
context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
292+
293+
if check_hostname is None:
281294
if cert_reqs == ssl.CERT_NONE:
282295
context.check_hostname = False
296+
else:
297+
context.check_hostname = check_hostname
298+
299+
if cert_reqs is not None:
283300
context.verify_mode = cert_reqs
301+
284302
if ca_certs is not None:
285303
context.load_verify_locations(ca_certs)
286304
if certfile is not None or keyfile is not None:
287305
context.load_cert_chain(certfile, keyfile)
306+
288307
if ciphers is not None:
289308
context.set_ciphers(ciphers)
290309
if ciphersuites is not None:
291310
context.set_ciphersuites(ciphersuites)
311+
292312
if min_version is not None:
293313
context.minimum_version = min_version
294314
if max_version is not None:
295315
context.maximum_version = max_version
296-
return context.wrap_socket(sock, **kwargs)
316+
317+
return context
318+
319+
320+
def test_wrap_socket(
321+
sock,
322+
*,
323+
server_side=False,
324+
check_hostname=None,
325+
cert_reqs=ssl.CERT_NONE,
326+
ca_certs=None, certfile=None, keyfile=None,
327+
ciphers=None, ciphersuites=None,
328+
min_version=None, max_version=None,
329+
**kwargs,
330+
):
331+
context = make_test_context(
332+
server_side=server_side,
333+
check_hostname=check_hostname,
334+
cert_reqs=cert_reqs,
335+
ca_certs=ca_certs, certfile=certfile, keyfile=keyfile,
336+
ciphers=ciphers, ciphersuites=ciphersuites,
337+
min_version=min_version, max_version=max_version,
338+
)
339+
if not server_side:
340+
kwargs.setdefault("server_hostname", SIGNED_CERTFILE_HOSTNAME)
341+
return context.wrap_socket(sock, server_side=server_side, **kwargs)
297342

298343

299344
USE_SAME_TEST_CONTEXT = False
@@ -1741,6 +1786,39 @@ def test_num_tickest(self):
17411786
with self.assertRaises(ValueError):
17421787
ctx.num_tickets = 1
17431788

1789+
@support.cpython_only
1790+
def test_refcycle_msg_callback(self):
1791+
# See https://github.com/python/cpython/issues/142516.
1792+
ctx = make_test_context()
1793+
def msg_callback(*args, _=ctx, **kwargs): ...
1794+
ctx._msg_callback = msg_callback
1795+
1796+
@support.cpython_only
1797+
@requires_keylog_setter
1798+
def test_refcycle_keylog_filename(self):
1799+
# See https://github.com/python/cpython/issues/142516.
1800+
self.addCleanup(os_helper.unlink, os_helper.TESTFN)
1801+
ctx = make_test_context()
1802+
class KeylogFilename(str): ...
1803+
ctx.keylog_filename = KeylogFilename(os_helper.TESTFN)
1804+
ctx.keylog_filename._ = ctx
1805+
1806+
@support.cpython_only
1807+
@unittest.skipUnless(ssl.HAS_PSK, 'requires TLS-PSK')
1808+
def test_refcycle_psk_client_callback(self):
1809+
# See https://github.com/python/cpython/issues/142516.
1810+
ctx = make_test_context()
1811+
def psk_client_callback(*args, _=ctx, **kwargs): ...
1812+
ctx.set_psk_client_callback(psk_client_callback)
1813+
1814+
@support.cpython_only
1815+
@unittest.skipUnless(ssl.HAS_PSK, 'requires TLS-PSK')
1816+
def test_refcycle_psk_server_callback(self):
1817+
# See https://github.com/python/cpython/issues/142516.
1818+
ctx = make_test_context(server_side=True)
1819+
def psk_server_callback(*args, _=ctx, **kwargs): ...
1820+
ctx.set_psk_server_callback(psk_server_callback)
1821+
17441822

17451823
class SSLErrorTests(unittest.TestCase):
17461824

@@ -5174,10 +5252,6 @@ def test_internal_chain_server(self):
51745252
self.assertEqual(res, b'\x02\n')
51755253

51765254

5177-
HAS_KEYLOG = hasattr(ssl.SSLContext, 'keylog_filename')
5178-
requires_keylog = unittest.skipUnless(
5179-
HAS_KEYLOG, 'test requires OpenSSL 1.1.1 with keylog callback')
5180-
51815255
class TestSSLDebug(unittest.TestCase):
51825256

51835257
def keylog_lines(self, fname=os_helper.TESTFN):
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
:mod:`ssl`: fix reference leaks in :class:`ssl.SSLContext` objects. Patch by
2+
Bénédikt Tran.

Modules/_ssl.c

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ typedef struct {
331331
int post_handshake_auth;
332332
#endif
333333
PyObject *msg_cb;
334-
PyObject *keylog_filename;
334+
PyObject *keylog_filename; // can be anything accepted by Py_fopen()
335335
BIO *keylog_bio;
336336
/* Cached module state, also used in SSLSocket and SSLSession code. */
337337
_sslmodulestate *state;
@@ -361,7 +361,7 @@ typedef struct {
361361
PySSLContext *ctx; /* weakref to SSL context */
362362
char shutdown_seen_zero;
363363
enum py_ssl_server_or_client socket_type;
364-
PyObject *owner; /* Python level "owner" passed to servername callback */
364+
PyObject *owner; /* weakref to Python level "owner" passed to servername callback */
365365
PyObject *server_hostname;
366366
} PySSLSocket;
367367

@@ -2436,6 +2436,17 @@ PySSL_traverse(PyObject *op, visitproc visit, void *arg)
24362436
return 0;
24372437
}
24382438

2439+
static int
2440+
PySSL_clear(PyObject *op)
2441+
{
2442+
PySSLSocket *self = PySSLSocket_CAST(op);
2443+
Py_CLEAR(self->Socket);
2444+
Py_CLEAR(self->ctx);
2445+
Py_CLEAR(self->owner);
2446+
Py_CLEAR(self->server_hostname);
2447+
return 0;
2448+
}
2449+
24392450
static void
24402451
PySSL_dealloc(PyObject *op)
24412452
{
@@ -2456,10 +2467,7 @@ PySSL_dealloc(PyObject *op)
24562467
SSL_set_shutdown(self->ssl, SSL_SENT_SHUTDOWN | SSL_get_shutdown(self->ssl));
24572468
SSL_free(self->ssl);
24582469
}
2459-
Py_XDECREF(self->Socket);
2460-
Py_XDECREF(self->ctx);
2461-
Py_XDECREF(self->server_hostname);
2462-
Py_XDECREF(self->owner);
2470+
(void)PySSL_clear(op);
24632471
PyObject_GC_Del(self);
24642472
Py_DECREF(tp);
24652473
}
@@ -3568,6 +3576,11 @@ context_traverse(PyObject *op, visitproc visit, void *arg)
35683576
PySSLContext *self = PySSLContext_CAST(op);
35693577
Py_VISIT(self->set_sni_cb);
35703578
Py_VISIT(self->msg_cb);
3579+
Py_VISIT(self->keylog_filename);
3580+
#ifndef OPENSSL_NO_PSK
3581+
Py_VISIT(self->psk_client_callback);
3582+
Py_VISIT(self->psk_server_callback);
3583+
#endif
35713584
Py_VISIT(Py_TYPE(self));
35723585
return 0;
35733586
}

0 commit comments

Comments
 (0)