Zebediah Figura : cryptnet: Check only the first successfully retrieved CRL in verify_cert_revocation_from_dist_points_ext().
Alexandre Julliard
julliard at winehq.org
Thu Jul 22 16:28:19 CDT 2021
Module: wine
Branch: master
Commit: 13a349540ae0230f83466c04e003209656a889c0
URL: https://source.winehq.org/git/wine.git/?a=commit;h=13a349540ae0230f83466c04e003209656a889c0
Author: Zebediah Figura <zfigura at codeweavers.com>
Date: Thu Jul 22 00:21:06 2021 -0500
cryptnet: Check only the first successfully retrieved CRL in verify_cert_revocation_from_dist_points_ext().
>From RFC 5280 § 4.2.1.13:
If the DistributionPointName contains multiple values, each name
describes a different mechanism to obtain the same CRL. For example,
the same CRL could be available for retrieval through both LDAP and
HTTP.
Steam attempts to validate a certificate containing what are apparently two
different mirrored URLs to the same 20 MB CRL, which currently takes over 400ms
to parse in Wine. According to my reading of the RFC, we should only need to
parse one of them, cutting the time in half.
Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard at winehq.org>
---
dlls/cryptnet/cryptnet_main.c | 111 ++++++++++++++++++++++--------------------
1 file changed, 58 insertions(+), 53 deletions(-)
diff --git a/dlls/cryptnet/cryptnet_main.c b/dlls/cryptnet/cryptnet_main.c
index f4088021a0e..b1e2829d881 100644
--- a/dlls/cryptnet/cryptnet_main.c
+++ b/dlls/cryptnet/cryptnet_main.c
@@ -1528,66 +1528,71 @@ static DWORD verify_cert_revocation_with_crl_online(const CERT_CONTEXT *cert,
return ERROR_SUCCESS;
}
+/* Try to retrieve a CRL from any one of the specified distribution points. */
+static const CRL_CONTEXT *retrieve_crl_from_dist_points(const CRYPT_URL_ARRAY *array,
+ DWORD verify_flags, DWORD timeout)
+{
+ DWORD retrieve_flags = 0;
+ const CRL_CONTEXT *crl;
+ DWORD i;
+
+ if (verify_flags & CERT_VERIFY_CACHE_ONLY_BASED_REVOCATION)
+ retrieve_flags |= CRYPT_CACHE_ONLY_RETRIEVAL;
+
+ /* Yes, this is a weird algorithm, but the documentation for
+ * CERT_CHAIN_REVOCATION_ACCUMULATIVE_TIMEOUT specifies this, and
+ * tests seem to bear it out for CertVerifyRevocation() as well. */
+ if (verify_flags & CERT_VERIFY_REV_ACCUMULATIVE_TIMEOUT_FLAG)
+ timeout /= 2;
+
+ for (i = 0; i < array->cUrl; ++i)
+ {
+ if (CryptRetrieveObjectByUrlW(array->rgwszUrl[i], CONTEXT_OID_CRL, retrieve_flags,
+ timeout, (void **)&crl, NULL, NULL, NULL, NULL))
+ return crl;
+
+ /* We don't check the current time here. This may result in less
+ * accurate timeouts, but this too seems to be true of Windows. */
+ if ((verify_flags & CERT_VERIFY_REV_ACCUMULATIVE_TIMEOUT_FLAG) && GetLastError() == ERROR_TIMEOUT)
+ timeout /= 2;
+ }
+
+ return NULL;
+}
+
static DWORD verify_cert_revocation_from_dist_points_ext(const CRYPT_DATA_BLOB *value, const CERT_CONTEXT *cert,
- FILETIME *pTime, DWORD dwFlags, const CERT_REVOCATION_PARA *pRevPara, CERT_REVOCATION_STATUS *pRevStatus)
+ FILETIME *time, DWORD flags, const CERT_REVOCATION_PARA *params, CERT_REVOCATION_STATUS *status)
{
- DWORD error = ERROR_SUCCESS, cbUrlArray;
+ DWORD url_array_size, error;
+ CRYPT_URL_ARRAY *url_array;
+ const CRL_CONTEXT *crl;
+ DWORD timeout = 0;
+
+ if (!CRYPT_GetUrlFromCRLDistPointsExt(value, NULL, &url_array_size, NULL, NULL))
+ return GetLastError();
+
+ if (!(url_array = CryptMemAlloc(url_array_size)))
+ return ERROR_OUTOFMEMORY;
- if (CRYPT_GetUrlFromCRLDistPointsExt(value, NULL, &cbUrlArray, NULL, NULL))
+ if (!CRYPT_GetUrlFromCRLDistPointsExt(value, url_array, &url_array_size, NULL, NULL))
{
- CRYPT_URL_ARRAY *urlArray = CryptMemAlloc(cbUrlArray);
+ CryptMemFree(url_array);
+ return GetLastError();
+ }
- if (urlArray)
- {
- DWORD j, retrievalFlags = 0, timeout = 0;
- BOOL ret;
-
- ret = CRYPT_GetUrlFromCRLDistPointsExt(value, urlArray,
- &cbUrlArray, NULL, NULL);
- if (dwFlags & CERT_VERIFY_CACHE_ONLY_BASED_REVOCATION)
- retrievalFlags |= CRYPT_CACHE_ONLY_RETRIEVAL;
-
- if (pRevPara && pRevPara->cbSize >= RTL_SIZEOF_THROUGH_FIELD(CERT_REVOCATION_PARA, dwUrlRetrievalTimeout))
- timeout = pRevPara->dwUrlRetrievalTimeout;
-
- /* Yes, this is a weird algorithm, but the documentation for
- * CERT_CHAIN_REVOCATION_ACCUMULATIVE_TIMEOUT specifies this, and
- * tests seem to bear it out for CertVerifyRevocation() as well. */
- if (dwFlags & CERT_VERIFY_REV_ACCUMULATIVE_TIMEOUT_FLAG)
- timeout /= 2;
-
- if (!ret)
- error = GetLastError();
- /* continue looping if one was offline; break if revoked or timed out */
- for (j = 0; (!error || error == CRYPT_E_REVOCATION_OFFLINE) && j < urlArray->cUrl; j++)
- {
- PCCRL_CONTEXT crl;
+ if (params && params->cbSize >= RTL_SIZEOF_THROUGH_FIELD(CERT_REVOCATION_PARA, dwUrlRetrievalTimeout))
+ timeout = params->dwUrlRetrievalTimeout;
- ret = CryptRetrieveObjectByUrlW(urlArray->rgwszUrl[j],
- CONTEXT_OID_CRL, retrievalFlags, timeout, (void **)&crl,
- NULL, NULL, NULL, NULL);
- if (ret)
- {
- error = verify_cert_revocation_with_crl_online(cert, crl, pTime, pRevStatus);
- CertFreeCRLContext(crl);
- }
- else
- {
- /* We don't check the current time here. This may result in
- * less accurate timeouts, but this too seems to be true of
- * Windows. */
- if ((dwFlags & CERT_VERIFY_REV_ACCUMULATIVE_TIMEOUT_FLAG) && GetLastError() == ERROR_TIMEOUT)
- timeout /= 2;
- error = CRYPT_E_REVOCATION_OFFLINE;
- }
- }
- CryptMemFree(urlArray);
- }
- else
- error = ERROR_OUTOFMEMORY;
+ if (!(crl = retrieve_crl_from_dist_points(url_array, flags, timeout)))
+ {
+ CryptMemFree(url_array);
+ return CRYPT_E_REVOCATION_OFFLINE;
}
- else
- error = GetLastError();
+
+ error = verify_cert_revocation_with_crl_online(cert, crl, time, status);
+
+ CertFreeCRLContext(crl);
+ CryptMemFree(url_array);
return error;
}
More information about the wine-cvs
mailing list