[PATCH v2 0/1] MR8785: rsaenh: Use TomCrypt for hash implementations.
Some apps unload bcrypt which causes a crash if rsaenh is used afterwards. -- v2: rsaenh: Use TomCrypt for hash implementations. https://gitlab.winehq.org/wine/wine/-/merge_requests/8785
From: Hans Leidekker <hans(a)codeweavers.com> Some apps unload bcrypt which causes a crash if rsaenh is used afterwards. --- dlls/rsaenh/Makefile.in | 2 +- dlls/rsaenh/implglue.c | 42 +++++++++++++++++++++++------------------ dlls/rsaenh/implglue.h | 14 +++++++++----- dlls/rsaenh/rsaenh.c | 35 +++++++++++++++------------------- 4 files changed, 49 insertions(+), 44 deletions(-) diff --git a/dlls/rsaenh/Makefile.in b/dlls/rsaenh/Makefile.in index 18fbb0fba20..ad77cd6a6da 100644 --- a/dlls/rsaenh/Makefile.in +++ b/dlls/rsaenh/Makefile.in @@ -1,6 +1,6 @@ MODULE = rsaenh.dll IMPORTLIB = rsaenh -IMPORTS = $(TOMCRYPT_PE_LIBS) bcrypt crypt32 advapi32 +IMPORTS = $(TOMCRYPT_PE_LIBS) crypt32 advapi32 EXTRAINCL = $(TOMCRYPT_PE_CFLAGS) SOURCES = \ diff --git a/dlls/rsaenh/implglue.c b/dlls/rsaenh/implglue.c index ffb6b3d49dc..900c620c9ef 100644 --- a/dlls/rsaenh/implglue.c +++ b/dlls/rsaenh/implglue.c @@ -34,52 +34,58 @@ prng_state prng = { 0 }; int wprng = 0; -BOOL init_hash_impl(ALG_ID aiAlgid, BCRYPT_HASH_HANDLE *hash_handle) +BOOL init_hash_impl( ALG_ID algid, struct hash *hash ) { - switch (aiAlgid) + memset( hash, 0, sizeof(*hash) ); + + switch (algid) { case CALG_MD2: - return !BCryptCreateHash(BCRYPT_MD2_ALG_HANDLE, hash_handle, NULL, 0, NULL, 0, 0); + hash->desc = &md2_desc; + break; case CALG_MD4: - return !BCryptCreateHash(BCRYPT_MD4_ALG_HANDLE, hash_handle, NULL, 0, NULL, 0, 0); + hash->desc = &md4_desc; + break; case CALG_MD5: - return !BCryptCreateHash(BCRYPT_MD5_ALG_HANDLE, hash_handle, NULL, 0, NULL, 0, 0); + hash->desc = &md5_desc; + break; case CALG_SHA: - return !BCryptCreateHash(BCRYPT_SHA1_ALG_HANDLE, hash_handle, NULL, 0, NULL, 0, 0); + hash->desc = &sha1_desc; + break; case CALG_SHA_256: - return !BCryptCreateHash(BCRYPT_SHA256_ALG_HANDLE, hash_handle, NULL, 0, NULL, 0, 0); + hash->desc = &sha256_desc; + break; case CALG_SHA_384: - return !BCryptCreateHash(BCRYPT_SHA384_ALG_HANDLE, hash_handle, NULL, 0, NULL, 0, 0); + hash->desc = &sha384_desc; + break; case CALG_SHA_512: - return !BCryptCreateHash(BCRYPT_SHA512_ALG_HANDLE, hash_handle, NULL, 0, NULL, 0, 0); + hash->desc = &sha512_desc; + break; default: return TRUE; } -} -BOOL update_hash_impl(BCRYPT_HASH_HANDLE hash_handle, const BYTE *pbData, DWORD dwDataLen) -{ - BCryptHashData(hash_handle, (UCHAR*)pbData, dwDataLen, 0); + hash->desc->init( &hash->state ); return TRUE; } -BOOL finalize_hash_impl(BCRYPT_HASH_HANDLE hash_handle, BYTE *hash_value, DWORD hash_size) +BOOL update_hash_impl( struct hash *hash, const BYTE *data, DWORD len ) { - BCryptFinishHash(hash_handle, hash_value, hash_size, 0); - BCryptDestroyHash(hash_handle); + hash->desc->process( &hash->state, data, len ); return TRUE; } -BOOL duplicate_hash_impl(BCRYPT_HASH_HANDLE src_hash_handle, BCRYPT_HASH_HANDLE *dest_hash_handle) +BOOL finalize_hash_impl( struct hash *hash, BYTE *hash_value, DWORD hash_size ) { - return !BCryptDuplicateHash(src_hash_handle, dest_hash_handle, NULL, 0, 0); + hash->desc->done( &hash->state, hash_value ); + return TRUE; } BOOL new_key_impl(ALG_ID aiAlgid, KEY_CONTEXT *pKeyContext, DWORD dwKeyLen) diff --git a/dlls/rsaenh/implglue.h b/dlls/rsaenh/implglue.h index ad497a81c1f..b0a518830ac 100644 --- a/dlls/rsaenh/implglue.h +++ b/dlls/rsaenh/implglue.h @@ -24,7 +24,6 @@ #ifndef __WINE_IMPLGLUE_H #define __WINE_IMPLGLUE_H -#include "bcrypt.h" #include "tomcrypt.h" #define RSAENH_MAX_HASH_SIZE 104 @@ -38,10 +37,15 @@ typedef union tagKEY_CONTEXT { extern prng_state prng; extern int wprng; -BOOL init_hash_impl(ALG_ID aiAlgid, BCRYPT_HASH_HANDLE *hash_handle); -BOOL update_hash_impl(BCRYPT_HASH_HANDLE hash_handle, const BYTE *pbData, DWORD dwDataLen); -BOOL finalize_hash_impl(BCRYPT_HASH_HANDLE hash_handle, BYTE *hash_value, DWORD hash_size); -BOOL duplicate_hash_impl(BCRYPT_HASH_HANDLE src_hash_handle, BCRYPT_HASH_HANDLE *dest_hash_handle); +struct hash +{ + const struct ltc_hash_descriptor *desc; + hash_state state; +}; + +BOOL init_hash_impl(ALG_ID algid, struct hash *hash); +BOOL update_hash_impl(struct hash *hash, const BYTE *data, DWORD len); +BOOL finalize_hash_impl(struct hash *hash, BYTE *hash_value, DWORD hash_size); BOOL new_key_impl(ALG_ID aiAlgid, KEY_CONTEXT *pKeyContext, DWORD dwKeyLen); BOOL free_key_impl(ALG_ID aiAlgid, KEY_CONTEXT *pKeyContext); diff --git a/dlls/rsaenh/rsaenh.c b/dlls/rsaenh/rsaenh.c index b3d55d9db16..98363fe5087 100644 --- a/dlls/rsaenh/rsaenh.c +++ b/dlls/rsaenh/rsaenh.c @@ -68,7 +68,7 @@ typedef struct tagCRYPTHASH HCRYPTPROV hProv; DWORD dwHashSize; DWORD dwState; - BCRYPT_HASH_HANDLE hash_handle; + struct hash hash; BYTE abHashValue[RSAENH_MAX_HASH_SIZE]; PHMAC_INFO pHMACInfo; RSAENH_TLS1PRF_PARAMS tpPRFParams; @@ -663,7 +663,6 @@ static void destroy_hash(OBJECTHDR *pObject) { CRYPTHASH *pCryptHash = (CRYPTHASH*)pObject; - BCryptDestroyHash(pCryptHash->hash_handle); free_hmac_info(pCryptHash->pHMACInfo); free_data_blob(&pCryptHash->tpPRFParams.blobLabel); free_data_blob(&pCryptHash->tpPRFParams.blobSeed); @@ -698,9 +697,8 @@ static inline BOOL init_hash(CRYPTHASH *pCryptHash) { return FALSE; } pCryptHash->dwHashSize = pAlgInfo->dwDefaultLen >> 3; - init_hash_impl(pCryptHash->pHMACInfo->HashAlgid, &pCryptHash->hash_handle); - update_hash_impl(pCryptHash->hash_handle, - pCryptHash->pHMACInfo->pbInnerString, + init_hash_impl(pCryptHash->pHMACInfo->HashAlgid, &pCryptHash->hash); + update_hash_impl(&pCryptHash->hash, pCryptHash->pHMACInfo->pbInnerString, pCryptHash->pHMACInfo->cbInnerString); } return TRUE; @@ -713,7 +711,7 @@ static inline BOOL init_hash(CRYPTHASH *pCryptHash) { return TRUE; default: - return init_hash_impl(pCryptHash->aiAlgid, &pCryptHash->hash_handle); + return init_hash_impl(pCryptHash->aiAlgid, &pCryptHash->hash); } } @@ -737,7 +735,7 @@ static inline void update_hash(CRYPTHASH *pCryptHash, const BYTE *pbData, DWORD { case CALG_HMAC: if (pCryptHash->pHMACInfo) - update_hash_impl(pCryptHash->hash_handle, pbData, dwDataLen); + update_hash_impl(&pCryptHash->hash, pbData, dwDataLen); break; case CALG_MAC: @@ -792,7 +790,7 @@ static inline void update_hash(CRYPTHASH *pCryptHash, const BYTE *pbData, DWORD break; default: - update_hash_impl(pCryptHash->hash_handle, pbData, dwDataLen); + update_hash_impl(&pCryptHash->hash, pbData, dwDataLen); } } @@ -816,16 +814,14 @@ static inline void finalize_hash(CRYPTHASH *pCryptHash) if (pCryptHash->pHMACInfo) { BYTE abHashValue[RSAENH_MAX_HASH_SIZE]; - finalize_hash_impl(pCryptHash->hash_handle, pCryptHash->abHashValue, pCryptHash->dwHashSize); + finalize_hash_impl(&pCryptHash->hash, pCryptHash->abHashValue, pCryptHash->dwHashSize); memcpy(abHashValue, pCryptHash->abHashValue, pCryptHash->dwHashSize); - init_hash_impl(pCryptHash->pHMACInfo->HashAlgid, &pCryptHash->hash_handle); - update_hash_impl(pCryptHash->hash_handle, - pCryptHash->pHMACInfo->pbOuterString, + init_hash_impl(pCryptHash->pHMACInfo->HashAlgid, &pCryptHash->hash); + update_hash_impl(&pCryptHash->hash, pCryptHash->pHMACInfo->pbOuterString, pCryptHash->pHMACInfo->cbOuterString); - update_hash_impl(pCryptHash->hash_handle, - abHashValue, pCryptHash->dwHashSize); - finalize_hash_impl(pCryptHash->hash_handle, pCryptHash->abHashValue, pCryptHash->dwHashSize); - pCryptHash->hash_handle = NULL; + update_hash_impl(&pCryptHash->hash, abHashValue, pCryptHash->dwHashSize); + finalize_hash_impl(&pCryptHash->hash, pCryptHash->abHashValue, pCryptHash->dwHashSize); + memset(&pCryptHash->hash, 0, sizeof(pCryptHash->hash)); } break; @@ -842,8 +838,8 @@ static inline void finalize_hash(CRYPTHASH *pCryptHash) break; default: - finalize_hash_impl(pCryptHash->hash_handle, pCryptHash->abHashValue, pCryptHash->dwHashSize); - pCryptHash->hash_handle = NULL; + finalize_hash_impl(&pCryptHash->hash, pCryptHash->abHashValue, pCryptHash->dwHashSize); + memset(&pCryptHash->hash, 0, sizeof(pCryptHash->hash)); } } @@ -2351,7 +2347,7 @@ BOOL WINAPI RSAENH_CPCreateHash(HCRYPTPROV hProv, ALG_ID Algid, HCRYPTKEY hKey, pCryptHash->hProv = hProv; pCryptHash->dwState = RSAENH_HASHSTATE_HASHING; pCryptHash->pHMACInfo = NULL; - pCryptHash->hash_handle = NULL; + memset(&pCryptHash->hash, 0, sizeof(pCryptHash->hash)); pCryptHash->dwHashSize = peaAlgidInfo->dwDefaultLen >> 3; pCryptHash->buffered_hash_bytes = 0; init_data_blob(&pCryptHash->tpPRFParams.blobLabel); @@ -2513,7 +2509,6 @@ BOOL WINAPI RSAENH_CPDuplicateHash(HCRYPTPROV hUID, HCRYPTHASH hHash, DWORD *pdw if (*phHash != (HCRYPTHASH)INVALID_HANDLE_VALUE) { *pDestHash = *pSrcHash; - duplicate_hash_impl(pSrcHash->hash_handle, &pDestHash->hash_handle); copy_hmac_info(&pDestHash->pHMACInfo, pSrcHash->pHMACInfo); copy_data_blob(&pDestHash->tpPRFParams.blobLabel, &pSrcHash->tpPRFParams.blobLabel); copy_data_blob(&pDestHash->tpPRFParams.blobSeed, &pSrcHash->tpPRFParams.blobSeed); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8785
Some apps unload bcrypt which causes a crash if rsaenh is used afterwards.
How does this happen? Bcrypt is currently a linked dependency. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8785#note_113263
The app calls FreeLibrary(bcrypt) in a loop. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8785#note_113264
Adding some traces to rsaenh tests, on Windows it shows that bcrypt is actually loaded, right after first test function test_RC4_salt(). So this doesn't look like a valid solution. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8785#note_113265
That doesn't necessarily mean that rsaenh loads bcrypt. The test calls advapi32 functions like CryptAcquireContext() which will in turn load an appropriate provider dll, such as rsaenh. Just a call to RegCreateKey() is sufficient to load bcrypt, so it's possible that advapi32 or one of its dependencies imports bcrypt. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8785#note_113381
On Thu Aug 21 08:44:30 2025 +0000, Hans Leidekker wrote:
That doesn't necessarily mean that rsaenh loads bcrypt. The test calls advapi32 functions like CryptAcquireContext() which will in turn load an appropriate provider dll, such as rsaenh. Just a call to RegCreateKey() is sufficient to load bcrypt, so it's possible that advapi32 or one of its dependencies imports bcrypt. That could be, but doing FreeLibrary() loop on it does not work as described for me, on Windows. FreeLibrary() never fails for it, assuming application in question is looping until it fails.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8785#note_113382
On Thu Aug 21 08:44:29 2025 +0000, Nikolay Sivov wrote:
That could be, but doing FreeLibrary() loop on it does not work as described for me, on Windows. FreeLibrary() never fails for it, assuming application in question is looping until it fails. And if advapi32 also uses it, it means we should be able to link more modules to it and it should continue to work.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8785#note_113383
On Thu Aug 21 08:46:27 2025 +0000, Nikolay Sivov wrote:
And if advapi32 also uses it, it means we should be able to link more modules to it and it should continue to work. This isn't meant to fix the FreeLibrary() issue. It avoids it but I think it's a good change regardless.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8785#note_113393
participants (3)
-
Hans Leidekker -
Hans Leidekker (＠hans)
-
Nikolay Sivov (＠nsivov)