On 14.07.2019 17:51, Evgeny Kotkov wrote: > Branko Čibej <br...@apache.org> writes: > >> Uh. I don't think so. >> >> First of all, the reference to Chromium source is a red herring ... that >> code disables CRL/OCSP checks *if some caller required it to*. You'll >> find that browsers do, indeed, check CRL or OCSP info, if it's available. > I went through an additional round of fact-checking to ensure that Chromium > browsers have never been doing online revocation checks by default. > > Back in 2014, this could be controlled by a user preference (disabled by > default), but since then the UI option was removed and the current state > is that such checks only operate from cache: > > > https://codereview.chromium.org/250583004/diff/20001/chrome/browser/net/ssl_config_service_manager_pref.cc > > https://chromium.googlesource.com/chromium/src/+/refs/tags/77.0.3852.2/chrome/browser/ssl/ssl_config_service_manager_pref.cc#243 > > On Windows, this can be additionally verified by examining the CryptoAPI > log where I can see the certificate chains being constructed with the > CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY flag. > >> Your change disables all online verification, regardless, and that seems >> quite wrong. > This change affects the Win32 callback used to *override specific* certificate > validation failures, namely SVN_AUTH_SSL_UNKNOWNCA. So in the current > design, the revocation checks are only supplemental, but not authoritative. > As an example, if the Serf/OpenSSL layer thinks that the certificate is OK > by itself (without performing a revocation check), this callback is not going > to be called at all. > > Since all infrastructure-related failures, stalls and timeouts during online > revocation checks appear to be fairly common, combined, this means that we > are putting the users through all these potential problems, but in the end > the whole checking is still non-exhaustive. Perhaps, the actual result of > that is just adding to the perception that Subversion is slow in general. > > With that in mind, I tend to think that it would be much better to just stick > to the locally available data for this particular callback, that is currently > used to only resolve a subset of certificate validation failures. > >> So ... -1, please revert > Will certainly do, unless this new data alters your opinion on the topic.
Hmm ... I wasn't aware that this check wasn't always performed. That changes the impact of you change considerably. Please leave it in for now ... but let's have that discussion anyway. -- Brane
