[PATCH v3 0/6] MR7117: crypt32: Some fixes for verify_ssl_policy().
-- v3: crypt32: Favour CERT_CHAIN_POLICY_IGNORE_WRONG_USAGE_FLAG in verify_ssl_policy(). crypt32: Only mind end certificate when checking revocation status in verify_ssl_policy(). crypt32: Favour CERT_CHAIN_POLICY_IGNORE_END_REV_UNKNOWN_FLAG in verify_ssl_policy(). crypt32: Check CERT_TRUST_REVOCATION_STATUS_UNKNOWN instead of CERT_TRUST_IS_OFFLINE_REVOCATION in verify_ssl_policy(). crypt32: Fix some error codes in verify_ssl_policy(). crypt32/tests: Add more tests for VerifyCertChainPolicy(). https://gitlab.winehq.org/wine/wine/-/merge_requests/7117
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/crypt32/tests/chain.c | 132 +++++++++++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/dlls/crypt32/tests/chain.c b/dlls/crypt32/tests/chain.c index 9717b184c14..2a4ae516903 100644 --- a/dlls/crypt32/tests/chain.c +++ b/dlls/crypt32/tests/chain.c @@ -5376,10 +5376,142 @@ static void testVerifyCertChainPolicy(void) check_msroot_policy(); } +static void test_VerifyCertChainPolicy_flags(void) +{ + static const struct + { + DWORD trust_status; + unsigned int index; + DWORD policy_flags; + DWORD ssl_policy_flags; + DWORD expected_error; + BOOL wine_todo; + } + tests[] = + { + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, 0, 0, CRYPT_E_REVOCATION_OFFLINE, TRUE }, + /* CERT_TRUST_REVOCATION_STATUS_UNKNOWN is only checked on the end certificate. */ + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 1, 0, 0, ERROR_SUCCESS, TRUE }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 2, 0, 0, ERROR_SUCCESS, TRUE }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_END_REV_UNKNOWN_FLAG, 0, 0, TRUE }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_CTL_SIGNER_REV_UNKNOWN_FLAG, 0, CRYPT_E_REVOCATION_OFFLINE, TRUE }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_CA_REV_UNKNOWN_FLAG, 0, CRYPT_E_REVOCATION_OFFLINE, TRUE }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_ROOT_REV_UNKNOWN_FLAG, 0, CRYPT_E_REVOCATION_OFFLINE, TRUE }, + /* CERT_TRUST_IS_OFFLINE_REVOCATION is ignored. */ + { CERT_TRUST_IS_OFFLINE_REVOCATION, 0, 0, 0, ERROR_SUCCESS, TRUE }, + { CERT_TRUST_IS_OFFLINE_REVOCATION, 1, 0, 0, ERROR_SUCCESS, TRUE }, + { CERT_TRUST_IS_OFFLINE_REVOCATION, 2, 0, 0, ERROR_SUCCESS, TRUE }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 0, 0, 0, CRYPT_E_REVOCATION_OFFLINE, TRUE }, + /* CERT_TRUST_REVOCATION_STATUS_UNKNOWN is only checked on the end certificate. */ + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 1, 0, 0, ERROR_SUCCESS }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 2, 0, 0, ERROR_SUCCESS }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 0, CERT_CHAIN_POLICY_IGNORE_END_REV_UNKNOWN_FLAG, 0, 0 }, + + { CERT_TRUST_IS_REVOKED, 0, 0, 0, CRYPT_E_REVOKED, TRUE }, + { CERT_TRUST_IS_REVOKED, 1, 0, 0, CRYPT_E_REVOKED, TRUE }, + { CERT_TRUST_IS_REVOKED, 2, 0, 0, CRYPT_E_REVOKED, TRUE }, + + { CERT_TRUST_IS_NOT_VALID_FOR_USAGE, 0, 0, 0, CERT_E_WRONG_USAGE }, + { CERT_TRUST_IS_NOT_VALID_FOR_USAGE, 1, 0, 0, CERT_E_WRONG_USAGE }, + { CERT_TRUST_IS_NOT_VALID_FOR_USAGE, 2, 0, 0, CERT_E_WRONG_USAGE }, + { CERT_TRUST_IS_NOT_VALID_FOR_USAGE, 0, 0, SECURITY_FLAG_IGNORE_WRONG_USAGE, ERROR_SUCCESS }, + { CERT_TRUST_IS_NOT_VALID_FOR_USAGE, 0, CERT_CHAIN_POLICY_IGNORE_WRONG_USAGE_FLAG, 0, ERROR_SUCCESS, TRUE }, + + { CERT_TRUST_IS_SELF_SIGNED, 0, 0, 0, TRUST_E_CERT_SIGNATURE }, + { CERT_TRUST_IS_SELF_SIGNED, 1, 0, 0, TRUST_E_CERT_SIGNATURE }, + { CERT_TRUST_IS_SELF_SIGNED, 2, 0, 0, TRUST_E_CERT_SIGNATURE }, + { CERT_TRUST_IS_SELF_SIGNED, 2, 0, SECURITY_FLAG_IGNORE_UNKNOWN_CA, TRUST_E_CERT_SIGNATURE }, + { CERT_TRUST_IS_SELF_SIGNED, 2, CERT_CHAIN_POLICY_ALLOW_UNKNOWN_CA_FLAG, 0, TRUST_E_CERT_SIGNATURE }, + }; + + BOOL ret; + PCCERT_CONTEXT cert; + CERT_CHAIN_PARA para = { 0 }; + PCCERT_CHAIN_CONTEXT chain; + FILETIME fileTime; + HCERTSTORE store; + static char one_two_three[] = "1.2.3"; + LPSTR oids[1]; + SSL_EXTRA_CERT_CHAIN_POLICY_PARA ssl_para; + CERT_CHAIN_POLICY_PARA policy_para; + CERT_CHAIN_POLICY_STATUS status; + CERT_REVOCATION_INFO rev_info[3]; + unsigned int i; + + store = CertOpenStore(CERT_STORE_PROV_MEMORY, 0, 0, CERT_STORE_CREATE_NEW_FLAG, NULL); + CertAddEncodedCertificateToStore(store, X509_ASN_ENCODING, geotrust_global_ca, + sizeof(geotrust_global_ca), CERT_STORE_ADD_ALWAYS, NULL); + CertAddEncodedCertificateToStore(store, X509_ASN_ENCODING, google_internet_authority, + sizeof(google_internet_authority), CERT_STORE_ADD_ALWAYS, NULL); + cert = CertCreateCertificateContext(X509_ASN_ENCODING, google_com, sizeof(google_com)); + SystemTimeToFileTime(&oct2009, &fileTime); + memset(¶, 0, sizeof(para)); + para.cbSize = sizeof(para); + oids[0] = one_two_three; + para.RequestedUsage.dwType = USAGE_MATCH_TYPE_AND; + para.RequestedUsage.Usage.rgpszUsageIdentifier = oids; + para.RequestedUsage.Usage.cUsageIdentifier = 1; + ret = CertGetCertificateChain(NULL, cert, &fileTime, store, ¶, CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY, NULL, &chain); + ok(ret, "got error %#lx.\n", GetLastError()); + ok(chain->cChain == 1, "got %lu.\n", chain->cChain); + ok(chain->rgpChain[0]->cElement == 3, "got %lu.\n", chain->rgpChain[0]->cElement); + + memset(&policy_para, 0, sizeof(policy_para)); + policy_para.cbSize = sizeof(policy_para); + memset(&ssl_para, 0, sizeof(ssl_para)); + ssl_para.cbSize = sizeof(ssl_para); + ssl_para.dwAuthType = AUTHTYPE_SERVER; + ssl_para.pwszServerName = (WCHAR *)L"www.google.com"; + policy_para.pvExtraPolicyPara = &ssl_para; + status.cbSize = sizeof(status); + + for (i = 0; i < chain->rgpChain[0]->cElement; ++i) + { + chain->rgpChain[0]->rgpElement[i]->TrustStatus.dwErrorStatus = 0; + memset(&rev_info[i], 0, sizeof(rev_info[i])); + rev_info[i].cbSize = sizeof(rev_info); + chain->rgpChain[0]->rgpElement[i]->pRevocationInfo = &rev_info[i]; + } + + for (i = 0; i < ARRAY_SIZE(tests); ++i) + { + winetest_push_context("test %u", i); + *(DWORD *)&chain->TrustStatus.dwErrorStatus = tests[i].trust_status; + chain->rgpChain[0]->TrustStatus.dwErrorStatus = chain->TrustStatus.dwErrorStatus; + chain->rgpChain[0]->rgpElement[tests[i].index]->TrustStatus.dwErrorStatus = chain->TrustStatus.dwErrorStatus; + policy_para.dwFlags = tests[i].policy_flags; + ssl_para.cbSize = sizeof(ssl_para); + ssl_para.fdwChecks = tests[i].ssl_policy_flags; + policy_para.pvExtraPolicyPara = &ssl_para; + ret = CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_SSL, chain, &policy_para, &status); + ok(ret, "got error %#lx.\n", GetLastError()); + todo_wine_if(tests[i].wine_todo) ok(status.dwError == tests[i].expected_error, "got %#lx, expected %#lx.\n", status.dwError, tests[i].expected_error); + if (status.dwError) + { + ok(!status.lChainIndex, "got %ld.\n", status.lChainIndex); + ok(status.lElementIndex == tests[i].index, "got %ld.\n", status.lElementIndex); + } + else + { + ok(status.lChainIndex == -1, "got %ld.\n", status.lChainIndex); + ok(status.lElementIndex == -1, "got %ld.\n", status.lElementIndex); + } + chain->rgpChain[0]->rgpElement[tests[i].index]->TrustStatus.dwErrorStatus = 0; + winetest_pop_context(); + } + for (i = 0; i < chain->rgpChain[0]->cElement; ++i) + chain->rgpChain[0]->rgpElement[i]->pRevocationInfo = NULL; + + CertFreeCertificateChain(chain); + CertFreeCertificateContext(cert); + CertCloseStore(store, 0); +} + START_TEST(chain) { testCreateCertChainEngine(); testVerifyCertChainPolicy(); testGetCertChain(); test_CERT_CHAIN_PARA_cbSize(); + test_VerifyCertChainPolicy_flags(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7117
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/crypt32/chain.c | 4 ++-- dlls/crypt32/tests/chain.c | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/dlls/crypt32/chain.c b/dlls/crypt32/chain.c index 5e4288eda37..b7072a2f8b4 100644 --- a/dlls/crypt32/chain.c +++ b/dlls/crypt32/chain.c @@ -3513,7 +3513,7 @@ static BOOL WINAPI verify_ssl_policy(LPCSTR szPolicyOID, else if (pChainContext->TrustStatus.dwErrorStatus & CERT_TRUST_IS_REVOKED && !(checks & SECURITY_FLAG_IGNORE_REVOCATION)) { - pPolicyStatus->dwError = CERT_E_REVOKED; + pPolicyStatus->dwError = CRYPT_E_REVOKED; find_element_with_error(pChainContext, CERT_TRUST_IS_REVOKED, &pPolicyStatus->lChainIndex, &pPolicyStatus->lElementIndex); @@ -3522,7 +3522,7 @@ static BOOL WINAPI verify_ssl_policy(LPCSTR szPolicyOID, CERT_TRUST_IS_OFFLINE_REVOCATION && !(checks & SECURITY_FLAG_IGNORE_REVOCATION)) { - pPolicyStatus->dwError = CERT_E_REVOCATION_FAILURE; + pPolicyStatus->dwError = CRYPT_E_REVOCATION_OFFLINE; find_element_with_error(pChainContext, CERT_TRUST_IS_OFFLINE_REVOCATION, &pPolicyStatus->lChainIndex, &pPolicyStatus->lElementIndex); diff --git a/dlls/crypt32/tests/chain.c b/dlls/crypt32/tests/chain.c index 2a4ae516903..9d3a22d5555 100644 --- a/dlls/crypt32/tests/chain.c +++ b/dlls/crypt32/tests/chain.c @@ -5389,14 +5389,14 @@ static void test_VerifyCertChainPolicy_flags(void) } tests[] = { - { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, 0, 0, CRYPT_E_REVOCATION_OFFLINE, TRUE }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, 0, 0, CRYPT_E_REVOCATION_OFFLINE }, /* CERT_TRUST_REVOCATION_STATUS_UNKNOWN is only checked on the end certificate. */ { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 1, 0, 0, ERROR_SUCCESS, TRUE }, { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 2, 0, 0, ERROR_SUCCESS, TRUE }, { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_END_REV_UNKNOWN_FLAG, 0, 0, TRUE }, - { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_CTL_SIGNER_REV_UNKNOWN_FLAG, 0, CRYPT_E_REVOCATION_OFFLINE, TRUE }, - { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_CA_REV_UNKNOWN_FLAG, 0, CRYPT_E_REVOCATION_OFFLINE, TRUE }, - { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_ROOT_REV_UNKNOWN_FLAG, 0, CRYPT_E_REVOCATION_OFFLINE, TRUE }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_CTL_SIGNER_REV_UNKNOWN_FLAG, 0, CRYPT_E_REVOCATION_OFFLINE }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_CA_REV_UNKNOWN_FLAG, 0, CRYPT_E_REVOCATION_OFFLINE }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_ROOT_REV_UNKNOWN_FLAG, 0, CRYPT_E_REVOCATION_OFFLINE }, /* CERT_TRUST_IS_OFFLINE_REVOCATION is ignored. */ { CERT_TRUST_IS_OFFLINE_REVOCATION, 0, 0, 0, ERROR_SUCCESS, TRUE }, { CERT_TRUST_IS_OFFLINE_REVOCATION, 1, 0, 0, ERROR_SUCCESS, TRUE }, @@ -5407,9 +5407,9 @@ static void test_VerifyCertChainPolicy_flags(void) { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 2, 0, 0, ERROR_SUCCESS }, { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 0, CERT_CHAIN_POLICY_IGNORE_END_REV_UNKNOWN_FLAG, 0, 0 }, - { CERT_TRUST_IS_REVOKED, 0, 0, 0, CRYPT_E_REVOKED, TRUE }, - { CERT_TRUST_IS_REVOKED, 1, 0, 0, CRYPT_E_REVOKED, TRUE }, - { CERT_TRUST_IS_REVOKED, 2, 0, 0, CRYPT_E_REVOKED, TRUE }, + { CERT_TRUST_IS_REVOKED, 0, 0, 0, CRYPT_E_REVOKED }, + { CERT_TRUST_IS_REVOKED, 1, 0, 0, CRYPT_E_REVOKED }, + { CERT_TRUST_IS_REVOKED, 2, 0, 0, CRYPT_E_REVOKED }, { CERT_TRUST_IS_NOT_VALID_FOR_USAGE, 0, 0, 0, CERT_E_WRONG_USAGE }, { CERT_TRUST_IS_NOT_VALID_FOR_USAGE, 1, 0, 0, CERT_E_WRONG_USAGE }, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7117
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/crypt32/chain.c | 8 +++----- dlls/crypt32/tests/chain.c | 14 +++++++------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/dlls/crypt32/chain.c b/dlls/crypt32/chain.c index b7072a2f8b4..fb2f391ff6e 100644 --- a/dlls/crypt32/chain.c +++ b/dlls/crypt32/chain.c @@ -2707,6 +2707,7 @@ static void CRYPT_VerifyChainRevocation(PCERT_CHAIN_CONTEXT chain, switch (revocationStatus.dwError) { + case CRYPT_E_REVOCATION_OFFLINE: case CRYPT_E_NO_REVOCATION_CHECK: case CRYPT_E_NO_REVOCATION_DLL: case CRYPT_E_NOT_IN_REVOCATION_DATABASE: @@ -2716,9 +2717,6 @@ static void CRYPT_VerifyChainRevocation(PCERT_CHAIN_CONTEXT chain, error = CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION; break; - case CRYPT_E_REVOCATION_OFFLINE: - error = CERT_TRUST_IS_OFFLINE_REVOCATION; - break; case CRYPT_E_REVOKED: error = CERT_TRUST_IS_REVOKED; break; @@ -3519,12 +3517,12 @@ static BOOL WINAPI verify_ssl_policy(LPCSTR szPolicyOID, &pPolicyStatus->lElementIndex); } else if (pChainContext->TrustStatus.dwErrorStatus & - CERT_TRUST_IS_OFFLINE_REVOCATION && + CERT_TRUST_REVOCATION_STATUS_UNKNOWN && !(checks & SECURITY_FLAG_IGNORE_REVOCATION)) { pPolicyStatus->dwError = CRYPT_E_REVOCATION_OFFLINE; find_element_with_error(pChainContext, - CERT_TRUST_IS_OFFLINE_REVOCATION, &pPolicyStatus->lChainIndex, + CERT_TRUST_REVOCATION_STATUS_UNKNOWN, &pPolicyStatus->lChainIndex, &pPolicyStatus->lElementIndex); } else if (pChainContext->TrustStatus.dwErrorStatus & diff --git a/dlls/crypt32/tests/chain.c b/dlls/crypt32/tests/chain.c index 9d3a22d5555..9c1623e206c 100644 --- a/dlls/crypt32/tests/chain.c +++ b/dlls/crypt32/tests/chain.c @@ -5398,14 +5398,14 @@ static void test_VerifyCertChainPolicy_flags(void) { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_CA_REV_UNKNOWN_FLAG, 0, CRYPT_E_REVOCATION_OFFLINE }, { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_ROOT_REV_UNKNOWN_FLAG, 0, CRYPT_E_REVOCATION_OFFLINE }, /* CERT_TRUST_IS_OFFLINE_REVOCATION is ignored. */ - { CERT_TRUST_IS_OFFLINE_REVOCATION, 0, 0, 0, ERROR_SUCCESS, TRUE }, - { CERT_TRUST_IS_OFFLINE_REVOCATION, 1, 0, 0, ERROR_SUCCESS, TRUE }, - { CERT_TRUST_IS_OFFLINE_REVOCATION, 2, 0, 0, ERROR_SUCCESS, TRUE }, - { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 0, 0, 0, CRYPT_E_REVOCATION_OFFLINE, TRUE }, + { CERT_TRUST_IS_OFFLINE_REVOCATION, 0, 0, 0, ERROR_SUCCESS }, + { CERT_TRUST_IS_OFFLINE_REVOCATION, 1, 0, 0, ERROR_SUCCESS }, + { CERT_TRUST_IS_OFFLINE_REVOCATION, 2, 0, 0, ERROR_SUCCESS }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 0, 0, 0, CRYPT_E_REVOCATION_OFFLINE }, /* CERT_TRUST_REVOCATION_STATUS_UNKNOWN is only checked on the end certificate. */ - { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 1, 0, 0, ERROR_SUCCESS }, - { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 2, 0, 0, ERROR_SUCCESS }, - { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 0, CERT_CHAIN_POLICY_IGNORE_END_REV_UNKNOWN_FLAG, 0, 0 }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 1, 0, 0, ERROR_SUCCESS, TRUE }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 2, 0, 0, ERROR_SUCCESS, TRUE }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 0, CERT_CHAIN_POLICY_IGNORE_END_REV_UNKNOWN_FLAG, 0, 0, TRUE }, { CERT_TRUST_IS_REVOKED, 0, 0, 0, CRYPT_E_REVOKED }, { CERT_TRUST_IS_REVOKED, 1, 0, 0, CRYPT_E_REVOKED }, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7117
From: Paul Gofman <pgofman(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56559 --- dlls/crypt32/chain.c | 2 +- dlls/crypt32/tests/chain.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dlls/crypt32/chain.c b/dlls/crypt32/chain.c index fb2f391ff6e..61f9e66f30c 100644 --- a/dlls/crypt32/chain.c +++ b/dlls/crypt32/chain.c @@ -3518,7 +3518,7 @@ static BOOL WINAPI verify_ssl_policy(LPCSTR szPolicyOID, } else if (pChainContext->TrustStatus.dwErrorStatus & CERT_TRUST_REVOCATION_STATUS_UNKNOWN && - !(checks & SECURITY_FLAG_IGNORE_REVOCATION)) + !(checks & SECURITY_FLAG_IGNORE_REVOCATION) && !(baseChecks & CERT_CHAIN_POLICY_IGNORE_END_REV_UNKNOWN_FLAG)) { pPolicyStatus->dwError = CRYPT_E_REVOCATION_OFFLINE; find_element_with_error(pChainContext, diff --git a/dlls/crypt32/tests/chain.c b/dlls/crypt32/tests/chain.c index 9c1623e206c..06d9f7d730e 100644 --- a/dlls/crypt32/tests/chain.c +++ b/dlls/crypt32/tests/chain.c @@ -5393,7 +5393,7 @@ static void test_VerifyCertChainPolicy_flags(void) /* CERT_TRUST_REVOCATION_STATUS_UNKNOWN is only checked on the end certificate. */ { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 1, 0, 0, ERROR_SUCCESS, TRUE }, { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 2, 0, 0, ERROR_SUCCESS, TRUE }, - { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_END_REV_UNKNOWN_FLAG, 0, 0, TRUE }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_END_REV_UNKNOWN_FLAG, 0, 0 }, { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_CTL_SIGNER_REV_UNKNOWN_FLAG, 0, CRYPT_E_REVOCATION_OFFLINE }, { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_CA_REV_UNKNOWN_FLAG, 0, CRYPT_E_REVOCATION_OFFLINE }, { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_ROOT_REV_UNKNOWN_FLAG, 0, CRYPT_E_REVOCATION_OFFLINE }, @@ -5405,7 +5405,7 @@ static void test_VerifyCertChainPolicy_flags(void) /* CERT_TRUST_REVOCATION_STATUS_UNKNOWN is only checked on the end certificate. */ { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 1, 0, 0, ERROR_SUCCESS, TRUE }, { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 2, 0, 0, ERROR_SUCCESS, TRUE }, - { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 0, CERT_CHAIN_POLICY_IGNORE_END_REV_UNKNOWN_FLAG, 0, 0, TRUE }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 0, CERT_CHAIN_POLICY_IGNORE_END_REV_UNKNOWN_FLAG, 0, 0 }, { CERT_TRUST_IS_REVOKED, 0, 0, 0, CRYPT_E_REVOKED }, { CERT_TRUST_IS_REVOKED, 1, 0, 0, CRYPT_E_REVOKED }, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7117
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/crypt32/chain.c | 25 +++++++++++++++++++++---- dlls/crypt32/tests/chain.c | 8 ++++---- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/dlls/crypt32/chain.c b/dlls/crypt32/chain.c index 61f9e66f30c..e8dc15ff7e9 100644 --- a/dlls/crypt32/chain.c +++ b/dlls/crypt32/chain.c @@ -2989,6 +2989,24 @@ static void find_element_with_error(PCCERT_CHAIN_CONTEXT chain, DWORD error, } } +static BOOL find_chain_first_element_with_error(PCCERT_CHAIN_CONTEXT chain, DWORD error, LONG *chain_idx, + LONG *element_idx) +{ + unsigned int i; + + for (i = 0; i < chain->cChain; i++) + { + if (!chain->rgpChain[i]->cElement) continue; + if (chain->rgpChain[i]->rgpElement[0]->TrustStatus.dwErrorStatus & error) + { + *chain_idx = i; + *element_idx = 0; + return TRUE; + } + } + return FALSE; +} + static BOOL WINAPI verify_base_policy(LPCSTR szPolicyOID, PCCERT_CHAIN_CONTEXT pChainContext, PCERT_CHAIN_POLICY_PARA pPolicyPara, PCERT_CHAIN_POLICY_STATUS pPolicyStatus) @@ -3518,12 +3536,11 @@ static BOOL WINAPI verify_ssl_policy(LPCSTR szPolicyOID, } else if (pChainContext->TrustStatus.dwErrorStatus & CERT_TRUST_REVOCATION_STATUS_UNKNOWN && - !(checks & SECURITY_FLAG_IGNORE_REVOCATION) && !(baseChecks & CERT_CHAIN_POLICY_IGNORE_END_REV_UNKNOWN_FLAG)) + !(checks & SECURITY_FLAG_IGNORE_REVOCATION) && !(baseChecks & CERT_CHAIN_POLICY_IGNORE_END_REV_UNKNOWN_FLAG) + && find_chain_first_element_with_error(pChainContext, CERT_TRUST_REVOCATION_STATUS_UNKNOWN, + &pPolicyStatus->lChainIndex, &pPolicyStatus->lElementIndex)) { pPolicyStatus->dwError = CRYPT_E_REVOCATION_OFFLINE; - find_element_with_error(pChainContext, - CERT_TRUST_REVOCATION_STATUS_UNKNOWN, &pPolicyStatus->lChainIndex, - &pPolicyStatus->lElementIndex); } else if (pChainContext->TrustStatus.dwErrorStatus & CERT_TRUST_HAS_NOT_SUPPORTED_CRITICAL_EXT) diff --git a/dlls/crypt32/tests/chain.c b/dlls/crypt32/tests/chain.c index 06d9f7d730e..268e03155d7 100644 --- a/dlls/crypt32/tests/chain.c +++ b/dlls/crypt32/tests/chain.c @@ -5391,8 +5391,8 @@ static void test_VerifyCertChainPolicy_flags(void) { { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, 0, 0, CRYPT_E_REVOCATION_OFFLINE }, /* CERT_TRUST_REVOCATION_STATUS_UNKNOWN is only checked on the end certificate. */ - { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 1, 0, 0, ERROR_SUCCESS, TRUE }, - { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 2, 0, 0, ERROR_SUCCESS, TRUE }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 1, 0, 0, ERROR_SUCCESS }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 2, 0, 0, ERROR_SUCCESS }, { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_END_REV_UNKNOWN_FLAG, 0, 0 }, { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_CTL_SIGNER_REV_UNKNOWN_FLAG, 0, CRYPT_E_REVOCATION_OFFLINE }, { CERT_TRUST_REVOCATION_STATUS_UNKNOWN | CERT_TRUST_IS_OFFLINE_REVOCATION, 0, CERT_CHAIN_POLICY_IGNORE_CA_REV_UNKNOWN_FLAG, 0, CRYPT_E_REVOCATION_OFFLINE }, @@ -5403,8 +5403,8 @@ static void test_VerifyCertChainPolicy_flags(void) { CERT_TRUST_IS_OFFLINE_REVOCATION, 2, 0, 0, ERROR_SUCCESS }, { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 0, 0, 0, CRYPT_E_REVOCATION_OFFLINE }, /* CERT_TRUST_REVOCATION_STATUS_UNKNOWN is only checked on the end certificate. */ - { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 1, 0, 0, ERROR_SUCCESS, TRUE }, - { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 2, 0, 0, ERROR_SUCCESS, TRUE }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 1, 0, 0, ERROR_SUCCESS }, + { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 2, 0, 0, ERROR_SUCCESS }, { CERT_TRUST_REVOCATION_STATUS_UNKNOWN, 0, CERT_CHAIN_POLICY_IGNORE_END_REV_UNKNOWN_FLAG, 0, 0 }, { CERT_TRUST_IS_REVOKED, 0, 0, 0, CRYPT_E_REVOKED }, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7117
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/crypt32/chain.c | 2 +- dlls/crypt32/tests/chain.c | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/dlls/crypt32/chain.c b/dlls/crypt32/chain.c index e8dc15ff7e9..fa74ade13d6 100644 --- a/dlls/crypt32/chain.c +++ b/dlls/crypt32/chain.c @@ -3519,7 +3519,7 @@ static BOOL WINAPI verify_ssl_policy(LPCSTR szPolicyOID, } else if (pChainContext->TrustStatus.dwErrorStatus & CERT_TRUST_IS_NOT_VALID_FOR_USAGE && - !(checks & SECURITY_FLAG_IGNORE_WRONG_USAGE)) + !(checks & SECURITY_FLAG_IGNORE_WRONG_USAGE) && !(baseChecks & CERT_CHAIN_POLICY_IGNORE_WRONG_USAGE_FLAG)) { pPolicyStatus->dwError = CERT_E_WRONG_USAGE; find_element_with_error(pChainContext, diff --git a/dlls/crypt32/tests/chain.c b/dlls/crypt32/tests/chain.c index 268e03155d7..5522ab4f01b 100644 --- a/dlls/crypt32/tests/chain.c +++ b/dlls/crypt32/tests/chain.c @@ -5385,7 +5385,6 @@ static void test_VerifyCertChainPolicy_flags(void) DWORD policy_flags; DWORD ssl_policy_flags; DWORD expected_error; - BOOL wine_todo; } tests[] = { @@ -5415,7 +5414,7 @@ static void test_VerifyCertChainPolicy_flags(void) { CERT_TRUST_IS_NOT_VALID_FOR_USAGE, 1, 0, 0, CERT_E_WRONG_USAGE }, { CERT_TRUST_IS_NOT_VALID_FOR_USAGE, 2, 0, 0, CERT_E_WRONG_USAGE }, { CERT_TRUST_IS_NOT_VALID_FOR_USAGE, 0, 0, SECURITY_FLAG_IGNORE_WRONG_USAGE, ERROR_SUCCESS }, - { CERT_TRUST_IS_NOT_VALID_FOR_USAGE, 0, CERT_CHAIN_POLICY_IGNORE_WRONG_USAGE_FLAG, 0, ERROR_SUCCESS, TRUE }, + { CERT_TRUST_IS_NOT_VALID_FOR_USAGE, 0, CERT_CHAIN_POLICY_IGNORE_WRONG_USAGE_FLAG, 0, ERROR_SUCCESS }, { CERT_TRUST_IS_SELF_SIGNED, 0, 0, 0, TRUST_E_CERT_SIGNATURE }, { CERT_TRUST_IS_SELF_SIGNED, 1, 0, 0, TRUST_E_CERT_SIGNATURE }, @@ -5485,7 +5484,7 @@ static void test_VerifyCertChainPolicy_flags(void) policy_para.pvExtraPolicyPara = &ssl_para; ret = CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_SSL, chain, &policy_para, &status); ok(ret, "got error %#lx.\n", GetLastError()); - todo_wine_if(tests[i].wine_todo) ok(status.dwError == tests[i].expected_error, "got %#lx, expected %#lx.\n", status.dwError, tests[i].expected_error); + ok(status.dwError == tests[i].expected_error, "got %#lx, expected %#lx.\n", status.dwError, tests[i].expected_error); if (status.dwError) { ok(!status.lChainIndex, "got %ld.\n", status.lChainIndex); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7117
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=150818 Your paranoid android. === debian11b (64 bit WoW report) === dnsapi: query.c:142: Test failed: expected record name L"testbot.winehq.org", got L"becky.ns.cloudflare.com" secur32: schannel.c:1206: Test failed: QueryContextAttributesW(SECPKG_ATTR_ENDPOINT_BINDINGS) failed: 00000057 schannel.c:1284: Test failed: got L"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384" schannel.c:1298: Test failed: got L"ECDSA" schannel.c:1307: Test failed: key_info.SignatureAlgorithm = 2203 schannel.c:1309: Test failed: key_info.sSignatureAlgorithmName = ECDSA user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000004200FE, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032 wininet: http.c:6917: Test failed: lpszSubjectInfo = test.winehq.org
v3: - remove leftover '//' comments; - fix a typo in comments in test; - fix the subject for the first patch. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7117#note_91716
participants (3)
-
Marvin -
Paul Gofman -
Paul Gofman (＠gofman)