Skip to content

Commit

Permalink
[CVE-2024-0397] Fix locking in cert_store_stats and get_ca_certs
Browse files Browse the repository at this point in the history
cert_store_stats and get_ca_certs query the SSLContext's
X509_STORE with X509_STORE_get0_objects, but reading the result
requires a lock. See gh#openssl/openssl#23224 for details.

Instead, use X509_STORE_get1_objects, newly added in that
PR. X509_STORE_get1_objects does not exist in current
OpenSSLs, but we can polyfill it with X509_STORE_lock and
X509_STORE_unlock.

From-PR: gh#python/cpython!114573
Fixes: gh#python#114572
Fixes: bsc#1226447 (CVE-2024-0397)
Patch: CVE-2024-0397-memrace_ssl.SSLContext_cert_store.patch
  • Loading branch information
sethmlarson authored and mcepl committed Jun 21, 2024
1 parent 878403d commit e701181
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
:meth:`ssl.SSLContext.cert_store_stats` and
:meth:`ssl.SSLContext.get_ca_certs` now correctly lock access to the
certificate store, when the :class:`ssl.SSLContext` is shared across
multiple threads.
98 changes: 95 additions & 3 deletions Modules/_ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ struct py_ssl_library_code {
# define HAVE_TLSv1_2 0
#endif

#if (OPENSSL_VERSION_NUMBER >= 0x30300000L) && !defined(LIBRESSL_VERSION_NUMBER)
# define OPENSSL_VERSION_3_3 1
#endif

/* SNI support (client- and server-side) appeared in OpenSSL 1.0.0 and 0.9.8f
* This includes the SSL_set_SSL_CTX() function.
*/
Expand Down Expand Up @@ -164,6 +168,16 @@ struct py_ssl_library_code {
#define HAVE_OPENSSL_CRYPTO_LOCK
#endif

/* OpenSSL 1.1+ allows locking X509_STORE, 1.0.2 doesn't. */
#ifdef OPENSSL_VERSION_1_1
#define HAVE_OPENSSL_X509_STORE_LOCK
#endif

/* OpenSSL 3.3 added the X509_STORE_get1_objects API */
#ifdef OPENSSL_VERSION_3_3
#define HAVE_OPENSSL_X509_STORE_GET1_OBJECTS 1
#endif

#if defined(OPENSSL_VERSION_1_1) && !defined(OPENSSL_NO_SSL2)
#define OPENSSL_NO_SSL2
#endif
Expand Down Expand Up @@ -4096,6 +4110,61 @@ _ssl__SSLContext_set_servername_callback(PySSLContext *self, PyObject *cb)
#endif
}

/* Shim of X509_STORE_get1_objects API from OpenSSL 3.3
* Only available with the X509_STORE_lock() API */
#if defined(HAVE_OPENSSL_X509_STORE_LOCK) && !defined(OPENSSL_VERSION_3_3)
#define HAVE_OPENSSL_X509_STORE_GET1_OBJECTS 1

static X509_OBJECT *x509_object_dup(const X509_OBJECT *obj)
{
int ok;
X509_OBJECT *ret = X509_OBJECT_new();
if (ret == NULL) {
return NULL;
}
switch (X509_OBJECT_get_type(obj)) {
case X509_LU_X509:
ok = X509_OBJECT_set1_X509(ret, X509_OBJECT_get0_X509(obj));
break;
case X509_LU_CRL:
/* X509_OBJECT_get0_X509_CRL was not const-correct prior to 3.0.*/
ok = X509_OBJECT_set1_X509_CRL(
ret, X509_OBJECT_get0_X509_CRL((X509_OBJECT *)obj));
break;
default:
/* We cannot duplicate unrecognized types in a polyfill, but it is
* safe to leave an empty object. The caller will ignore it. */
ok = 1;
break;
}
if (!ok) {
X509_OBJECT_free(ret);
return NULL;
}
return ret;
}

static STACK_OF(X509_OBJECT) *
X509_STORE_get1_objects(X509_STORE *store)
{
STACK_OF(X509_OBJECT) *ret;
if (!X509_STORE_lock(store)) {
return NULL;
}
ret = sk_X509_OBJECT_deep_copy(X509_STORE_get0_objects(store),
x509_object_dup, X509_OBJECT_free);
X509_STORE_unlock(store);
return ret;
}
#endif

PyDoc_STRVAR(PySSLContext_sni_callback_doc,
"Set a callback that will be called when a server name is provided by the SSL/TLS client in the SNI extension.\n\
\n\
If the argument is None then the callback is disabled. The method is called\n\
with the SSLSocket, the server name as a string, and the SSLContext object.\n\
See RFC 6066 for details of the SNI extension.");

/*[clinic input]
_ssl._SSLContext.cert_store_stats
Expand All @@ -4118,7 +4187,15 @@ _ssl__SSLContext_cert_store_stats_impl(PySSLContext *self)
int x509 = 0, crl = 0, ca = 0, i;

store = SSL_CTX_get_cert_store(self->ctx);
#if HAVE_OPENSSL_X509_STORE_GET1_OBJECTS
objs = X509_STORE_get1_objects(store);
if (objs == NULL) {
PyErr_SetString(PyExc_MemoryError, "failed to query cert store");
return NULL;
}
#else
objs = X509_STORE_get0_objects(store);
#endif
for (i = 0; i < sk_X509_OBJECT_num(objs); i++) {
obj = sk_X509_OBJECT_value(objs, i);
switch (X509_OBJECT_get_type(obj)) {
Expand All @@ -4132,12 +4209,13 @@ _ssl__SSLContext_cert_store_stats_impl(PySSLContext *self)
crl++;
break;
default:
/* Ignore X509_LU_FAIL, X509_LU_RETRY, X509_LU_PKEY.
* As far as I can tell they are internal states and never
* stored in a cert store */
/* Ignore unrecognized types. */
break;
}
}
#if HAVE_OPENSSL_X509_STORE_GET1_OBJECTS
sk_X509_OBJECT_pop_free(objs, X509_OBJECT_free);
#endif
return Py_BuildValue("{sisisi}", "x509", x509, "crl", crl,
"x509_ca", ca);
}
Expand Down Expand Up @@ -4169,7 +4247,15 @@ _ssl__SSLContext_get_ca_certs_impl(PySSLContext *self, int binary_form)
}

store = SSL_CTX_get_cert_store(self->ctx);
#if HAVE_OPENSSL_X509_STORE_GET1_OBJECTS
objs = X509_STORE_get1_objects(store);
if (objs == NULL) {
PyErr_SetString(PyExc_MemoryError, "failed to query cert store");
return NULL;
}
#else
objs = X509_STORE_get0_objects(store);
#endif
for (i = 0; i < sk_X509_OBJECT_num(objs); i++) {
X509_OBJECT *obj;
X509 *cert;
Expand Down Expand Up @@ -4197,9 +4283,15 @@ _ssl__SSLContext_get_ca_certs_impl(PySSLContext *self, int binary_form)
}
Py_CLEAR(ci);
}
#if HAVE_OPENSSL_X509_STORE_GET1_OBJECTS
sk_X509_OBJECT_pop_free(objs, X509_OBJECT_free);
#endif
return rlist;

error:
#if HAVE_OPENSSL_X509_STORE_GET1_OBJECTS
sk_X509_OBJECT_pop_free(objs, X509_OBJECT_free);
#endif
Py_XDECREF(ci);
Py_XDECREF(rlist);
return NULL;
Expand Down

0 comments on commit e701181

Please sign in to comment.