From 70c81734c51a7f3257da7273cf7738173fc82caa Mon Sep 17 00:00:00 2001 From: Jean Flach Date: Tue, 13 Feb 2018 17:29:48 +0100 Subject: [PATCH] Add timeout for TLS handshakes --- lib/base/stream.cpp | 25 ++++++++++++++++++++----- lib/base/stream.hpp | 4 +++- lib/base/tlsstream.cpp | 13 +++++++++++-- lib/remote/apilistener.cpp | 12 +++++++++--- lib/remote/httpserverconnection.cpp | 8 +++++++- lib/remote/pkiutility.cpp | 6 ++++-- 6 files changed, 54 insertions(+), 14 deletions(-) diff --git a/lib/base/stream.cpp b/lib/base/stream.cpp index 57791d305..4e0ad72dc 100644 --- a/lib/base/stream.cpp +++ b/lib/base/stream.cpp @@ -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(); } diff --git a/lib/base/stream.hpp b/lib/base/stream.hpp index 6c39e79aa..63ac31599 100644 --- a/lib/base/stream.hpp +++ b/lib/base/stream.hpp @@ -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; diff --git a/lib/base/tlsstream.cpp b/lib/base/tlsstream.cpp index 78b1851da..953b2b8f0 100644 --- a/lib/base/tlsstream.cpp +++ b/lib/base/tlsstream.cpp @@ -28,6 +28,9 @@ # include #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.")); diff --git a/lib/remote/apilistener.cpp b/lib/remote/apilistener.cpp index b00c37f6a..107c142b7 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -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; } diff --git a/lib/remote/httpserverconnection.cpp b/lib/remote/httpserverconnection.cpp index 955862756..706ee33a2 100644 --- a/lib/remote/httpserverconnection.cpp +++ b/lib/remote/httpserverconnection.cpp @@ -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(); diff --git a/lib/remote/pkiutility.cpp b/lib/remote/pkiutility.cpp index 6e8443bd5..b9da04ebe 100644 --- a/lib/remote/pkiutility.cpp +++ b/lib/remote/pkiutility.cpp @@ -121,8 +121,10 @@ boost::shared_ptr 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(); } return stream->GetPeerCertificate(); -- 2.40.0