[PATCH v2 0/2] MR10521: Certsetcertificatecontextproperty cert ncrypt key handle prop id
This is part of my attempt to run the [Niko Home Control programming software](https://appdb.winehq.org/objectManager.php?sClass=application&iId=21635) under Wine. This (.NET 8 / Electron app) is performing mTLS with a [Connected controller voor Niko Home Control II](https://products.niko.eu/nl-be/article/550-00003). Doing this, it calls `CertSetCertificateContextProperty` with `CERT_NCRYPT_KEY_HANDLE_PROP_ID`. Wine currently does not implement setting this property so it falls through to the default case, with the \``FIXME("%ld: stub\n", dwPropId);` that lead me to try to implement it. This patch seems to work for the Niko Home Control app so I added test to contribute it upstream. According to [the documentation](https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cer...), `dwKeySpec` should be set to `CERT_NCRYPT_KEY_SPEC`. This value didn't seem to be defined yet in Wine so I added its definition. [Its value seems to be \`0xffffffff\`](https://android.googlesource.com/toolchain/mingw/+/refs/heads/main/mingw-w64...). I added tests. Before the changes in `dlls/crypt32/cert.c`, running the tests with `wine 11.5-1` (current version on ArchLinux) the tests give 7 failures: ``` $ wine dlls/crypt32/tests/x86_64-windows/crypt32_test-stripped.exe cert 0024:fixme:crypt:add_cert_to_store Unimplemented add disposition 0 0024:fixme:crypt:add_cert_to_store Unimplemented add disposition 0 0024:fixme:crypt:Collection_release Unimplemented flags 2 0024:fixme:crypt:CertContext_SetProperty 78: stub cert.c:553: Test failed: CertSetCertificateContextProperty failed: 80092004 cert.c:559: Test failed: CertGetCertificateContextProperty failed: 80092004 cert.c:560: Test failed: Expected hCryptProv 0xBEEF1234, got 0 cert.c:562: Test failed: Expected dwKeySpec CERT_NCRYPT_KEY_SPEC, got 2 cert.c:569: Test failed: CertGetCertificateContextProperty failed: 80092004 cert.c:570: Test failed: Expected handle 0xBEEF1234, got 0 0024:fixme:crypt:CertContext_SetProperty 78: stub cert.c:587: Test failed: CertSetCertificateContextProperty failed: 80092004 0024:fixme:crypt:CertAddCertificateLinkToStore (00007FFFFE871940, 00007FFFFE8530D8, 00000001, 00007FFFFE1FFB50): semi-stub 0024:fixme:crypt:Collection_release Unimplemented flags 2 0024:fixme:crypt:ProvStore_release Unimplemented flags 2 0024:fixme:crypt:CRYPT_RegCloseStore Unimplemented flags: 00000002 0024:fixme:crypt:ProvStore_release Unimplemented flags 2 0024:fixme:crypt:CRYPT_RegCloseStore Unimplemented flags: 00000002 0024:fixme:bcrypt:BCryptOpenAlgorithmProvider algorithm L"_SHOULDNOTEXIST_" not supported 0024:fixme:cryptasn:CryptDecodeObjectEx Unimplemented decoder for lpszStructType OID 1 0024:fixme:cryptasn:CryptDecodeObjectEx Unimplemented decoder for lpszStructType OID 1 0024:fixme:crypt:CryptVerifyCertificateSignatureEx unimplemented for NULL signer 0024:fixme:cryptasn:CRYPT_GetBuiltinEncoder Unimplemented encoder for lpszStructType OID 0 0024:fixme:cryptasn:CryptDecodeObjectEx Unimplemented decoder for lpszStructType OID 1 0024:fixme:crypt:CertGetPublicKeyLength unimplemented for DH public keys 0024:fixme:cryptasn:CryptDecodeObjectEx Unimplemented decoder for lpszStructType OID 7 0020:cert: 649 tests executed (0 marked as todo, 0 as flaky, 7 failures), 0 skipped. ``` After applying my patches like so ``` sudo cp dlls/crypt32/x86_64-windows/crypt32.dll /usr/lib/wine/x86_64-windows/crypt32.dll sudo cp dlls/crypt32/crypt32.so /usr/lib/wine/x86_64-unix/crypt32.so ``` I get 0 failures: ``` $ wine dlls/crypt32/tests/x86_64-windows/crypt32_test-stripped.exe cert 0024:fixme:crypt:add_cert_to_store Unimplemented add disposition 0 0024:fixme:crypt:add_cert_to_store Unimplemented add disposition 0 0024:fixme:crypt:Collection_release Unimplemented flags 2 0024:fixme:crypt:CertAddCertificateLinkToStore (00007FFFFE872F70, 00007FFFFE854988, 00000001, 00007FFFFE1FFB50): semi-stub 0024:fixme:crypt:Collection_release Unimplemented flags 2 0024:fixme:crypt:ProvStore_release Unimplemented flags 2 0024:fixme:crypt:CRYPT_RegCloseStore Unimplemented flags: 00000002 0024:fixme:crypt:ProvStore_release Unimplemented flags 2 0024:fixme:crypt:CRYPT_RegCloseStore Unimplemented flags: 00000002 0024:fixme:bcrypt:BCryptOpenAlgorithmProvider algorithm L"_SHOULDNOTEXIST_" not supported 0024:fixme:cryptasn:CryptDecodeObjectEx Unimplemented decoder for lpszStructType OID 1 0024:fixme:cryptasn:CryptDecodeObjectEx Unimplemented decoder for lpszStructType OID 1 0024:fixme:crypt:CryptVerifyCertificateSignatureEx unimplemented for NULL signer 0024:fixme:cryptasn:CRYPT_GetBuiltinEncoder Unimplemented encoder for lpszStructType OID 0 0024:fixme:cryptasn:CryptDecodeObjectEx Unimplemented decoder for lpszStructType OID 1 0024:fixme:crypt:CertGetPublicKeyLength unimplemented for DH public keys 0024:fixme:cryptasn:CryptDecodeObjectEx Unimplemented decoder for lpszStructType OID 7 0020:cert: 649 tests executed (0 marked as todo, 0 as flaky, 0 failures), 0 skipped. ``` Let me know if there is anything else I can do to help, this is my first contribution to this project. -- v2: Try fixing the tests to match Windows behavior https://gitlab.winehq.org/wine/wine/-/merge_requests/10521
From: Benoît Legat <benoit.legat@gmail.com> --- dlls/crypt32/cert.c | 51 ++++++++++++++++++++++- dlls/crypt32/tests/cert.c | 88 +++++++++++++++++++++++++++++++++++++++ include/wincrypt.h | 1 + 3 files changed, 138 insertions(+), 2 deletions(-) diff --git a/dlls/crypt32/cert.c b/dlls/crypt32/cert.c index 11e436b48c7..08517456112 100644 --- a/dlls/crypt32/cert.c +++ b/dlls/crypt32/cert.c @@ -652,8 +652,38 @@ BOOL WINAPI CertGetCertificateContextProperty(PCCERT_CONTEXT pCertContext, ret = CertContext_GetProperty(cert, CERT_KEY_CONTEXT_PROP_ID, &keyContext, &size); if (ret) - ret = CertContext_CopyParam(pvData, pcbData, &keyContext.hCryptProv, - sizeof(keyContext.hCryptProv)); + { + if (keyContext.dwKeySpec == CERT_NCRYPT_KEY_SPEC) + { + SetLastError(CRYPT_E_NOT_FOUND); + ret = FALSE; + } + else + ret = CertContext_CopyParam(pvData, pcbData, &keyContext.hCryptProv, + sizeof(keyContext.hCryptProv)); + } + break; + } + case CERT_NCRYPT_KEY_HANDLE_PROP_ID: + { + CERT_KEY_CONTEXT keyContext; + DWORD size = sizeof(keyContext); + + ret = CertContext_GetProperty(cert, + CERT_KEY_CONTEXT_PROP_ID, &keyContext, &size); + if (ret) + { + if (keyContext.dwKeySpec != CERT_NCRYPT_KEY_SPEC) + { + SetLastError(CRYPT_E_NOT_FOUND); + ret = FALSE; + } + else + ret = CertContext_CopyParam(pvData, pcbData, &keyContext.hCryptProv, + sizeof(keyContext.hCryptProv)); + } + else + SetLastError(CRYPT_E_NOT_FOUND); break; } case CERT_KEY_PROV_INFO_PROP_ID: @@ -838,6 +868,23 @@ static BOOL CertContext_SetProperty(cert_t *cert, DWORD dwPropId, 0, &keyContext); break; } + case CERT_NCRYPT_KEY_HANDLE_PROP_ID: + { + CERT_KEY_CONTEXT keyContext; + + if (!pvData) + { + ContextPropertyList_RemoveProperty(cert->base.properties, + CERT_KEY_CONTEXT_PROP_ID); + ret = TRUE; + break; + } + keyContext.cbSize = sizeof(keyContext); + keyContext.hCryptProv = *(const HCRYPTPROV *)pvData; + keyContext.dwKeySpec = CERT_NCRYPT_KEY_SPEC; + ret = CertContext_SetKeyContextProperty(cert->base.properties, &keyContext); + break; + } default: FIXME("%ld: stub\n", dwPropId); ret = FALSE; diff --git a/dlls/crypt32/tests/cert.c b/dlls/crypt32/tests/cert.c index aeab4c02713..a82b83f54be 100644 --- a/dlls/crypt32/tests/cert.c +++ b/dlls/crypt32/tests/cert.c @@ -529,6 +529,94 @@ static void testCertProperties(void) GetLastError()); ok(keyContext.hCryptProv == 0, "Expected no hCryptProv\n"); + /* Remove the key context so we start fresh for NCrypt tests */ + ret = CertSetCertificateContextProperty(context, CERT_KEY_CONTEXT_PROP_ID, + 0, NULL); + ok(ret, "CertSetCertificateContextProperty failed: %08lx\n", GetLastError()); + + /* Test CERT_NCRYPT_KEY_HANDLE_PROP_ID (78) */ + /* Getting it when no key context is set should fail */ + size = sizeof(HCRYPTPROV); + SetLastError(0xdeadbeef); + ret = CertGetCertificateContextProperty(context, + CERT_NCRYPT_KEY_HANDLE_PROP_ID, NULL, &size); + ok(!ret && GetLastError() == CRYPT_E_NOT_FOUND, + "Expected CRYPT_E_NOT_FOUND, got %08lx\n", GetLastError()); + + /* Set an NCrypt key handle via property 78 */ + { + HCRYPTPROV_OR_NCRYPT_KEY_HANDLE ncryptHandle = 0xBEEF1234; + HCRYPTPROV_OR_NCRYPT_KEY_HANDLE retrievedHandle = 0; + + ret = CertSetCertificateContextProperty(context, + CERT_NCRYPT_KEY_HANDLE_PROP_ID, 0, &ncryptHandle); + ok(ret, "CertSetCertificateContextProperty failed: %08lx\n", GetLastError()); + + /* Verify the key context was set with CERT_NCRYPT_KEY_SPEC */ + size = sizeof(keyContext); + ret = CertGetCertificateContextProperty(context, + CERT_KEY_CONTEXT_PROP_ID, &keyContext, &size); + ok(ret, "CertGetCertificateContextProperty failed: %08lx\n", GetLastError()); + ok(keyContext.hCryptProv == 0xBEEF1234, + "Expected hCryptProv 0xBEEF1234, got %Ix\n", keyContext.hCryptProv); + ok(keyContext.dwKeySpec == CERT_NCRYPT_KEY_SPEC, + "Expected dwKeySpec CERT_NCRYPT_KEY_SPEC, got %lx\n", keyContext.dwKeySpec); + + /* Get the NCrypt handle back via property 78 */ + size = sizeof(retrievedHandle); + ret = CertGetCertificateContextProperty(context, + CERT_NCRYPT_KEY_HANDLE_PROP_ID, &retrievedHandle, &size); + ok(ret, "CertGetCertificateContextProperty failed: %08lx\n", GetLastError()); + ok(retrievedHandle == 0xBEEF1234, + "Expected handle 0xBEEF1234, got %Ix\n", retrievedHandle); + ok(size == sizeof(retrievedHandle), + "Expected size %Iu, got %lu\n", sizeof(retrievedHandle), size); + + /* Getting CERT_KEY_PROV_HANDLE_PROP_ID should fail since this is an NCrypt key */ + size = sizeof(retrievedHandle); + SetLastError(0xdeadbeef); + ret = CertGetCertificateContextProperty(context, + CERT_KEY_PROV_HANDLE_PROP_ID, &retrievedHandle, &size); + ok(!ret && GetLastError() == CRYPT_E_NOT_FOUND, + "Expected CRYPT_E_NOT_FOUND for CAPI handle on NCrypt key, got %08lx\n", + GetLastError()); + + /* Delete the NCrypt key handle by setting property 78 to NULL */ + ret = CertSetCertificateContextProperty(context, + CERT_NCRYPT_KEY_HANDLE_PROP_ID, 0, NULL); + ok(ret, "CertSetCertificateContextProperty failed: %08lx\n", GetLastError()); + + /* Verify it was deleted */ + size = sizeof(retrievedHandle); + SetLastError(0xdeadbeef); + ret = CertGetCertificateContextProperty(context, + CERT_NCRYPT_KEY_HANDLE_PROP_ID, &retrievedHandle, &size); + ok(!ret && GetLastError() == CRYPT_E_NOT_FOUND, + "Expected CRYPT_E_NOT_FOUND after delete, got %08lx\n", GetLastError()); + } + + /* Test that CERT_NCRYPT_KEY_HANDLE_PROP_ID fails for non-NCrypt key contexts */ + keyContext.cbSize = sizeof(keyContext); + keyContext.hCryptProv = 0xCAFE; + keyContext.dwKeySpec = AT_KEYEXCHANGE; + ret = CertSetCertificateContextProperty(context, CERT_KEY_CONTEXT_PROP_ID, + 0, &keyContext); + ok(ret, "CertSetCertificateContextProperty failed: %08lx\n", GetLastError()); + { + HCRYPTPROV_OR_NCRYPT_KEY_HANDLE retrievedHandle = 0xdeadbeef; + + size = sizeof(retrievedHandle); + SetLastError(0xdeadbeef); + ret = CertGetCertificateContextProperty(context, + CERT_NCRYPT_KEY_HANDLE_PROP_ID, &retrievedHandle, &size); + ok(!ret && GetLastError() == CRYPT_E_NOT_FOUND, + "Expected CRYPT_E_NOT_FOUND for CAPI key, got %08lx\n", GetLastError()); + } + /* Clean up */ + ret = CertSetCertificateContextProperty(context, CERT_KEY_CONTEXT_PROP_ID, + 0, NULL); + ok(ret, "CertSetCertificateContextProperty failed: %08lx\n", GetLastError()); + /* According to MSDN the subject key id can be stored as a property, * as a subject key extension, or as the SHA1 hash of the public key, * but this cert has none of them: diff --git a/include/wincrypt.h b/include/wincrypt.h index 9c42586700f..6ee077cd552 100644 --- a/include/wincrypt.h +++ b/include/wincrypt.h @@ -1942,6 +1942,7 @@ static const WCHAR MS_ENH_RSA_AES_PROV_XP_W[] = { 'M','i','c','r','o','s','o','f /* Key Specs*/ #define AT_KEYEXCHANGE 1 #define AT_SIGNATURE 2 +#define CERT_NCRYPT_KEY_SPEC 0xffffffff /* Provider Types */ #define PROV_RSA_FULL 1 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10521
From: Benoît Legat <benoit.legat@gmail.com> --- dlls/crypt32/tests/cert.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/crypt32/tests/cert.c b/dlls/crypt32/tests/cert.c index a82b83f54be..a50c4bdcb98 100644 --- a/dlls/crypt32/tests/cert.c +++ b/dlls/crypt32/tests/cert.c @@ -557,8 +557,8 @@ static void testCertProperties(void) ret = CertGetCertificateContextProperty(context, CERT_KEY_CONTEXT_PROP_ID, &keyContext, &size); ok(ret, "CertGetCertificateContextProperty failed: %08lx\n", GetLastError()); - ok(keyContext.hCryptProv == 0xBEEF1234, - "Expected hCryptProv 0xBEEF1234, got %Ix\n", keyContext.hCryptProv); + ok(keyContext.hCryptProv != 0, + "Expected non-zero hCryptProv, got 0\n"); ok(keyContext.dwKeySpec == CERT_NCRYPT_KEY_SPEC, "Expected dwKeySpec CERT_NCRYPT_KEY_SPEC, got %lx\n", keyContext.dwKeySpec); @@ -567,8 +567,8 @@ static void testCertProperties(void) ret = CertGetCertificateContextProperty(context, CERT_NCRYPT_KEY_HANDLE_PROP_ID, &retrievedHandle, &size); ok(ret, "CertGetCertificateContextProperty failed: %08lx\n", GetLastError()); - ok(retrievedHandle == 0xBEEF1234, - "Expected handle 0xBEEF1234, got %Ix\n", retrievedHandle); + ok(retrievedHandle == keyContext.hCryptProv, + "Expected handle %Ix, got %Ix\n", keyContext.hCryptProv, retrievedHandle); ok(size == sizeof(retrievedHandle), "Expected size %Iu, got %lu\n", sizeof(retrievedHandle), size); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10521
Hans Leidekker (@hans) commented about dlls/crypt32/tests/cert.c:
+ CERT_KEY_CONTEXT_PROP_ID, &keyContext, &size); + ok(ret, "CertGetCertificateContextProperty failed: %08lx\n", GetLastError()); + ok(keyContext.hCryptProv == 0xBEEF1234, + "Expected hCryptProv 0xBEEF1234, got %Ix\n", keyContext.hCryptProv); + ok(keyContext.dwKeySpec == CERT_NCRYPT_KEY_SPEC, + "Expected dwKeySpec CERT_NCRYPT_KEY_SPEC, got %lx\n", keyContext.dwKeySpec); + + /* Get the NCrypt handle back via property 78 */ + size = sizeof(retrievedHandle); + ret = CertGetCertificateContextProperty(context, + CERT_NCRYPT_KEY_HANDLE_PROP_ID, &retrievedHandle, &size); + ok(ret, "CertGetCertificateContextProperty failed: %08lx\n", GetLastError()); + ok(retrievedHandle == 0xBEEF1234, + "Expected handle 0xBEEF1234, got %Ix\n", retrievedHandle); + ok(size == sizeof(retrievedHandle), + "Expected size %Iu, got %lu\n", sizeof(retrievedHandle), size); You should initialize the fields before the call:
/* Verify the key context was set with CERT_NCRYPT_KEY_SPEC */
size = sizeof(keyContext);
keyContext.hCryptProv = keyContext.dwKeySpec = 0;
ret = CertGetCertificateContextProperty(context,
CERT_KEY_CONTEXT_PROP_ID, &keyContext, &size);
ok(ret, "CertGetCertificateContextProperty failed: %08lx\n", GetLastError());
ok(keyContext.hCryptProv != 0, "Expected non-zero hCryptProv, got 0\n");
ok(keyContext.dwKeySpec == CERT_NCRYPT_KEY_SPEC,
"Expected dwKeySpec CERT_NCRYPT_KEY_SPEC, got %lx\n", keyContext.dwKeySpec);
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10521#note_134526
Looks good, thanks. Can you please squash the commits? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10521#note_134527
participants (3)
-
Benoît Legat -
Benoît Legat (＠blegat)
-
Hans Leidekker (＠hans)