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

@@ -286,7 +286,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

@@ -566,7 +566,27 @@ when defineSsl:
     discard newCTX.SSLCTXSetMode(SSL_MODE_AUTO_RETRY)
     newCTX.loadCertificates(certFile, keyFile)
 
-    result = SslContext(context: newCTX, referencedData: initSet[int](),
+    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) != VerifySuccess:
+            raise newException(IOError, "Failed to load SSL/TLS CA certificate(s).")
+
+        else:
+          # Scan for certs in known locations. For CVerifyPeerUseEnvVars also scan
+          # 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, nil) == VerifySuccess:
+              found = true
+              break
+          if not found:
+            raise newException(IOError, "No SSL/TLS CA certificates found.")
+
+    result = SslContext(context: newCTX, referencedData: initHashSet[int](),
       extraInternal: new(SslContextExtraInternal))
 
   proc getExtraInternal(ctx: SslContext): SslContextExtraInternal =

lib/pure/ssl_certs.nim

@@ -0,0 +1,161 @@
+#
+#
+#            Nim's Runtime Library
+#        (c) Copyright 2017 Nim contributors
+#
+#    See the file "copying.txt", included in this
+#    distribution, for details about the copyright.
+#
+## Scan for SSL/TLS CA certificates on disk
+## The default locations can be overridden using the SSL_CERT_FILE and
+## SSL_CERT_DIR environment variables.
+
+import os, strutils
+
+# FWIW look for files before scanning entire dirs.
+
+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
+    "/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",
+    # FreeBSD (security/ca-root-nss package)
+    "/usr/local/share/certs/ca-root-nss.crt",
+    # CentOS/RHEL 7
+    "/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem",
+    # OpenBSD, FreeBSD (optional symlink)
+    "/etc/ssl/cert.pem",
+    # Fedora/RHEL
+    "/etc/pki/tls/certs",
+    # Android
+    "/system/etc/security/cacerts",
+    # FreeBSD
+    "/usr/local/share/certs",
+    # NetBSD
+    "/etc/openssl/certs",
+  ]
+
+when defined(haiku):
+  const
+    B_FIND_PATH_EXISTING_ONLY = 0x4
+    B_FIND_PATH_DATA_DIRECTORY = 6
+
+  proc find_paths_etc(architecture: cstring, baseDirectory: cint,
+                      subPath: cstring, flags: uint32,
+                      paths: var ptr UncheckedArray[cstring],
+                      pathCount: var csize): int32
+                     {.importc, header: "<FindDirectory.h>".}
+  proc free(p: pointer) {.importc, header: "<stdlib.h>".}
+
+iterator scanSSLCertificates*(useEnvVars = false): string =
+  ## Scan for SSL/TLS CA certificates on disk.
+  ##
+  ## 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 useEnvVars and existsEnv("SSL_CERT_FILE"):
+    yield getEnv("SSL_CERT_FILE")
+
+  elif useEnvVars and existsEnv("SSL_CERT_DIR"):
+    let p = getEnv("SSL_CERT_DIR")
+    for fn in joinPath(p, "*").walkFiles():
+      yield fn
+
+  else:
+    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 fileExists(p):
+            yield p
+        elif dirExists(p):
+          for fn in joinPath(p, "*").walkFiles():
+            yield fn
+    else:
+      var
+        paths: ptr UncheckedArray[cstring]
+        size: csize
+      let err = find_paths_etc(
+        nil, B_FIND_PATH_DATA_DIRECTORY, "ssl/CARootCertificates.pem",
+        B_FIND_PATH_EXISTING_ONLY, paths, size
+      )
+      if err == 0:
+        defer: free(paths)
+        for i in 0 ..< size:
+          yield $paths[i]
+
+# Certificates management on windows
+# when defined(windows) or defined(nimdoc):
+#
+#   import openssl
+#
+#   type
+#     PCCertContext {.final, pure.} = pointer
+#     X509 {.final, pure.} = pointer
+#     CertStore {.final, pure.} = pointer
+#
+#   # OpenSSL cert store
+#
+#   {.push stdcall, dynlib: "kernel32", importc.}
+#
+#   proc CertOpenSystemStore*(hprov: pointer=nil, szSubsystemProtocol: cstring): CertStore
+#
+#   proc CertEnumCertificatesInStore*(hCertStore: CertStore, pPrevCertContext: PCCertContext): pointer
+#
+#   proc CertFreeCertificateContext*(pContext: PCCertContext): bool
+#
+#   proc CertCloseStore*(hCertStore:CertStore, flags:cint): bool
+#
+#   {.pop.}

tests/stdlib/thttpclient_ssl.nim

@@ -0,0 +1,131 @@
+discard """
+  cmd: "nim $target --threads:on -d:ssl $options $file"
+  disabled: "openbsd"
+"""
+
+#            Nim - Basic SSL integration tests
+#        (c) Copyright 2018 Nim contributors
+#
+#    See the file "copying.txt", included in this
+#    distribution, for details about the copyright.
+#
+## Warning: this test performs local networking.
+## Test with:
+## ./bin/nim c -d:ssl -p:. --threads:on -r tests/stdlib/thttpclient_ssl.nim
+
+when not defined(windows):
+  # Disabled on Windows due to old OpenSSL version
+
+  import
+    httpclient,
+    net,
+    openssl,
+    os,
+    strutils,
+    threadpool,
+    times,
+    unittest
+
+  # bogus self-signed certificate
+  const
+    certFile = "tests/stdlib/thttpclient_ssl_cert.pem"
+    keyFile = "tests/stdlib/thttpclient_ssl_key.pem"
+
+  proc log(msg: string) =
+    when defined(ssldebug):
+      echo "    [" & $epochTime() & "] " & msg
+    # FIXME
+    echo "    [" & $epochTime() & "] " & msg
+    discard
+
+  proc runServer(port: Port): bool {.thread.} =
+    ## Run a trivial HTTPS server in a {.thread.}
+    ## Exit after serving one request
+
+    var socket = newSocket()
+    socket.setSockOpt(OptReusePort, true)
+    socket.bindAddr(port)
+
+    var ctx = newContext(certFile=certFile, keyFile=keyFile)
+
+    ##  Handle one connection
+    socket.listen()
+
+    var client: Socket
+    var address = ""
+
+    log "server: ready"
+    socket.acceptAddr(client, address)
+    log "server: incoming connection"
+
+    var ssl: SslPtr = SSL_new(ctx.context)
+    discard SSL_set_fd(ssl, client.getFd())
+    log "server: accepting connection"
+    ErrClearError()
+    if SSL_accept(ssl) <= 0:
+      ERR_print_errors_fp(stderr)
+    else:
+      const reply = "HTTP/1.0 200 OK\r\nServer: test\r\nContent-type: text/html\r\nContent-Length: 0\r\n\r\n"
+      log "server: sending reply"
+      discard SSL_write(ssl, reply.cstring, reply.len)
+
+    log "server: receiving a line"
+    let line = client.recvLine()
+    log "server: received $# bytes" % $line.len
+    log "closing"
+    SSL_free(ssl)
+    close(client)
+    close(socket)
+    log "server: exited"
+
+
+  suite "SSL self signed certificate check":
+
+    test "TCP socket":
+      const port = 12347.Port
+      let t = spawn runServer(port)
+      sleep(100)
+      var sock = newSocket()
+      var ctx = newContext()
+      ctx.wrapSocket(sock)
+      try:
+        log "client: connect"
+        sock.connect("127.0.0.1", port)
+        fail()
+      except:
+        let msg = getCurrentExceptionMsg()
+        check(msg.contains("certificate verify failed"))
+
+    test "HttpClient default: no check":
+      const port = 12345.Port
+      let t = spawn runServer(port)
+      sleep(100)
+
+      var client = newHttpClient(sslContext=newContext(verifyMode=CVerifyNone))
+      try:
+        log "client: connect"
+        discard client.getContent("https://127.0.0.1:12345")
+      except:
+        let msg = getCurrentExceptionMsg()
+        log "client: unexpected exception: " & msg
+        fail()
+
+    test "HttpClient with CVerifyPeer":
+      const port = 12346.Port
+      let t = spawn runServer(port)
+      sleep(100)
+
+      var client = newHttpClient(sslContext=newContext(verifyMode=CVerifyPeer))
+      try:
+        log "client: connect"
+        discard client.getContent("https://127.0.0.1:12346")
+        log "getContent should have raised an exception"
+        fail()
+      except:
+        let msg = getCurrentExceptionMsg()
+        log "client: exception: " & msg
+        # SSL_shutdown:shutdown while in init
+        if not (msg.contains("alert number 48") or
+          msg.contains("certificate verify failed")):
+          echo "CVerifyPeer exception: " & msg
+          check(false)