Author: kotkov Date: Sat Jul 13 21:31:25 2019 New Revision: 1863018 URL: http://svn.apache.org/viewvc?rev=1863018&view=rev Log: Win32: tweak the SSL certificate validation override to avoid hitting the wire for URL based objects and revocation checks.
The primary purpose of this callback is to resolve SVN_AUTH_SSL_UNKNOWNCA failures using CryptoAPI and Windows local certificate stores. To do so, we should be fine with just using the immediately available data on the local machine. Doing the opposite of that appears to be troublesome, as always connecting to remote CRL and OCSP endpoints can result in spurious errors or significant (user-reported) network stalls caused by timeouts if the endpoints are inaccessible or unreliable. The new approach should also be in par with the default basic behavior of several major browsers, for example: https://chromium.googlesource.com/chromium/src/net/+/3d1dad1c17ae3ff59e7c35841af94b66f4bca1ba/cert/cert_verify_proc_win.cc#919 * subversion/libsvn_subr/win32_crypto.c (windows_validate_certificate): Use the CERT_CHAIN_CACHE_ONLY_URL_RETRIEVAL and CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY flags when preparing the certificate chain. Ignore errors in obtaining valid revocation information when verifying the chain, as they could be induced by the new cache-only behavior. Modified: subversion/trunk/subversion/libsvn_subr/win32_crypto.c Modified: subversion/trunk/subversion/libsvn_subr/win32_crypto.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/win32_crypto.c?rev=1863018&r1=1863017&r2=1863018&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_subr/win32_crypto.c (original) +++ subversion/trunk/subversion/libsvn_subr/win32_crypto.c Sat Jul 13 21:31:25 2019 @@ -395,16 +395,29 @@ windows_validate_certificate(svn_boolean memset(&chain_para, 0, sizeof(chain_para)); chain_para.cbSize = sizeof(chain_para); + /* Don't hit the wire for URL based objects and revocation checks, as + that may cause stalls, network timeouts or spurious errors in cases + such as with the remote OCSP and CRL endpoints being inaccessible or + unreliable. + + For this particular case of the SVN_AUTH_SSL_UNKNOWNCA cert failure + override we should be okay with just the data that we have immediately + available on the local machine. + */ if (CertGetCertificateChain(NULL, cert_context, NULL, NULL, &chain_para, CERT_CHAIN_CACHE_END_CERT | - CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT, + CERT_CHAIN_CACHE_ONLY_URL_RETRIEVAL | + CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT | + CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY, NULL, &chain_context)) { CERT_CHAIN_POLICY_PARA policy_para; CERT_CHAIN_POLICY_STATUS policy_status; policy_para.cbSize = sizeof(policy_para); - policy_para.dwFlags = 0; + /* We only use the local data for revocation checks, so they may + fail with errors like CRYPT_E_REVOCATION_OFFLINE; ignore those. */ + policy_para.dwFlags = CERT_CHAIN_POLICY_IGNORE_ALL_REV_UNKNOWN_FLAGS; policy_para.pvExtraPolicyPara = NULL; policy_status.cbSize = sizeof(policy_status);
