[PATCH v2 0/6] MR7117: crypt32: Some fixes for verify_ssl_policy().
-- v2: 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..ac4ccbf5cf5 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 cheked 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 cheked 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 ac4ccbf5cf5..528ba1060c0 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 cheked 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 528ba1060c0..3b3d870a278 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 cheked 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 3b3d870a278..c4f9af3bf6e 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 cheked 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 cheked 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 c4f9af3bf6e..1f344e7e635 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 cheked 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 cheked 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 1f344e7e635..bd40028b0c8 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=150817 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 wininet: http.c:6917: Test failed: lpszSubjectInfo = test.winehq.org
participants (3)
-
Marvin -
Paul Gofman -
Paul Gofman (＠gofman)