On 14.07.2019 17:51, Evgeny Kotkov wrote:
> Branko Čibej <brane_at_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
Received on 2019-07-14 18:23:40 CEST