[PATCH 1/2] crypt32: Implement CryptHashCertificate2
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46516 Signed-off-by: Orhan Kavrakoğlu <aibok42(a)gmail.com> --- dlls/crypt32/cert.c | 62 +++++++++++++++++++++++++++++++++++++++ dlls/crypt32/crypt32.spec | 1 + include/wincrypt.h | 4 +++ 3 files changed, 67 insertions(+) diff --git a/dlls/crypt32/cert.c b/dlls/crypt32/cert.c index eb646cd5f6..9e5dec68f5 100644 --- a/dlls/crypt32/cert.c +++ b/dlls/crypt32/cert.c @@ -2213,6 +2213,68 @@ BOOL WINAPI CryptHashCertificate(HCRYPTPROV_LEGACY hCryptProv, ALG_ID Algid, return ret; } +BOOL WINAPI CryptHashCertificate2(LPCWSTR pwszCNGHashAlgid, DWORD dwFlags, + void *pvReserved, const BYTE *pbEncoded, DWORD cbEncoded, BYTE *pbComputedHash, + DWORD *pcbComputedHash) +{ + BCRYPT_HASH_HANDLE hash = NULL; + BCRYPT_ALG_HANDLE alg = NULL; + NTSTATUS status; + DWORD hash_len; + DWORD hash_len_size; + + TRACE("(%s, %08x, %p, %p, %d, %p, %p)\n", debugstr_w(pwszCNGHashAlgid), + dwFlags, pvReserved, pbEncoded, cbEncoded, pbComputedHash, pcbComputedHash); + + /* Windows does not check for this, causes access violation instead */ + if (0) { + if (!pwszCNGHashAlgid | !pcbComputedHash) + { + status = STATUS_INVALID_PARAMETER; + goto done; + } + } + + if ((status = BCryptOpenAlgorithmProvider(&alg, pwszCNGHashAlgid, NULL, 0))) + { + if (status == STATUS_NOT_IMPLEMENTED) + status = STATUS_NOT_FOUND; + goto done; + } + + if ((status = BCryptCreateHash(alg, &hash, NULL, 0, NULL, 0, 0))) + goto done; + + if ((status = BCryptGetProperty(hash, BCRYPT_HASH_LENGTH, (BYTE *)&hash_len, sizeof(hash_len), &hash_len_size, 0))) + goto done; + + if (!pbComputedHash) + { + *pcbComputedHash = hash_len; + goto done; + } + + if (*pcbComputedHash < hash_len) + { + status = ERROR_MORE_DATA; + goto done; + } + + *pcbComputedHash = hash_len; + + if ((status = BCryptHashData(hash, (BYTE *)pbEncoded, cbEncoded, 0))) + goto done; + + if ((status = BCryptFinishHash(hash, pbComputedHash, hash_len, 0))) + goto done; + +done: + if (hash) BCryptDestroyHash(hash); + if (alg) BCryptCloseAlgorithmProvider(alg, 0); + if (status) SetLastError(status); + return !status; +} + BOOL WINAPI CryptHashPublicKeyInfo(HCRYPTPROV_LEGACY hCryptProv, ALG_ID Algid, DWORD dwFlags, DWORD dwCertEncodingType, PCERT_PUBLIC_KEY_INFO pInfo, BYTE *pbComputedHash, DWORD *pcbComputedHash) diff --git a/dlls/crypt32/crypt32.spec b/dlls/crypt32/crypt32.spec index fc32570b52..fa507dfc65 100644 --- a/dlls/crypt32/crypt32.spec +++ b/dlls/crypt32/crypt32.spec @@ -136,6 +136,7 @@ @ stdcall CryptGetOIDFunctionAddress(long long str long ptr ptr) @ stdcall CryptGetOIDFunctionValue(long str str wstr ptr ptr ptr) @ stdcall CryptHashCertificate(long long long ptr long ptr ptr) +@ stdcall CryptHashCertificate2(wstr long ptr ptr long ptr ptr) @ stdcall CryptHashMessage(ptr long long ptr ptr ptr ptr ptr ptr) @ stdcall CryptHashPublicKeyInfo(long long long long ptr ptr ptr) @ stdcall CryptHashToBeSigned(ptr long ptr long ptr ptr) diff --git a/include/wincrypt.h b/include/wincrypt.h index a1b1305902..cbc76d7f10 100644 --- a/include/wincrypt.h +++ b/include/wincrypt.h @@ -4379,6 +4379,10 @@ BOOL WINAPI CryptHashCertificate(HCRYPTPROV_LEGACY hCryptProv, ALG_ID Algid, DWORD dwFlags, const BYTE *pbEncoded, DWORD cbEncoded, BYTE *pbComputedHash, DWORD *pcbComputedHash); +BOOL WINAPI CryptHashCertificate2(LPCWSTR pwszCNGHashAlgid, DWORD dwFlags, + void *pvReserved, const BYTE *pbEncoded, DWORD cbEncoded, BYTE *pbComputedHash, + DWORD *pcbComputedHash); + BOOL WINAPI CryptHashPublicKeyInfo(HCRYPTPROV_LEGACY hCryptProv, ALG_ID Algid, DWORD dwFlags, DWORD dwCertEncodingType, PCERT_PUBLIC_KEY_INFO pInfo, BYTE *pbComputedHash, DWORD *pcbComputedHash); -- 2.19.1
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46516 Signed-off-by: Orhan Kavrakoğlu <aibok42(a)gmail.com> --- dlls/crypt32/tests/cert.c | 77 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/dlls/crypt32/tests/cert.c b/dlls/crypt32/tests/cert.c index f653741ea7..54bb70c634 100644 --- a/dlls/crypt32/tests/cert.c +++ b/dlls/crypt32/tests/cert.c @@ -21,6 +21,8 @@ #include <stdio.h> #include <stdarg.h> +#include <ntstatus.h> +#define WIN32_NO_STATUS #include <windef.h> #include <winbase.h> #include <winreg.h> @@ -33,6 +35,7 @@ static PCCERT_CONTEXT (WINAPI *pCertCreateSelfSignCertificate)(HCRYPTPROV_OR_NCR static BOOL (WINAPI *pCertGetValidUsages)(DWORD,PCCERT_CONTEXT*,int*,LPSTR*,DWORD*); static BOOL (WINAPI *pCryptAcquireCertificatePrivateKey)(PCCERT_CONTEXT,DWORD,void*,HCRYPTPROV_OR_NCRYPT_KEY_HANDLE*,DWORD*,BOOL*); static BOOL (WINAPI *pCryptEncodeObjectEx)(DWORD,LPCSTR,const void*,DWORD,PCRYPT_ENCODE_PARA,void*,DWORD*); +static BOOL (WINAPI *pCryptHashCertificate2)(LPCWSTR, DWORD, void*, const BYTE*, DWORD, BYTE*, DWORD*); static BOOL (WINAPI * pCryptVerifyCertificateSignatureEx) (HCRYPTPROV, DWORD, DWORD, void *, DWORD, void *, DWORD, void *); @@ -53,6 +56,7 @@ static void init_function_pointers(void) GET_PROC(hCrypt32, CertGetValidUsages) GET_PROC(hCrypt32, CryptAcquireCertificatePrivateKey) GET_PROC(hCrypt32, CryptEncodeObjectEx) + GET_PROC(hCrypt32, CryptHashCertificate2) GET_PROC(hCrypt32, CryptVerifyCertificateSignatureEx) GET_PROC(hAdvapi32, CryptAcquireContextA) @@ -1784,6 +1788,78 @@ static void testCryptHashCert(void) ok(!memcmp(hash, knownHash, sizeof(knownHash)), "Unexpected hash\n"); } +static void testCryptHashCert2(void) +{ + static const BYTE emptyHash[] = { 0xda, 0x39, 0xa3, 0xee, 0x5e, 0x6b, 0x4b, + 0x0d, 0x32, 0x55, 0xbf, 0xef, 0x95, 0x60, 0x18, 0x90, 0xaf, 0xd8, 0x07, + 0x09 }; + static const BYTE knownHash[] = { 0xae, 0x9d, 0xbf, 0x6d, 0xf5, 0x46, 0xee, + 0x8b, 0xc5, 0x7a, 0x13, 0xba, 0xc2, 0xb1, 0x04, 0xf2, 0xbf, 0x52, 0xa8, + 0xa2 }; + static const BYTE toHash[] = "abcdefghijklmnopqrstuvwxyz0123456789.,;!?:"; + BOOL ret; + BYTE hash[20]; + DWORD hashLen; + const WCHAR SHA1[] = { 'S', 'H', 'A', '1', '\0' }; + const WCHAR invalidAlgorithm[] = { '_', 'S', 'H', 'O', 'U', 'L', 'D', + 'N', 'O', 'T', + 'E', 'X', 'I', 'S', 'T', '_', '\0' }; + + if (!pCryptHashCertificate2) + { + win_skip("CryptHashCertificate2() is not available\n"); + return; + } + + /* Test empty hash */ + hashLen = sizeof(hash); + ret = pCryptHashCertificate2(SHA1, 0, NULL, NULL, 0, hash, &hashLen); + ok(ret, "CryptHashCertificate2 failed: %08x\n", GetLastError()); + ok(hashLen == sizeof(hash), "Got unexpected size of hash %d\n", hashLen); + ok(!memcmp(hash, emptyHash, sizeof(emptyHash)), "Unexpected hash of nothing\n"); + + /* Test known hash */ + hashLen = sizeof(hash); + ret = pCryptHashCertificate2(SHA1, 0, NULL, toHash, sizeof(toHash), hash, &hashLen); + ok(ret, "CryptHashCertificate2 failed: %08x\n", GetLastError()); + ok(hashLen == sizeof(hash), "Got unexpected size of hash %d\n", hashLen); + ok(!memcmp(hash, knownHash, sizeof(knownHash)), "Unexpected hash\n"); + + /* Test null hash size pointer just sets hash size */ + hashLen = 0; + ret = pCryptHashCertificate2(SHA1, 0, NULL, toHash, sizeof(toHash), NULL, &hashLen); + ok(ret, "CryptHashCertificate2 failed: %08x\n", GetLastError()); + ok(hashLen == sizeof(hash), "Hash size not set correctly (%d)\n", hashLen); + + /* Null algorithm ID crashes Windows implementations */ + if (0) { + /* Test null algorithm ID */ + hashLen = sizeof(hash); + ret = pCryptHashCertificate2(NULL, 0, NULL, toHash, sizeof(toHash), hash, &hashLen); + } + + /* Test invalid algorithm */ + hashLen = sizeof(hash); + ret = pCryptHashCertificate2(invalidAlgorithm, 0, NULL, toHash, sizeof(toHash), hash, &hashLen); + ok(!ret && GetLastError() == STATUS_NOT_FOUND, + "Expected STATUS_NOT_FOUND (0x%08x), got 0x%08x\n", STATUS_NOT_FOUND, GetLastError()); + + /* Test hash buffer too small */ + hashLen = sizeof(hash) / 2; + ret = pCryptHashCertificate2(SHA1, 0, NULL, toHash, sizeof(toHash), hash, &hashLen); + ok(!ret && GetLastError() == ERROR_MORE_DATA, + "Expected ERROR_MORE_DATA (%d), got %d\n", ERROR_MORE_DATA, GetLastError()); + + /* Null hash length crashes Windows implementations */ + if (0) { + /* Test hashLen null with hash */ + ret = pCryptHashCertificate2(SHA1, 0, NULL, toHash, sizeof(toHash), hash, NULL); + + /* Test hashLen null with no hash */ + ret = pCryptHashCertificate2(SHA1, 0, NULL, toHash, sizeof(toHash), NULL, NULL); + } +} + static void verifySig(HCRYPTPROV csp, const BYTE *toSign, size_t toSignLen, const BYTE *sig, unsigned int sigLen) { @@ -4074,6 +4150,7 @@ START_TEST(cert) testLinkCert(); testCryptHashCert(); + testCryptHashCert2(); testCertSigs(); testSignAndEncodeCert(); testCreateSelfSignCert(); -- 2.19.1
On Fri, 2019-01-25 at 21:12 +0300, Orhan 'aib' Kavrakoğlu wrote:
+ /* Test invalid algorithm */ + hashLen = sizeof(hash); + ret = pCryptHashCertificate2(invalidAlgorithm, 0, NULL, toHash, sizeof(toHash), hash, &hashLen); + ok(!ret && GetLastError() == STATUS_NOT_FOUND, + "Expected STATUS_NOT_FOUND (0x%08x), got 0x%08x\n", STATUS_NOT_FOUND, GetLastError());
You should set last error to a known value (0xdeadbeef) right before calling the function. This way we are certain it's this function call that sets it.
+ /* Test hash buffer too small */ + hashLen = sizeof(hash) / 2; + ret = pCryptHashCertificate2(SHA1, 0, NULL, toHash, sizeof(toHash), hash, &hashLen); + ok(!ret && GetLastError() == ERROR_MORE_DATA, + "Expected ERROR_MORE_DATA (%d), got %d\n", ERROR_MORE_DATA, GetLastError());
Same here.
+BOOL WINAPI CryptHashCertificate2(LPCWSTR pwszCNGHashAlgid, DWORD dwFlags, + void *pvReserved, const BYTE *pbEncoded, DWORD cbEncoded, BYTE *pbComputedHash, + DWORD *pcbComputedHash) +{ + BCRYPT_HASH_HANDLE hash = NULL; + BCRYPT_ALG_HANDLE alg = NULL; + NTSTATUS status; + DWORD hash_len; + DWORD hash_len_size; + + TRACE("(%s, %08x, %p, %p, %d, %p, %p)\n", debugstr_w(pwszCNGHashAlgid), + dwFlags, pvReserved, pbEncoded, cbEncoded, pbComputedHash, pcbComputedHash); + + /* Windows does not check for this, causes access violation instead */ + if (0) { + if (!pwszCNGHashAlgid | !pcbComputedHash) + { + status = STATUS_INVALID_PARAMETER; + goto done; + } + }
Please avoid adding dead code. You include the same information in the tests, which is generally a better place to document these things. The patch looks good otherwise.
participants (2)
-
Hans Leidekker -
Orhan 'aib' Kavrakoğlu