Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 35.2k
gh-114572: Fix locking in cert_store_stats and get_ca_certs#114573
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| 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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -4553,6 +4553,50 @@ set_sni_callback(PySSLContext *self, PyObject *arg, void *c) | ||
| return 0; | ||
| } | ||
| #if OPENSSL_VERSION_NUMBER < 0x30300000L | ||
| 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) * | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noting this is a faithful backport of openssl/openssl@08cecb4 | ||
| 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\ | ||
| @@ -4582,7 +4626,12 @@ _ssl__SSLContext_cert_store_stats_impl(PySSLContext *self) | ||
| int x509 = 0, crl = 0, ca = 0, i; | ||
| store = SSL_CTX_get_cert_store(self->ctx); | ||
| objs = X509_STORE_get0_objects(store); | ||
| objs = X509_STORE_get1_objects(store); | ||
| if (objs == NULL) { | ||
| PyErr_SetString(PyExc_MemoryError, "failed to query cert store"); | ||
| return NULL; | ||
| } | ||
| for (i = 0; i < sk_X509_OBJECT_num(objs); i++) { | ||
| obj = sk_X509_OBJECT_value(objs, i); | ||
| switch (X509_OBJECT_get_type(obj)) { | ||
| @@ -4596,12 +4645,11 @@ _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; | ||
| } | ||
| } | ||
| sk_X509_OBJECT_pop_free(objs, X509_OBJECT_free); | ||
| return Py_BuildValue("{sisisi}", "x509", x509, "crl", crl, | ||
| "x509_ca", ca); | ||
| } | ||
| @@ -4633,7 +4681,12 @@ _ssl__SSLContext_get_ca_certs_impl(PySSLContext *self, int binary_form) | ||
| } | ||
| store = SSL_CTX_get_cert_store(self->ctx); | ||
| objs = X509_STORE_get0_objects(store); | ||
| objs = X509_STORE_get1_objects(store); | ||
davidben marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if (objs == NULL) { | ||
| PyErr_SetString(PyExc_MemoryError, "failed to query cert store"); | ||
| goto error; | ||
| } | ||
| for (i = 0; i < sk_X509_OBJECT_num(objs); i++) { | ||
| X509_OBJECT *obj; | ||
| X509 *cert; | ||
| @@ -4661,9 +4714,11 @@ _ssl__SSLContext_get_ca_certs_impl(PySSLContext *self, int binary_form) | ||
| } | ||
| Py_CLEAR(ci); | ||
| } | ||
| sk_X509_OBJECT_pop_free(objs, X509_OBJECT_free); | ||
| return rlist; | ||
| error: | ||
| sk_X509_OBJECT_pop_free(objs, X509_OBJECT_free); | ||
| Py_XDECREF(ci); | ||
| Py_XDECREF(rlist); | ||
| return NULL; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From looking at all the OpenSSL docs this appears to be correct? I'm less versed with the OpenSSL API than the submitter :)