[PATCH 0/5] MR7150: crypt32: Do not delete root certs which were not imported from host.
Currently the logic of syncing of certificates with host effectively assumes that all the root certificates come from host and doesn't mind the certificates added by the app (those erroneously get deleted during host sync).
That fixes Battle.net being unable to complete game installs / update after Battle net update on 14 Jan 2025.
This is not for code freeze obviously.
The issue is that Battle.net fails to verify certificate chain which depends on an ephemeral certificate marked valid for DNS:localbattle.net (which resolves to 127.0.0.1) server auth. The certificate is self signed and is added to system root storage by Battle.bet setup (and also possibly later if it is missing). The addition of certificate to system root storage works per se, but then upon syncing host root certificates (in another process or new prefix start) the certificate gets wiped from registry.
From: Paul Gofman pgofman@codeweavers.com
--- dlls/crypt32/regstore.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/dlls/crypt32/regstore.c b/dlls/crypt32/regstore.c index 8567604c39b..962bd6a0382 100644 --- a/dlls/crypt32/regstore.c +++ b/dlls/crypt32/regstore.c @@ -237,6 +237,15 @@ BOOL CRYPT_SerializeContextsToReg(HKEY key, DWORD flags, return ret; }
+static void CRYPT_RegDeleteFromReg(HKEY key, const BYTE *sha1_hash) +{ + WCHAR hash[20 * 2 + 1]; + + CRYPT_HashToStr(sha1_hash, hash); + TRACE("Removing %s\n", debugstr_w(hash)); + RegDeleteKeyW(key, hash); +} + static BOOL CRYPT_RegWriteToReg(WINE_REGSTOREINFO *store) { static const WCHAR * const subKeys[] = { L"Certificates", L"CRLs", L"CTLs" }; @@ -258,22 +267,12 @@ static BOOL CRYPT_RegWriteToReg(WINE_REGSTOREINFO *store) if (listToDelete[i]) { WINE_HASH_TO_DELETE *toDelete, *next; - WCHAR asciiHash[20 * 2 + 1];
EnterCriticalSection(&store->cs); LIST_FOR_EACH_ENTRY_SAFE(toDelete, next, listToDelete[i], WINE_HASH_TO_DELETE, entry) { - LONG rc; - - CRYPT_HashToStr(toDelete->hash, asciiHash); - TRACE("Removing %s\n", debugstr_w(asciiHash)); - rc = RegDeleteKeyW(key, asciiHash); - if (rc != ERROR_SUCCESS && rc != ERROR_FILE_NOT_FOUND) - { - SetLastError(rc); - ret = FALSE; - } + CRYPT_RegDeleteFromReg(key, toDelete->hash); list_remove(&toDelete->entry); CryptMemFree(toDelete); }
From: Paul Gofman pgofman@codeweavers.com
--- dlls/crypt32/regstore.c | 56 ++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 26 deletions(-)
diff --git a/dlls/crypt32/regstore.c b/dlls/crypt32/regstore.c index 962bd6a0382..43ffa106c76 100644 --- a/dlls/crypt32/regstore.c +++ b/dlls/crypt32/regstore.c @@ -197,6 +197,35 @@ static BOOL CRYPT_WriteSerializedToReg(HKEY key, DWORD flags, const BYTE *hash, return ret; }
+static BOOL CRYPT_SerializeContextToReg(HKEY key, DWORD flags, const WINE_CONTEXT_INTERFACE *context_iface, + const void *context) +{ + BYTE hash[20]; + DWORD hash_size = sizeof(hash); + DWORD size = 0; + BYTE *buf; + BOOL ret; + + if (!context_iface->getProp(context, CERT_HASH_PROP_ID, hash, &hash_size)) + return FALSE; + + context_iface->serialize(context, 0, NULL, &size); + if (!size) + return FALSE; + + if (!(buf = CryptMemAlloc(size))) + return FALSE; + + if (!(context_iface->serialize(context, 0, buf, &size))) + { + CryptMemFree(buf); + return FALSE; + } + ret = CRYPT_WriteSerializedToReg(key, flags, hash, buf, size); + CryptMemFree(buf); + return ret; +} + BOOL CRYPT_SerializeContextsToReg(HKEY key, DWORD flags, const WINE_CONTEXT_INTERFACE *contextInterface, HCERTSTORE memStore) { @@ -205,32 +234,7 @@ BOOL CRYPT_SerializeContextsToReg(HKEY key, DWORD flags,
do { context = contextInterface->enumContextsInStore(memStore, context); - if (context) - { - BYTE hash[20]; - DWORD hashSize = sizeof(hash); - - ret = contextInterface->getProp(context, CERT_HASH_PROP_ID, hash, - &hashSize); - if (ret) - { - DWORD size = 0; - LPBYTE buf = NULL; - - ret = contextInterface->serialize(context, 0, NULL, &size); - if (size) - buf = CryptMemAlloc(size); - if (buf) - { - ret = contextInterface->serialize(context, 0, buf, &size); - if (ret) - ret = CRYPT_WriteSerializedToReg(key, flags, hash, buf, size); - } - CryptMemFree(buf); - } - } - else - ret = TRUE; + ret = !context || CRYPT_SerializeContextToReg(key, flags, contextInterface, context); } while (ret && context != NULL); if (context) Context_Release(context_from_ptr(context));
From: Paul Gofman pgofman@codeweavers.com
The other certificates in constructed chain are either in 'cached' set and will be persisted anyway, or come from CA / My / Trust stores and should not be duplicated to the ROOT store. --- dlls/crypt32/rootstore.c | 98 ++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 59 deletions(-)
diff --git a/dlls/crypt32/rootstore.c b/dlls/crypt32/rootstore.c index cfddcc143ac..6d049af2edf 100644 --- a/dlls/crypt32/rootstore.c +++ b/dlls/crypt32/rootstore.c @@ -98,74 +98,54 @@ static const char *get_cert_common_name(PCCERT_CONTEXT cert) return name; }
-static void check_and_store_certs(HCERTSTORE cached, HCERTSTORE new, HCERTSTORE to) +static void check_and_store_certs( HCERTSTORE cached, HCERTSTORE new, HCERTSTORE to ) { DWORD root_count = 0; - CERT_CHAIN_ENGINE_CONFIG chainEngineConfig = - { sizeof(chainEngineConfig), 0 }; + CERT_CHAIN_ENGINE_CONFIG chainEngineConfig = { sizeof(chainEngineConfig), 0 }; HCERTCHAINENGINE engine; + PCCERT_CONTEXT cert = NULL;
TRACE("\n");
- CertDuplicateStore(to); - engine = CRYPT_CreateChainEngine(to, CERT_SYSTEM_STORE_CURRENT_USER, &chainEngineConfig); - if (engine) + CertDuplicateStore( to ); + if (!(engine = CRYPT_CreateChainEngine( to, CERT_SYSTEM_STORE_CURRENT_USER, &chainEngineConfig ))) + return; + + while ((cert = CertEnumCertificatesInStore( new, cert ))) { - PCCERT_CONTEXT cert = NULL; + const DWORD allowed_errors = CERT_TRUST_IS_UNTRUSTED_ROOT | CERT_TRUST_IS_NOT_VALID_FOR_USAGE + | CERT_TRUST_INVALID_BASIC_CONSTRAINTS | CERT_TRUST_IS_NOT_TIME_VALID; + CERT_CHAIN_PARA chainPara = { sizeof(chainPara), { 0 } }; + PCCERT_CHAIN_CONTEXT chain; + BOOL ret; + + ret = CertGetCertificateChain( engine, cert, NULL, cached, &chainPara, CERT_CHAIN_CACHE_ONLY_URL_RETRIEVAL, NULL, &chain ); + if (!ret) + { + TRACE( "rejecting %s: chain creation failed.\n", get_cert_common_name( cert )); + continue; + }
- do { - cert = CertEnumCertificatesInStore(new, cert); - if (cert) - { - CERT_CHAIN_PARA chainPara = { sizeof(chainPara), { 0 } }; - PCCERT_CHAIN_CONTEXT chain; - BOOL ret; - - ret = CertGetCertificateChain(engine, cert, NULL, cached, - &chainPara, CERT_CHAIN_CACHE_ONLY_URL_RETRIEVAL, NULL, &chain); - if (!ret) - TRACE("rejecting %s: %s\n", get_cert_common_name(cert), - "chain creation failed"); - else - { - DWORD allowedErrors = CERT_TRUST_IS_UNTRUSTED_ROOT | - CERT_TRUST_IS_NOT_VALID_FOR_USAGE | - CERT_TRUST_INVALID_BASIC_CONSTRAINTS | - CERT_TRUST_IS_NOT_TIME_VALID; - - /* The certificate chain verification only allows certain - * invalid CA certs if they're installed locally: CA - * certs missing the key usage extension, and CA certs - * missing the basic constraints extension. Of course - * there's a chicken and egg problem: we have to accept - * them here in order for them to be accepted later. - * Expired, locally installed certs are also allowed here, - * because we don't know (yet) what date will be checked - * for an item signed by one of these certs. - * Thus, accept certs with any of the allowed errors. - */ - if (chain->TrustStatus.dwErrorStatus & ~allowedErrors) - TRACE("rejecting %s: %s\n", get_cert_common_name(cert), - trust_status_to_str(chain->TrustStatus.dwErrorStatus & - ~CERT_TRUST_IS_UNTRUSTED_ROOT)); - else - { - DWORD i, j; - - for (i = 0; i < chain->cChain; i++) - for (j = 0; j < chain->rgpChain[i]->cElement; j++) - if (CertAddCertificateContextToStore(to, - chain->rgpChain[i]->rgpElement[j]->pCertContext, - CERT_STORE_ADD_NEW, NULL)) - root_count++; - } - CertFreeCertificateChain(chain); - } - } - } while (cert); - CertFreeCertificateChainEngine(engine); + /* The certificate chain verification only allows certain + * invalid CA certs if they're installed locally: CA + * certs missing the key usage extension, and CA certs + * missing the basic constraints extension. Of course + * there's a chicken and egg problem: we have to accept + * them here in order for them to be accepted later. + * Expired, locally installed certs are also allowed here, + * because we don't know (yet) what date will be checked + * for an item signed by one of these certs. + * Thus, accept certs with any of the allowed errors. + */ + if (chain->TrustStatus.dwErrorStatus & ~allowed_errors) + TRACE( "rejecting %s: %s\n", get_cert_common_name(cert), + trust_status_to_str( chain->TrustStatus.dwErrorStatus & ~CERT_TRUST_IS_UNTRUSTED_ROOT )); + else if (CertAddCertificateContextToStore( to, cert, CERT_STORE_ADD_NEW, NULL )) + root_count++; + CertFreeCertificateChain(chain); } - TRACE("Added %ld root certificates\n", root_count); + CertFreeCertificateChainEngine( engine ); + TRACE( "Added %ld root certificates\n", root_count ); }
static const BYTE authenticode[] = {
From: Paul Gofman pgofman@codeweavers.com
--- dlls/crypt32/crypt32_private.h | 5 +- dlls/crypt32/regstore.c | 6 +- dlls/crypt32/rootstore.c | 145 ++++++++++++++------------------- 3 files changed, 69 insertions(+), 87 deletions(-)
diff --git a/dlls/crypt32/crypt32_private.h b/dlls/crypt32/crypt32_private.h index 827594d8489..1f0ec39d200 100644 --- a/dlls/crypt32/crypt32_private.h +++ b/dlls/crypt32/crypt32_private.h @@ -349,10 +349,11 @@ WINECRYPT_CERTSTORE *CRYPT_FileNameOpenStoreW(HCRYPTPROV hCryptProv, DWORD dwFlags, const void *pvPara);
void CRYPT_ImportSystemRootCertsToReg(void); -BOOL CRYPT_SerializeContextsToReg(HKEY key, DWORD flags, const WINE_CONTEXT_INTERFACE *contextInterface, - HCERTSTORE memStore); +BOOL CRYPT_SerializeContextToReg(HKEY key, DWORD flags, const WINE_CONTEXT_INTERFACE *context_iface, + const void *context); void CRYPT_RegReadSerializedFromReg(HKEY key, DWORD contextType, HCERTSTORE store, DWORD disposition); +void CRYPT_RegDeleteFromReg(HKEY key, const BYTE *sha1_hash);
DWORD CRYPT_IsCertificateSelfSigned(const CERT_CONTEXT *cert);
diff --git a/dlls/crypt32/regstore.c b/dlls/crypt32/regstore.c index 43ffa106c76..fbc2909dd2c 100644 --- a/dlls/crypt32/regstore.c +++ b/dlls/crypt32/regstore.c @@ -197,7 +197,7 @@ static BOOL CRYPT_WriteSerializedToReg(HKEY key, DWORD flags, const BYTE *hash, return ret; }
-static BOOL CRYPT_SerializeContextToReg(HKEY key, DWORD flags, const WINE_CONTEXT_INTERFACE *context_iface, +BOOL CRYPT_SerializeContextToReg(HKEY key, DWORD flags, const WINE_CONTEXT_INTERFACE *context_iface, const void *context) { BYTE hash[20]; @@ -226,7 +226,7 @@ static BOOL CRYPT_SerializeContextToReg(HKEY key, DWORD flags, const WINE_CONTEX return ret; }
-BOOL CRYPT_SerializeContextsToReg(HKEY key, DWORD flags, +static BOOL CRYPT_SerializeContextsToReg(HKEY key, DWORD flags, const WINE_CONTEXT_INTERFACE *contextInterface, HCERTSTORE memStore) { const void *context = NULL; @@ -241,7 +241,7 @@ BOOL CRYPT_SerializeContextsToReg(HKEY key, DWORD flags, return ret; }
-static void CRYPT_RegDeleteFromReg(HKEY key, const BYTE *sha1_hash) +void CRYPT_RegDeleteFromReg(HKEY key, const BYTE *sha1_hash) { WCHAR hash[20 * 2 + 1];
diff --git a/dlls/crypt32/rootstore.c b/dlls/crypt32/rootstore.c index 6d049af2edf..bf4804783e2 100644 --- a/dlls/crypt32/rootstore.c +++ b/dlls/crypt32/rootstore.c @@ -58,6 +58,7 @@ static const char *trust_status_to_str(DWORD status) { CERT_TRUST_HAS_EXCLUDED_NAME_CONSTRAINT, "excluded name constraint" }, { CERT_TRUST_IS_OFFLINE_REVOCATION, "revocation server offline" }, { CERT_TRUST_NO_ISSUANCE_CHAIN_POLICY, "no issuance policy" }, + { CERT_TRUST_IS_PARTIAL_CHAIN, "partial chain" }, }; static char buf[1024]; int i, pos = 0; @@ -98,7 +99,7 @@ static const char *get_cert_common_name(PCCERT_CONTEXT cert) return name; }
-static void check_and_store_certs( HCERTSTORE cached, HCERTSTORE new, HCERTSTORE to ) +static void check_and_store_certs( HCERTSTORE cached, HKEY key ) { DWORD root_count = 0; CERT_CHAIN_ENGINE_CONFIG chainEngineConfig = { sizeof(chainEngineConfig), 0 }; @@ -107,19 +108,29 @@ static void check_and_store_certs( HCERTSTORE cached, HCERTSTORE new, HCERTSTORE
TRACE("\n");
- CertDuplicateStore( to ); - if (!(engine = CRYPT_CreateChainEngine( to, CERT_SYSTEM_STORE_CURRENT_USER, &chainEngineConfig ))) + if (!(engine = CRYPT_CreateChainEngine( cached, CERT_SYSTEM_STORE_CURRENT_USER, &chainEngineConfig ))) return;
- while ((cert = CertEnumCertificatesInStore( new, cert ))) + while ((cert = CertEnumCertificatesInStore( cached, cert ))) { const DWORD allowed_errors = CERT_TRUST_IS_UNTRUSTED_ROOT | CERT_TRUST_IS_NOT_VALID_FOR_USAGE | CERT_TRUST_INVALID_BASIC_CONSTRAINTS | CERT_TRUST_IS_NOT_TIME_VALID; CERT_CHAIN_PARA chainPara = { sizeof(chainPara), { 0 } }; PCCERT_CHAIN_CONTEXT chain; + DWORD size; + int is_new; BOOL ret;
- ret = CertGetCertificateChain( engine, cert, NULL, cached, &chainPara, CERT_CHAIN_CACHE_ONLY_URL_RETRIEVAL, NULL, &chain ); + size = sizeof(is_new); + if (!CertGetCertificateContextProperty( cert, CERT_FIRST_USER_PROP_ID, &is_new, &size )) + { + ERR( "CERT_FIRST_USER_PROP_ID property absent for cert %p.\n", cert ); + continue; + } + if (!is_new) + continue; + + ret = CertGetCertificateChain( engine, cert, NULL, NULL, &chainPara, CERT_CHAIN_CACHE_ONLY_URL_RETRIEVAL, NULL, &chain ); if (!ret) { TRACE( "rejecting %s: chain creation failed.\n", get_cert_common_name( cert )); @@ -140,8 +151,13 @@ static void check_and_store_certs( HCERTSTORE cached, HCERTSTORE new, HCERTSTORE if (chain->TrustStatus.dwErrorStatus & ~allowed_errors) TRACE( "rejecting %s: %s\n", get_cert_common_name(cert), trust_status_to_str( chain->TrustStatus.dwErrorStatus & ~CERT_TRUST_IS_UNTRUSTED_ROOT )); - else if (CertAddCertificateContextToStore( to, cert, CERT_STORE_ADD_NEW, NULL )) + else + { + /* Clear custom property so it is not serialized and seen by apps. */ + CertSetCertificateContextProperty( cert, CERT_FIRST_USER_PROP_ID, 0, NULL ); + CRYPT_SerializeContextToReg( key, 0, pCertInterface, cert ); root_count++; + } CertFreeCertificateChain(chain); } CertFreeCertificateChainEngine( engine ); @@ -575,26 +591,32 @@ static const struct CONST_BLOB { { rootcertauthority2011, sizeof(rootcertauthority2011) }, };
-static void add_ms_root_certs(HCERTSTORE to, HCERTSTORE cached) +static void add_ms_root_certs(HKEY key, HCERTSTORE cached) { PCCERT_CONTEXT cert, existing; DWORD i; + int is_new; + const CRYPT_DATA_BLOB exists_blob = { sizeof(is_new), (BYTE *)&is_new };
TRACE("\n");
for (i = 0; i < ARRAY_SIZE(msRootCerts); i++) { - if (!CertAddEncodedCertificateToStore(to, X509_ASN_ENCODING, - msRootCerts[i].pb, msRootCerts[i].cb, CERT_STORE_ADD_NEW, &cert)) + if (!(cert = CertCreateCertificateContext(X509_ASN_ENCODING, msRootCerts[i].pb, msRootCerts[i].cb))) { WARN("adding root cert %ld failed: %08lx\n", i, GetLastError()); continue; } if ((existing = CertFindCertificateInStore(cached, X509_ASN_ENCODING, 0, CERT_FIND_EXISTING, cert, NULL))) { - CertDeleteCertificateFromStore(existing); + is_new = 0; + CertSetCertificateContextProperty(existing, CERT_FIRST_USER_PROP_ID, 0, &exists_blob); CertFreeCertificateContext(existing); } + else + { + CRYPT_SerializeContextToReg(key, 0, pCertInterface, cert); + } CertFreeCertificateContext(cert); } } @@ -604,9 +626,8 @@ static void add_ms_root_certs(HCERTSTORE to, HCERTSTORE cached) * adding redundant certificates, e.g. when both a certificate bundle and * individual certificates exist in the same directory. */ -static void read_trusted_roots_from_known_locations(HCERTSTORE store, HCERTSTORE cached, BOOL *delete) +static void sync_trusted_roots_from_known_locations( HKEY key, HCERTSTORE cached ) { - HCERTSTORE new; DWORD needed, size; struct enum_root_certs_params params = { NULL, 2048, &needed }; HCRYPTPROV prov; @@ -614,17 +635,11 @@ static void read_trusted_roots_from_known_locations(HCERTSTORE store, HCERTSTORE BYTE hashval[20]; DWORD hashlen; CRYPT_HASH_BLOB hash_blob = { sizeof(hashval), hashval }; - CRYPT_DATA_BLOB exists_blob = { 0, NULL }; + int is_new; + CRYPT_DATA_BLOB exists_blob = { sizeof(is_new), (BYTE *)&is_new }; PCCERT_CONTEXT cert, existing; - unsigned int existing_count = 0, new_count = 0; - unsigned int cached_count = 0; - - new = CertOpenStore( CERT_STORE_PROV_MEMORY, X509_ASN_ENCODING, 0, CERT_STORE_CREATE_NEW_FLAG, NULL ); - if (!new) return; - - existing = NULL; - while ((existing = CertEnumCertificatesInStore( cached, existing ))) - ++cached_count; + unsigned int existing_count = 0, new_count = 0, deleted_count = 0; + BYTE sha1_hash[20];
CryptAcquireContextW( &prov, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT ); params.buffer = CryptMemAlloc( params.size ); @@ -649,6 +664,7 @@ static void read_trusted_roots_from_known_locations(HCERTSTORE store, HCERTSTORE * is found among host imports. */ if (!CertGetCertificateContextProperty( existing, CERT_FIRST_USER_PROP_ID, NULL, &size )) { + is_new = 0; if (!CertSetCertificateContextProperty( existing, CERT_FIRST_USER_PROP_ID, 0, &exists_blob )) ERR( "Failed to set property.\n" ); ++existing_count; @@ -656,9 +672,9 @@ static void read_trusted_roots_from_known_locations(HCERTSTORE store, HCERTSTORE CertFreeCertificateContext( existing ); continue; } - CertAddEncodedCertificateToStore( new, X509_ASN_ENCODING, params.buffer, needed, CERT_STORE_ADD_ALWAYS, &cert ); - /* Add to cached so we can catch duplicates and check_and_store_certs() has the full chains. */ - CertAddCertificateContextToStore( cached, cert, CERT_STORE_ADD_ALWAYS, NULL ); + + CertAddEncodedCertificateToStore( cached, X509_ASN_ENCODING, params.buffer, needed, CERT_STORE_ADD_ALWAYS, &cert ); + is_new = 1; if (!CertSetCertificateContextProperty( cert, CERT_FIRST_USER_PROP_ID, 0, &exists_blob )) ERR("Failed to set property.\n"); CertFreeCertificateContext( cert ); @@ -667,61 +683,35 @@ static void read_trusted_roots_from_known_locations(HCERTSTORE store, HCERTSTORE CryptMemFree( params.buffer ); CryptReleaseContext( prov, 0 );
- if (existing_count < cached_count) + cert = NULL; + while ((cert = CertEnumCertificatesInStore( cached, cert ))) { - /* Some certs were removed on host. Clean up the cache and add all the certificates so cert chains - * get revalidated. The certs present on host are now in 'cached' store and are marked with - * CERT_FIRST_USER_PROP_ID property. */ - TRACE( "Some keys were removed, reimporting, cached %u, existing %u, new %u.\n", - cached_count, existing_count, new_count ); - *delete = TRUE; - existing_count = 0; - existing = NULL; - while ((existing = CertEnumCertificatesInStore( cached, existing ))) - { - if (!CertGetCertificateContextProperty( existing, CERT_FIRST_USER_PROP_ID, NULL, &size )) - continue; - CertAddCertificateContextToStore( new, existing, CERT_STORE_ADD_NEW, NULL ); - ++new_count; - } - } - if (new_count) - { - /* Clear custom property so it is not serialized and seen by apps. */ + if (CertGetCertificateContextProperty( cert, CERT_FIRST_USER_PROP_ID, NULL, &size )) + continue; + + size = sizeof(sha1_hash); + CertGetCertificateContextProperty( cert, CERT_HASH_PROP_ID, sha1_hash, &size ); + ++deleted_count; + CRYPT_RegDeleteFromReg( key, sha1_hash ); + /* Delete from cached so deleted certs do not participate in chain verification. */ + CertDeleteCertificateFromStore( cert ); + /* Restart enumeration as it is broken by deleting cert from store. */ + CertFreeCertificateContext( cert ); cert = NULL; - while ((cert = CertEnumCertificatesInStore( new, cert ))) - CertSetCertificateContextProperty( cert, CERT_FIRST_USER_PROP_ID, 0, NULL ); - check_and_store_certs( cached, new, store ); }
- CertCloseStore( new, 0 ); - TRACE( "existing %u, new %u.\n", existing_count, new_count ); -} - -static HCERTSTORE create_root_store(HCERTSTORE cached, BOOL *delete) -{ - HCERTSTORE memStore = CertOpenStore(CERT_STORE_PROV_MEMORY, - X509_ASN_ENCODING, 0, CERT_STORE_CREATE_NEW_FLAG, NULL); - - - *delete = FALSE; - if (memStore) - { - add_ms_root_certs(memStore, cached); - read_trusted_roots_from_known_locations(memStore, cached, delete); - } + if (new_count) + check_and_store_certs( cached, key );
- TRACE("returning %p\n", memStore); - return memStore; + TRACE( "existing %u, deleted %u, new %u.\n", existing_count, deleted_count, new_count ); }
void CRYPT_ImportSystemRootCertsToReg(void) { - HCERTSTORE store = NULL, reg = NULL; + HCERTSTORE cached = NULL; HKEY key = NULL; LONG rc; HANDLE hsem; - BOOL delete;
static BOOL root_certs_imported = FALSE;
@@ -746,27 +736,18 @@ void CRYPT_ImportSystemRootCertsToReg(void) if (rc) goto done;
- if (!(reg = CertOpenStore(CERT_STORE_PROV_MEMORY, X509_ASN_ENCODING, 0, CERT_STORE_CREATE_NEW_FLAG, NULL))) + if (!(cached = CertOpenStore(CERT_STORE_PROV_MEMORY, X509_ASN_ENCODING, 0, CERT_STORE_CREATE_NEW_FLAG, NULL))) { ERR("Failed to create memory store.\n"); goto done; } - CRYPT_RegReadSerializedFromReg(key, CERT_STORE_CERTIFICATE_CONTEXT_FLAG, reg, CERT_STORE_ADD_ALWAYS); - - if (!(store = create_root_store(reg, &delete))) - { - ERR("Failed to create root store\n"); - goto done; - } - if (delete && RegDeleteTreeW(key, NULL)) - ERR("Error deleting key.\n"); - if (!CRYPT_SerializeContextsToReg(key, 0, pCertInterface, store)) - ERR("Failed to import system certs into registry, %08lx\n", GetLastError()); + CRYPT_RegReadSerializedFromReg(key, CERT_STORE_CERTIFICATE_CONTEXT_FLAG, cached, CERT_STORE_ADD_ALWAYS); + add_ms_root_certs(key, cached); + sync_trusted_roots_from_known_locations(key, cached);
done: RegCloseKey(key); - CertCloseStore(store, 0); - CertCloseStore(reg, 0); + CertCloseStore(cached, 0); root_certs_imported = TRUE; ReleaseSemaphore(hsem, 1, NULL); CloseHandle(hsem);
From: Paul Gofman pgofman@codeweavers.com
--- dlls/crypt32/crypt32_private.h | 1 + dlls/crypt32/regstore.c | 2 +- dlls/crypt32/rootstore.c | 57 +++++++++++++++++++++++++++++++--- 3 files changed, 55 insertions(+), 5 deletions(-)
diff --git a/dlls/crypt32/crypt32_private.h b/dlls/crypt32/crypt32_private.h index 1f0ec39d200..a5aeae63168 100644 --- a/dlls/crypt32/crypt32_private.h +++ b/dlls/crypt32/crypt32_private.h @@ -354,6 +354,7 @@ BOOL CRYPT_SerializeContextToReg(HKEY key, DWORD flags, const WINE_CONTEXT_INTER void CRYPT_RegReadSerializedFromReg(HKEY key, DWORD contextType, HCERTSTORE store, DWORD disposition); void CRYPT_RegDeleteFromReg(HKEY key, const BYTE *sha1_hash); +void CRYPT_HashToStr(const BYTE *hash, LPWSTR asciiHash);
DWORD CRYPT_IsCertificateSelfSigned(const CERT_CONTEXT *cert);
diff --git a/dlls/crypt32/regstore.c b/dlls/crypt32/regstore.c index fbc2909dd2c..218a7ea3269 100644 --- a/dlls/crypt32/regstore.c +++ b/dlls/crypt32/regstore.c @@ -45,7 +45,7 @@ typedef struct _WINE_REGSTOREINFO struct list ctlsToDelete; } WINE_REGSTOREINFO;
-static void CRYPT_HashToStr(const BYTE *hash, LPWSTR asciiHash) +void CRYPT_HashToStr(const BYTE *hash, LPWSTR asciiHash) { DWORD i;
diff --git a/dlls/crypt32/rootstore.c b/dlls/crypt32/rootstore.c index bf4804783e2..e960f1d61bc 100644 --- a/dlls/crypt32/rootstore.c +++ b/dlls/crypt32/rootstore.c @@ -99,7 +99,26 @@ static const char *get_cert_common_name(PCCERT_CONTEXT cert) return name; }
-static void check_and_store_certs( HCERTSTORE cached, HKEY key ) +static void get_cert_context_hash( const CERT_CONTEXT *cert, BYTE *sha1_hash, WCHAR *hash_str ) +{ + DWORD size; + + size = 20; + CertGetCertificateContextProperty( cert, CERT_HASH_PROP_ID, sha1_hash, &size ); + CRYPT_HashToStr( sha1_hash, hash_str ); +} + +static void mark_cert_imported( HKEY import_key, const CERT_CONTEXT *cert ) +{ + WCHAR hash_str[20 * 2 + 1]; + BYTE sha1_hash[20]; + DWORD value = 1; + + get_cert_context_hash( cert, sha1_hash, hash_str ); + RegSetValueExW( import_key, hash_str, 0, REG_DWORD, (BYTE *)&value, sizeof(value) ); +} + +static void check_and_store_certs( HCERTSTORE cached, HKEY key, HKEY import_key ) { DWORD root_count = 0; CERT_CHAIN_ENGINE_CONFIG chainEngineConfig = { sizeof(chainEngineConfig), 0 }; @@ -155,6 +174,7 @@ static void check_and_store_certs( HCERTSTORE cached, HKEY key ) { /* Clear custom property so it is not serialized and seen by apps. */ CertSetCertificateContextProperty( cert, CERT_FIRST_USER_PROP_ID, 0, NULL ); + mark_cert_imported( import_key, cert ); CRYPT_SerializeContextToReg( key, 0, pCertInterface, cert ); root_count++; } @@ -640,6 +660,29 @@ static void sync_trusted_roots_from_known_locations( HKEY key, HCERTSTORE cached PCCERT_CONTEXT cert, existing; unsigned int existing_count = 0, new_count = 0, deleted_count = 0; BYTE sha1_hash[20]; + HKEY import_key; + WCHAR hash_str[20 * 2 + 1]; + DWORD value; + + if (RegOpenKeyExW( HKEY_LOCAL_MACHINE, L"Software\Wine\HostImportedCertificates", 0, KEY_ALL_ACCESS, &import_key )) + { + if (RegCreateKeyExW( HKEY_LOCAL_MACHINE, L"Software\Wine\HostImportedCertificates_tmp", 0, NULL, 0, + KEY_ALL_ACCESS, NULL, &import_key, NULL )) + return; + + /* If the key is absent existing certificates were added by an older Wine version, mark all cached certificates + * as imported (as it was previously assumed) so they can be deleted when are deleted on host. */ + cert = NULL; + while ((cert = CertEnumCertificatesInStore( cached, cert ))) + mark_cert_imported( import_key, cert ); + + if ((value = RegRenameKey( import_key, NULL, L"HostImportedCertificates" ))) + { + ERR( "Error renaming key %#lx.\n", value ); + RegCloseKey( import_key ); + return; + } + }
CryptAcquireContextW( &prov, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT ); params.buffer = CryptMemAlloc( params.size ); @@ -689,8 +732,13 @@ static void sync_trusted_roots_from_known_locations( HKEY key, HCERTSTORE cached if (CertGetCertificateContextProperty( cert, CERT_FIRST_USER_PROP_ID, NULL, &size )) continue;
- size = sizeof(sha1_hash); - CertGetCertificateContextProperty( cert, CERT_HASH_PROP_ID, sha1_hash, &size ); + get_cert_context_hash( cert, sha1_hash, hash_str ); + size = sizeof(value); + if (RegQueryValueExW( import_key, hash_str, NULL, NULL, (BYTE *)&value, &size )) + { + TRACE( "key %s is not imported, not deleting.\n", debugstr_w(hash_str) ); + continue; + } ++deleted_count; CRYPT_RegDeleteFromReg( key, sha1_hash ); /* Delete from cached so deleted certs do not participate in chain verification. */ @@ -701,8 +749,9 @@ static void sync_trusted_roots_from_known_locations( HKEY key, HCERTSTORE cached }
if (new_count) - check_and_store_certs( cached, key ); + check_and_store_certs( cached, key, import_key );
+ RegCloseKey( import_key ); TRACE( "existing %u, deleted %u, new %u.\n", existing_count, deleted_count, new_count ); }
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=150906
Your paranoid android.
=== debian11b (64 bit WoW report) ===
mfmediaengine: mfmediaengine.c:2640: Test failed: Got unexpected refcount 1.
-
Marvin -
Paul Gofman -
Paul Gofman (＠gofman)