basic cleanups regarding SSL handling (#16940) [backport:1.0]

* basic cleanups regarding SSL handling
* enabled certificate checking on Windows
* updated the SSL test
* quoting helps

(cherry picked from commit abac35e743)

lib/pure/httpclient.nim

@@ -317,7 +317,7 @@ proc getDefaultSSL(): SslContext =
   result = defaultSslContext
   when defined(ssl):
     if result == nil:
-      defaultSslContext = newContext(verifyMode = CVerifyNone)
+      defaultSslContext = newContext(verifyMode = CVerifyPeer)
       result = defaultSslContext
       doAssert result != nil, "failure to initialize the SSL context"
 

lib/pure/net.nim

@@ -587,11 +587,13 @@ when defineSsl:
     discard newCTX.SSLCTXSetMode(SSL_MODE_AUTO_RETRY)
     newCTX.loadCertificates(certFile, keyFile)
 
-    when not defined(nimDisableCertificateValidation) and not defined(windows):
+    const VerifySuccess = 1 # SSL_CTX_load_verify_locations returns 1 on success.
+
+    when not defined(nimDisableCertificateValidation):
       if verifyMode != CVerifyNone:
         # Use the caDir and caFile parameters if set
         if caDir != "" or caFile != "":
-          if newCTX.SSL_CTX_load_verify_locations(caFile, caDir) != 0:
+          if newCTX.SSL_CTX_load_verify_locations(caFile, caDir) != VerifySuccess:
             raise newException(IOError, "Failed to load SSL/TLS CA certificate(s).")
 
         else:
@@ -599,7 +601,7 @@ when defineSsl:
           # the SSL_CERT_FILE and SSL_CERT_DIR env vars
           var found = false
           for fn in scanSSLCertificates():
-            if newCTX.SSL_CTX_load_verify_locations(fn, "") == 0:
+            if newCTX.SSL_CTX_load_verify_locations(fn, nil) == VerifySuccess:
               found = true
               break
           if not found:

lib/pure/ssl_certs.nim

@@ -11,16 +11,48 @@
 ## SSL_CERT_DIR environment variables.
 
 import os, strutils
-from ospaths import existsEnv, getEnv
-import strutils
-
-# SECURITY: this unnecessarily scans through dirs/files regardless of the
-# actual host OS/distribution. Hopefully all the paths are writeble only by
-# root.
 
 # FWIW look for files before scanning entire dirs.
 
-const certificate_paths = [
+when defined(macosx):
+  const certificatePaths = [
+    "/etc/ssl/cert.pem",
+    "/System/Library/OpenSSL/certs/cert.pem"
+  ]
+elif defined(linux):
+  const certificatePaths = [
+    # Debian, Ubuntu, Arch: maintained by update-ca-certificates, SUSE, Gentoo
+    # NetBSD (security/mozilla-rootcerts)
+    # SLES10/SLES11, https://golang.org/issue/12139
+    "/etc/ssl/certs/ca-certificates.crt",
+    # OpenSUSE
+    "/etc/ssl/ca-bundle.pem",
+    # Red Hat 5+, Fedora, Centos
+    "/etc/pki/tls/certs/ca-bundle.crt",
+    # Red Hat 4
+    "/usr/share/ssl/certs/ca-bundle.crt",
+    # Fedora/RHEL
+    "/etc/pki/tls/certs",
+    # Android
+    "/system/etc/security/cacerts",
+  ]
+elif defined(bsd):
+  const certificatePaths = [
+    # Debian, Ubuntu, Arch: maintained by update-ca-certificates, SUSE, Gentoo
+    # NetBSD (security/mozilla-rootcerts)
+    # SLES10/SLES11, https://golang.org/issue/12139
+    "/etc/ssl/certs/ca-certificates.crt",
+    # FreeBSD (security/ca-root-nss package)
+    "/usr/local/share/certs/ca-root-nss.crt",
+    # OpenBSD, FreeBSD (optional symlink)
+    "/etc/ssl/cert.pem",
+    # FreeBSD
+    "/usr/local/share/certs",
+    # NetBSD
+    "/etc/openssl/certs",
+  ]
+else:
+  const certificatePaths = [
     # Debian, Ubuntu, Arch: maintained by update-ca-certificates, SUSE, Gentoo
     # NetBSD (security/mozilla-rootcerts)
     # SLES10/SLES11, https://golang.org/issue/12139
@@ -37,8 +69,6 @@ const certificate_paths = [
     "/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem",
     # OpenBSD, FreeBSD (optional symlink)
     "/etc/ssl/cert.pem",
-    # Mac OS X
-    "/System/Library/OpenSSL/certs/cert.pem",
     # Fedora/RHEL
     "/etc/pki/tls/certs",
     # Android
@@ -47,7 +77,7 @@ const certificate_paths = [
     "/usr/local/share/certs",
     # NetBSD
     "/etc/openssl/certs",
-]
+  ]
 
 when defined(haiku):
   const
@@ -67,17 +97,26 @@ iterator scanSSLCertificates*(useEnvVars = false): string =
   ## if `useEnvVars` is true, the SSL_CERT_FILE and SSL_CERT_DIR
   ## environment variables can be used to override the certificate
   ## directories to scan or specify a CA certificate file.
-  if existsEnv("SSL_CERT_FILE"):
+  if useEnvVars and existsEnv("SSL_CERT_FILE"):
     yield getEnv("SSL_CERT_FILE")
 
-  elif existsEnv("SSL_CERT_DIR"):
+  elif useEnvVars and existsEnv("SSL_CERT_DIR"):
     let p = getEnv("SSL_CERT_DIR")
     for fn in joinPath(p, "*").walkFiles():
       yield fn
 
   else:
-    when not defined(haiku):
-      for p in certificate_paths:
+    when defined(windows):
+      let pem = getAppDir() / "cacert.pem"
+      # We download the certificates according to https://curl.se/docs/caextract.html
+      # These are the certificates from Firefox. The 'bitsadmin.exe' tool ships with every
+      # recent version of Windows (Windows 8, Windows XP, etc.)
+      if not fileExists(pem):
+        discard os.execShellCmd("""bitsadmin.exe /rawreturn /transfer "JobName" /priority FOREGROUND https://curl.se/ca/cacert.pem """ &
+          quoteShell(pem))
+      yield pem
+    elif not defined(haiku):
+      for p in certificatePaths:
         if p.endsWith(".pem") or p.endsWith(".crt"):
           if existsFile(p):
             yield p

tests/stdlib/thttpclient_ssl.nim

@@ -99,7 +99,7 @@ when not defined(windows):
       let t = spawn runServer(port)
       sleep(100)
 
-      var client = newHttpClient()
+      var client = newHttpClient(sslContext=newContext(verifyMode=CVerifyNone))
       try:
         log "client: connect"
         discard client.getContent("https://127.0.0.1:12345")