]> granicus.if.org Git - icinga2/commitdiff
Add timeout for TLS handshakes
authorJean Flach <jean-marcel.flach@icinga.com>
Tue, 13 Feb 2018 16:29:48 +0000 (17:29 +0100)
committerJean Flach <jean-marcel.flach@icinga.com>
Fri, 23 Feb 2018 09:09:26 +0000 (10:09 +0100)
lib/base/stream.cpp
lib/base/stream.hpp
lib/base/tlsstream.cpp
lib/remote/apilistener.cpp
lib/remote/httpserverconnection.cpp
lib/remote/pkiutility.cpp

index 57791d30522b0e2ab39012bcfb25ea361bb2f8a9..4e0ad72dcb49ddfc26124ecfe3d4267c932e57f2 100644 (file)
@@ -60,7 +60,7 @@ void Stream::SignalDataAvailable(void)
        }
 }
 
-bool Stream::WaitForData(int timeout)
+bool Stream::WaitForData()
 {
        if (!SupportsWaiting())
                BOOST_THROW_EXCEPTION(std::runtime_error("Stream does not support waiting."));
@@ -68,10 +68,25 @@ bool Stream::WaitForData(int timeout)
        boost::mutex::scoped_lock lock(m_Mutex);
 
        while (!IsDataAvailable() && !IsEof())
-               if (timeout < 0)
-                       m_CV.wait(lock);
-               else
-                       m_CV.timed_wait(lock, boost::posix_time::milliseconds(timeout * 1000));
+               m_CV.wait(lock);
+
+       return IsDataAvailable() || IsEof();
+}
+
+bool Stream::WaitForData(int timeout)
+{
+       if (!SupportsWaiting())
+               BOOST_THROW_EXCEPTION(std::runtime_error("Stream does not support waiting."));
+
+       if (timeout < 0)
+               BOOST_THROW_EXCEPTION(std::runtime_error("Timeout can't be negative"));
+
+       boost::system_time const point_of_timeout = boost::get_system_time() + boost::posix_time::seconds(timeout);
+
+       boost::mutex::scoped_lock lock(m_Mutex);
+
+       while (!IsDataAvailable() && !IsEof() && point_of_timeout > boost::get_system_time())
+               m_CV.timed_wait(lock, point_of_timeout);
 
        return IsDataAvailable() || IsEof();
 }
index 6c39e79aa19b56a90ebd9c0b069dabe6a703b137..63ac31599029cbc93eedb7c792c7bc3ef16cf07c 100644 (file)
@@ -124,8 +124,10 @@ public:
 
        /**
         * Waits until data can be read from the stream.
+        * Optionally with a timeout.
         */
-       bool WaitForData(int timeout = -1);
+       bool WaitForData();
+       bool WaitForData(int timeout);
 
        virtual bool SupportsWaiting(void) const;
 
index 78b1851daf865f1622963a96074e7e98771664c5..953b2b8f0ddd0a0ad5b8ad18bfeba8a98eb28850 100644 (file)
@@ -28,6 +28,9 @@
 #      include <poll.h>
 #endif /* _WIN32 */
 
+#define TLS_TIMEOUT_SECONDS 10
+#define TLS_TIMEOUT_STEP_SECONDS 1
+
 using namespace icinga;
 
 int I2_EXPORT TlsStream::m_SSLIndex;
@@ -286,8 +289,14 @@ void TlsStream::Handshake(void)
        m_CurrentAction = TlsActionHandshake;
        ChangeEvents(POLLOUT);
 
-       while (!m_HandshakeOK && !m_ErrorOccurred && !m_Eof)
-               m_CV.wait(lock);
+       boost::system_time const timeout = boost::get_system_time() + boost::posix_time::seconds(TLS_TIMEOUT_SECONDS);
+
+       while (!m_HandshakeOK && !m_ErrorOccurred && !m_Eof && timeout > boost::get_system_time())
+               m_CV.timed_wait(lock, timeout);
+
+       // We should _NOT_ (underline, bold, itallic and wordart) throw an exception for a timeout.
+       if (timeout < boost::get_system_time())
+               BOOST_THROW_EXCEPTION(std::runtime_error("Timeout during handshake."));
 
        if (m_Eof)
                BOOST_THROW_EXCEPTION(std::runtime_error("Socket was closed during TLS handshake."));
index b00c37f6a3d952e00277df23619d08fe88c28196..107c142b7ca749baad56350efaceb191742a8ef1 100644 (file)
@@ -511,11 +511,17 @@ void ApiListener::NewClientHandlerInternal(const Socket::Ptr& client, const Stri
                JsonRpc::SendMessage(tlsStream, message);
                ctype = ClientJsonRpc;
        } else {
-               tlsStream->WaitForData(5);
+               tlsStream->WaitForData(10);
 
                if (!tlsStream->IsDataAvailable()) {
-                       Log(LogWarning, "ApiListener")
-                           << "No data received on new API connection for identity '" << identity << "'. Ensure that the remote endpoints are properly configured in a cluster setup.";
+                       if (identity.IsEmpty())
+                               Log(LogInformation, "ApiListener")
+                                       << "No data received on new API connection. "
+                                       << "Ensure that the remote endpoints are properly configured in a cluster setup.";
+                       else
+                               Log(LogWarning, "ApiListener")
+                                       << "No data received on new API connection for identity '" << identity << "'. "
+                                       << "Ensure that the remote endpoints are properly configured in a cluster setup.";
                        return;
                }
 
index 955862756d69f9044ac24cc3aa38819437cb4b02..706ee33a2a2de7b94645b39bd2374669609ee30e 100644 (file)
@@ -52,7 +52,7 @@ void HttpServerConnection::StaticInitialize(void)
 {
        l_HttpServerConnectionTimeoutTimer = new Timer();
        l_HttpServerConnectionTimeoutTimer->OnTimerExpired.connect(boost::bind(&HttpServerConnection::TimeoutTimerHandler));
-       l_HttpServerConnectionTimeoutTimer->SetInterval(15);
+       l_HttpServerConnectionTimeoutTimer->SetInterval(5);
        l_HttpServerConnectionTimeoutTimer->Start();
 }
 
@@ -76,6 +76,12 @@ TlsStream::Ptr HttpServerConnection::GetStream(void) const
 
 void HttpServerConnection::Disconnect(void)
 {
+       boost::mutex::scoped_try_lock lock(m_DataHandlerMutex);
+       if (!lock.owns_lock()) {
+               Log(LogInformation, "HttpServerConnection", "Unable to disconnect Http client, I/O thread busy");
+               return;
+       }
+
        Log(LogDebug, "HttpServerConnection", "Http client disconnected");
 
        ApiListener::Ptr listener = ApiListener::GetInstance();
index 6e8443bd55d4c9c2428b7a3a153a74588986316e..b9da04ebebcc78d5e636f342a5c7cdf5552d9542 100644 (file)
@@ -121,8 +121,10 @@ boost::shared_ptr<X509> PkiUtility::FetchCert(const String& host, const String&
 
        try {
                stream->Handshake();
-       } catch (...) {
-
+       } catch (const std::exception& ex) {
+               Log(LogCritical, "pki")
+                       << "Client TLS handshake failed. (" << ex.what() << ")";
+               return std::shared_ptr<X509>();
        }
 
        return stream->GetPeerCertificate();