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

diff --git a/docs/cmdline-opts/Makefile.inc b/docs/cmdline-opts/Makefile.inc
index 829551ff627133f642b93ea43dd5105f81f45ba8..f8df916b762d5298af6927a1d9d8a0b1f21b8b0e 100644 (file)
--- a/docs/cmdline-opts/Makefile.inc
+++ b/docs/cmdline-opts/Makefile.inc
@@ -179,6 +179,7 @@ DPAGES =                                    \
   ssl-allow-beast.d                            \
   ssl-no-revoke.d                              \
   ssl-reqd.d                                   \
+  ssl-revoke-best-effort.d                     \
   ssl.d                                                \
   sslv2.d sslv3.d                              \
   stderr.d                                     \

diff --git a/docs/cmdline-opts/ssl-revoke-best-effort.d b/docs/cmdline-opts/ssl-revoke-best-effort.d
new file mode 100644 (file)
index 0000000..9b55699
--- /dev/null
+++ b/docs/cmdline-opts/ssl-revoke-best-effort.d
@@ -0,0 +1,7 @@
+Long: ssl-revoke-best-effort
+Help: Ignore missing/offline cert CRL distribution points (Schannel)
+Added: 7.70.0
+---
+(Schannel) This option tells curl to ignore certificate revocation checks when
+they failed due to missing/offline distribution points for the revocation check
+lists.

diff --git a/docs/libcurl/opts/CURLOPT_PROXY_SSL_OPTIONS.3 b/docs/libcurl/opts/CURLOPT_PROXY_SSL_OPTIONS.3
index ded54c7b7e3813dc637cdce8a9d60734961d59cf..9c877c2a2dc7440c809767dd9e2afd52149275f6 100644 (file)
--- a/docs/libcurl/opts/CURLOPT_PROXY_SSL_OPTIONS.3
+++ b/docs/libcurl/opts/CURLOPT_PROXY_SSL_OPTIONS.3
@@ -49,6 +49,13 @@ Tells libcurl to not accept "partial" certificate chains, which it otherwise
 does by default. This option is only supported for OpenSSL and will fail the
 certificate verification if the chain ends with an intermediate certificate
 and not with a root cert. (Added in 7.68.0)
+
+.IP CURLSSLOPT_REVOKE_BEST_EFFORT
+Tells libcurl to ignore certificate revocation checks in case of missing or
+offline distribution points for those SSL backends where such behavior is
+present. This option is only supported for Schannel (the native Windows SSL
+library). If combined with \fICURLSSLOPT_NO_REVOKE\fP, the latter takes
+precedence. (Added in 7.70.0)
 .SH DEFAULT
 0
 .SH PROTOCOLS

diff --git a/docs/libcurl/opts/CURLOPT_SSL_OPTIONS.3 b/docs/libcurl/opts/CURLOPT_SSL_OPTIONS.3
index fd8504fb674f821382f92c7ff874354269c5c92b..2a915a8c657940c3eca48ab54dbe6fc99ddb3f6e 100644 (file)
--- a/docs/libcurl/opts/CURLOPT_SSL_OPTIONS.3
+++ b/docs/libcurl/opts/CURLOPT_SSL_OPTIONS.3
@@ -49,6 +49,12 @@ Tells libcurl to not accept "partial" certificate chains, which it otherwise
 does by default. This option is only supported for OpenSSL and will fail the
 certificate verification if the chain ends with an intermediate certificate
 and not with a root cert. (Added in 7.68.0)
+.IP CURLSSLOPT_REVOKE_BEST_EFFORT
+Tells libcurl to ignore certificate revocation checks in case of missing or
+offline distribution points for those SSL backends where such behavior is
+present. This option is only supported for Schannel (the native Windows SSL
+library). If combined with \fICURLSSLOPT_NO_REVOKE\fP, the latter takes
+precedence. (Added in 7.70.0)
 .SH DEFAULT
 0
 .SH PROTOCOLS

diff --git a/docs/libcurl/symbols-in-versions b/docs/libcurl/symbols-in-versions
index 93930c435880c8537225af45c2ba3b8abc275f3d..7882895f8abd8dcfacce4b0f1b988ef4c68bfb79 100644 (file)
--- a/docs/libcurl/symbols-in-versions
+++ b/docs/libcurl/symbols-in-versions
@@ -746,6 +746,7 @@ CURLSSLOPT_NO_PARTIALCHAIN      7.68.0
 CURLSSLOPT_NO_REVOKE            7.44.0
 CURLSSLSET_NO_BACKENDS          7.56.0
 CURLSSLSET_OK                   7.56.0
+CURLSSLOPT_REVOKE_BEST_EFFORT   7.70.0
 CURLSSLSET_TOO_LATE             7.56.0
 CURLSSLSET_UNKNOWN_BACKEND      7.56.0
 CURLUE_BAD_HANDLE               7.62.0

diff --git a/include/curl/curl.h b/include/curl/curl.h
index b7cb30a581b031e1fd575e874a1395fc3c0cc1e5..9da97924f9c232978fab1cfa6d03936720e4e8a7 100644 (file)
--- a/include/curl/curl.h
+++ b/include/curl/curl.h
@@ -833,6 +833,11 @@ typedef enum {
    if possible. The OpenSSL backend has this ability. */
 #define CURLSSLOPT_NO_PARTIALCHAIN (1<<2)
 
+/* - REVOKE_BEST_EFFORT tells libcurl to ignore certificate revocation offline
+   checks and ignore missing revocation list for those SSL backends where such
+   behavior is present. */
+#define CURLSSLOPT_REVOKE_BEST_EFFORT (1<<3)
+
 /* The default connection attempt delay in milliseconds for happy eyeballs.
    CURLOPT_HAPPY_EYEBALLS_TIMEOUT_MS.3 and happy-eyeballs-timeout-ms.d document
    this value, keep them in sync. */

diff --git a/lib/doh.c b/lib/doh.c
index aaa8f15ca6fc6f5f11052a16bbb6a4a5e23a0215..dd2bbf125375f3c7e4752f2498339d98d9d40500 100644 (file)
--- 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 4648c872b4f194ac6b368ce480a20fdb741c1af4..04785a6820ed503d9f32411042fe3f6fd978de33 100644 (file)
--- 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 374bf437125762884c2b3bdce7c5959fc0b8bf87..2a36c114701e36543ad8cdcfc4d90cc6bd6114d6 100644 (file)
--- 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 3b9aef47c2a154d6864adf200bf7fec638857539..64719fe702682bc771079aaed11a4b3ab6373a84 100644 (file)
--- 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 e75132caddd23daf49c5e8346274366962890c49..3dbc11f0566060dd812a1879b86014b2d4d5a502 100644 (file)
--- 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"

diff --git a/packages/OS400/curl.inc.in b/packages/OS400/curl.inc.in
index a160f108501e8572a1a6604582fdd7c274a28dbc..72acbe3fa72127a9bdf7ae9408c8d39a14ee576e 100644 (file)
--- a/packages/OS400/curl.inc.in
+++ b/packages/OS400/curl.inc.in
@@ -770,6 +770,8 @@
      d                 c                   X'0002'
      d CURLSSLOPT_NO_PARTIALCHAIN...
      d                 c                   X'0004'
+     d CURLSSLOPT_REVOKE_BEST_EFFORT...
+     d                 c                   X'0008'
       *
      d CURL_HET_DEFAULT...
      d                 c                   200

diff --git a/src/tool_cfgable.h b/src/tool_cfgable.h
index e093b2c842eded1e263e67702e300d9742a2ac70..2ae7944e375b3b3abc10f7297a25bcbbb093e003 100644 (file)
--- a/src/tool_cfgable.h
+++ b/src/tool_cfgable.h
@@ -254,6 +254,9 @@ struct OperationConfig {
   bool ssl_no_revoke;       /* disable SSL certificate revocation checks */
   /*bool proxy_ssl_no_revoke; */
 
+  bool ssl_revoke_best_effort; /* ignore SSL revocation offline/missing
+                                  revocation list errors */
+
   bool use_metalink;        /* process given URLs as metalink XML file */
   metalinkfile *metalinkfile_list; /* point to the first node */
   metalinkfile *metalinkfile_last; /* point to the last/current node */

diff --git a/src/tool_getparam.c b/src/tool_getparam.c
index 764caa203acde663904c0003b19e94d21e264aee..0c555cc96677706dc632e65da6fa1dc6ef60a166 100644 (file)
--- a/src/tool_getparam.c
+++ b/src/tool_getparam.c
@@ -249,6 +249,7 @@ static const struct LongShort aliases[]= {
   {"Eq", "cert-status",              ARG_BOOL},
   {"Er", "false-start",              ARG_BOOL},
   {"Es", "ssl-no-revoke",            ARG_BOOL},
+  {"ES", "ssl-revoke-best-effort",   ARG_BOOL},
   {"Et", "tcp-fastopen",             ARG_BOOL},
   {"Eu", "proxy-tlsuser",            ARG_STRING},
   {"Ev", "proxy-tlspassword",        ARG_STRING},
@@ -1606,6 +1607,11 @@ ParameterError getparameter(const char *flag, /* f or -long-flag */
           config->ssl_no_revoke = TRUE;
         break;
 
+      case 'S': /* --ssl-revoke-best-effort */
+        if(curlinfo->features & CURL_VERSION_SSL)
+          config->ssl_revoke_best_effort = TRUE;
+        break;
+
       case 't': /* --tcp-fastopen */
         config->tcp_fastopen = TRUE;
         break;

diff --git a/src/tool_help.c b/src/tool_help.c
index 9ee99d174935d8483d5b31516e0743ee9e8d0618..4fe9e4ce0aeadbcf49ed6f460c0552766544c7e3 100644 (file)
--- a/src/tool_help.c
+++ b/src/tool_help.c
@@ -437,6 +437,8 @@ static const struct helptxt helptext[] = {
    "Allow security flaw to improve interop"},
   {"    --ssl-no-revoke",
    "Disable cert revocation checks (Schannel)"},
+  {"    --ssl-revoke-best-effort",
+   "Ignore revocation offline or missing revocation list errors (Schannel)"},
   {"    --ssl-reqd",
    "Require SSL/TLS"},
   {"-2, --sslv2",

diff --git a/src/tool_operate.c b/src/tool_operate.c
index ab06b71c5b6c77ccca97dc564adfe46aac613ab9..3c7dd6829fdbe86b2eb720a2e8288e47dacfac3d 100644 (file)
--- a/src/tool_operate.c
+++ b/src/tool_operate.c
@@ -1898,9 +1898,11 @@ static CURLcode single_transfer(struct GlobalConfig *global,
           my_setopt_str(curl, CURLOPT_GSSAPI_DELEGATION,
                         config->gssapi_delegation);
 
-        /* new in 7.25.0 and 7.44.0 */
+        /* new in 7.25.0, 7.44.0 and 7.70.0 */
         {
           long mask = (config->ssl_allow_beast ? CURLSSLOPT_ALLOW_BEAST : 0) |
+                      (config->ssl_revoke_best_effort ?
+                       CURLSSLOPT_REVOKE_BEST_EFFORT : 0) |
                       (config->ssl_no_revoke ? CURLSSLOPT_NO_REVOKE : 0);
           if(mask)
             my_setopt_bitmask(curl, CURLOPT_SSL_OPTIONS, mask);

diff --git a/src/tool_setopt.c b/src/tool_setopt.c
index 9b308bf4a930c53bb3920c1f08a73973b3298322..ef90e6e4aa10543ba03f67dc9166618129eaaa3e 100644 (file)
--- a/src/tool_setopt.c
+++ b/src/tool_setopt.c
@@ -125,6 +125,7 @@ const NameValueUnsigned setopt_nv_CURLSSLOPT[] = {
   NV(CURLSSLOPT_ALLOW_BEAST),
   NV(CURLSSLOPT_NO_REVOKE),
   NV(CURLSSLOPT_NO_PARTIALCHAIN),
+  NV(CURLSSLOPT_REVOKE_BEST_EFFORT),
   NVEND,
 };
 

diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc
index 0e606e560760a872bfabcf95718b98370a533414..8451bb011d916bc226f99c73fa1ca715b9486492 100644 (file)
--- a/tests/data/Makefile.inc
+++ b/tests/data/Makefile.inc
@@ -207,6 +207,7 @@ test2040 test2041 test2042 test2043 test2044 test2045 test2046 test2047 \
 test2048 test2049 test2050 test2051 test2052 test2053 test2054 test2055 \
 test2056 test2057 test2058 test2059 test2060 test2061 test2062 test2063 \
 test2064 test2065 test2066 test2067 test2068 test2069 \
+test2064 test2065 test2066 test2067 test2068 test2069 test2070 \
          test2071 test2072 test2073 test2074 test2075 test2076 test2077 \
 test2078 \
 test2080 \

diff --git a/tests/data/test2070 b/tests/data/test2070
new file mode 100644 (file)
index 0000000..4a21512
--- /dev/null
+++ b/tests/data/test2070
@@ -0,0 +1,62 @@
+<testcase>
+<info>
+<keywords>
+HTTPS
+HTTP GET
+PEM certificate
+</keywords>
+</info>type
+
+#
+# Server-side
+<reply>
+<data>
+HTTP/1.1 200 OK
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake
+Content-Length: 7
+
+MooMoo
+</data>
+</reply>
+
+#
+# Client-side
+<client>
+<features>
+WinSSL
+!MinGW
+</features>
+<server>
+https Server-localhost-sv.pem
+</server>
+ <name>
+Ignore certificate revocation "best effort" strategy
+ </name>
+  <setenv>
+# This test is pointless if we're not using the schannel backend
+CURL_SSL_BACKEND=schannel
+ </setenv>
+ <command>
+--cacert %SRCDIR/certs/EdelCurlRoot-ca.crt --ssl-revoke-best-effort https://localhost:%HTTPSPORT/2070
+</command>
+# Ensure that we're running on localhost because we're checking the host name
+<precheck>
+perl -e "print 'Test requires default test server host' if ( '%HOSTIP' ne '127.0.0.1' );"
+</precheck>
+</client>
+
+#
+# Verify data after the test has been "shot"
+<verify>
+<strip>
+^User-Agent:.*
+</strip>
+<protocol>
+GET /2070 HTTP/1.1\r
+Host: localhost:%HTTPSPORT\r
+Accept: */*\r
+\r
+</protocol>
+</verify>
+</testcase>

diff --git a/tests/runtests.pl b/tests/runtests.pl
index ec5462be5c5ec58c87d7e6ee1d36bb575d0e5cfb..9b0609e29969f71293140fe0c854f0e5e0787307 100755 (executable)
--- a/tests/runtests.pl
+++ b/tests/runtests.pl
@@ -249,6 +249,7 @@ my $has_ldpreload;  # set if curl is built for systems supporting LD_PRELOAD
 my $has_multissl;   # set if curl is build with MultiSSL support
 my $has_manual;     # set if curl is built with built-in manual
 my $has_win32;      # set if curl is built for Windows
+my $has_mingw;      # set if curl is built with MinGW (as opposed to MinGW-w64)
 
 # this version is decided by the particular nghttp2 library that is being used
 my $h2cver = "h2c";
@@ -272,7 +273,6 @@ my $resolver;       # name of the resolver backend (for human presentation)
 
 my $has_textaware;  # set if running on a system that has a text mode concept
                     # on files. Windows for example
-
 my @protocols;   # array of lowercase supported protocol servers
 
 my $skipped=0;  # number of tests skipped; reported in main loop
@@ -2659,6 +2659,7 @@ sub setupfeatures {
     $feature{"manual"} = $has_manual;
     $feature{"unix-sockets"} = $has_unix;
     $feature{"win32"} = $has_win32;
+    $feature{"MinGW"} = $has_mingw;
 
     # make each protocol an enabled "feature"
     for my $p (@protocols) {
@@ -2739,6 +2740,7 @@ sub checksystem {
                 $pwd = pathhelp::sys_native_current_path();
                 $has_textaware = 1;
                 $has_win32 = 1;
+                $has_mingw = 1 if ($curl =~ /-pc-mingw32/);
             }
            if ($libcurl =~ /(winssl|schannel)/i) {
                $has_winssl=1;