Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-116810: fix memory leak in ssl module #123249

Merged
merged 15 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
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.
76 changes: 13 additions & 63 deletions Modules/_ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -2251,6 +2251,17 @@ PySSL_dealloc(PySSLSocket *self)
PyTypeObject *tp = Py_TYPE(self);
PyObject_GC_UnTrack(self);
if (self->ssl) {
// If we free the SSL socket object without having called SSL_shutdown,
// OpenSSL will invalidate the linked SSL session object. While this
// behavior is strictly RFC-compliant, it makes session reuse less
// likely and it would also break compatibility with older stdlib
// versions (which used an ugly workaround of duplicating the
// SSL_SESSION object).
// Therefore, we ensure the socket is marked as shutdown in any case.
//
// See elaborate explanation at
// https://github.com/python/cpython/pull/123249#discussion_r1766164530
SSL_set_shutdown(self->ssl, SSL_SENT_SHUTDOWN);
gpshead marked this conversation as resolved.
Show resolved Hide resolved
SSL_free(self->ssl);
}
Py_XDECREF(self->Socket);
Expand Down Expand Up @@ -2795,64 +2806,13 @@ _ssl__SSLSocket_verify_client_post_handshake_impl(PySSLSocket *self)
#endif
}

static SSL_SESSION*
_ssl_session_dup(SSL_SESSION *session) {
SSL_SESSION *newsession = NULL;
int slen;
unsigned char *senc = NULL, *p;
const unsigned char *const_p;

if (session == NULL) {
PyErr_SetString(PyExc_ValueError, "Invalid session");
goto error;
}

/* get length */
slen = i2d_SSL_SESSION(session, NULL);
if (slen == 0 || slen > 0xFF00) {
PyErr_SetString(PyExc_ValueError, "i2d() failed");
goto error;
}
if ((senc = PyMem_Malloc(slen)) == NULL) {
PyErr_NoMemory();
goto error;
}
p = senc;
if (!i2d_SSL_SESSION(session, &p)) {
PyErr_SetString(PyExc_ValueError, "i2d() failed");
goto error;
}
const_p = senc;
newsession = d2i_SSL_SESSION(NULL, &const_p, slen);
if (newsession == NULL) {
PyErr_SetString(PyExc_ValueError, "d2i() failed");
goto error;
}
PyMem_Free(senc);
return newsession;
error:
if (senc != NULL) {
PyMem_Free(senc);
}
return NULL;
}

static PyObject *
PySSL_get_session(PySSLSocket *self, void *closure) {
/* get_session can return sessions from a server-side connection,
* it does not check for handshake done or client socket. */
PySSLSession *pysess;
SSL_SESSION *session;

/* duplicate session as workaround for session bug in OpenSSL 1.1.0,
* https://github.com/openssl/openssl/issues/1550 */
session = SSL_get0_session(self->ssl); /* borrowed reference */
if (session == NULL) {
Py_RETURN_NONE;
}
if ((session = _ssl_session_dup(session)) == NULL) {
return NULL;
}
session = SSL_get1_session(self->ssl);
if (session == NULL) {
Py_RETURN_NONE;
Expand All @@ -2871,11 +2831,8 @@ PySSL_get_session(PySSLSocket *self, void *closure) {
}

static int PySSL_set_session(PySSLSocket *self, PyObject *value,
void *closure)
{
void *closure) {
PySSLSession *pysess;
SSL_SESSION *session;
int result;

if (!Py_IS_TYPE(value, get_state_sock(self)->PySSLSession_Type)) {
PyErr_SetString(PyExc_TypeError, "Value is not a SSLSession.");
Expand All @@ -2898,14 +2855,7 @@ static int PySSL_set_session(PySSLSocket *self, PyObject *value,
"Cannot set session after handshake.");
return -1;
}
/* duplicate session */
if ((session = _ssl_session_dup(pysess->session)) == NULL) {
return -1;
}
result = SSL_set_session(self->ssl, session);
/* free duplicate, SSL_set_session() bumps ref count */
SSL_SESSION_free(session);
if (result == 0) {
if (SSL_set_session(self->ssl, pysess->session) == 0) {
_setSSLError(get_state_sock(self), NULL, 0, __FILE__, __LINE__);
return -1;
}
Expand Down
Loading