From: Donat Enikeev Subject: [PATCH 2/2] crypt32: importing system root certs into volatile registry keys instead of dedicated RootStore Message-Id: <1477667802-12090-1-git-send-email-donat@enikeev.net> Date: Fri, 28 Oct 2016 18:16:42 +0300 In-Reply-To: References: Fixes bug: https://bugs.winehq.org/show_bug.cgi?id=30187 Follows proposals of Jacek Caban @wine-devel Superceeds: https://source.winehq.org/patches/data/127163 v2: 1. importing system root certs into volatile registry keys using existing functions 2. RootStore removal v3: Mutex, clean-ups Signed-off-by: Donat Enikeev --- dlls/crypt32/crypt32_private.h | 6 ++- dlls/crypt32/main.c | 1 - dlls/crypt32/regstore.c | 10 ++-- dlls/crypt32/rootstore.c | 114 ++++++++++++----------------------------- dlls/crypt32/store.c | 12 ++--- 5 files changed, 47 insertions(+), 96 deletions(-) diff --git a/dlls/crypt32/crypt32_private.h b/dlls/crypt32/crypt32_private.h index ad4a827..addff9b 100644 --- a/dlls/crypt32/crypt32_private.h +++ b/dlls/crypt32/crypt32_private.h @@ -336,7 +336,11 @@ WINECRYPT_CERTSTORE *CRYPT_FileNameOpenStoreA(HCRYPTPROV hCryptProv, DWORD dwFlags, const void *pvPara) DECLSPEC_HIDDEN; WINECRYPT_CERTSTORE *CRYPT_FileNameOpenStoreW(HCRYPTPROV hCryptProv, DWORD dwFlags, const void *pvPara) DECLSPEC_HIDDEN; -WINECRYPT_CERTSTORE *CRYPT_RootOpenStore(HCRYPTPROV hCryptProv, DWORD dwFlags) DECLSPEC_HIDDEN; + +void CRYPT_ImportSystemRootCertsToReg(void) DECLSPEC_HIDDEN; +BOOL CRYPT_SerializeContextsToReg(HKEY key, DWORD dwFlags, const WINE_CONTEXT_INTERFACE *contextInterface, + HCERTSTORE memStore) DECLSPEC_HIDDEN; + BOOL CRYPT_IsCertificateSelfSigned(PCCERT_CONTEXT cert) DECLSPEC_HIDDEN; /* Allocates and initializes a certificate chain engine, but without creating diff --git a/dlls/crypt32/main.c b/dlls/crypt32/main.c index 241a1d9..9ff7592 100644 --- a/dlls/crypt32/main.c +++ b/dlls/crypt32/main.c @@ -49,7 +49,6 @@ BOOL WINAPI DllMain(HINSTANCE hInst, DWORD fdwReason, PVOID pvReserved) if (pvReserved) break; crypt_oid_free(); crypt_sip_free(); - root_store_free(); default_chain_engine_free(); if (hDefProv) CryptReleaseContext(hDefProv, 0); break; diff --git a/dlls/crypt32/regstore.c b/dlls/crypt32/regstore.c index b79387f..750f315 100644 --- a/dlls/crypt32/regstore.c +++ b/dlls/crypt32/regstore.c @@ -179,7 +179,7 @@ static void CRYPT_RegReadFromReg(HKEY key, HCERTSTORE store) } /* Hash is assumed to be 20 bytes in length (a SHA-1 hash) */ -static BOOL CRYPT_WriteSerializedToReg(HKEY key, const BYTE *hash, const BYTE *buf, +static BOOL CRYPT_WriteSerializedToReg(HKEY key, DWORD dwFlags, const BYTE *hash, const BYTE *buf, DWORD len) { WCHAR asciiHash[20 * 2 + 1]; @@ -188,7 +188,7 @@ static BOOL CRYPT_WriteSerializedToReg(HKEY key, const BYTE *hash, const BYTE *b BOOL ret; CRYPT_HashToStr(hash, asciiHash); - rc = RegCreateKeyExW(key, asciiHash, 0, NULL, 0, KEY_ALL_ACCESS, NULL, + rc = RegCreateKeyExW(key, asciiHash, 0, NULL, dwFlags, KEY_ALL_ACCESS, NULL, &subKey, NULL); if (!rc) { @@ -205,7 +205,7 @@ static BOOL CRYPT_WriteSerializedToReg(HKEY key, const BYTE *hash, const BYTE *b return ret; } -static BOOL CRYPT_SerializeContextsToReg(HKEY key, +BOOL CRYPT_SerializeContextsToReg(HKEY key, DWORD dwFlags, const WINE_CONTEXT_INTERFACE *contextInterface, HCERTSTORE memStore) { const void *context = NULL; @@ -232,7 +232,7 @@ static BOOL CRYPT_SerializeContextsToReg(HKEY key, { ret = contextInterface->serialize(context, 0, buf, &size); if (ret) - ret = CRYPT_WriteSerializedToReg(key, hash, buf, size); + ret = CRYPT_WriteSerializedToReg(key, dwFlags, hash, buf, size); } CryptMemFree(buf); } @@ -287,7 +287,7 @@ static BOOL CRYPT_RegWriteToReg(WINE_REGSTOREINFO *store) } LeaveCriticalSection(&store->cs); } - ret = CRYPT_SerializeContextsToReg(key, interfaces[i], + ret = CRYPT_SerializeContextsToReg(key, 0, interfaces[i], store->memStore); RegCloseKey(key); } diff --git a/dlls/crypt32/rootstore.c b/dlls/crypt32/rootstore.c index 5cee235..5bf77db 100644 --- a/dlls/crypt32/rootstore.c +++ b/dlls/crypt32/rootstore.c @@ -435,53 +435,6 @@ static BOOL import_certs_from_path(LPCSTR path, HCERTSTORE store, return ret; } -static BOOL WINAPI CRYPT_RootWriteCert(HCERTSTORE hCertStore, - PCCERT_CONTEXT cert, DWORD dwFlags) -{ - /* The root store can't have certs added */ - return FALSE; -} - -static BOOL WINAPI CRYPT_RootDeleteCert(HCERTSTORE hCertStore, - PCCERT_CONTEXT cert, DWORD dwFlags) -{ - /* The root store can't have certs deleted */ - return FALSE; -} - -static BOOL WINAPI CRYPT_RootWriteCRL(HCERTSTORE hCertStore, - PCCRL_CONTEXT crl, DWORD dwFlags) -{ - /* The root store can have CRLs added. At worst, a malicious application - * can DoS itself, as the changes aren't persisted in any way. - */ - return TRUE; -} - -static BOOL WINAPI CRYPT_RootDeleteCRL(HCERTSTORE hCertStore, - PCCRL_CONTEXT crl, DWORD dwFlags) -{ - /* The root store can't have CRLs deleted */ - return FALSE; -} - -static void *rootProvFuncs[] = { - NULL, /* CERT_STORE_PROV_CLOSE_FUNC */ - NULL, /* CERT_STORE_PROV_READ_CERT_FUNC */ - CRYPT_RootWriteCert, - CRYPT_RootDeleteCert, - NULL, /* CERT_STORE_PROV_SET_CERT_PROPERTY_FUNC */ - NULL, /* CERT_STORE_PROV_READ_CRL_FUNC */ - CRYPT_RootWriteCRL, - CRYPT_RootDeleteCRL, - NULL, /* CERT_STORE_PROV_SET_CRL_PROPERTY_FUNC */ - NULL, /* CERT_STORE_PROV_READ_CTL_FUNC */ - NULL, /* CERT_STORE_PROV_WRITE_CTL_FUNC */ - NULL, /* CERT_STORE_PROV_DELETE_CTL_FUNC */ - NULL, /* CERT_STORE_PROV_SET_CTL_PROPERTY_FUNC */ - NULL, /* CERT_STORE_PROV_CONTROL_FUNC */ -}; - static const char * const CRYPT_knownLocations[] = { "/etc/ssl/certs/ca-certificates.crt", "/etc/ssl/certs", @@ -790,55 +743,56 @@ static void read_trusted_roots_from_known_locations(HCERTSTORE store) static HCERTSTORE create_root_store(void) { - HCERTSTORE root = NULL; HCERTSTORE memStore = CertOpenStore(CERT_STORE_PROV_MEMORY, X509_ASN_ENCODING, 0, CERT_STORE_CREATE_NEW_FLAG, NULL); if (memStore) { - CERT_STORE_PROV_INFO provInfo = { - sizeof(CERT_STORE_PROV_INFO), - sizeof(rootProvFuncs) / sizeof(rootProvFuncs[0]), - rootProvFuncs, - NULL, - 0, - NULL - }; - read_trusted_roots_from_known_locations(memStore); add_ms_root_certs(memStore); - root = CRYPT_ProvCreateStore(0, memStore, &provInfo); } - TRACE("returning %p\n", root); - return root; + + TRACE("returning %p\n", memStore); + return memStore; } -static WINECRYPT_CERTSTORE *CRYPT_rootStore; +static const WCHAR certs_root_pathW[] = + {'S','o','f','t','w','a','r','e','\\','M','i','c','r','o','s','o','f','t','\\', + 'S','y','s','t','e','m','C','e','r','t','i','f','i','c','a','t','e','s','\\', + 'R','o','o','t','\\', 'C','e','r','t','i','f','i','c','a','t','e','s', 0}; +static const WCHAR mutex_nameW[] = + {'_','_','W','I','N','E','_','C','R','Y','P','T','_','M','U','T','E','X','_','_','\0'}; + -WINECRYPT_CERTSTORE *CRYPT_RootOpenStore(HCRYPTPROV hCryptProv, DWORD dwFlags) +void CRYPT_ImportSystemRootCertsToReg(void) { - TRACE("(%ld, %08x)\n", hCryptProv, dwFlags); + HCERTSTORE store = NULL; + HKEY key; + LONG rc; + HANDLE mutex; - if (dwFlags & CERT_STORE_DELETE_FLAG) + TRACE("\n"); + + mutex = CreateMutexW( NULL, FALSE, mutex_nameW); + if (!mutex) { - WARN("root store can't be deleted\n"); - SetLastError(ERROR_ACCESS_DENIED); - return NULL; + ERR("Failed to create mutex, %08x\n", GetLastError()); + return; } - if (!CRYPT_rootStore) + if ( GetLastError() == ERROR_ALREADY_EXISTS ) + return; + + if (!(store = create_root_store())) + return; + + rc = RegCreateKeyExW(HKEY_LOCAL_MACHINE, certs_root_pathW, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &key, 0); + if (!rc) { - HCERTSTORE root = create_root_store(); + if (!CRYPT_SerializeContextsToReg(key, REG_OPTION_VOLATILE, pCertInterface, store)) + WARN("Failed to import system certs into registry, %08x\n", GetLastError()); - InterlockedCompareExchangePointer((PVOID *)&CRYPT_rootStore, root, - NULL); - if (CRYPT_rootStore != root) - CertCloseStore(root, 0); + RegCloseKey(key); } - CRYPT_rootStore->vtbl->addref(CRYPT_rootStore); - return CRYPT_rootStore; -} -void root_store_free(void) -{ - CertCloseStore(CRYPT_rootStore, 0); -} + CertCloseStore(store, 0); +} \ No newline at end of file diff --git a/dlls/crypt32/store.c b/dlls/crypt32/store.c index 356712b..19eca22 100644 --- a/dlls/crypt32/store.c +++ b/dlls/crypt32/store.c @@ -424,21 +424,15 @@ static WINECRYPT_CERTSTORE *CRYPT_SysRegOpenStoreW(HCRYPTPROV hCryptProv, SetLastError(E_INVALIDARG); return NULL; } - /* FIXME: In Windows, the root store (even the current user location) is - * protected: adding to it or removing from it present a user interface, - * and the keys are owned by the system process, not the current user. - * Wine's registry doesn't implement access controls, so a similar - * mechanism isn't possible yet. - */ - if ((dwFlags & CERT_SYSTEM_STORE_LOCATION_MASK) == - CERT_SYSTEM_STORE_LOCAL_MACHINE && !lstrcmpiW(storeName, rootW)) - return CRYPT_RootOpenStore(hCryptProv, dwFlags); switch (dwFlags & CERT_SYSTEM_STORE_LOCATION_MASK) { case CERT_SYSTEM_STORE_LOCAL_MACHINE: root = HKEY_LOCAL_MACHINE; base = CERT_LOCAL_MACHINE_SYSTEM_STORE_REGPATH; + /* If the HKLM\Root certs are requested, expressing system certs into the registry */ + if (!lstrcmpiW(storeName, rootW)) + CRYPT_ImportSystemRootCertsToReg(); break; case CERT_SYSTEM_STORE_CURRENT_USER: root = HKEY_CURRENT_USER; -- 2.7.4