From 82092b3bb7ead1414f673ed7f43e9fa1dcf8c5f4 Mon Sep 17 00:00:00 2001
From: Leorize <leorize+oss@disroot.org>
Date: Mon, 1 Jun 2020 17:10:02 -0500
Subject: [PATCH] asyncnet, net: call SSL_shutdown only when connection
 established

This commit prevents "SSL_shutdown while in init" errors from happening.

See https://github.com/openssl/openssl/issues/710#issuecomment-253897666
---
 lib/pure/asyncnet.nim            |  9 ++++++++-
 lib/pure/net.nim                 | 22 +++++++++++++---------
 lib/wrappers/openssl.nim         | 28 ++++++++++++++++++++++++++++
 tests/stdlib/thttpclient_ssl.nim |  5 +++--
 4 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/lib/pure/asyncnet.nim b/lib/pure/asyncnet.nim
index 7ccb469cfe..8df839c14e 100644
--- a/lib/pure/asyncnet.nim
+++ b/lib/pure/asyncnet.nim
@@ -713,7 +713,14 @@ proc close*(socket: AsyncSocket) =
     socket.fd.AsyncFD.closeSocket()
   when defineSsl:
     if socket.isSsl:
-      let res = SSL_shutdown(socket.sslHandle)
+      let res =
+        # Don't call SSL_shutdown if the connection has not been fully
+        # established, see:
+        # https://github.com/openssl/openssl/issues/710#issuecomment-253897666
+        if SSL_in_init(socket.sslHandle) == 0:
+          SSL_shutdown(socket.sslHandle)
+        else:
+          0
       SSL_free(socket.sslHandle)
       if res == 0:
         discard
diff --git a/lib/pure/net.nim b/lib/pure/net.nim
index b08629a96e..f7aacfb0c3 100644
--- a/lib/pure/net.nim
+++ b/lib/pure/net.nim
@@ -1007,15 +1007,19 @@ proc close*(socket: Socket) =
     when defineSsl:
       if socket.isSsl and socket.sslHandle != nil:
         ErrClearError()
-        # As we are closing the underlying socket immediately afterwards,
-        # it is valid, under the TLS standard, to perform a unidirectional
-        # shutdown i.e not wait for the peers "close notify" alert with a second
-        # call to SSL_shutdown
-        let res = SSL_shutdown(socket.sslHandle)
-        if res == 0:
-          discard
-        elif res != 1:
-          socketError(socket, res)
+        # Don't call SSL_shutdown if the connection has not been fully
+        # established, see:
+        # https://github.com/openssl/openssl/issues/710#issuecomment-253897666
+        if SSL_in_init(socket.sslHandle) == 0:
+          # As we are closing the underlying socket immediately afterwards,
+          # it is valid, under the TLS standard, to perform a unidirectional
+          # shutdown i.e not wait for the peers "close notify" alert with a second
+          # call to SSL_shutdown
+          let res = SSL_shutdown(socket.sslHandle)
+          if res == 0:
+            discard
+          elif res != 1:
+            socketError(socket, res)
   finally:
     when defineSsl:
       if socket.isSsl and socket.sslHandle != nil:
diff --git a/lib/wrappers/openssl.nim b/lib/wrappers/openssl.nim
index 5aaa0a0e8c..5fd4def0ba 100644
--- a/lib/wrappers/openssl.nim
+++ b/lib/wrappers/openssl.nim
@@ -201,6 +201,9 @@ const
   SSL_OP_ALL* = 0x000FFFFF
   SSL_VERIFY_NONE* = 0x00000000
   SSL_VERIFY_PEER* = 0x00000001
+  SSL_ST_CONNECT* = 0x1000
+  SSL_ST_ACCEPT* = 0x2000
+  SSL_ST_INIT* = SSL_ST_CONNECT or SSL_ST_ACCEPT
   OPENSSL_DES_DECRYPT* = 0
   OPENSSL_DES_ENCRYPT* = 1
   X509_V_OK* = 0
@@ -282,6 +285,13 @@ when compileOption("dynlibOverride", "ssl") or defined(noOpenSSLHacks):
       # Static linking against OpenSSL < 1.1.0 is not supported
       discard
 
+  when defined(libressl) or defined(openssl10):
+    proc SSL_state(ssl: SslPtr): cint {.cdecl, dynlib: DLLSSLName, importc.}
+    proc SSL_in_init*(ssl: SslPtr): cint {.inline.} =
+      SSl_state(ssl) and SSL_ST_INIT
+  else:
+    proc SSL_in_init*(ssl: SslPtr): cint {.cdecl, dynlib: DLLSSLName, importc.}
+
   template OpenSSL_add_all_algorithms*() = discard
 
   proc SSLv23_client_method*(): PSSL_METHOD {.cdecl, dynlib: DLLSSLName, importc.}
@@ -382,6 +392,24 @@ else:
       if theProc.isNil: 0.culong
       else: theProc()
 
+  proc SSL_in_init*(ssl: SslPtr): cint =
+    # A compatibility wrapper for `SSL_in_init()` for OpenSSL 1.0, 1.1 and LibreSSL
+    const MainProc = "SSL_in_init"
+    let
+      theProc {.global.} = cast[proc(ssl: SslPtr): cint {.cdecl.}](sslSymNullable(MainProc))
+      # Fallback
+      sslState {.global.} = cast[proc(ssl: SslPtr): cint {.cdecl.}](sslSymNullable("SSL_state"))
+
+    # FIXME: This shouldn't be needed, but the compiler is complaining about
+    #        `sslState` being GC-ed memory???
+    {.gcsafe.}:
+      if not theProc.isNil:
+        theProc(ssl)
+      elif not sslState.isNil:
+        sslState(ssl) and SSL_ST_INIT
+      else:
+        raiseInvalidLibrary MainProc
+
 proc ERR_load_BIO_strings*(){.cdecl, dynlib: DLLUtilName, importc.}
 
 proc SSL_new*(context: SslCtx): SslPtr{.cdecl, dynlib: DLLSSLName, importc.}
diff --git a/tests/stdlib/thttpclient_ssl.nim b/tests/stdlib/thttpclient_ssl.nim
index 2e9d1c5be4..f06c1eddd7 100644
--- a/tests/stdlib/thttpclient_ssl.nim
+++ b/tests/stdlib/thttpclient_ssl.nim
@@ -124,7 +124,8 @@ when not defined(windows):
         let msg = getCurrentExceptionMsg()
         log "client: exception: " & msg
         # SSL_shutdown:shutdown while in init
-        if not (msg.contains("shutdown while in init") or msg.contains("alert number 48") or
-          msg.contains("routines:CONNECT_CR_CERT:certificate verify failed")):
+        if not (msg.contains("alert number 48") or
+          msg.contains("routines:CONNECT_CR_CERT:certificate verify failed") or
+          msg.contains("routines:tls_process_server_certificate:certificate verify failed")):
           echo "CVerifyPeer exception: " & msg
           check(false)