CURLOPT_SSL_VERIFYHOST: stop supporting the 1 value

After a research team wrote a document[1] that found several live source
codes out there in the wild that misused the CURLOPT_SSL_VERIFYHOST
option thinking it was a boolean, this change now bans 1 as a value and
will make libcurl return error for it.

1 was never a sensible value to use in production but was introduced
back in the days to help debugging. It was always documented clearly
this way.

1 was never supported by all SSL backends in libcurl, so this cleanup
makes the treatment of it unified.

The report's list of mistakes for this option were all PHP code and
while there's a binding layer between libcurl and PHP, the PHP team has
decided that they have an as thin layer as possible on top of libcurl so
they will not alter or specifically filter a 'TRUE' value for this
particular option. I sympathize with that position.

[1] = http://daniel.haxx.se/blog/2012/10/25/libcurl-claimed-to-be-dangerous/

diff --git a/docs/libcurl/curl_easy_setopt.3 b/docs/libcurl/curl_easy_setopt.3
index 3b0dc6c9d9f6820c3a0b12e51ccb75b4ff1292f8..a388c22206d78c9562cbde255d55610bea3e28c9 100644 (file)
--- a/docs/libcurl/curl_easy_setopt.3
+++ b/docs/libcurl/curl_easy_setopt.3
@@ -2323,8 +2323,9 @@ Curl considers the server the intended one when the Common Name field or a
 Subject Alternate Name field in the certificate matches the host name in the
 URL to which you told Curl to connect.
 
-When the value is 1, the certificate must contain a Common Name field, but it
-doesn't matter what name it says.  (This is not ordinarily a useful setting).
+When the value is 1, libcurl will return a failure. It was previously (in
+7.28.0 and earlier) a debug option of some sorts, but it is no longer
+supported due to frequently leading to programmer mistakes.
 
 When the value is 0, the connection succeeds regardless of the names in the
 certificate.

diff --git a/lib/curl_darwinssl.c b/lib/curl_darwinssl.c
index 31531dbccb41a346dbf1492678422270a611ce3a..334944f0985ceaab2ad265c91abb1d113733d718 100644 (file)
--- a/lib/curl_darwinssl.c
+++ b/lib/curl_darwinssl.c
@@ -632,7 +632,6 @@ static CURLcode darwinssl_connect_step1(struct connectdata *conn,
   struct SessionHandle *data = conn->data;
   curl_socket_t sockfd = conn->sock[sockindex];
   struct ssl_connect_data *connssl = &conn->ssl[sockindex];
-  bool sni = true;
 #ifdef ENABLE_IPV6
   struct in6_addr addr;
 #else
@@ -809,7 +808,7 @@ static CURLcode darwinssl_connect_step1(struct connectdata *conn,
 #ifdef ENABLE_IPV6
      (0 == Curl_inet_pton(AF_INET6, conn->host.name, &addr)) &&
 #endif
-     sni) {
+     data->set.ssl.verifyhost) {
     err = SSLSetPeerDomainName(connssl->ssl_ctx, conn->host.name,
                                strlen(conn->host.name));
     if(err != noErr) {

diff --git a/lib/curl_schannel.c b/lib/curl_schannel.c
index 75fa071cea2e50947045f7f55755626d28a2eadb..ef6b1adb0c0d79d955c2127178fdcd9e4f909b90 100644 (file)
--- a/lib/curl_schannel.c
+++ b/lib/curl_schannel.c
@@ -160,7 +160,7 @@ schannel_connect_step1(struct connectdata *conn, int sockindex)
 #ifdef ENABLE_IPV6
        Curl_inet_pton(AF_INET6, conn->host.name, &addr6) ||
 #endif
-       data->set.ssl.verifyhost < 2) {
+       !data->set.ssl.verifyhost) {
       schannel_cred.dwFlags |= SCH_CRED_NO_SERVERNAME_CHECK;
       infof(data, "schannel: using IP address, disable SNI servername "
             "check\n");
@@ -1238,10 +1238,7 @@ static CURLcode verify_certificate(struct connectdata *conn, int sockindex)
   }
 
   if(result == CURLE_OK) {
-    if(data->set.ssl.verifyhost == 1) {
-      infof(data, "warning: ignoring unsupported value (1) ssl.verifyhost\n");
-    }
-    else if(data->set.ssl.verifyhost == 2) {
+    if(data->set.ssl.verifyhost) {
       TCHAR cert_hostname_buff[128];
       xcharp_u hostname;
       xcharp_u cert_hostname;

diff --git a/lib/gtls.c b/lib/gtls.c
index a17d9eeda2a67e0594b1c68ec9a1156a18a841ff..2920ee2f92222e1ed0451191622c28f89c22e9b2 100644 (file)
--- a/lib/gtls.c
+++ b/lib/gtls.c
@@ -681,7 +681,7 @@ gtls_connect_step3(struct connectdata *conn,
   rc = gnutls_x509_crt_check_hostname(x509_cert, conn->host.name);
 
   if(!rc) {
-    if(data->set.ssl.verifyhost > 1) {
+    if(data->set.ssl.verifyhost) {
       failf(data, "SSL: certificate subject name (%s) does not match "
             "target host name '%s'", certbuf, conn->host.dispname);
       gnutls_x509_crt_deinit(x509_cert);

diff --git a/lib/nss.c b/lib/nss.c
index 56290f4b772913b8d898b579443c04d9c1a2871f..22b53bfd85f06c2f05211c0a64afc6528a0d1e21 100644 (file)
--- a/lib/nss.c
+++ b/lib/nss.c
@@ -1316,8 +1316,6 @@ CURLcode Curl_nss_connect(struct connectdata *conn, int sockindex)
 
   if(!data->set.ssl.verifypeer && data->set.ssl.verifyhost)
     infof(data, "warning: ignoring value of ssl.verifyhost\n");
-  else if(data->set.ssl.verifyhost == 1)
-    infof(data, "warning: ignoring unsupported value (1) of ssl.verifyhost\n");
 
   /* bypass the default SSL_AuthCertificate() hook in case we do not want to
    * verify peer */

diff --git a/lib/ssluse.c b/lib/ssluse.c
index a701131faa41a22c3b49c601f60b152b04b27359..745fc88f943b7f9167dd03a05a9c62f64677cb77 100644 (file)
--- a/lib/ssluse.c
+++ b/lib/ssluse.c
@@ -1292,14 +1292,9 @@ static CURLcode verifyhost(struct connectdata *conn,
       res = CURLE_PEER_FAILED_VERIFICATION;
     }
     else if(!cert_hostcheck((const char *)peer_CN, conn->host.name)) {
-      if(data->set.ssl.verifyhost > 1) {
-        failf(data, "SSL: certificate subject name '%s' does not match "
-              "target host name '%s'", peer_CN, conn->host.dispname);
-        res = CURLE_PEER_FAILED_VERIFICATION;
-      }
-      else
-        infof(data, "\t common name: %s (does not match '%s')\n",
-              peer_CN, conn->host.dispname);
+      failf(data, "SSL: certificate subject name '%s' does not match "
+            "target host name '%s'", peer_CN, conn->host.dispname);
+      res = CURLE_PEER_FAILED_VERIFICATION;
     }
     else {
       infof(data, "\t common name: %s (matched)\n", peer_CN);

diff --git a/lib/url.c b/lib/url.c
index b30a4fa511e60eeeb56ffcca4e239a7e1804d499..5ffaa730562c30dcbb2b3bc3523dd82a47aaf347 100644 (file)
--- a/lib/url.c
+++ b/lib/url.c
@@ -708,7 +708,7 @@ CURLcode Curl_init_userdefined(struct UserDefined *set)
    * switched off unless wanted.
    */
   set->ssl.verifypeer = TRUE;
-  set->ssl.verifyhost = 2;
+  set->ssl.verifyhost = TRUE;
 #ifdef USE_TLS_SRP
   set->ssl.authtype = CURL_TLSAUTH_NONE;
 #endif
@@ -2049,13 +2049,25 @@ CURLcode Curl_setopt(struct SessionHandle *data, CURLoption option,
     /*
      * Enable peer SSL verifying.
      */
-    data->set.ssl.verifypeer = va_arg(param, long);
+    data->set.ssl.verifypeer = (0 != va_arg(param, long))?TRUE:FALSE;
     break;
   case CURLOPT_SSL_VERIFYHOST:
     /*
-     * Enable verification of the CN contained in the peer certificate
+     * Enable verification of the host name in the peer certificate
      */
-    data->set.ssl.verifyhost = va_arg(param, long);
+    arg = va_arg(param, long);
+
+    /* Obviously people are not reading documentation and too many thought
+       this argument took a boolean when it wasn't and misused it. We thus ban
+       1 as a sensible input and we warn about its use. Then we only have the
+       2 action internally stored as TRUE. */
+
+    if(1 == arg) {
+      failf(data, "CURLOPT_SSL_VERIFYHOST no longer supports 1 as value!");
+      return CURLE_BAD_FUNCTION_ARGUMENT;
+    }
+
+    data->set.ssl.verifyhost = (0 != arg)?TRUE:FALSE;
     break;
 #ifdef USE_SSLEAY
     /* since these two options are only possible to use on an OpenSSL-

diff --git a/lib/urldata.h b/lib/urldata.h
index 5f893c92ee97c7294e542c43229a550501f00d4c..4116c341f6a1f749b15c089280848606bbb81fd5 100644 (file)
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -332,10 +332,9 @@ struct ssl_connect_data {
 struct ssl_config_data {
   long version;          /* what version the client wants to use */
   long certverifyresult; /* result from the certificate verification */
-  long verifypeer;       /* set TRUE if this is desired */
-  long verifyhost;       /* 0: no verify
-                            1: check that CN exists
-                            2: CN must match hostname */
+
+  bool verifypeer;       /* set TRUE if this is desired */
+  bool verifyhost;       /* set TRUE if CN/SAN must match hostname */
   char *CApath;          /* certificate dir (doesn't work on windows) */
   char *CAfile;          /* certificate to verify peer against */
   const char *CRLfile;   /* CRL to check certificate revocation */
@@ -994,8 +993,8 @@ struct connectdata {
   int socks5_gssapi_enctype;
 #endif
 
-  long verifypeer;
-  long verifyhost;
+  bool verifypeer;
+  bool verifyhost;
 
   /* When this connection is created, store the conditions for the local end
      bind. This is stored before the actual bind and before any connection is