Merge topic 'curl-tls-verify'

4e62bc943c ctest: Verify TLS server certificate by default
8e92ee34f6 file(DOWNLOAD/UPLOAD): Verify TLS server certificate by default
dcaea54898 cmCTestCurl: Clarify names and logic using optional<bool>
03d37ae3ff cmFileCommand: Clarify names and logic using optional<bool>

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !9843

3 files changed, 79 insertions, 44 deletions

diff --git a/Source/CTest/cmCTestCurl.cxx b/Source/CTest/cmCTestCurl.cxx
index 7137e63..d9dc3b2 100644
--- a/Source/CTest/cmCTestCurl.cxx
+++ b/Source/CTest/cmCTestCurl.cxx
@@ -14,6 +14,10 @@
 #include "cmSystemTools.h"
 #include "cmValue.h"
 
+namespace {
+const bool TLS_VERIFY_DEFAULT = true;
+}
+
 cmCTestCurl::cmCTestCurl(cmCTest* ctest)
   : CTest(ctest)
   , CurlOpts(ctest)
@@ -76,6 +80,9 @@ cmCTestCurlOpts::cmCTestCurlOpts(cmCTest* ctest)
       }
     }
   }
+  if (!this->TLSVerifyOpt.has_value()) {
+    this->TLSVerifyOpt = TLS_VERIFY_DEFAULT;
+  }
 }
 
 bool cmCTestCurl::InitCurl()
@@ -84,11 +91,11 @@ bool cmCTestCurl::InitCurl()
     return false;
   }
   cmCurlSetCAInfo(this->Curl);
-  if (this->CurlOpts.TLSVersionOpt) {
+  if (this->CurlOpts.TLSVersionOpt.has_value()) {
     curl_easy_setopt(this->Curl, CURLOPT_SSLVERSION,
                      *this->CurlOpts.TLSVersionOpt);
   }
-  if (this->CurlOpts.TLSVerifyOpt) {
+  if (this->CurlOpts.TLSVerifyOpt.has_value()) {
     curl_easy_setopt(this->Curl, CURLOPT_SSL_VERIFYPEER,
                      *this->CurlOpts.TLSVerifyOpt ? 1 : 0);
   }
diff --git a/Source/CTest/cmCTestSubmitHandler.cxx b/Source/CTest/cmCTestSubmitHandler.cxx
index f05b874..91dea55 100644
--- a/Source/CTest/cmCTestSubmitHandler.cxx
+++ b/Source/CTest/cmCTestSubmitHandler.cxx
@@ -181,7 +181,7 @@ bool cmCTestSubmitHandler::SubmitUsingHTTP(
     curl = cm_curl_easy_init();
     if (curl) {
       cmCurlSetCAInfo(curl);
-      if (curlOpts.TLSVersionOpt) {
+      if (curlOpts.TLSVersionOpt.has_value()) {
         cm::optional<std::string> tlsVersionStr =
           cmCurlPrintTLSVersion(*curlOpts.TLSVersionOpt);
         cmCTestOptionalLog(
@@ -191,7 +191,7 @@ bool cmCTestSubmitHandler::SubmitUsingHTTP(
           this->Quiet);
         curl_easy_setopt(curl, CURLOPT_SSLVERSION, *curlOpts.TLSVersionOpt);
       }
-      if (curlOpts.TLSVerifyOpt) {
+      if (curlOpts.TLSVerifyOpt.has_value()) {
         cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT,
                            "  Set CURLOPT_SSL_VERIFYPEER to "
                              << (*curlOpts.TLSVerifyOpt ? "on" : "off")
diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx
index 48ea01d..30d92ca 100644
--- a/Source/cmFileCommand.cxx
+++ b/Source/cmFileCommand.cxx
@@ -1740,6 +1740,8 @@ bool HandleNativePathCommand(std::vector<std::string> const& args,
 
 #if !defined(CMAKE_BOOTSTRAP)
 
+const bool TLS_VERIFY_DEFAULT = true;
+
 // Stuff for curl download/upload
 using cmFileCommandVectorOfChar = std::vector<char>;
 
@@ -1932,8 +1934,8 @@ bool HandleDownloadCommand(std::vector<std::string> const& args,
   long inactivity_timeout = 0;
   std::string logVar;
   std::string statusVar;
-  cm::optional<std::string> tls_version;
-  cm::optional<bool> tls_verify;
+  cm::optional<std::string> tlsVersionOpt;
+  cm::optional<bool> tlsVerifyOpt;
   cmValue cainfo = status.GetMakefile().GetDefinition("CMAKE_TLS_CAINFO");
   std::string netrc_level =
     status.GetMakefile().GetSafeDefinition("CMAKE_NETRC");
@@ -1982,7 +1984,7 @@ bool HandleDownloadCommand(std::vector<std::string> const& args,
     } else if (*i == "TLS_VERSION") {
       ++i;
       if (i != args.end()) {
-        tls_version = *i;
+        tlsVersionOpt = *i;
       } else {
         status.SetError("DOWNLOAD missing value for TLS_VERSION.");
         return false;
@@ -1990,7 +1992,7 @@ bool HandleDownloadCommand(std::vector<std::string> const& args,
     } else if (*i == "TLS_VERIFY") {
       ++i;
       if (i != args.end()) {
-        tls_verify = cmIsOn(*i);
+        tlsVerifyOpt = cmIsOn(*i);
       } else {
         status.SetError("DOWNLOAD missing bool value for TLS_VERIFY.");
         return false;
@@ -2098,27 +2100,32 @@ bool HandleDownloadCommand(std::vector<std::string> const& args,
     ++i;
   }
 
-  if (!tls_verify) {
+  if (!tlsVerifyOpt.has_value()) {
     if (cmValue v = status.GetMakefile().GetDefinition("CMAKE_TLS_VERIFY")) {
-      tls_verify = v.IsOn();
+      tlsVerifyOpt = v.IsOn();
     }
   }
-  if (!tls_verify) {
+  if (!tlsVerifyOpt.has_value()) {
     if (cm::optional<std::string> v =
           cmSystemTools::GetEnvVar("CMAKE_TLS_VERIFY")) {
-      tls_verify = cmIsOn(*v);
+      tlsVerifyOpt = cmIsOn(*v);
     }
   }
+  bool tlsVerifyDefaulted = false;
+  if (!tlsVerifyOpt.has_value()) {
+    tlsVerifyOpt = TLS_VERIFY_DEFAULT;
+    tlsVerifyDefaulted = true;
+  }
 
-  if (!tls_version) {
+  if (!tlsVersionOpt.has_value()) {
     if (cmValue v = status.GetMakefile().GetDefinition("CMAKE_TLS_VERSION")) {
-      tls_version = *v;
+      tlsVersionOpt = *v;
     }
   }
-  if (!tls_version) {
+  if (!tlsVersionOpt.has_value()) {
     if (cm::optional<std::string> v =
           cmSystemTools::GetEnvVar("CMAKE_TLS_VERSION")) {
-      tls_version = std::move(v);
+      tlsVersionOpt = std::move(v);
     }
   }
 
@@ -2202,21 +2209,21 @@ bool HandleDownloadCommand(std::vector<std::string> const& args,
                            cmFileCommandCurlDebugCallback);
   check_curl_result(res, "DOWNLOAD cannot set debug function: ");
 
-  if (tls_version) {
-    if (cm::optional<int> v = cmCurlParseTLSVersion(*tls_version)) {
+  if (tlsVersionOpt.has_value()) {
+    if (cm::optional<int> v = cmCurlParseTLSVersion(*tlsVersionOpt)) {
       res = ::curl_easy_setopt(curl, CURLOPT_SSLVERSION, *v);
-      check_curl_result(
-        res,
-        cmStrCat("DOWNLOAD cannot set TLS/SSL version ", *tls_version, ": "));
+      check_curl_result(res,
+                        cmStrCat("DOWNLOAD cannot set TLS/SSL version ",
+                                 *tlsVersionOpt, ": "));
     } else {
       status.SetError(
-        cmStrCat("DOWNLOAD given unknown TLS/SSL version ", *tls_version));
+        cmStrCat("DOWNLOAD given unknown TLS/SSL version ", *tlsVersionOpt));
       return false;
     }
   }
 
   // check to see if TLS verification is requested
-  if (tls_verify && *tls_verify) {
+  if (tlsVerifyOpt.has_value() && tlsVerifyOpt.value()) {
     res = ::curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1);
     check_curl_result(res, "DOWNLOAD cannot set TLS/SSL Verify on: ");
   } else {
@@ -2317,9 +2324,17 @@ bool HandleDownloadCommand(std::vector<std::string> const& args,
   ::curl_easy_cleanup(curl);
 
   if (!statusVar.empty()) {
+    std::string m = curl_easy_strerror(res);
+    if ((res == CURLE_SSL_CONNECT_ERROR ||
+         res == CURLE_PEER_FAILED_VERIFICATION) &&
+        tlsVerifyDefaulted) {
+      m = cmStrCat(
+        std::move(m),
+        ".  If this is due to https certificate verification failure, one may "
+        "set environment variable CMAKE_TLS_VERIFY=0 to suppress it.");
+    }
     status.GetMakefile().AddDefinition(
-      statusVar,
-      cmStrCat(static_cast<int>(res), ";\"", ::curl_easy_strerror(res), "\""));
+      statusVar, cmStrCat(static_cast<int>(res), ";\"", std::move(m), "\""));
   }
 
   ::curl_global_cleanup();
@@ -2404,8 +2419,8 @@ bool HandleUploadCommand(std::vector<std::string> const& args,
   std::string logVar;
   std::string statusVar;
   bool showProgress = false;
-  cm::optional<std::string> tls_version;
-  cm::optional<bool> tls_verify;
+  cm::optional<std::string> tlsVersionOpt;
+  cm::optional<bool> tlsVerifyOpt;
   cmValue cainfo = status.GetMakefile().GetDefinition("CMAKE_TLS_CAINFO");
   std::string userpwd;
   std::string netrc_level =
@@ -2451,7 +2466,7 @@ bool HandleUploadCommand(std::vector<std::string> const& args,
     } else if (*i == "TLS_VERSION") {
       ++i;
       if (i != args.end()) {
-        tls_version = *i;
+        tlsVersionOpt = *i;
       } else {
         status.SetError("UPLOAD missing value for TLS_VERSION.");
         return false;
@@ -2459,7 +2474,7 @@ bool HandleUploadCommand(std::vector<std::string> const& args,
     } else if (*i == "TLS_VERIFY") {
       ++i;
       if (i != args.end()) {
-        tls_verify = cmIsOn(*i);
+        tlsVerifyOpt = cmIsOn(*i);
       } else {
         status.SetError("UPLOAD missing bool value for TLS_VERIFY.");
         return false;
@@ -2511,27 +2526,32 @@ bool HandleUploadCommand(std::vector<std::string> const& args,
     ++i;
   }
 
-  if (!tls_verify) {
+  if (!tlsVerifyOpt.has_value()) {
     if (cmValue v = status.GetMakefile().GetDefinition("CMAKE_TLS_VERIFY")) {
-      tls_verify = v.IsOn();
+      tlsVerifyOpt = v.IsOn();
     }
   }
-  if (!tls_verify) {
+  if (!tlsVerifyOpt.has_value()) {
     if (cm::optional<std::string> v =
           cmSystemTools::GetEnvVar("CMAKE_TLS_VERIFY")) {
-      tls_verify = cmIsOn(*v);
+      tlsVerifyOpt = cmIsOn(*v);
     }
   }
+  bool tlsVerifyDefaulted = false;
+  if (!tlsVerifyOpt.has_value()) {
+    tlsVerifyOpt = TLS_VERIFY_DEFAULT;
+    tlsVerifyDefaulted = true;
+  }
 
-  if (!tls_version) {
+  if (!tlsVersionOpt.has_value()) {
     if (cmValue v = status.GetMakefile().GetDefinition("CMAKE_TLS_VERSION")) {
-      tls_version = *v;
+      tlsVersionOpt = *v;
     }
   }
-  if (!tls_version) {
+  if (!tlsVersionOpt.has_value()) {
     if (cm::optional<std::string> v =
           cmSystemTools::GetEnvVar("CMAKE_TLS_VERSION")) {
-      tls_version = std::move(v);
+      tlsVersionOpt = std::move(v);
     }
   }
 
@@ -2580,21 +2600,21 @@ bool HandleUploadCommand(std::vector<std::string> const& args,
                            cmFileCommandCurlDebugCallback);
   check_curl_result(res, "UPLOAD cannot set debug function: ");
 
-  if (tls_version) {
-    if (cm::optional<int> v = cmCurlParseTLSVersion(*tls_version)) {
+  if (tlsVersionOpt.has_value()) {
+    if (cm::optional<int> v = cmCurlParseTLSVersion(*tlsVersionOpt)) {
       res = ::curl_easy_setopt(curl, CURLOPT_SSLVERSION, *v);
       check_curl_result(
         res,
-        cmStrCat("UPLOAD cannot set TLS/SSL version ", *tls_version, ": "));
+        cmStrCat("UPLOAD cannot set TLS/SSL version ", *tlsVersionOpt, ": "));
     } else {
       status.SetError(
-        cmStrCat("UPLOAD given unknown TLS/SSL version ", *tls_version));
+        cmStrCat("UPLOAD given unknown TLS/SSL version ", *tlsVersionOpt));
       return false;
     }
   }
 
   // check to see if TLS verification is requested
-  if (tls_verify && *tls_verify) {
+  if (tlsVerifyOpt.has_value() && tlsVerifyOpt.value()) {
     res = ::curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1);
     check_curl_result(res, "UPLOAD cannot set TLS/SSL Verify on: ");
   } else {
@@ -2697,9 +2717,17 @@ bool HandleUploadCommand(std::vector<std::string> const& args,
   ::curl_easy_cleanup(curl);
 
   if (!statusVar.empty()) {
+    std::string m = curl_easy_strerror(res);
+    if ((res == CURLE_SSL_CONNECT_ERROR ||
+         res == CURLE_PEER_FAILED_VERIFICATION) &&
+        tlsVerifyDefaulted) {
+      m = cmStrCat(
+        std::move(m),
+        ".  If this is due to https certificate verification failure, one may "
+        "set environment variable CMAKE_TLS_VERIFY=0 to suppress it.");
+    }
     status.GetMakefile().AddDefinition(
-      statusVar,
-      cmStrCat(static_cast<int>(res), ";\"", ::curl_easy_strerror(res), "\""));
+      statusVar, cmStrCat(static_cast<int>(res), ";\"", std::move(m), "\""));
   }
 
   ::curl_global_cleanup();