From d93fbbf88e4abdd24a0a55e3ddf85b8420c62052 Mon Sep 17 00:00:00 2001 From: kctherookie <48805853+kctherookie@users.noreply.github.com> Date: Fri, 29 Mar 2019 00:59:06 +0700 Subject: [PATCH] bpo-35941: Fix ssl certificate enumeration for windows (GH-12486) Add a function to collect certificates from several certificate stores into one certificate collection store that is then enumerated. This ensures we load as many certificates as we can access. --- .../2019-03-28-03-51-16.bpo-35941.UnlAEE.rst | 3 + Modules/_ssl.c | 109 ++++++++++++++---- 2 files changed, 92 insertions(+), 20 deletions(-) create mode 100644 Misc/NEWS.d/next/Windows/2019-03-28-03-51-16.bpo-35941.UnlAEE.rst diff --git a/Misc/NEWS.d/next/Windows/2019-03-28-03-51-16.bpo-35941.UnlAEE.rst b/Misc/NEWS.d/next/Windows/2019-03-28-03-51-16.bpo-35941.UnlAEE.rst new file mode 100644 index 0000000000..cda654bfa5 --- /dev/null +++ b/Misc/NEWS.d/next/Windows/2019-03-28-03-51-16.bpo-35941.UnlAEE.rst @@ -0,0 +1,3 @@ +enum_certificates function of the ssl module now returns certificates from all available certificate stores inside windows in a query instead of returning only certificates from the system wide certificate store. +This includes certificates from these certificate stores: local machine, local machine enterprise, local machine group policy, current user, current user group policy, services, users. +ssl.enum_crls() function is changed in the same way to return all certificate revocation lists inside the windows certificate revocation list stores. \ No newline at end of file diff --git a/Modules/_ssl.c b/Modules/_ssl.c index f2ce7fa627..f8ae916735 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -5400,6 +5400,68 @@ parseKeyUsage(PCCERT_CONTEXT pCertCtx, DWORD flags) return retval; } +static HCERTSTORE +ssl_collect_certificates(const char *store_name) +{ +/* this function collects the system certificate stores listed in + * system_stores into a collection certificate store for being + * enumerated. The store must be readable to be added to the + * store collection. + */ + + HCERTSTORE hCollectionStore = NULL, hSystemStore = NULL; + static DWORD system_stores[] = { + CERT_SYSTEM_STORE_LOCAL_MACHINE, + CERT_SYSTEM_STORE_LOCAL_MACHINE_ENTERPRISE, + CERT_SYSTEM_STORE_LOCAL_MACHINE_GROUP_POLICY, + CERT_SYSTEM_STORE_CURRENT_USER, + CERT_SYSTEM_STORE_CURRENT_USER_GROUP_POLICY, + CERT_SYSTEM_STORE_SERVICES, + CERT_SYSTEM_STORE_USERS}; + size_t i, storesAdded; + BOOL result; + + hCollectionStore = CertOpenStore(CERT_STORE_PROV_COLLECTION, 0, + (HCRYPTPROV)NULL, 0, NULL); + if (!hCollectionStore) { + return NULL; + } + storesAdded = 0; + for (i = 0; i < sizeof(system_stores) / sizeof(DWORD); i++) { + hSystemStore = CertOpenStore(CERT_STORE_PROV_SYSTEM_A, 0, + (HCRYPTPROV)NULL, + CERT_STORE_READONLY_FLAG | + system_stores[i], store_name); + if (hSystemStore) { + result = CertAddStoreToCollection(hCollectionStore, hSystemStore, + CERT_PHYSICAL_STORE_ADD_ENABLE_FLAG, 0); + if (result) { + ++storesAdded; + } + } + } + if (storesAdded == 0) { + CertCloseStore(hCollectionStore, CERT_CLOSE_STORE_FORCE_FLAG); + return NULL; + } + + return hCollectionStore; +} + +/* code from Objects/listobject.c */ + +static int +list_contains(PyListObject *a, PyObject *el) +{ + Py_ssize_t i; + int cmp; + + for (i = 0, cmp = 0 ; cmp == 0 && i < Py_SIZE(a); ++i) + cmp = PyObject_RichCompareBool(el, PyList_GET_ITEM(a, i), + Py_EQ); + return cmp; +} + /*[clinic input] _ssl.enum_certificates store_name: str @@ -5417,7 +5479,7 @@ static PyObject * _ssl_enum_certificates_impl(PyObject *module, const char *store_name) /*[clinic end generated code: output=5134dc8bb3a3c893 input=915f60d70461ea4e]*/ { - HCERTSTORE hStore = NULL; + HCERTSTORE hCollectionStore = NULL; PCCERT_CONTEXT pCertCtx = NULL; PyObject *keyusage = NULL, *cert = NULL, *enc = NULL, *tup = NULL; PyObject *result = NULL; @@ -5426,15 +5488,13 @@ _ssl_enum_certificates_impl(PyObject *module, const char *store_name) if (result == NULL) { return NULL; } - hStore = CertOpenStore(CERT_STORE_PROV_SYSTEM_A, 0, (HCRYPTPROV)NULL, - CERT_STORE_READONLY_FLAG | CERT_SYSTEM_STORE_LOCAL_MACHINE, - store_name); - if (hStore == NULL) { + hCollectionStore = ssl_collect_certificates(store_name); + if (hCollectionStore == NULL) { Py_DECREF(result); return PyErr_SetFromWindowsErr(GetLastError()); } - while (pCertCtx = CertEnumCertificatesInStore(hStore, pCertCtx)) { + while (pCertCtx = CertEnumCertificatesInStore(hCollectionStore, pCertCtx)) { cert = PyBytes_FromStringAndSize((const char*)pCertCtx->pbCertEncoded, pCertCtx->cbCertEncoded); if (!cert) { @@ -5464,9 +5524,11 @@ _ssl_enum_certificates_impl(PyObject *module, const char *store_name) enc = NULL; PyTuple_SET_ITEM(tup, 2, keyusage); keyusage = NULL; - if (PyList_Append(result, tup) < 0) { - Py_CLEAR(result); - break; + if (!list_contains((PyListObject*)result, tup)) { + if (PyList_Append(result, tup) < 0) { + Py_CLEAR(result); + break; + } } Py_CLEAR(tup); } @@ -5481,11 +5543,15 @@ _ssl_enum_certificates_impl(PyObject *module, const char *store_name) Py_XDECREF(keyusage); Py_XDECREF(tup); - if (!CertCloseStore(hStore, 0)) { + /* CERT_CLOSE_STORE_FORCE_FLAG forces freeing of memory for all contexts + associated with the store, in this case our collection store and the + associated system stores. */ + if (!CertCloseStore(hCollectionStore, CERT_CLOSE_STORE_FORCE_FLAG)) { /* This error case might shadow another exception.*/ Py_XDECREF(result); return PyErr_SetFromWindowsErr(GetLastError()); } + return result; } @@ -5505,7 +5571,7 @@ static PyObject * _ssl_enum_crls_impl(PyObject *module, const char *store_name) /*[clinic end generated code: output=bce467f60ccd03b6 input=a1f1d7629f1c5d3d]*/ { - HCERTSTORE hStore = NULL; + HCERTSTORE hCollectionStore = NULL; PCCRL_CONTEXT pCrlCtx = NULL; PyObject *crl = NULL, *enc = NULL, *tup = NULL; PyObject *result = NULL; @@ -5514,15 +5580,13 @@ _ssl_enum_crls_impl(PyObject *module, const char *store_name) if (result == NULL) { return NULL; } - hStore = CertOpenStore(CERT_STORE_PROV_SYSTEM_A, 0, (HCRYPTPROV)NULL, - CERT_STORE_READONLY_FLAG | CERT_SYSTEM_STORE_LOCAL_MACHINE, - store_name); - if (hStore == NULL) { + hCollectionStore = ssl_collect_certificates(store_name); + if (hCollectionStore == NULL) { Py_DECREF(result); return PyErr_SetFromWindowsErr(GetLastError()); } - while (pCrlCtx = CertEnumCRLsInStore(hStore, pCrlCtx)) { + while (pCrlCtx = CertEnumCRLsInStore(hCollectionStore, pCrlCtx)) { crl = PyBytes_FromStringAndSize((const char*)pCrlCtx->pbCrlEncoded, pCrlCtx->cbCrlEncoded); if (!crl) { @@ -5542,9 +5606,11 @@ _ssl_enum_crls_impl(PyObject *module, const char *store_name) PyTuple_SET_ITEM(tup, 1, enc); enc = NULL; - if (PyList_Append(result, tup) < 0) { - Py_CLEAR(result); - break; + if (!list_contains((PyListObject*)result, tup)) { + if (PyList_Append(result, tup) < 0) { + Py_CLEAR(result); + break; + } } Py_CLEAR(tup); } @@ -5558,7 +5624,10 @@ _ssl_enum_crls_impl(PyObject *module, const char *store_name) Py_XDECREF(enc); Py_XDECREF(tup); - if (!CertCloseStore(hStore, 0)) { + /* CERT_CLOSE_STORE_FORCE_FLAG forces freeing of memory for all contexts + associated with the store, in this case our collection store and the + associated system stores. */ + if (!CertCloseStore(hCollectionStore, CERT_CLOSE_STORE_FORCE_FLAG)) { /* This error case might shadow another exception.*/ Py_XDECREF(result); return PyErr_SetFromWindowsErr(GetLastError()); -- 2.40.0