]> 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>
Tue, 20 Feb 2018 12:32:04 +0000 (13:32 +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 1e6b9baf13c382ed68eb566d525fecbdc0565190..72ca82c1a021dc67b878a508cb5089bb61ace30f 100644 (file)
@@ -60,7 +60,7 @@ void Stream::SignalDataAvailable()
        }
 }
 
-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 58569246cafc1a29adc06c3df68f73ce4708c72b..8a140a58607395c74d318aae6ffa53d2a72af497 100644 (file)
@@ -122,8 +122,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() const;
 
index 029c9b446f960a26b975d527858f41b2748062e6..e97f68b605c046e7869f7cee9f27d138a2fa82ac 100644 (file)
@@ -27,6 +27,9 @@
 #      include <poll.h>
 #endif /* _WIN32 */
 
+#define TLS_TIMEOUT_SECONDS 10
+#define TLS_TIMEOUT_STEP_SECONDS 1
+
 using namespace icinga;
 
 int TlsStream::m_SSLIndex;
@@ -285,8 +288,14 @@ void TlsStream::Handshake()
        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 251aff7916e9512e06692e3194eb7769216a8e3e..bc0df4449bdf0103751bf0f53fdb896ad8be994e 100644 (file)
@@ -512,11 +512,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 c22f0ee155bfb06869497bea13983cd43f8419d6..7c86f9be9a17a813a47782a2170628fda1d04fbb 100644 (file)
@@ -52,7 +52,7 @@ void HttpServerConnection::StaticInitialize()
 {
        l_HttpServerConnectionTimeoutTimer = new Timer();
        l_HttpServerConnectionTimeoutTimer->OnTimerExpired.connect(std::bind(&HttpServerConnection::TimeoutTimerHandler));
-       l_HttpServerConnectionTimeoutTimer->SetInterval(15);
+       l_HttpServerConnectionTimeoutTimer->SetInterval(5);
        l_HttpServerConnectionTimeoutTimer->Start();
 }
 
@@ -76,6 +76,12 @@ TlsStream::Ptr HttpServerConnection::GetStream() const
 
 void HttpServerConnection::Disconnect()
 {
+       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 6b896dab1262652e5102b13ef7dd85909fb1a5a5..20d9ca6c2fe877ff8dc4dfee0d437bddfa8f4084 100644 (file)
@@ -121,8 +121,10 @@ std::shared_ptr<X509> PkiUtility::FetchCert(const String& host, const String& po
 
        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();