[PATCH v3 0/1] MR5516: wintrust: Add support for the PE image hash in CryptCATAdminCalcHashFromFileHandle().
-- v3: wintrust: Add support for the PE image hash in CryptCATAdminCalcHashFromFileHandle(). https://gitlab.winehq.org/wine/wine/-/merge_requests/5516
From: Hans Leidekker <hans(a)codeweavers.com> --- dlls/wintrust/crypt.c | 76 +++++++++++++++++++++++++++++++++-- dlls/wintrust/tests/softpub.c | 34 ++++++++++++++++ 2 files changed, 106 insertions(+), 4 deletions(-) diff --git a/dlls/wintrust/crypt.c b/dlls/wintrust/crypt.c index 7321b84eea4..1be27fed856 100644 --- a/dlls/wintrust/crypt.c +++ b/dlls/wintrust/crypt.c @@ -214,11 +214,75 @@ HCATINFO WINAPI CryptCATAdminAddCatalog(HCATADMIN catAdmin, PWSTR catalogFile, return ci; } +static BOOL pe_image_hash( HANDLE file, HCRYPTHASH hash ) +{ + UINT32 size, offset, file_size, header_size, sig_pos; + HANDLE mapping; + BYTE *view; + IMAGE_NT_HEADERS *nt; + BOOL ret = FALSE; + + if ((file_size = GetFileSize( file, NULL )) == INVALID_FILE_SIZE) return FALSE; + + if ((mapping = CreateFileMappingW( file, NULL, PAGE_READONLY, 0, 0, NULL )) == INVALID_HANDLE_VALUE) + return FALSE; + + if (!(view = MapViewOfFile( mapping, FILE_MAP_READ, 0, 0, 0 )) || !(nt = ImageNtHeader( view ))) goto done; + header_size = (BYTE *)nt - view; + + if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) + { + const IMAGE_NT_HEADERS64 *nt64 = (const IMAGE_NT_HEADERS64 *)nt; + + /* offset from start of file to checksum */ + offset = header_size + FIELD_OFFSET( IMAGE_NT_HEADERS64, OptionalHeader.CheckSum ); + + /* area between checksum and security directory entry */ + size = FIELD_OFFSET( IMAGE_OPTIONAL_HEADER64, DataDirectory[IMAGE_DIRECTORY_ENTRY_SECURITY] ) - + FIELD_OFFSET( IMAGE_OPTIONAL_HEADER64, Subsystem ); + + sig_pos = nt64->OptionalHeader.DataDirectory[IMAGE_FILE_SECURITY_DIRECTORY].VirtualAddress; + } + else if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) + { + const IMAGE_NT_HEADERS32 *nt32 = (const IMAGE_NT_HEADERS32 *)nt; + + /* offset from start of file to checksum */ + offset = header_size + FIELD_OFFSET( IMAGE_NT_HEADERS32, OptionalHeader.CheckSum ); + + /* area between checksum and security directory entry */ + size = FIELD_OFFSET( IMAGE_OPTIONAL_HEADER32, DataDirectory[IMAGE_DIRECTORY_ENTRY_SECURITY] ) - + FIELD_OFFSET( IMAGE_OPTIONAL_HEADER32, Subsystem ); + + sig_pos = nt32->OptionalHeader.DataDirectory[IMAGE_FILE_SECURITY_DIRECTORY].VirtualAddress; + } + else goto done; + + if (!CryptHashData( hash, view, offset, 0 )) goto done; + offset += sizeof(DWORD); /* skip checksum */ + if (!CryptHashData( hash, view + offset, size, 0 )) goto done; + + offset += size + sizeof(IMAGE_DATA_DIRECTORY); /* skip security entry */ + size = sig_pos ? sig_pos - offset : file_size - offset; /* exclude signature */ + if (!CryptHashData( hash, view + offset, size, 0 )) goto done; + ret = TRUE; + + if (!sig_pos && (size = file_size % 8)) + { + static const BYTE pad[7]; + ret = CryptHashData( hash, pad, 8 - size, 0 ); + } + +done: + UnmapViewOfFile( view ); + CloseHandle( mapping ); + return ret; +} + /*********************************************************************** * CryptCATAdminCalcHashFromFileHandle (WINTRUST.@) */ -BOOL WINAPI CryptCATAdminCalcHashFromFileHandle(HANDLE hFile, DWORD* pcbHash, - BYTE* pbHash, DWORD dwFlags ) +BOOL WINAPI CryptCATAdminCalcHashFromFileHandle(HANDLE hFile, DWORD *pcbHash, BYTE *pbHash, DWORD dwFlags) { BOOL ret = FALSE; @@ -262,9 +326,13 @@ BOOL WINAPI CryptCATAdminCalcHashFromFileHandle(HANDLE hFile, DWORD* pcbHash, CryptReleaseContext(prov, 0); return FALSE; } - while ((ret = ReadFile(hFile, buffer, 4096, &bytes_read, NULL)) && bytes_read) + + if (!(ret = pe_image_hash(hFile, hash))) { - CryptHashData(hash, buffer, bytes_read, 0); + while ((ret = ReadFile(hFile, buffer, 4096, &bytes_read, NULL)) && bytes_read) + { + CryptHashData(hash, buffer, bytes_read, 0); + } } if (ret) ret = CryptGetHashParam(hash, HP_HASHVAL, pbHash, pcbHash, 0); diff --git a/dlls/wintrust/tests/softpub.c b/dlls/wintrust/tests/softpub.c index 46861766621..9654c296a78 100644 --- a/dlls/wintrust/tests/softpub.c +++ b/dlls/wintrust/tests/softpub.c @@ -1892,6 +1892,39 @@ static void test_multiple_signatures(void) DeleteFileW(pathW); } +static BOOL (WINAPI *pCryptCATAdminCalcHashFromFileHandle)(HANDLE,DWORD*,BYTE*,DWORD); + +static void test_pe_image_hash(void) +{ + static const char expected[] = + {0x8a,0xd5,0x45,0x53,0x3d,0x67,0xdf,0x2f,0x78,0xe0,0x55,0x0a,0xe0,0xd9,0x7a,0x28,0x3e,0xbf,0x45,0x2b}; + WCHAR path[MAX_PATH]; + HANDLE file; + BYTE sha1[20]; + DWORD size, count; + HMODULE wintrust = GetModuleHandleA("wintrust.dll"); + BOOL ret; + + pCryptCATAdminCalcHashFromFileHandle = (void *)GetProcAddress(wintrust, "CryptCATAdminCalcHashFromFileHandle"); + if (!pCryptCATAdminCalcHashFromFileHandle) + { + win_skip("hash function missing\n"); + return; + } + + file = create_temp_file(path); + WriteFile(file, &bin, sizeof(bin), &count, NULL); + + size = sizeof(sha1); + memset(sha1, 0, sizeof(sha1)); + ret = pCryptCATAdminCalcHashFromFileHandle(file, &size, sha1, 0); + ok(ret, "got %lu\n", GetLastError()); + ok(!memcmp(sha1, expected, sizeof(sha1)), "wrong hash\n"); + + CloseHandle(file); + DeleteFileW(path); +} + START_TEST(softpub) { InitFunctionPtrs(); @@ -1901,4 +1934,5 @@ START_TEST(softpub) test_wintrust_digest(); test_get_known_usages(); test_multiple_signatures(); + test_pe_image_hash(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5516
On Mon Apr 22 17:01:54 2024 +0000, Hans Leidekker wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/5516/diffs?diff_id=110615&start_sha=e302b931c1622238283bb5b4fa30b73d25f9bcd4#a60ccf80094fe5e19aea80805fa499ca75ce8a6f_234_231) Done, thanks.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5516#note_68460
On Wed Apr 24 15:30:39 2024 +0000, eric pouech wrote:
this doesn't make sense to me... if you need to skip the _content_ of the IMAGE_FILE_SECURITY_DIRECTORY, the actual size is OptionalHeader.DataDirectory[IMAGE_FILE_SECURITY_DIRECTORY].Size The entry itself must skipped. The hash should be the same whether the image is signed or not.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5516#note_68461
This could also validate 'size' and 'offset' before reading. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5516#note_68545
participants (3)
-
Hans Leidekker -
Hans Leidekker (＠hans)
-
Nikolay Sivov (＠nsivov)