[gnurl] 41/264: schannel: add "best effort" revocation check option

[gnunet]

This is an automated email from the git hooks/post-receive script.

nikita pushed a commit to branch master
in repository gnurl.

commit 54504284918a4ba19bc7b1efb486a64629d376aa
Author: Johannes Schindelin <address@hidden>
AuthorDate: Wed Feb 26 11:24:26 2020 +0100

    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 <address@hidden>
    Co-authored-by: Markus Olsson <address@hidden>
    Signed-off-by: Johannes Schindelin <address@hidden>
    
    Closes https://github.com/curl/curl/pull/4981
---
 docs/cmdline-opts/Makefile.inc                |  1 +
 docs/cmdline-opts/ssl-revoke-best-effort.d    |  7 +++
 docs/libcurl/opts/CURLOPT_PROXY_SSL_OPTIONS.3 |  7 +++
 docs/libcurl/opts/CURLOPT_SSL_OPTIONS.3       |  6 +++
 docs/libcurl/symbols-in-versions              |  1 +
 include/curl/curl.h                           |  5 +++
 lib/doh.c                                     |  5 +++
 lib/setopt.c                                  |  3 ++
 lib/urldata.h                                 |  2 +
 lib/vtls/schannel.c                           |  7 +++
 lib/vtls/schannel_verify.c                    |  9 ++++
 packages/OS400/curl.inc.in                    |  2 +
 src/tool_cfgable.h                            |  3 ++
 src/tool_getparam.c                           |  6 +++
 src/tool_help.c                               |  2 +
 src/tool_operate.c                            |  4 +-
 src/tool_setopt.c                             |  1 +
 tests/data/Makefile.inc                       |  1 +
 tests/data/test2070                           | 62 +++++++++++++++++++++++++++
 tests/runtests.pl                             |  4 +-
 20 files changed, 136 insertions(+), 2 deletions(-)

diff --git a/docs/cmdline-opts/Makefile.inc b/docs/cmdline-opts/Makefile.inc
index 829551ff6..f8df916b7 100644
--- 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
index 000000000..9b55699db
--- /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 ded54c7b7..9c877c2a2 100644
--- 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 fd8504fb6..2a915a8c6 100644
--- 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 93930c435..7882895f8 100644
--- 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 b7cb30a58..9da97924f 100644
--- 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 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"
diff --git a/packages/OS400/curl.inc.in b/packages/OS400/curl.inc.in
index a160f1085..72acbe3fa 100644
--- 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 e093b2c84..2ae7944e3 100644
--- 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 764caa203..0c555cc96 100644
--- 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 9ee99d174..4fe9e4ce0 100644
--- 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 ab06b71c5..3c7dd6829 100644
--- 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 9b308bf4a..ef90e6e4a 100644
--- 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 0e606e560..8451bb011 100644
--- 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
index 000000000..4a21512ba
--- /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
+Host: localhost:%HTTPSPORT
+Accept: */*
+
+</protocol>
+</verify>
+</testcase>
diff --git a/tests/runtests.pl b/tests/runtests.pl
index ec5462be5..9b0609e29 100755
--- 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;

-- 
To stop receiving notification emails like this one, please contact
address@hidden.