Skip to content

Commit 1decc7e

Browse files
authored
[3.14] gh-142516: fix reference leaks in ssl.SSLContext objects (GH-143685) (#145075)
* [3.14] gh-142516: fix reference leaks in `ssl.SSLContext` objects (GH-143685) (cherry picked from commit 3a2a686) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> * fix backport
1 parent dcf96d0 commit 1decc7e

File tree

3 files changed

+108
-20
lines changed

3 files changed

+108
-20
lines changed

Lib/test/test_ssl.py

Lines changed: 94 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,16 @@
5050
IS_OPENSSL_3_0_0 = ssl.OPENSSL_VERSION_INFO >= (3, 0, 0)
5151
PY_SSL_DEFAULT_CIPHERS = sysconfig.get_config_var('PY_SSL_DEFAULT_CIPHERS')
5252

53+
HAS_KEYLOG = hasattr(ssl.SSLContext, 'keylog_filename')
54+
requires_keylog = unittest.skipUnless(
55+
HAS_KEYLOG, 'test requires OpenSSL 1.1.1 with keylog callback')
56+
CAN_SET_KEYLOG = HAS_KEYLOG and os.name != "nt"
57+
requires_keylog_setter = unittest.skipUnless(
58+
CAN_SET_KEYLOG,
59+
"cannot set 'keylog_filename' on Windows"
60+
)
61+
62+
5363
PROTOCOL_TO_TLS_VERSION = {}
5464
for proto, ver in (
5565
("PROTOCOL_SSLv3", "SSLv3"),
@@ -258,26 +268,67 @@ def utc_offset(): #NOTE: ignore issues like #1647654
258268
)
259269

260270

261-
def test_wrap_socket(sock, *,
262-
cert_reqs=ssl.CERT_NONE, ca_certs=None,
263-
ciphers=None, certfile=None, keyfile=None,
264-
**kwargs):
265-
if not kwargs.get("server_side"):
266-
kwargs["server_hostname"] = SIGNED_CERTFILE_HOSTNAME
267-
context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
268-
else:
271+
def make_test_context(
272+
*,
273+
server_side=False,
274+
check_hostname=None,
275+
cert_reqs=ssl.CERT_NONE,
276+
ca_certs=None, certfile=None, keyfile=None,
277+
ciphers=None,
278+
min_version=None, max_version=None,
279+
):
280+
if server_side:
269281
context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
270-
if cert_reqs is not None:
282+
else:
283+
context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
284+
285+
if check_hostname is None:
271286
if cert_reqs == ssl.CERT_NONE:
272287
context.check_hostname = False
288+
else:
289+
context.check_hostname = check_hostname
290+
291+
if cert_reqs is not None:
273292
context.verify_mode = cert_reqs
293+
274294
if ca_certs is not None:
275295
context.load_verify_locations(ca_certs)
276296
if certfile is not None or keyfile is not None:
277297
context.load_cert_chain(certfile, keyfile)
298+
278299
if ciphers is not None:
279300
context.set_ciphers(ciphers)
280-
return context.wrap_socket(sock, **kwargs)
301+
302+
if min_version is not None:
303+
context.minimum_version = min_version
304+
if max_version is not None:
305+
context.maximum_version = max_version
306+
307+
return context
308+
309+
310+
def test_wrap_socket(
311+
sock,
312+
*,
313+
server_side=False,
314+
check_hostname=None,
315+
cert_reqs=ssl.CERT_NONE,
316+
ca_certs=None, certfile=None, keyfile=None,
317+
ciphers=None,
318+
min_version=None, max_version=None,
319+
**kwargs,
320+
):
321+
context = make_test_context(
322+
server_side=server_side,
323+
check_hostname=check_hostname,
324+
cert_reqs=cert_reqs,
325+
ca_certs=ca_certs, certfile=certfile, keyfile=keyfile,
326+
ciphers=ciphers,
327+
min_version=min_version, max_version=max_version,
328+
)
329+
if not server_side:
330+
kwargs.setdefault("server_hostname", SIGNED_CERTFILE_HOSTNAME)
331+
return context.wrap_socket(sock, server_side=server_side, **kwargs)
281332

282333

283334
USE_SAME_TEST_CONTEXT = False
@@ -1665,6 +1716,39 @@ def test_num_tickest(self):
16651716
with self.assertRaises(ValueError):
16661717
ctx.num_tickets = 1
16671718

1719+
@support.cpython_only
1720+
def test_refcycle_msg_callback(self):
1721+
# See https://github.com/python/cpython/issues/142516.
1722+
ctx = make_test_context()
1723+
def msg_callback(*args, _=ctx, **kwargs): ...
1724+
ctx._msg_callback = msg_callback
1725+
1726+
@support.cpython_only
1727+
@requires_keylog_setter
1728+
def test_refcycle_keylog_filename(self):
1729+
# See https://github.com/python/cpython/issues/142516.
1730+
self.addCleanup(os_helper.unlink, os_helper.TESTFN)
1731+
ctx = make_test_context()
1732+
class KeylogFilename(str): ...
1733+
ctx.keylog_filename = KeylogFilename(os_helper.TESTFN)
1734+
ctx.keylog_filename._ = ctx
1735+
1736+
@support.cpython_only
1737+
@unittest.skipUnless(ssl.HAS_PSK, 'requires TLS-PSK')
1738+
def test_refcycle_psk_client_callback(self):
1739+
# See https://github.com/python/cpython/issues/142516.
1740+
ctx = make_test_context()
1741+
def psk_client_callback(*args, _=ctx, **kwargs): ...
1742+
ctx.set_psk_client_callback(psk_client_callback)
1743+
1744+
@support.cpython_only
1745+
@unittest.skipUnless(ssl.HAS_PSK, 'requires TLS-PSK')
1746+
def test_refcycle_psk_server_callback(self):
1747+
# See https://github.com/python/cpython/issues/142516.
1748+
ctx = make_test_context(server_side=True)
1749+
def psk_server_callback(*args, _=ctx, **kwargs): ...
1750+
ctx.set_psk_server_callback(psk_server_callback)
1751+
16681752

16691753
class SSLErrorTests(unittest.TestCase):
16701754

@@ -4914,10 +4998,6 @@ def test_internal_chain_server(self):
49144998
self.assertEqual(res, b'\x02\n')
49154999

49165000

4917-
HAS_KEYLOG = hasattr(ssl.SSLContext, 'keylog_filename')
4918-
requires_keylog = unittest.skipUnless(
4919-
HAS_KEYLOG, 'test requires OpenSSL 1.1.1 with keylog callback')
4920-
49215001
class TestSSLDebug(unittest.TestCase):
49225002

49235003
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: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ typedef struct {
301301
int post_handshake_auth;
302302
#endif
303303
PyObject *msg_cb;
304-
PyObject *keylog_filename;
304+
PyObject *keylog_filename; // can be anything accepted by Py_fopen()
305305
BIO *keylog_bio;
306306
/* Cached module state, also used in SSLSocket and SSLSession code. */
307307
_sslmodulestate *state;
@@ -331,7 +331,7 @@ typedef struct {
331331
PySSLContext *ctx; /* weakref to SSL context */
332332
char shutdown_seen_zero;
333333
enum py_ssl_server_or_client socket_type;
334-
PyObject *owner; /* Python level "owner" passed to servername callback */
334+
PyObject *owner; /* weakref to Python level "owner" passed to servername callback */
335335
PyObject *server_hostname;
336336
_PySSLError err; /* last seen error from various sources */
337337
/* Some SSL callbacks don't have error reporting. Callback wrappers
@@ -2345,6 +2345,10 @@ static int
23452345
PySSL_clear(PyObject *op)
23462346
{
23472347
PySSLSocket *self = PySSLSocket_CAST(op);
2348+
Py_CLEAR(self->Socket);
2349+
Py_CLEAR(self->ctx);
2350+
Py_CLEAR(self->owner);
2351+
Py_CLEAR(self->server_hostname);
23482352
Py_CLEAR(self->exc);
23492353
return 0;
23502354
}
@@ -2369,10 +2373,7 @@ PySSL_dealloc(PyObject *op)
23692373
SSL_set_shutdown(self->ssl, SSL_SENT_SHUTDOWN | SSL_get_shutdown(self->ssl));
23702374
SSL_free(self->ssl);
23712375
}
2372-
Py_XDECREF(self->Socket);
2373-
Py_XDECREF(self->ctx);
2374-
Py_XDECREF(self->server_hostname);
2375-
Py_XDECREF(self->owner);
2376+
(void)PySSL_clear(op);
23762377
PyObject_GC_Del(self);
23772378
Py_DECREF(tp);
23782379
}
@@ -3309,6 +3310,11 @@ context_traverse(PyObject *op, visitproc visit, void *arg)
33093310
PySSLContext *self = PySSLContext_CAST(op);
33103311
Py_VISIT(self->set_sni_cb);
33113312
Py_VISIT(self->msg_cb);
3313+
Py_VISIT(self->keylog_filename);
3314+
#ifndef OPENSSL_NO_PSK
3315+
Py_VISIT(self->psk_client_callback);
3316+
Py_VISIT(self->psk_server_callback);
3317+
#endif
33123318
Py_VISIT(Py_TYPE(self));
33133319
return 0;
33143320
}

0 commit comments

Comments
 (0)