[PATCH v6 0/1] MR5516: wintrust: Add support for the PE image hash in CryptCATAdminCalcHashFromFileHandle().
-- v6: 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 | 80 +++++++++++++++++++++++++++++++++-- dlls/wintrust/tests/softpub.c | 34 +++++++++++++++ 2 files changed, 110 insertions(+), 4 deletions(-) diff --git a/dlls/wintrust/crypt.c b/dlls/wintrust/crypt.c index 7321b84eea4..36b72840ce7 100644 --- a/dlls/wintrust/crypt.c +++ b/dlls/wintrust/crypt.c @@ -214,11 +214,79 @@ 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 ); + + if (nt64->OptionalHeader.NumberOfRvaAndSizes < IMAGE_FILE_SECURITY_DIRECTORY + 1) goto done; + 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 ); + + if (nt32->OptionalHeader.NumberOfRvaAndSizes < IMAGE_FILE_SECURITY_DIRECTORY + 1) goto done; + 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 (offset + size > file_size) goto done; + + 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 +330,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 Wed Apr 24 15:30:39 2024 +0000, eric pouech wrote:
right, but this means that you never include in the hash what's after the content of the security directory? (and another nitpick, not sure it does matter so much for real images, but it should be checked against nth->OptionalHeader.NumberOfRvaAndSizes that security header in present) The signature is specified to come last. I added a check for NumberOfRvaAndSizes.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5516#note_68715
On Wed Apr 24 15:30:39 2024 +0000, Hans Leidekker wrote:
The signature is specified to come last. I added a check for NumberOfRvaAndSizes. hmmm... this is not what I see from kernel32 64bit in Win10
``` winedump dump -x kernel32.dll <snip> EXPORT rva: 0x9a370 size: 0xdf0c IMPORT rva: 0xa827c size: 0x794 RESOURCE rva: 0xbd000 size: 0x520 EXCEPTION rva: 0xb6000 size: 0x5634 SECURITY rva: 0xb8c00 size: 0x4088 BASERELOC rva: 0xbe000 size: 0x314 DEBUG rva: 0x87b90 size: 0x70 ARCHITECTURE rva: 0x0 size: 0x0 GLOBALPTR rva: 0x0 size: 0x0 TLS rva: 0x0 size: 0x0 LOAD_CONFIG rva: 0x807f0 size: 0x118 Bound IAT rva: 0x0 size: 0x0 IAT rva: 0x817c0 size: 0x2a70 Delay IAT rva: 0x9a128 size: 0x60 CLR Header rva: 0x0 size: 0x0 rva: 0x0 size: 0x0 <snip> objdump kernel32.dll <snip> The Data Directory Entry 0 000000000009a370 0000df0c Export Directory [.edata (or where ever we found it)] Entry 1 00000000000a827c 00000794 Import Directory [parts of .idata] Entry 2 00000000000bd000 00000520 Resource Directory [.rsrc] Entry 3 00000000000b6000 00005634 Exception Directory [.pdata] Entry 4 00000000000b8c00 00004088 Security Directory Entry 5 00000000000be000 00000314 Base Relocation Directory [.reloc] Entry 6 0000000000087b90 00000070 Debug Directory Entry 7 0000000000000000 00000000 Description Directory Entry 8 0000000000000000 00000000 Special Directory Entry 9 0000000000000000 00000000 Thread Storage Directory [.tls] Entry a 00000000000807f0 00000118 Load Configuration Directory Entry b 0000000000000000 00000000 Bound Import Directory Entry c 00000000000817c0 00002a70 Import Address Table Directory Entry d 000000000009a128 00000060 Delay Import Directory Entry e 0000000000000000 00000000 CLR Runtime Header Entry f 0000000000000000 00000000 Reserved <snip> ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5516#note_68722
On Wed Apr 24 15:30:39 2024 +0000, eric pouech wrote:
hmmm... this is not what I see from kernel32 64bit in Win10 ``` winedump dump -x kernel32.dll <snip> EXPORT rva: 0x9a370 size: 0xdf0c IMPORT rva: 0xa827c size: 0x794 RESOURCE rva: 0xbd000 size: 0x520 EXCEPTION rva: 0xb6000 size: 0x5634 SECURITY rva: 0xb8c00 size: 0x4088 BASERELOC rva: 0xbe000 size: 0x314 DEBUG rva: 0x87b90 size: 0x70 ARCHITECTURE rva: 0x0 size: 0x0 GLOBALPTR rva: 0x0 size: 0x0 TLS rva: 0x0 size: 0x0 LOAD_CONFIG rva: 0x807f0 size: 0x118 Bound IAT rva: 0x0 size: 0x0 IAT rva: 0x817c0 size: 0x2a70 Delay IAT rva: 0x9a128 size: 0x60 CLR Header rva: 0x0 size: 0x0 rva: 0x0 size: 0x0 <snip> objdump kernel32.dll <snip> The Data Directory Entry 0 000000000009a370 0000df0c Export Directory [.edata (or where ever we found it)] Entry 1 00000000000a827c 00000794 Import Directory [parts of .idata] Entry 2 00000000000bd000 00000520 Resource Directory [.rsrc] Entry 3 00000000000b6000 00005634 Exception Directory [.pdata] Entry 4 00000000000b8c00 00004088 Security Directory Entry 5 00000000000be000 00000314 Base Relocation Directory [.reloc] Entry 6 0000000000087b90 00000070 Debug Directory Entry 7 0000000000000000 00000000 Description Directory Entry 8 0000000000000000 00000000 Special Directory Entry 9 0000000000000000 00000000 Thread Storage Directory [.tls] Entry a 00000000000807f0 00000118 Load Configuration Directory Entry b 0000000000000000 00000000 Bound Import Directory Entry c 00000000000817c0 00002a70 Import Address Table Directory Entry d 000000000009a128 00000060 Delay Import Directory Entry e 0000000000000000 00000000 CLR Runtime Header Entry f 0000000000000000 00000000 Reserved <snip> ``` Interesting. This patch produces the same hash for Windows 10 kernel32.dll as native however. osslsigncode also makes this assumption.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5516#note_68724
On Wed Apr 24 15:30:39 2024 +0000, Hans Leidekker wrote:
Interesting. This patch produces the same hash for Windows 10 kernel32.dll as native however. osslsigncode also makes this assumption. Note that you can't compare RVAs to find out which is last, because the security directory address is a raw file offset, not an RVA.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5516#note_68725
On Wed Apr 24 15:30:39 2024 +0000, Alexandre Julliard wrote:
Note that you can't compare RVAs to find out which is last, because the security directory address is a raw file offset, not an RVA. got it... sorry for the noise :-(
it's clearer here https://www.sciencedirect.com/science/article/pii/S2666281720300123?via%3Dih... The digital signature data in an embedded Authenticode-signed Windows PE file is appended to the file. The offset and size of the embedded signature is stored in the Security directory entry within the Data directories array of the PE optional header. The Data directories array contains offsets and sizes of different structures within the PE file, such as the export, import, or relocation directories, among others. All directories but the security directory store their offsets as relative virtual address (RVA) offsets, which means that they are the virtual addresses from the PE file once it is loaded into memory. On the contrary, the security directory stores its offset as a file offset. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5516#note_68750
participants (4)
-
Alexandre Julliard (＠julliard) -
eric pouech (＠epo)
-
Hans Leidekker
-
Hans Leidekker (＠hans)