SSL certificate verify GitHub action (#13697)

* Implement SSL/TLS certificate checking #782

* SSL: Add nimDisableCertificateValidation

Remove NIM_SSL_CERT_VALIDATION env var
tests/untestable/thttpclient_ssl.nim ran successfully on Linux with libssl 1.1.1d

* SSL: update integ test to skip flapping tests

* Revert .travis.yml change

* nimDisableCertificateValidation disable imports

Prevent loading symbols that are not defined on older SSL libs

* SSL: disable verification in net.nim

..when nimDisableCertificateValidation is set

* Update changelog

* Fix peername type

* Add define check for windows

* Disable test on windows

* Add exprimental GitHub action CI for SSL

* Test nimDisableCertificateValidation

4 files changed, 253 insertions, 12 deletions

diff --git a/lib/pure/httpclient.nim b/lib/pure/httpclient.nim
index b7bdc8d17..e3e5a5c11 100644
--- a/lib/pure/httpclient.nim
+++ b/lib/pure/httpclient.nim
@@ -119,6 +119,14 @@
 ## You will also have to compile with ``ssl`` defined like so:
 ## ``nim c -d:ssl ...``.
 ##
+## Certificate validation is NOT performed by default.
+## This will change in future.
+##
+## A set of directories and files from the `ssl_certs <ssl_certs.html>`_
+## module are scanned to locate CA certificates.
+##
+## See `newContext <net.html#newContext>`_ to tweak or disable certificate validation.
+##
 ## Timeouts
 ## ========
 ##
@@ -552,6 +560,7 @@ proc newHttpClient*(userAgent = defUserAgent, maxRedirects = 5,
   ## default is 5.
   ##
   ## ``sslContext`` specifies the SSL context to use for HTTPS requests.
+  ## See `SSL/TLS support <##ssl-tls-support>`_
   ##
   ## ``proxy`` specifies an HTTP proxy to use for this HTTP client's
   ## connections.
diff --git a/lib/pure/net.nim b/lib/pure/net.nim
index 0d4440242..ca3d37259 100644
--- a/lib/pure/net.nim
+++ b/lib/pure/net.nim
@@ -67,6 +67,8 @@
 {.deadCodeElim: on.} # dce option deprecated
 import nativesockets, os, strutils, parseutils, times, sets, options,
   std/monotimes
+from ospaths import getEnv
+from ssl_certs import scanSSLCertificates
 export nativesockets.Port, nativesockets.`$`, nativesockets.`==`
 export Domain, SockType, Protocol
 
@@ -83,7 +85,7 @@ when defineSsl:
     SslError* = object of Exception
 
     SslCVerifyMode* = enum
-      CVerifyNone, CVerifyPeer
+      CVerifyNone, CVerifyPeer, CVerifyPeerUseEnvVars
 
     SslProtVersion* = enum
       protSSLv2, protSSLv3, protTLSv1, protSSLv23
@@ -517,17 +519,30 @@ when defineSsl:
         raiseSSLError("Verification of private key file failed.")
 
   proc newContext*(protVersion = protSSLv23, verifyMode = CVerifyPeer,
-      certFile = "", keyFile = "", cipherList = "ALL"): SslContext =
+                   certFile = "", keyFile = "", cipherList = "ALL",
+                   caDir = "", caFile = ""): SSLContext =
     ## Creates an SSL context.
     ##
     ## Protocol version specifies the protocol to use. SSLv2, SSLv3, TLSv1
     ## are available with the addition of ``protSSLv23`` which allows for
     ## compatibility with all of them.
     ##
-    ## There are currently only two options for verify mode;
-    ## one is ``CVerifyNone`` and with it certificates will not be verified
-    ## the other is ``CVerifyPeer`` and certificates will be verified for
-    ## it, ``CVerifyPeer`` is the safest choice.
+    ## There are three options for verify mode:
+    ## ``CVerifyNone``: certificates are not verified;
+    ## ``CVerifyPeer``: certificates are verified;
+    ## ``CVerifyPeerUseEnvVars``: certificates are verified and the optional
+    ## environment variables SSL_CERT_FILE and SSL_CERT_DIR are also used to
+    ## locate certificates
+    ##
+    ## The `nimDisableCertificateValidation` define overrides verifyMode and
+    ## disables certificate verification globally!
+    ##
+    ## CA certificates will be loaded, in the following order, from:
+    ##
+    ##  - caFile, caDir, parameters, if set
+    ##  - if `verifyMode` is set to ``CVerifyPeerUseEnvVars``,
+    ##    the SSL_CERT_FILE and SSL_CERT_DIR environment variables are used
+    ##  - a set of files and directories from the `ssl_certs <ssl_certs.html>`_ file.
     ##
     ## The last two parameters specify the certificate file path and the key file
     ## path, a server socket will most likely not work without these.
@@ -550,18 +565,41 @@ when defineSsl:
 
     if newCTX.SSL_CTX_set_cipher_list(cipherList) != 1:
       raiseSSLError()
-    case verifyMode
-    of CVerifyPeer:
-      newCTX.SSL_CTX_set_verify(SSL_VERIFY_PEER, nil)
-    of CVerifyNone:
+
+    when defined(nimDisableCertificateValidation) or defined(windows):
       newCTX.SSL_CTX_set_verify(SSL_VERIFY_NONE, nil)
+    else:
+      case verifyMode
+      of CVerifyPeer, CVerifyPeerUseEnvVars:
+        newCTX.SSL_CTX_set_verify(SSL_VERIFY_PEER, nil)
+      of CVerifyNone:
+        newCTX.SSL_CTX_set_verify(SSL_VERIFY_NONE, nil)
+
     if newCTX == nil:
       raiseSSLError()
 
     discard newCTX.SSLCTXSetMode(SSL_MODE_AUTO_RETRY)
     newCTX.loadCertificates(certFile, keyFile)
 
-    result = SslContext(context: newCTX, referencedData: initSet[int](),
+    when not defined(nimDisableCertificateValidation) and not defined(windows):
+      if verifyMode != CVerifyNone:
+        # Use the caDir and caFile parameters if set
+        if caDir != "" or caFile != "":
+          if newCTX.SSL_CTX_load_verify_locations(caDir, caFile) != 0:
+            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, "") == 0:
+              found = true
+              break
+          if not found:
+            raise newException(IOError, "No SSL/TLS CA certificates found.")
+
+    result = SSLContext(context: newCTX, referencedData: initSet[int](),
       extraInternal: new(SslContextExtraInternal))
 
   proc getExtraInternal(ctx: SslContext): SslContextExtraInternal =
@@ -645,6 +683,7 @@ when defineSsl:
     ## This must be called on an unconnected socket; an SSL session will
     ## be started when the socket is connected.
     ##
+    ## FIXME:
     ## **Disclaimer**: This code is not well tested, may be very unsafe and
     ## prone to security vulnerabilities.
 
@@ -660,7 +699,25 @@ when defineSsl:
     if SSL_set_fd(socket.sslHandle, socket.fd) != 1:
       raiseSSLError()
 
-  proc wrapConnectedSocket*(ctx: SslContext, socket: Socket,
+  proc checkCertName(socket: Socket, hostname: string) =
+    ## Check if the certificate Subject Alternative Name (SAN) or Subject CommonName (CN) matches hostname.
+    ## Wildcards match only in the left-most label.
+    ## When name starts with a dot it will be matched by a certificate valid for any subdomain
+    when not defined(nimDisableCertificateValidation) and not defined(windows):
+      assert socket.isSSL
+      let certificate = socket.sslHandle.SSL_get_peer_certificate()
+      if certificate.isNil:
+        raiseSSLError("No SSL certificate found.")
+
+      const X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT = 0x1.cuint
+      const size = 1024
+      var peername: string = newString(size)
+      let match = certificate.X509_check_host(hostname.cstring, hostname.len.cint,
+        X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT, peername)
+      if match != 1:
+        raiseSSLError("SSL Certificate check failed.")
+
+  proc wrapConnectedSocket*(ctx: SSLContext, socket: Socket,
                             handshake: SslHandshakeType,
                             hostname: string = "") =
     ## Wraps a connected socket in an SSL context. This function effectively
@@ -671,6 +728,7 @@ when defineSsl:
     ## This should be called on a connected socket, and will perform
     ## an SSL handshake immediately.
     ##
+    ## FIXME:
     ## **Disclaimer**: This code is not well tested, may be very unsafe and
     ## prone to security vulnerabilities.
     wrapSocket(ctx, socket)
@@ -682,6 +740,9 @@ when defineSsl:
         discard SSL_set_tlsext_host_name(socket.sslHandle, hostname)
       let ret = SSL_connect(socket.sslHandle)
       socketError(socket, ret)
+      when not defined(nimDisableCertificateValidation) and not defined(windows):
+        if hostname.len > 0 and not isIpAddress(hostname):
+          socket.checkCertName(hostname)
     of handshakeAsServer:
       let ret = SSL_accept(socket.sslHandle)
       socketError(socket, ret)
@@ -1638,6 +1699,9 @@ proc connect*(socket: Socket, address: string,
 
       let ret = SSL_connect(socket.sslHandle)
       socketError(socket, ret)
+      when not defined(nimDisableCertificateValidation) and not defined(windows):
+        if not isIpAddress(address):
+          socket.checkCertName(address)
 
 proc connectAsync(socket: Socket, name: string, port = Port(0),
                   af: Domain = AF_INET) {.tags: [ReadIOEffect].} =
diff --git a/lib/pure/ssl_certs.nim b/lib/pure/ssl_certs.nim
new file mode 100644
index 000000000..7b1550004
--- /dev/null
+++ b/lib/pure/ssl_certs.nim
@@ -0,0 +1,97 @@
+#
+#
+#            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
+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 = [
+    # 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",
+    # Mac OS X
+    "/System/Library/OpenSSL/certs/cert.pem",
+    # Fedora/RHEL
+    "/etc/pki/tls/certs",
+    # Android
+    "/system/etc/security/cacerts",
+    # FreeBSD
+    "/usr/local/share/certs",
+    # NetBSD
+    "/etc/openssl/certs",
+]
+
+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 existsEnv("SSL_CERT_FILE"):
+    yield getEnv("SSL_CERT_FILE")
+
+  elif existsEnv("SSL_CERT_DIR"):
+    let p = getEnv("SSL_CERT_DIR")
+    for fn in joinPath(p, "*").walkFiles():
+      yield fn
+
+  else:
+    for p in certificate_paths:
+      if p.endsWith(".pem") or p.endsWith(".crt"):
+        if existsFile(p):
+          yield p
+      elif existsDir(p):
+        for fn in joinPath(p, "*").walkFiles():
+          yield fn
+
+# 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.}
diff --git a/lib/wrappers/openssl.nim b/lib/wrappers/openssl.nim
index 4a53fba80..789b3611c 100644
--- a/lib/wrappers/openssl.nim
+++ b/lib/wrappers/openssl.nim
@@ -647,3 +647,74 @@ proc md5_Str*(str: string): string =
 
 when defined(nimHasStyleChecks):
   {.pop.}
+
+
+# Certificate validation
+# On old openSSL version some of these symbols are not available
+when not defined(nimDisableCertificateValidation) and not defined(windows):
+
+  proc SSL_get_peer_certificate*(ssl: SslCtx): PX509{.cdecl, dynlib: DLLSSLName,
+      importc.}
+
+  proc X509_get_subject_name*(a: PX509): PX509_NAME{.cdecl, dynlib: DLLSSLName, importc.}
+
+  proc X509_get_issuer_name*(a: PX509): PX509_NAME{.cdecl, dynlib: DLLUtilName, importc.}
+
+  proc X509_NAME_oneline*(a: PX509_NAME, buf: cstring, size: cint): cstring {.
+    cdecl, dynlib:DLLSSLName, importc.}
+
+  proc X509_NAME_get_text_by_NID*(subject:cstring, NID: cint, buf: cstring, size: cint): cint{.
+    cdecl, dynlib:DLLSSLName, importc.}
+
+  proc X509_check_host*(cert: PX509, name: cstring, namelen: cint, flags:cuint, peername: cstring): cint {.cdecl, dynlib: DLLSSLName, importc.}
+
+  # Certificates store
+
+  type PX509_STORE* = SslPtr
+  type PX509_OBJECT* = SslPtr
+
+  {.push callconv:cdecl, dynlib:DLLUtilName, importc.}
+
+  proc X509_OBJECT_new*(): PX509_OBJECT
+  proc X509_OBJECT_free*(a: PX509_OBJECT)
+
+  proc X509_STORE_new*(): PX509_STORE
+  proc X509_STORE_free*(v: PX509_STORE)
+  proc X509_STORE_lock*(ctx: PX509_STORE): cint
+  proc X509_STORE_unlock*(ctx: PX509_STORE): cint
+  proc X509_STORE_up_ref*(v: PX509_STORE): cint
+  proc X509_STORE_set_flags*(ctx: PX509_STORE; flags: culong): cint
+  proc X509_STORE_set_purpose*(ctx: PX509_STORE; purpose: cint): cint
+  proc X509_STORE_set_trust*(ctx: PX509_STORE; trust: cint): cint
+  proc X509_STORE_add_cert*(ctx: PX509_STORE; x: PX509): cint
+
+  proc d2i_X509*(px: ptr PX509, i: ptr ptr cuchar, len: cint): PX509
+
+  proc i2d_X509*(cert: PX509; o: ptr ptr cuchar): cint
+
+  {.pop.}
+
+  proc d2i_X509*(b: string): PX509 =
+    ## decode DER/BER bytestring into X.509 certificate struct
+    var bb = b.cstring
+    let i = cast[ptr ptr cuchar](addr bb)
+    let ret = d2i_X509(addr result, i, b.len.cint)
+    if ret.isNil:
+      raise newException(Exception, "X.509 certificate decoding failed")
+
+  proc i2d_X509*(cert: PX509): string =
+    ## encode `cert` to DER string
+    let encoded_length = i2d_X509(cert, nil)
+    result = newString(encoded_length)
+    var q = result.cstring
+    let o = cast[ptr ptr cuchar](addr q)
+    let length = i2d_X509(cert, o)
+    if length.int <= 0:
+      raise newException(Exception, "X.509 certificate encoding failed")
+
+  when isMainModule:
+    # A simple certificate test
+    let certbytes = readFile("certificate.der")
+    let cert = d2i_X509(certbytes)
+    let encoded = cert.i2d_X509()
+    assert encoded == certbytes