(websocket+tls) fix hang in tls handshake which could lead to ANR, discovered through unittesting.

docs/CHANGELOG.md

@@ -1,6 +1,10 @@
 # Changelog
 All changes to this project will be documented in this file.
 
+## [8.3.2] - 2020-03-20
+
+(websocket+tls) fix hang in tls handshake which could lead to ANR, discovered through unittesting.
+
 ## [8.3.1] - 2020-03-20
 
 (cobra) CobraMetricsPublisher can be configure with an ix::CobraConfig + more unittest use SSL in server + client

ixwebsocket/IXSocketAppleSSL.cpp

@@ -164,6 +164,26 @@ namespace ix
         return false;
     }
 
+    OSStatus SocketAppleSSL::tlsHandShake(std::string& errMsg,
+                                          const CancellationRequest& isCancellationRequested)
+    {
+        OSStatus status;
+
+        do
+        {
+            status = SSLHandshake(_sslContext);
+
+            // Interrupt the handshake
+            if (isCancellationRequested())
+            {
+                errMsg = "Cancellation requested";
+                return errSSLInternal;
+            }
+        } while (status == errSSLWouldBlock || status == errSSLServerAuthCompleted);
+
+        return status;
+    }
+
     // No wait support
     bool SocketAppleSSL::connect(const std::string& host,
                                  int port,
@@ -190,26 +210,17 @@ namespace ix
                 Boolean option(1);
                 SSLSetSessionOption(_sslContext, kSSLSessionOptionBreakOnServerAuth, option);
 
-                do
+                status = tlsHandShake(errMsg, isCancellationRequested);
-                {
-                    status = SSLHandshake(_sslContext);
-                } while (status == errSSLWouldBlock || status == errSSLServerAuthCompleted);
 
                 if (status == errSSLServerAuthCompleted)
                 {
                     // proceed with the handshake
-                    do
+                    status = tlsHandShake(errMsg, isCancellationRequested);
-                    {
-                        status = SSLHandshake(_sslContext);
-                    } while (status == errSSLWouldBlock || status == errSSLServerAuthCompleted);
                 }
             }
             else
             {
-                do
+                status = tlsHandShake(errMsg, isCancellationRequested);
-                {
-                    status = SSLHandshake(_sslContext);
-                } while (status == errSSLWouldBlock || status == errSSLServerAuthCompleted);
             }
         }
 

ixwebsocket/IXSocketAppleSSL.h

@@ -37,6 +37,10 @@ namespace ix
         static OSStatus writeToSocket(SSLConnectionRef connection, const void* data, size_t* len);
         static OSStatus readFromSocket(SSLConnectionRef connection, void* data, size_t* len);
 
+        OSStatus tlsHandShake(
+            std::string& errMsg,
+            const CancellationRequest& isCancellationRequested);
+
         SSLContextRef _sslContext;
         mutable std::mutex _mutex; // AppleSSL routines are not thread-safe
 

ixwebsocket/IXSocketMbedTLS.cpp

@@ -195,8 +195,17 @@ namespace ix
         int res;
         do
         {
-            std::lock_guard<std::mutex> lock(_mutex);
+            {
-            res = mbedtls_ssl_handshake(&_ssl);
+                std::lock_guard<std::mutex> lock(_mutex);
+                res = mbedtls_ssl_handshake(&_ssl);
+            }
+
+            if (isCancellationRequested())
+            {
+                errMsg = "Cancellation requested";
+                close();
+                return false;
+            }
         } while (res == MBEDTLS_ERR_SSL_WANT_READ || res == MBEDTLS_ERR_SSL_WANT_WRITE);
 
         if (res != 0)

ixwebsocket/IXSocketOpenSSL.cpp

@@ -224,7 +224,10 @@ namespace ix
         return true;
     }
 
-    bool SocketOpenSSL::openSSLClientHandshake(const std::string& host, std::string& errMsg)
+    bool SocketOpenSSL::openSSLClientHandshake(
+        const std::string& host,
+        std::string& errMsg,
+        const CancellationRequest& isCancellationRequested)
     {
         while (true)
         {
@@ -233,6 +236,12 @@ namespace ix
                 return false;
             }
 
+            if (isCancellationRequested())
+            {
+                errMsg = "Cancellation requested";
+                return false;
+            }
+
             ERR_clear_error();
             int connect_result = SSL_connect(_ssl_connection);
             if (connect_result == 1)
@@ -577,7 +586,7 @@ namespace ix
             X509_VERIFY_PARAM* param = SSL_get0_param(_ssl_connection);
             X509_VERIFY_PARAM_set1_host(param, host.c_str(), 0);
 #endif
-            handshakeSuccessful = openSSLClientHandshake(host, errMsg);
+            handshakeSuccessful = openSSLClientHandshake(host, errMsg, isCancellationRequested);
         }
 
         if (!handshakeSuccessful)

ixwebsocket/IXSocketOpenSSL.h

@@ -39,7 +39,9 @@ namespace ix
         void openSSLInitialize();
         std::string getSSLError(int ret);
         SSL_CTX* openSSLCreateContext(std::string& errMsg);
-        bool openSSLClientHandshake(const std::string& hostname, std::string& errMsg);
+        bool openSSLClientHandshake(const std::string& hostname,
+                                    std::string& errMsg,
+                                    const CancellationRequest& isCancellationRequested);
         bool openSSLCheckServerCert(SSL* ssl, const std::string& hostname, std::string& errMsg);
         bool checkHost(const std::string& host, const char* pattern);
         bool handleTLSOptions(std::string& errMsg);

ixwebsocket/IXWebSocketVersion.h

@@ -6,4 +6,4 @@
 
 #pragma once
 
-#define IX_WEBSOCKET_VERSION "8.3.1"
+#define IX_WEBSOCKET_VERSION "8.3.2"

test/IXCobraMetricsPublisherTest.cpp

@@ -147,7 +147,7 @@ namespace
 TEST_CASE("Cobra_Metrics_Publisher", "[cobra]")
 {
     int port = getFreePort();
-    bool preferTLS = false;
+    bool preferTLS = true;
     snake::AppConfig appConfig = makeSnakeServerConfig(port, preferTLS);
 
     // Start a redis server