bpo-37702: Fix SSL certificate-store-handles leak

Misc/NEWS.d/next/Windows/2019-07-29-16-49-31.bpo-37702.Lj2f5e.rst

@@ -0,0 +1,2 @@
+Fix memory leak on Windows in creating an SSLContext object or
+running urllib.request.urlopen('https://...').

Modules/_ssl.c

@@ -5543,51 +5543,67 @@ parseKeyUsage(PCCERT_CONTEXT pCertCtx, DWORD flags)
     return retval;
 }
 
+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
+};
+
+#define SYSTEM_STORES_COUNT (sizeof(system_stores) / sizeof(DWORD))
+
+static BOOL
+cert_close_stores(HCERTSTORE hCollectionStore, HCERTSTORE *system_store_handles)
+{
+    size_t i;
+    BOOL success = CertCloseStore(hCollectionStore, 0);
+    for (i = 0; i < SYSTEM_STORES_COUNT; i++) {
+        if (system_store_handles[i] &&
+                !CertCloseStore(system_store_handles[i], 0)) {
+            success = 0;
+        }
+    }
+    return success;
+}
+
 static HCERTSTORE
-ssl_collect_certificates(const char *store_name)
+ssl_collect_certificates(const char *store_name, HCERTSTORE *system_store_handles)
 {
 /* 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};
+    HCERTSTORE hCollectionStore, hSystemStore;
     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++) {
+    for (i = 0; i < SYSTEM_STORES_COUNT; i++) {
+        system_store_handles[i] = NULL;
         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;
+            system_store_handles[i] = hSystemStore;
+            if (CertAddStoreToCollection(hCollectionStore, hSystemStore,
+                                    CERT_PHYSICAL_STORE_ADD_ENABLE_FLAG, 0)) {
+                storesAdded = 1;
             }
         }
     }
-    if (storesAdded == 0) {
-        CertCloseStore(hCollectionStore, CERT_CLOSE_STORE_FORCE_FLAG);
+    if (!storesAdded) {
+        cert_close_stores(hCollectionStore, system_store_handles);
         return NULL;
     }
-
     return hCollectionStore;
 }
 
@@ -5622,7 +5638,8 @@ static PyObject *
 _ssl_enum_certificates_impl(PyObject *module, const char *store_name)
 /*[clinic end generated code: output=5134dc8bb3a3c893 input=915f60d70461ea4e]*/
 {
-    HCERTSTORE hCollectionStore = NULL;
+    HCERTSTORE hCollectionStore;
+    HCERTSTORE system_store_handles[SYSTEM_STORES_COUNT];
     PCCERT_CONTEXT pCertCtx = NULL;
     PyObject *keyusage = NULL, *cert = NULL, *enc = NULL, *tup = NULL;
     PyObject *result = NULL;
@@ -5631,7 +5648,7 @@ _ssl_enum_certificates_impl(PyObject *module, const char *store_name)
     if (result == NULL) {
         return NULL;
     }
-    hCollectionStore = ssl_collect_certificates(store_name);
+    hCollectionStore = ssl_collect_certificates(store_name, system_store_handles);
     if (hCollectionStore == NULL) {
         Py_DECREF(result);
         return PyErr_SetFromWindowsErr(GetLastError());
@@ -5686,15 +5703,11 @@ _ssl_enum_certificates_impl(PyObject *module, const char *store_name)
     Py_XDECREF(keyusage);
     Py_XDECREF(tup);
 
-    /* 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)) {
+    if (!cert_close_stores(hCollectionStore, system_store_handles)) {
         /* This error case might shadow another exception.*/
         Py_XDECREF(result);
         return PyErr_SetFromWindowsErr(GetLastError());
     }
-
     return result;
 }
 
@@ -5714,7 +5727,8 @@ static PyObject *
 _ssl_enum_crls_impl(PyObject *module, const char *store_name)
 /*[clinic end generated code: output=bce467f60ccd03b6 input=a1f1d7629f1c5d3d]*/
 {
-    HCERTSTORE hCollectionStore = NULL;
+    HCERTSTORE hCollectionStore;
+    HCERTSTORE system_store_handles[SYSTEM_STORES_COUNT];
     PCCRL_CONTEXT pCrlCtx = NULL;
     PyObject *crl = NULL, *enc = NULL, *tup = NULL;
     PyObject *result = NULL;
@@ -5723,7 +5737,7 @@ _ssl_enum_crls_impl(PyObject *module, const char *store_name)
     if (result == NULL) {
         return NULL;
     }
-    hCollectionStore = ssl_collect_certificates(store_name);
+    hCollectionStore = ssl_collect_certificates(store_name, system_store_handles);
     if (hCollectionStore == NULL) {
         Py_DECREF(result);
         return PyErr_SetFromWindowsErr(GetLastError());
@@ -5767,10 +5781,7 @@ _ssl_enum_crls_impl(PyObject *module, const char *store_name)
     Py_XDECREF(enc);
     Py_XDECREF(tup);
 
-    /* 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)) {
+    if (!cert_close_stores(hCollectionStore, system_store_handles)) {
         /* This error case might shadow another exception.*/
         Py_XDECREF(result);
         return PyErr_SetFromWindowsErr(GetLastError());