[PATCH 0/1] MR7744: rsaenh: Ignore reserved field in import_key
The game "Marvel Heroes" uses AES for some session validation on login. It uses `CryptImportKey` to configure the key, but this fails in Wine (see for example the documentation of MHServerEmu: https://github.com/Crypto137/MHServerEmu/blob/f250fbd8d62a8d18afa6592e39058f...) because it leaves garbage(?) data in the reserved field of the key blob header. This is ignored on Windows (tested on Windows 10, but seems to be the same on other Windows versions, since the game runs without problem), but Wine verifies this value and exits early. This removes the check and adds a barebones test to verify this behavior. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7744
From: Tobias Gruetzmacher <tobias-git(a)23.gs> --- dlls/advapi32/tests/crypt.c | 43 +++++++++++++++++++++++++++++++++++++ dlls/rsaenh/rsaenh.c | 8 +++---- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/dlls/advapi32/tests/crypt.c b/dlls/advapi32/tests/crypt.c index 64067bea8ee..02c5a59f1d6 100644 --- a/dlls/advapi32/tests/crypt.c +++ b/dlls/advapi32/tests/crypt.c @@ -478,6 +478,48 @@ static void test_incorrect_api_usage(void) ok (!result && GetLastError() == ERROR_INVALID_PARAMETER, "%ld\n", GetLastError()); } +static void test_garbage_data(void) +{ + struct KeyBlob + { + BLOBHEADER header; + DWORD key_size; + BYTE key_data[2048]; + } key_blob; + + BOOL result; + HCRYPTPROV hProv; + HCRYPTKEY hkey = 0; + + /* When verifying logins in "Marvel Heroes", the application passes garbage data + * in a reserved field that should always be 0 (according to documentation). + * + * This doesn't lead to any error on Windows. + */ + + result = CryptAcquireContextA(&hProv, 0, 0, PROV_RSA_AES, CRYPT_VERIFYCONTEXT); + ok (result, "%08lx\n", GetLastError()); + if (!result) return; + + /* Looks like native handles are just pointers. */ + ok(!!*(void **)hProv, "Got zero *(void **)hProv.\n"); + + key_blob.header.bType = PLAINTEXTKEYBLOB; + key_blob.header.bVersion = CUR_BLOB_VERSION; + key_blob.header.reserved = 29806; // Not allowed, but accepted? + key_blob.header.aiKeyAlg = CALG_AES_128; + key_blob.key_size = 16; + + result = CryptImportKey(hProv, (BYTE *)&key_blob, sizeof(BLOBHEADER) + sizeof(DWORD) + key_blob.key_size, 0, 0, &hkey); + ok(result, "CryptImportKey failed: %08lx\n", GetLastError()); + + CryptDestroyKey(hkey); + + result = CryptReleaseContext(hProv, 0); + ok(result, "got %lu\n", GetLastError()); + +} + static const BYTE privKey[] = { 0x07, 0x02, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x52, 0x53, 0x41, 0x32, 0x00, 0x02, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x79, 0x10, 0x1c, 0xd0, 0x6b, 0x10, @@ -1293,6 +1335,7 @@ START_TEST(crypt) test_CryptReleaseContext(); test_acquire_context(); test_incorrect_api_usage(); + test_garbage_data(); test_verify_sig(); test_machine_guid(); test_container_sd(); diff --git a/dlls/rsaenh/rsaenh.c b/dlls/rsaenh/rsaenh.c index 5f37141e65b..b3d55d9db16 100644 --- a/dlls/rsaenh/rsaenh.c +++ b/dlls/rsaenh/rsaenh.c @@ -3490,14 +3490,14 @@ static BOOL import_key(HCRYPTPROV hProv, const BYTE *pbData, DWORD dwDataLen, HC return FALSE; if (dwDataLen < sizeof(BLOBHEADER) || - pBlobHeader->bVersion != CUR_BLOB_VERSION || - pBlobHeader->reserved != 0) + pBlobHeader->bVersion != CUR_BLOB_VERSION) { - TRACE("bVersion = %d, reserved = %d\n", pBlobHeader->bVersion, - pBlobHeader->reserved); + TRACE("bVersion = %d", pBlobHeader->bVersion); SetLastError(NTE_BAD_DATA); return FALSE; } + if (pBlobHeader->reserved != 0) + WARN("reserved != 0: %d\n", pBlobHeader->reserved); /* If this is a verify-only context, the key is not persisted regardless of * fStoreKey's original value. -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7744
Hans Leidekker (@hans) commented about dlls/advapi32/tests/crypt.c:
+ BYTE key_data[2048]; + } key_blob; + + BOOL result; + HCRYPTPROV hProv; + HCRYPTKEY hkey = 0; + + /* When verifying logins in "Marvel Heroes", the application passes garbage data + * in a reserved field that should always be 0 (according to documentation). + * + * This doesn't lead to any error on Windows. + */ + + result = CryptAcquireContextA(&hProv, 0, 0, PROV_RSA_AES, CRYPT_VERIFYCONTEXT); + ok (result, "%08lx\n", GetLastError()); + if (!result) return; I know there are more tests like this but there's no need to return early here, this call should always succeed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7744#note_100107
Hans Leidekker (@hans) commented about dlls/advapi32/tests/crypt.c:
+ BOOL result; + HCRYPTPROV hProv; + HCRYPTKEY hkey = 0; + + /* When verifying logins in "Marvel Heroes", the application passes garbage data + * in a reserved field that should always be 0 (according to documentation). + * + * This doesn't lead to any error on Windows. + */ + + result = CryptAcquireContextA(&hProv, 0, 0, PROV_RSA_AES, CRYPT_VERIFYCONTEXT); + ok (result, "%08lx\n", GetLastError()); + if (!result) return; + + /* Looks like native handles are just pointers. */ + ok(!!*(void **)hProv, "Got zero *(void **)hProv.\n"); You're testing an implementation detail of CryptAcquireContextA() here which belongs somewhere else, if it's relevant at all. Otherwise this patch looks good, thanks!
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7744#note_100109
On Mon Apr 7 09:10:53 2025 +0000, Hans Leidekker wrote:
You're testing an implementation detail of CryptAcquireContextA() here which belongs somewhere else, if it's relevant at all. Otherwise this patch looks good, thanks! Looks like a leftover thing I copy & pasted from somewhere else. :slight_smile:
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7744#note_100129
participants (3)
-
Hans Leidekker (＠hans) -
Tobias Gruetzmacher
-
Tobias Gruetzmacher (＠TobiX)