aboutsummaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
authorJohannes Schindelin <johannes.schindelin@gmx.de>2020-02-26 11:24:26 +0100
committerJay Satiro <raysatiro@yahoo.com>2020-03-18 03:23:39 -0400
commit54504284918a4ba19bc7b1efb486a64629d376aa (patch)
treed945df3518472bcb02efcd95caf16bbdadd52d33 /lib
parenta268ad5d7e84189d8dfec3705d84a88ae064f7fc (diff)
schannel: add "best effort" revocation check option
- Implement new option CURLSSLOPT_REVOKE_BEST_EFFORT and --ssl-revoke-best-effort to allow a "best effort" revocation check. A best effort revocation check ignores errors that the revocation check was unable to take place. The reasoning is described in detail below and discussed further in the PR. --- When running e.g. with Fiddler, the schannel backend fails with an unhelpful error message: Unknown error (0x80092012) - The revocation function was unable to check revocation for the certificate. Sadly, many enterprise users who are stuck behind MITM proxies suffer the very same problem. This has been discussed in plenty of issues: https://github.com/curl/curl/issues/3727, https://github.com/curl/curl/issues/264, for example. In the latter, a Microsoft Edge developer even made the case that the common behavior is to ignore issues when a certificate has no recorded distribution point for revocation lists, or when the server is offline. This is also known as "best effort" strategy and addresses the Fiddler issue. Unfortunately, this strategy was not chosen as the default for schannel (and is therefore a backend-specific behavior: OpenSSL seems to happily ignore the offline servers and missing distribution points). To maintain backward-compatibility, we therefore add a new flag (`CURLSSLOPT_REVOKE_BEST_EFFORT`) and a new option (`--ssl-revoke-best-effort`) to select the new behavior. Due to the many related issues Git for Windows and GitHub Desktop, the plan is to make this behavior the default in these software packages. The test 2070 was added to verify this behavior, adapted from 310. Based-on-work-by: georgeok <giorgos.n.oikonomou@gmail.com> Co-authored-by: Markus Olsson <j.markus.olsson@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Closes https://github.com/curl/curl/pull/4981
Diffstat (limited to 'lib')
-rw-r--r--lib/doh.c5
-rw-r--r--lib/setopt.c3
-rw-r--r--lib/urldata.h2
-rw-r--r--lib/vtls/schannel.c7
-rw-r--r--lib/vtls/schannel_verify.c9
5 files changed, 26 insertions, 0 deletions
diff --git a/lib/doh.c b/lib/doh.c
index aaa8f15ca..dd2bbf125 100644
--- a/lib/doh.c
+++ b/lib/doh.c
@@ -318,6 +318,9 @@ static CURLcode dohprobe(struct Curl_easy *data,
}
if(data->set.proxy_ssl.no_revoke)
ERROR_CHECK_SETOPT(CURLOPT_PROXY_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);
+ else if(data->set.proxy_ssl.revoke_best_effort)
+ ERROR_CHECK_SETOPT(CURLOPT_PROXY_SSL_OPTIONS,
+ CURLSSLOPT_REVOKE_BEST_EFFORT);
if(data->set.str[STRING_SSL_CAPATH_PROXY]) {
ERROR_CHECK_SETOPT(CURLOPT_PROXY_CAPATH,
data->set.str[STRING_SSL_CAPATH_PROXY]);
@@ -351,6 +354,8 @@ static CURLcode dohprobe(struct Curl_easy *data,
}
if(data->set.ssl.no_revoke)
ERROR_CHECK_SETOPT(CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);
+ else if(data->set.ssl.revoke_best_effort)
+ ERROR_CHECK_SETOPT(CURLOPT_SSL_OPTIONS, CURLSSLOPT_REVOKE_BEST_EFFORT);
if(data->set.ssl.fsslctx)
ERROR_CHECK_SETOPT(CURLOPT_SSL_CTX_FUNCTION, data->set.ssl.fsslctx);
if(data->set.ssl.fsslctxp)
diff --git a/lib/setopt.c b/lib/setopt.c
index 4648c872b..04785a682 100644
--- a/lib/setopt.c
+++ b/lib/setopt.c
@@ -2134,6 +2134,7 @@ CURLcode Curl_vsetopt(struct Curl_easy *data, CURLoption option, va_list param)
(bool)((arg&CURLSSLOPT_ALLOW_BEAST) ? TRUE : FALSE);
data->set.ssl.no_revoke = !!(arg & CURLSSLOPT_NO_REVOKE);
data->set.ssl.no_partialchain = !!(arg & CURLSSLOPT_NO_PARTIALCHAIN);
+ data->set.ssl.revoke_best_effort = !!(arg & CURLSSLOPT_REVOKE_BEST_EFFORT);
break;
#ifndef CURL_DISABLE_PROXY
@@ -2143,6 +2144,8 @@ CURLcode Curl_vsetopt(struct Curl_easy *data, CURLoption option, va_list param)
(bool)((arg&CURLSSLOPT_ALLOW_BEAST) ? TRUE : FALSE);
data->set.proxy_ssl.no_revoke = !!(arg & CURLSSLOPT_NO_REVOKE);
data->set.proxy_ssl.no_partialchain = !!(arg & CURLSSLOPT_NO_PARTIALCHAIN);
+ data->set.proxy_ssl.revoke_best_effort =
+ !!(arg & CURLSSLOPT_REVOKE_BEST_EFFORT);
break;
#endif
diff --git a/lib/urldata.h b/lib/urldata.h
index 374bf4371..2a36c1147 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -258,6 +258,8 @@ struct ssl_config_data {
BIT(enable_beast); /* allow this flaw for interoperability's sake*/
BIT(no_revoke); /* disable SSL certificate revocation checks */
BIT(no_partialchain); /* don't accept partial certificate chains */
+ BIT(revoke_best_effort); /* ignore SSL revocation offline/missing revocation
+ list errors */
};
struct ssl_general_config {
diff --git a/lib/vtls/schannel.c b/lib/vtls/schannel.c
index 3b9aef47c..64719fe70 100644
--- a/lib/vtls/schannel.c
+++ b/lib/vtls/schannel.c
@@ -520,8 +520,15 @@ schannel_connect_step1(struct connectdata *conn, int sockindex)
DEBUGF(infof(data, "schannel: disabled server certificate revocation "
"checks\n"));
}
+ else if(data->set.ssl.revoke_best_effort) {
+ schannel_cred.dwFlags |= SCH_CRED_IGNORE_NO_REVOCATION_CHECK |
+ SCH_CRED_IGNORE_REVOCATION_OFFLINE | SCH_CRED_REVOCATION_CHECK_CHAIN;
+
+ DEBUGF(infof(data, "schannel: ignore revocation offline errors"));
+ }
else {
schannel_cred.dwFlags |= SCH_CRED_REVOCATION_CHECK_CHAIN;
+
DEBUGF(infof(data,
"schannel: checking server certificate revocation\n"));
}
diff --git a/lib/vtls/schannel_verify.c b/lib/vtls/schannel_verify.c
index e75132cad..3dbc11f05 100644
--- a/lib/vtls/schannel_verify.c
+++ b/lib/vtls/schannel_verify.c
@@ -636,6 +636,15 @@ CURLcode Curl_verify_certificate(struct connectdata *conn, int sockindex)
CERT_SIMPLE_CHAIN *pSimpleChain = pChainContext->rgpChain[0];
DWORD dwTrustErrorMask = ~(DWORD)(CERT_TRUST_IS_NOT_TIME_NESTED);
dwTrustErrorMask &= pSimpleChain->TrustStatus.dwErrorStatus;
+
+ if(data->set.ssl.revoke_best_effort) {
+ /* Ignore errors when root certificates are missing the revocation
+ * list URL, or when the list could not be downloaded because the
+ * server is currently unreachable. */
+ dwTrustErrorMask &= ~(DWORD)(CERT_TRUST_REVOCATION_STATUS_UNKNOWN |
+ CERT_TRUST_IS_OFFLINE_REVOCATION);
+ }
+
if(dwTrustErrorMask) {
if(dwTrustErrorMask & CERT_TRUST_IS_REVOKED)
failf(data, "schannel: CertGetCertificateChain trust error"