8000 [3.12] gh-116810: fix memory leak in ssl module (GH-123249) (GH-124801) · python/cpython@186cc40 · GitHub
[go: up one dir, main page]
Skip to content

Commit 186cc40

Browse files
miss-islingtonjeffvanvoorstblurb-it[bot]ZeroIntensitygpshead
authored
[3.12] gh-116810: fix memory leak in ssl module (GH-123249) (GH-124801)
gh-116810: fix memory leak in ssl module (GH-123249) Resolve a memory leak introduced in CPython 3.10's :mod:`ssl` when the :attr:`ssl.SSLSocket.session` property was accessed. Speeds up read and write access to said property by no longer unnecessarily cloning session objects via serialization. (cherry picked from commit 7e7223e) Co-authored-by: Jeffrey R. Van Voorst <jeff.vanvoorst@gmail.com> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Antoine Pitrou <antoine@python.org>
1 parent 3b20c4a commit 186cc40

File tree

2 files changed

+17
-63
lines changed

2 files changed

+17
-63
lines changed

‎Misc/NEWS.d/next/Library/2024-08-23-15-49-10.gh-issue-116810.QLBUU8.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Resolve a memory leak introduced in CPython 3.10's :mod:`ssl` when the
2+
:attr:`ssl.SSLSocket.session` property was accessed. Speeds up read and
3+
write access to said property by no longer unnecessarily cloning session
4+
objects via serialization.

‎Modules/_ssl.c

Lines changed: 13 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -2224,6 +2224,17 @@ PySSL_dealloc(PySSLSocket *self)
22242224
PyTypeObject *tp = Py_TYPE(self);
22252225
PyObject_GC_UnTrack(self);
22262226
if (self->ssl) {
2227+
// If we free the SSL socket object without having called SSL_shutdown,
2228+
// OpenSSL will invalidate the linked SSL session object. While this
2229+
// behavior is strictly RFC-compliant, it makes session reuse less
2230+
// likely and it would also break compatibility with older stdlib
2231+
// versions (which used an ugly workaround of duplicating the
2232+
// SSL_SESSION object).
2233+
// Therefore, we ensure the socket is marked as shutdown in any case.
2234+
//
2235+
// See elaborate explanation at
2236+
// https://github.com/python/cpython/pull/123249#discussion_r1766164530
2237+
SSL_set_shutdown(self->ssl, SSL_SENT_SHUTDOWN | SSL_get_shutdown(self->ssl));
22272238
SSL_free(self->ssl);
22282239
}
22292240
Py_XDECREF(self->Socket);
@@ -2768,64 +2779,13 @@ _ssl__SSLSocket_verify_client_post_handshake_impl(PySSLSocket *self)
27682779
#endif
27692780
}
27702781

2771-
static SSL_SESSION*
2772-
_ssl_session_dup(SSL_SESSION *session) {
2773-
SSL_SESSION *newsession = NULL;
2774-
int slen;
2775-
unsigned char *senc = NULL, *p;
2776-
const unsigned char *const_p;
2777-
2778-
if (session == NULL) {
2779-
PyErr_SetString(PyExc_ValueError, "Invalid session");
2780-
goto error;
2781-
}
2782-
2783-
/* get length */
2784-
slen = i2d_SSL_SESSION(session, NULL);
2785-
if (slen == 0 || slen > 0xFF00) {
2786-
PyErr_SetString(PyExc_ValueError, "i2d() failed");
2787-
goto error;
2788-
}
2789-
if ((senc = PyMem_Malloc(slen)) == NULL) {
2790-
PyErr_NoMemory();
2791-
goto error;
2792-
}
2793-
p = senc;
2794-
if (!i2d_SSL_SESSION(session, &p)) {
2795-
PyErr_SetString(PyExc_ValueError, "i2d() failed");
2796-
goto error;
2797-
}
2798-
const_p = senc;
2799-
newsession = d2i_SSL_SESSION(NULL, &const_p, slen);
2800-
if (newsession == NULL) {
2801-
PyErr_SetString(PyExc_ValueError, "d2i() failed");
2802-
goto error;
2803-
}
2804-
PyMem_Free(senc);
2805-
return newsession;
2806-
error:
2807-
if (senc != NULL) {
2808-
PyMem_Free(senc);
2809-
}
2810-
return NULL;
2811-
}
2812-
28132782
static PyObject *
28142783
PySSL_get_session(PySSLSocket *self, void *closure) {
28152784
/* get_session can return sessions from a server-side connection,
28162785
* it does not check for handshake done or client socket. */
28172786
PySSLSession *pysess;
28182787
SSL_SESSION *session;
28192788

2820-
/* duplicate session as workaround for session bug in OpenSSL 1.1.0,
2821-
* https://github.com/openssl/openssl/issues/1550 */
2822-
session = SSL_get0_session(self->ssl); /* borrowed reference */
2823-
if (session == NULL) {
2824-
Py_RETURN_NONE;
2825-
}
2826-
if ((session = _ssl_session_dup(session)) == NULL) {
2827-
return NULL;
2828-
}
28292789
session = SSL_get1_session(self->ssl);
28302790
if (session == NULL) {
28312791
Py_RETURN_NONE;
@@ -2844,11 +2804,8 @@ PySSL_get_session(PySSLSocket *self, void *closure) {
28442804
}
28452805

28462806
static int PySSL_set_session(PySSLSocket *self, PyObject *value,
2847-
void *closure)
2848-
{
2807+
void *closure) {
28492808
PySSLSession *pysess;
2850-
SSL_SESSION *session;
2851-
int result;
28522809

28532810
if (!Py_IS_TYPE(value, get_state_sock(self)->PySSLSession_Type)) {
28542811
PyErr_SetString(PyExc_TypeError, "Value is not a SSLSession.");
@@ -2871,14 +2828,7 @@ static int PySSL_set_session(PySSLSocket *self, PyObject *value,
28712828
"Cannot set session after handshake.");
28722829
return -1;
28732830
}
2874-
/* duplicate session */
2875-
if ((session = _ssl_session_dup(pysess->session)) == NULL) {
2876-
return -1;
2877-
}
2878-
result = SSL_set_session(self->ssl, session);
2879-
/* free duplicate, SSL_set_session() bumps ref count */
2880-
SSL_SESSION_free(session);
2881-
if (result == 0) {
2831+
if (SSL_set_session(self->ssl, pysess->session) == 0) {
28822832
_setSSLError(get_state_sock(self), NULL, 0, __FILE__, __LINE__);
28832833
return -1;
28842834
}

0 commit comments

Comments
 (0)
0