[PATCH 0/2] MR3619: wintrust: Generate CRYPTCATMEMBER pIndirectData from SubjectIdentifier if cat file member has no SPC_INDIRECT_DATA attribute
Signed-off-by: Torge Matthies <openglfreak(a)googlemail.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3619
From: Torge Matthies <openglfreak(a)googlemail.com> Signed-off-by: Torge Matthies <openglfreak(a)googlemail.com> --- dlls/wintrust/tests/crypt.c | 99 +++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/dlls/wintrust/tests/crypt.c b/dlls/wintrust/tests/crypt.c index ca246776046..6f33d3a642a 100644 --- a/dlls/wintrust/tests/crypt.c +++ b/dlls/wintrust/tests/crypt.c @@ -24,6 +24,7 @@ #include "windows.h" #include "wincrypt.h" +#include "wintrust.h" #include "mscat.h" #include "wine/test.h" @@ -36,6 +37,10 @@ static CHAR catroot2[MAX_PATH]; static const WCHAR hashmeW[] = {'h','a','s','h','m','e',0}; static const WCHAR attr1W[] = {'a','t','t','r','1',0}; static const WCHAR attr2W[] = {'a','t','t','r','2',0}; +static const WCHAR member_tag[] = { + 'B','E','E','C','B','E','9','6','5','6','E','D','2','2','C','6','3','B','5','D','E','E','1','4', + '4','C','D','C','E','F','A','7','E','F','C','B','F','D','B','5',0 +}; /* * Minimalistic catalog file. To reconstruct, save text below as winetest.cdf, @@ -97,6 +102,17 @@ static const BYTE test_catalog[] = { 0x00, 0x74, 0x00, 0x74, 0x00, 0x72, 0x00, 0x31, 0x02, 0x04, 0x10, 0x01, 0x00, 0x01, 0x04, 0x0e, 0x76, 0x00, 0x61, 0x00, 0x6c, 0x00, 0x75, 0x00, 0x65, 0x00, 0x31, 0x00, 0x00, 0x00, 0x31, 0x00, }; +static const BYTE test_catalog_missing_hash[] = { + 0x30, 0x81, 0x87, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x02, 0xa0, 0x7a, + 0x30, 0x78, 0x02, 0x01, 0x01, 0x31, 0x00, 0x30, 0x6f, 0x06, 0x09, 0x2b, 0x06, 0x01, 0x04, 0x01, + 0x82, 0x37, 0x0a, 0x01, 0xa0, 0x62, 0x30, 0x60, 0x30, 0x0c, 0x06, 0x0a, 0x2b, 0x06, 0x01, 0x04, + 0x01, 0x82, 0x37, 0x0c, 0x01, 0x01, 0x04, 0x03, 0x66, 0x6f, 0x6f, 0x17, 0x0d, 0x32, 0x33, 0x30, + 0x33, 0x31, 0x37, 0x31, 0x37, 0x32, 0x35, 0x33, 0x30, 0x5a, 0x30, 0x0e, 0x06, 0x0a, 0x2b, 0x06, + 0x01, 0x04, 0x01, 0x82, 0x37, 0x0c, 0x01, 0x03, 0x05, 0x00, 0x30, 0x2c, 0x30, 0x2a, 0x04, 0x14, + 0xbe, 0xec, 0xbe, 0x96, 0x56, 0xed, 0x22, 0xc6, 0x3b, 0x5d, 0xee, 0x14, 0x4c, 0xdc, 0xef, 0xa7, + 0xef, 0xcb, 0xfd, 0xb5, 0x31, 0x12, 0x30, 0x10, 0x06, 0x0a, 0x2b, 0x06, 0x01, 0x04, 0x01, 0x82, + 0x37, 0x0c, 0x02, 0x03, 0x31, 0x02, 0x82, 0x00, 0x31, 0x00 +}; static BOOL (WINAPI * pCryptCATAdminAcquireContext)(HCATADMIN*, const GUID*, DWORD); static BOOL (WINAPI * pCryptCATAdminReleaseContext)(HCATADMIN, DWORD); @@ -1336,6 +1352,88 @@ static void test_sip(void) DeleteFileW(nameW); } +static void test_catalog_generated_indirect_data(void) +{ + static const GUID subject = {0xde351a42,0x8e59,0x11d0,{0x8c,0x47,0x00,0xc0,0x4f,0xc2,0x95,0xee}}; + + HANDLE file; + DWORD written; + HANDLE hcat; + CRYPTCATMEMBER *m; + CRYPTCATATTRIBUTE *attr; + char catalog[MAX_PATH]; + WCHAR catalogW[MAX_PATH]; + BOOL ret; + int attrcount = 0, membercount = 0; + + if (!GetTempFileNameA(CURR_DIR, "cat", 0, catalog)) return; + file = CreateFileA(catalog, GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, NULL); + ok(file != INVALID_HANDLE_VALUE, "CreateFileA failed %lu\n", GetLastError()); + WriteFile(file, test_catalog_missing_hash, sizeof(test_catalog_missing_hash), &written, NULL); + CloseHandle(file); + + MultiByteToWideChar(CP_ACP, 0, catalog, -1, catalogW, MAX_PATH); + + hcat = pCryptCATOpen(catalogW, 0, 0, 0, 0); + ok(hcat != INVALID_HANDLE_VALUE, "CryptCATOpen failed %lu\n", GetLastError()); + + m = NULL; + while ((m = pCryptCATEnumerateMember(hcat, m))) + { + ok(m->cbStruct == sizeof(CRYPTCATMEMBER), "unexpected size %lu\n", m->cbStruct); + ok(!lstrcmpW(m->pwszReferenceTag, member_tag), "unexpected tag\n"); + ok(!memcmp(&m->gSubjectType, &subject, sizeof(subject)), "guid differs\n"); + ok(!m->fdwMemberFlags, "got %lx expected 0\n", m->fdwMemberFlags); + todo_wine ok(m->dwCertVersion == 0x200, "got %lx expected 0x200\n", m->dwCertVersion); + ok(!m->dwReserved, "got %lx expected 0\n", m->dwReserved); + ok(m->hReserved == NULL, "got %p expected NULL\n", m->hReserved); + + ok(m->sEncodedIndirectData.cbData == 0, "got %lu expected 0\n", m->sEncodedIndirectData.cbData); + ok(m->sEncodedIndirectData.pbData == NULL, "got %p expected NULL\n", m->sEncodedIndirectData.pbData); + ok(m->pIndirectData != NULL, "expected non-NULL\n"); + ok(m->pIndirectData->Data.pszObjId != NULL, "expected non-NULL\n"); + ok(!strcmp(m->pIndirectData->Data.pszObjId, SPC_CAB_DATA_OBJID), "got %s expected %s\n", + m->pIndirectData->Data.pszObjId, SPC_CAB_DATA_OBJID); + /* Native wintrust bug: The generated SPC_LINK is not encoded even though it should be. */ + ok(m->pIndirectData->Data.Value.cbData == sizeof(SPC_LINK), "got %lu expected %Iu\n", + m->pIndirectData->Data.Value.cbData, sizeof(SPC_LINK)); + ok(m->pIndirectData->Data.Value.pbData != NULL, "expected non-NULL\n"); + if (m->pIndirectData->Data.Value.pbData) + { + SPC_LINK *link = (void*)m->pIndirectData->Data.Value.pbData; + ok(link->dwLinkChoice == SPC_FILE_LINK_CHOICE, "got %lx expected %d\n", link->dwLinkChoice, + SPC_FILE_LINK_CHOICE); + ok(link->pwszFile != NULL, "expected non-NULL\n"); + if (link->pwszFile) + ok(link->pwszFile[0] == 0, "got %Iu-char string expected empty string\n", wcslen(link->pwszFile)); + } + ok(!strcmp(m->pIndirectData->DigestAlgorithm.pszObjId, szOID_OIWSEC_sha1), "got %s expected %s\n", + m->pIndirectData->DigestAlgorithm.pszObjId, szOID_OIWSEC_sha1); + ok(m->pIndirectData->DigestAlgorithm.Parameters.cbData == 0, "got %lu expected 0\n", + m->pIndirectData->DigestAlgorithm.Parameters.cbData); + ok(m->pIndirectData->DigestAlgorithm.Parameters.pbData == NULL, "got %p expected NULL\n", + m->pIndirectData->DigestAlgorithm.Parameters.pbData); + ok(m->pIndirectData->Digest.cbData == 20, "got %lu expected 20\n", m->pIndirectData->Digest.cbData); + ok(m->pIndirectData->Digest.pbData != NULL, "expected non-NULL\n"); + + attr = pCryptCATEnumerateAttr(hcat, m, NULL); + ok(attr == NULL, "CryptCATEnumerateAttr succeeded\n"); + + membercount++; + } + todo_wine + ok(membercount == 1, "Expected 1 member, got %d\n", membercount); + + attr = NULL; + while ((attr = pCryptCATEnumerateCatAttr(hcat, attr))) + attrcount++; + ok(attrcount == 0, "Expected no catalog attributes, got %d\n", attrcount); + + ret = pCryptCATClose(hcat); + ok(ret, "CryptCATClose failed\n"); + DeleteFileA( catalog ); +} + START_TEST(crypt) { char** myARGV; @@ -1371,4 +1469,5 @@ START_TEST(crypt) test_create_catalog_file(); test_CryptCATAdminAddRemoveCatalog(); test_sip(); + test_catalog_generated_indirect_data(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3619
From: Torge Matthies <openglfreak(a)googlemail.com> Native does this when the SubjectIdentifier has the correct size for an SHA1 hash. Note that pIndirectData->Data.Value is supposed to point to an encoded blob, but in this case it points to the raw unencoded SPC_LINK structure. This seems to be a bug in native wintrust. Signed-off-by: Torge Matthies <openglfreak(a)googlemail.com> --- dlls/wintrust/crypt.c | 27 ++++++++++++++++++++++++++- dlls/wintrust/tests/crypt.c | 1 - 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/dlls/wintrust/crypt.c b/dlls/wintrust/crypt.c index ac3ac9251f1..705d4ff4e8b 100644 --- a/dlls/wintrust/crypt.c +++ b/dlls/wintrust/crypt.c @@ -757,7 +757,32 @@ CRYPTCATMEMBER * WINAPI CryptCATEnumerateMember(HANDLE hCatalog, CRYPTCATMEMBER FIXME("unhandled object id \"%s\"\n", attr->pszObjId); } - if (!member->sEncodedMemberInfo.cbData || !member->sEncodedIndirectData.cbData) + if (!member->pIndirectData && entry->SubjectIdentifier.cbData == 20) + { + SPC_LINK *link; + + member->sEncodedIndirectData.cbData = 0; + member->sEncodedIndirectData.pbData = NULL; + + size = sizeof(SIP_INDIRECT_DATA) + sizeof(SPC_LINK); + if (!(member->pIndirectData = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, size))) + { + SetLastError(ERROR_OUTOFMEMORY); + goto error; + } + + link = (void*)(member->pIndirectData + 1); + link->dwLinkChoice = SPC_FILE_LINK_CHOICE; + link->pwszFile = (LPWSTR)L""; + + member->pIndirectData->Data.pszObjId = (LPSTR)SPC_CAB_DATA_OBJID; + member->pIndirectData->Data.Value.cbData = sizeof(SPC_LINK); + member->pIndirectData->Data.Value.pbData = (BYTE*)link; + member->pIndirectData->DigestAlgorithm.pszObjId = (LPSTR)szOID_OIWSEC_sha1; + member->pIndirectData->Digest = entry->SubjectIdentifier; + } + + if (!member->sEncodedMemberInfo.cbData || !member->pIndirectData) { ERR("Corrupted catalog entry?\n"); SetLastError(CRYPT_E_ATTRIBUTES_MISSING); diff --git a/dlls/wintrust/tests/crypt.c b/dlls/wintrust/tests/crypt.c index 6f33d3a642a..5759953827b 100644 --- a/dlls/wintrust/tests/crypt.c +++ b/dlls/wintrust/tests/crypt.c @@ -1421,7 +1421,6 @@ static void test_catalog_generated_indirect_data(void) membercount++; } - todo_wine ok(membercount == 1, "Expected 1 member, got %d\n", membercount); attr = NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3619
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=136512 Your paranoid android. === w7u_2qxl (32 bit report) === wintrust: crypt.c:1408: Test failed: got 14-char string expected empty string === w7u_adm (32 bit report) === wintrust: crypt.c:1408: Test failed: got 14-char string expected empty string === w7u_el (32 bit report) === wintrust: crypt.c:1408: Test failed: got 14-char string expected empty string === w7pro64 (64 bit report) === wintrust: crypt.c:1408: Test failed: got 14-char string expected empty string === debian11 (32 bit report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11 (32 bit ar:MA report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11 (32 bit de report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11 (32 bit fr report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11 (32 bit he:IL report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11 (32 bit hi:IN report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11 (32 bit ja:JP report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11 (32 bit zh:CN report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11b (32 bit WoW report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11b (64 bit WoW report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0
On Mon Aug 21 04:33:57 2023 +0000, **** wrote:
Marvin replied on the mailing list: ``` 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=136512 Your paranoid android. === w7u_2qxl (32 bit report) === wintrust: crypt.c:1408: Test failed: got 14-char string expected empty string === w7u_adm (32 bit report) === wintrust: crypt.c:1408: Test failed: got 14-char string expected empty string === w7u_el (32 bit report) === wintrust: crypt.c:1408: Test failed: got 14-char string expected empty string === w7pro64 (64 bit report) === wintrust: crypt.c:1408: Test failed: got 14-char string expected empty string === debian11 (32 bit report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11 (32 bit ar:MA report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11 (32 bit de report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11 (32 bit fr report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11 (32 bit he:IL report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11 (32 bit hi:IN report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11 (32 bit ja:JP report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11 (32 bit zh:CN report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11b (32 bit WoW report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 === debian11b (64 bit WoW report) === wintrust: crypt.c:1424: Test failed: Expected 1 member, got 0 ``` Sorry, looks like the tests here depend on another commit that I still need to submit.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3619#note_42833
participants (3)
-
Marvin -
Torge Matthies -
Torge Matthies (＠tmatthies)