From: Benoît Legat <benoit.legat@gmail.com> 1. PFXImport #1: CryptAcquireContextW(NULL container, CRYPT_NEWKEYSET) -> CSP creates container "blegat" (the default user name) and imports key K1 into it. set_key_prov_info then writes pwszContainerName="blegat" onto cert1. 2. PFXImport #2: same call fails with NTE_EXISTS (the default container already exists), the existing retry opens it without CRYPT_NEWKEYSET, then CryptImportKey writes K2 into that container, overwriting K1. set_key_prov_info writes pwszContainerName="blegat" onto cert2. 3. State: container "blegat" holds K2. Cert1's CRYPT_KEY_PROV_INFO.pwszContainerName still says "blegat". Anyone asking "what's cert1's private key?" reads its container name, opens "blegat", and gets K2, the second cert's key. --- dlls/crypt32/pfx.c | 50 ++++++++++++++++++++++++++-------- dlls/crypt32/tests/store.c | 56 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 12 deletions(-) diff --git a/dlls/crypt32/pfx.c b/dlls/crypt32/pfx.c index b7419e4f446..9d5952e4fcc 100644 --- a/dlls/crypt32/pfx.c +++ b/dlls/crypt32/pfx.c @@ -25,12 +25,17 @@ #include "bcrypt.h" #include "ncrypt.h" #include "snmp.h" +#include "rpc.h" #include "crypt32_private.h" #include "wine/debug.h" WINE_DEFAULT_DEBUG_CHANNEL(crypt); +typedef RPC_STATUS (RPC_ENTRY *UuidCreateFunc)( UUID * ); +typedef RPC_STATUS (RPC_ENTRY *UuidToStringFunc)( UUID *, unsigned char ** ); +typedef RPC_STATUS (RPC_ENTRY *RpcStringFreeFunc)( unsigned char ** ); + static HCRYPTPROV import_key( cert_store_data_t data, DWORD flags ) { HCRYPTPROV prov = 0; @@ -38,20 +43,36 @@ static HCRYPTPROV import_key( cert_store_data_t data, DWORD flags ) DWORD size, acquire_flags; void *key; struct import_store_key_params params = { data, NULL, &size }; - - if (CRYPT32_CALL( import_store_key, ¶ms ) != STATUS_BUFFER_TOO_SMALL) return 0; + /* Use a unique container name per import. With a NULL container + + * CRYPT_NEWKEYSET the CSP falls back to its per-user default container + * ("<USERNAME>"), so a second PFX import overwrites the first import's + * private key inside that shared container; any cert from the first + * import still held will then find the second import's key when its + * CRYPT_KEY_PROV_INFO is followed back to the container. Follow the + * same UuidCreate-via-rpcrt4 pattern that CRYPT_CreateKeyProv + * (cert.c) uses for the same problem on the synthesise-a-key path. */ + HMODULE rpcrt = LoadLibraryW( L"rpcrt4" ); + UuidCreateFunc uuidCreate; + UuidToStringFunc uuidToString; + RpcStringFreeFunc rpcStringFree; + UUID uuid; + unsigned char *uuid_str = NULL; + + if (!rpcrt) return 0; + uuidCreate = (UuidCreateFunc) GetProcAddress( rpcrt, "UuidCreate" ); + uuidToString = (UuidToStringFunc) GetProcAddress( rpcrt, "UuidToStringA" ); + rpcStringFree = (RpcStringFreeFunc)GetProcAddress( rpcrt, "RpcStringFreeA" ); + if (!uuidCreate || !uuidToString || !rpcStringFree) goto done; + if (uuidCreate( &uuid ) != RPC_S_OK && uuidCreate( &uuid ) != RPC_S_UUID_LOCAL_ONLY) goto done; + if (uuidToString( &uuid, &uuid_str ) != RPC_S_OK) goto done; + + if (CRYPT32_CALL( import_store_key, ¶ms ) != STATUS_BUFFER_TOO_SMALL) goto done; acquire_flags = (flags & CRYPT_MACHINE_KEYSET) | CRYPT_NEWKEYSET; - if (!CryptAcquireContextW( &prov, NULL, MS_ENHANCED_PROV_W, PROV_RSA_FULL, acquire_flags )) + if (!CryptAcquireContextA( &prov, (LPCSTR)uuid_str, MS_ENHANCED_PROV_A, PROV_RSA_FULL, acquire_flags )) { - if (GetLastError() != NTE_EXISTS) return 0; - - acquire_flags &= ~CRYPT_NEWKEYSET; - if (!CryptAcquireContextW( &prov, NULL, MS_ENHANCED_PROV_W, PROV_RSA_FULL, acquire_flags )) - { - WARN( "CryptAcquireContextW failed %08lx\n", GetLastError() ); - return 0; - } + WARN( "CryptAcquireContextA failed %08lx\n", GetLastError() ); + goto done; } params.buf = key = CryptMemAlloc( size ); @@ -60,11 +81,16 @@ static HCRYPTPROV import_key( cert_store_data_t data, DWORD flags ) { WARN( "CryptImportKey failed %08lx\n", GetLastError() ); CryptReleaseContext( prov, 0 ); + prov = 0; CryptMemFree( key ); - return 0; + goto done; } CryptDestroyKey( cryptkey ); CryptMemFree( key ); + +done: + if (uuid_str) rpcStringFree( &uuid_str ); + FreeLibrary( rpcrt ); return prov; } diff --git a/dlls/crypt32/tests/store.c b/dlls/crypt32/tests/store.c index 10b861675ca..81da1fad4e4 100644 --- a/dlls/crypt32/tests/store.c +++ b/dlls/crypt32/tests/store.c @@ -3541,6 +3541,61 @@ static void test_PFXImportCertStore(void) CertCloseStore( store, 0 ); } +static void test_PFXImportCertStore_unique_containers(void) +{ + /* Two persistent (CRYPT_USER_KEYSET) PFX imports of the *same* PKCS#12 + * blob must land in two *distinct* CSP key containers; otherwise the + * second import overwrites the first import's private key inside the + * shared container, and any cert still held from the first import + * then dereferences (via CRYPT_KEY_PROV_INFO) to the second import's + * key. The container-name format is internal to the importer; only + * its uniqueness across calls is observable to applications, and that + * uniqueness is what this test checks. */ + + CRYPT_DATA_BLOB pfx = { sizeof(pfxdata), (BYTE *)pfxdata }; + BYTE buf1[512], buf2[512]; + CRYPT_KEY_PROV_INFO *info1 = (CRYPT_KEY_PROV_INFO *)buf1; + CRYPT_KEY_PROV_INFO *info2 = (CRYPT_KEY_PROV_INFO *)buf2; + HCERTSTORE store1, store2; + const CERT_CONTEXT *cert1, *cert2; + DWORD size; + BOOL ret; + + store1 = PFXImportCertStore( &pfx, NULL, CRYPT_EXPORTABLE | CRYPT_USER_KEYSET ); + ok( store1 != NULL, "first PFXImportCertStore failed: %lu\n", GetLastError() ); + store2 = PFXImportCertStore( &pfx, NULL, CRYPT_EXPORTABLE | CRYPT_USER_KEYSET ); + ok( store2 != NULL, "second PFXImportCertStore failed: %lu\n", GetLastError() ); + if (!store1 || !store2) goto done; + + cert1 = CertFindCertificateInStore( store1, X509_ASN_ENCODING, 0, CERT_FIND_ANY, NULL, NULL ); + cert2 = CertFindCertificateInStore( store2, X509_ASN_ENCODING, 0, CERT_FIND_ANY, NULL, NULL ); + ok( cert1 != NULL, "cert1 lookup failed: %08lx\n", GetLastError() ); + ok( cert2 != NULL, "cert2 lookup failed: %08lx\n", GetLastError() ); + if (!cert1 || !cert2) goto done_close; + + size = sizeof(buf1); + ret = CertGetCertificateContextProperty( cert1, CERT_KEY_PROV_INFO_PROP_ID, info1, &size ); + ok( ret, "cert1 has no KEY_PROV_INFO: %08lx\n", GetLastError() ); + size = sizeof(buf2); + ret = CertGetCertificateContextProperty( cert2, CERT_KEY_PROV_INFO_PROP_ID, info2, &size ); + ok( ret, "cert2 has no KEY_PROV_INFO: %08lx\n", GetLastError() ); + if (!info1->pwszContainerName || !info2->pwszContainerName) goto done_certs; + + ok( wcscmp( info1->pwszContainerName, info2->pwszContainerName ) != 0, + "two PFX imports collided into the same container %s — the second " + "import would have overwritten the first cert's private key\n", + wine_dbgstr_w( info1->pwszContainerName ) ); + +done_certs: + CertFreeCertificateContext( cert1 ); + CertFreeCertificateContext( cert2 ); +done_close: + CertCloseStore( store1, 0 ); + CertCloseStore( store2, 0 ); +done: + ; +} + static void test_PFXExportCertStoreEx(void) { HCERTSTORE store, store2; @@ -3769,6 +3824,7 @@ START_TEST(store) test_I_UpdateStore(); test_PFXImportCertStore(); + test_PFXImportCertStore_unique_containers(); test_PFXExportCertStoreEx(); test_CryptQueryObject(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11268