[PATCH] crypt32/tests: Fix a registerOIDInfo() failure when missing elevated privileges.
And only accept the access denied errors when running without elevated privileges.
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- dlls/crypt32/tests/oid.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/dlls/crypt32/tests/oid.c b/dlls/crypt32/tests/oid.c index ae03bba90e8..d135092a3ad 100644 --- a/dlls/crypt32/tests/oid.c +++ b/dlls/crypt32/tests/oid.c @@ -30,6 +30,22 @@ #include "wine/test.h"
+static BOOL is_process_elevated(void) +{ + HANDLE token; + if (OpenProcessToken( GetCurrentProcess(), TOKEN_QUERY, &token )) + { + TOKEN_ELEVATION_TYPE type; + DWORD size; + BOOL ret; + + ret = GetTokenInformation( token, TokenElevationType, &type, sizeof(type), &size ); + CloseHandle( token ); + return (ret && type == TokenElevationTypeFull); + } + return FALSE; +} + static BOOL (WINAPI *pCryptEnumOIDInfo)(DWORD,DWORD,void*,PFN_CRYPT_ENUM_OID_INFO);
@@ -314,7 +330,7 @@ static void test_registerOIDFunction(void) SetLastError(0xdeadbeef); ret = CryptRegisterOIDFunction(X509_ASN_ENCODING, "CryptDllEncodeObject", "1.2.3.4.5.6.7.8.9.10", bogusDll, NULL); - if (!ret && GetLastError() == ERROR_ACCESS_DENIED) + if (!ret && GetLastError() == ERROR_ACCESS_DENIED && !is_process_elevated()) { skip("Need admin rights\n"); return; @@ -393,7 +409,7 @@ static void test_registerDefaultOIDFunction(void) SetLastError(0xdeadbeef); ret = CryptRegisterDefaultOIDFunction(0, "CertDllOpenStoreProv", 0, bogusDll); - if (!ret && GetLastError() == ERROR_ACCESS_DENIED) + if (!ret && GetLastError() == ERROR_ACCESS_DENIED && !is_process_elevated()) { skip("Need admin rights\n"); return; @@ -499,7 +515,7 @@ static void test_getDefaultOIDFunctionAddress(void) SetLastError(0xdeadbeef); ret = CryptRegisterDefaultOIDFunction(0, "CertDllOpenStoreProv", 0, bogusDll); - if (!ret && GetLastError() == ERROR_ACCESS_DENIED) + if (!ret && GetLastError() == ERROR_ACCESS_DENIED && !is_process_elevated()) skip("Need admin rights\n"); else ok(ret, "CryptRegisterDefaultOIDFunction failed: %08x\n", GetLastError()); @@ -645,7 +661,11 @@ static void test_registerOIDInfo(void) info1.pszOID = test_oid; SetLastError(0xdeadbeef); ret = CryptUnregisterOIDInfo(&info1); - ok(!ret, "should fail\n"); + if (!ret && GetLastError() == ERROR_ACCESS_DENIED && !is_process_elevated()) + { + skip("Need admin rights FIXME\n"); + return; + } ok(!ret, "should fail\n"); ok(GetLastError() == ERROR_FILE_NOT_FOUND, "got %u\n", GetLastError());
info2 = CryptFindOIDInfo(CRYPT_OID_INFO_OID_KEY, (void *)test_oid, 0); @@ -668,7 +688,7 @@ static void test_registerOIDInfo(void) info1.dwGroupId = CRYPT_HASH_ALG_OID_GROUP_ID; SetLastError(0xdeadbeef); ret = CryptRegisterOIDInfo(&info1, CRYPT_INSTALL_OID_INFO_BEFORE_FLAG); - if (!ret && GetLastError() == ERROR_ACCESS_DENIED) + if (!ret && GetLastError() == ERROR_ACCESS_DENIED && !is_process_elevated()) { skip("Need admin rights\n"); return;
Francois Gouget fgouget@codeweavers.com wrote:
@@ -314,7 +330,7 @@ static void test_registerOIDFunction(void) SetLastError(0xdeadbeef); ret = CryptRegisterOIDFunction(X509_ASN_ENCODING, "CryptDllEncodeObject", "1.2.3.4.5.6.7.8.9.10", bogusDll, NULL);
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED)
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED && !is_process_elevated()) { skip("Need admin rights\n"); return;
@@ -393,7 +409,7 @@ static void test_registerDefaultOIDFunction(void) SetLastError(0xdeadbeef); ret = CryptRegisterDefaultOIDFunction(0, "CertDllOpenStoreProv", 0, bogusDll);
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED)
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED && !is_process_elevated()) { skip("Need admin rights\n"); return;
@@ -499,7 +515,7 @@ static void test_getDefaultOIDFunctionAddress(void) SetLastError(0xdeadbeef); ret = CryptRegisterDefaultOIDFunction(0, "CertDllOpenStoreProv", 0, bogusDll);
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED)
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED && !is_process_elevated()) skip("Need admin rights\n"); else ok(ret, "CryptRegisterDefaultOIDFunction failed: %08x\n", GetLastError());
In which cases the changes above are needed? It seems that this only complicates things without a reason and could potentially slow down the tests execution.
@@ -645,7 +661,11 @@ static void test_registerOIDInfo(void) info1.pszOID = test_oid; SetLastError(0xdeadbeef); ret = CryptUnregisterOIDInfo(&info1);
- ok(!ret, "should fail\n");
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED && !is_process_elevated())
- {
skip("Need admin rights FIXME\n");return;- } ok(!ret, "should fail\n");
This change doesn't look right. Most likely it's not needed here.
On Mon, 14 Dec 2020, Dmitry Timoshkov wrote:
Francois Gouget fgouget@codeweavers.com wrote:
@@ -314,7 +330,7 @@ static void test_registerOIDFunction(void) SetLastError(0xdeadbeef); ret = CryptRegisterOIDFunction(X509_ASN_ENCODING, "CryptDllEncodeObject", "1.2.3.4.5.6.7.8.9.10", bogusDll, NULL);
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED)
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED && !is_process_elevated()) { skip("Need admin rights\n"); return;
@@ -393,7 +409,7 @@ static void test_registerDefaultOIDFunction(void) SetLastError(0xdeadbeef); ret = CryptRegisterDefaultOIDFunction(0, "CertDllOpenStoreProv", 0, bogusDll);
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED)
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED && !is_process_elevated()) { skip("Need admin rights\n"); return;
@@ -499,7 +515,7 @@ static void test_getDefaultOIDFunctionAddress(void) SetLastError(0xdeadbeef); ret = CryptRegisterDefaultOIDFunction(0, "CertDllOpenStoreProv", 0, bogusDll);
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED)
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED && !is_process_elevated()) skip("Need admin rights\n"); else ok(ret, "CryptRegisterDefaultOIDFunction failed: %08x\n", GetLastError());
In which cases the changes above are needed? It seems that this only complicates things without a reason and could potentially slow down the tests execution.
These are not strictly needed. Rather the goal is to verify that we only get ERROR_ACCESS_DENIED when missing elevated privileges rather than blindly assuming the access denied error is justified.
@@ -645,7 +661,11 @@ static void test_registerOIDInfo(void) info1.pszOID = test_oid; SetLastError(0xdeadbeef); ret = CryptUnregisterOIDInfo(&info1);
- ok(!ret, "should fail\n");
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED && !is_process_elevated())
- {
skip("Need admin rights FIXME\n");return;- } ok(!ret, "should fail\n");
This change doesn't look right. Most likely it's not needed here.
It is needed: https://test.winehq.org/data/3acb0b3326c4120ea0c4c6076bd03c9cfe82c744/win10_...
Francois Gouget fgouget@codeweavers.com wrote:
@@ -499,7 +515,7 @@ static void test_getDefaultOIDFunctionAddress(void) SetLastError(0xdeadbeef); ret = CryptRegisterDefaultOIDFunction(0, "CertDllOpenStoreProv", 0, bogusDll);
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED)
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED && !is_process_elevated()) skip("Need admin rights\n"); else ok(ret, "CryptRegisterDefaultOIDFunction failed: %08x\n", GetLastError());
In which cases the changes above are needed? It seems that this only complicates things without a reason and could potentially slow down the tests execution.
These are not strictly needed. Rather the goal is to verify that we only get ERROR_ACCESS_DENIED when missing elevated privileges rather than blindly assuming the access denied error is justified.
If the intent to add is_process_elevated() checks to every place where ERROR_ACCESS_DENIED error returned then it's not justified.
@@ -645,7 +661,11 @@ static void test_registerOIDInfo(void) info1.pszOID = test_oid; SetLastError(0xdeadbeef); ret = CryptUnregisterOIDInfo(&info1);
- ok(!ret, "should fail\n");
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED && !is_process_elevated())
- {
skip("Need admin rights FIXME\n");return;- } ok(!ret, "should fail\n");
This change doesn't look right. Most likely it's not needed here.
It is needed: https://test.winehq.org/data/3acb0b3326c4120ea0c4c6076bd03c9cfe82c744/win10_...
In addition to broken indentation and strange skip message the test bot results seem not related, in both cases !ret is the only accepted value.
On Mon, 14 Dec 2020, Dmitry Timoshkov wrote: [...]
If the intent to add is_process_elevated() checks to every place where ERROR_ACCESS_DENIED error returned then it's not justified.
Ok.
@@ -645,7 +661,11 @@ static void test_registerOIDInfo(void) info1.pszOID = test_oid; SetLastError(0xdeadbeef); ret = CryptUnregisterOIDInfo(&info1);
- ok(!ret, "should fail\n");
- if (!ret && GetLastError() == ERROR_ACCESS_DENIED && !is_process_elevated())
- {
skip("Need admin rights FIXME\n");return;- } ok(!ret, "should fail\n");
This change doesn't look right. Most likely it's not needed here.
It is needed: https://test.winehq.org/data/3acb0b3326c4120ea0c4c6076bd03c9cfe82c744/win10_...
In addition to broken indentation and strange skip message the test bot results seem not related, in both cases !ret is the only accepted value.
Oh, right. I missed the cleanup step and mixed it up with the fix for another test too. I will resubmit it.
-
Dmitry Timoshkov -
Francois Gouget