From 7b8dc1d97e9371e0e32c8551a260d02a497dee96 Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Mon, 4 Aug 2014 14:03:37 +0200 Subject: [PATCH 1/5] Fix non-existing endpoint on ApiListener error refs #6724 --- lib/base/application.cpp | 7 ++++--- lib/icinga/apievents.cpp | 24 +++++++++++++++++++++--- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/lib/base/application.cpp b/lib/base/application.cpp index 03559c421..14ea9f6ff 100644 --- a/lib/base/application.cpp +++ b/lib/base/application.cpp @@ -282,7 +282,7 @@ mainloop: lastLoop = now; } - + if (m_RequestRestart) { m_RequestRestart = false; // we are now handling the request, once is enough @@ -295,7 +295,7 @@ mainloop: goto mainloop; } - + Log(LogInformation, "Application", "Shutting down Icinga..."); DynamicObject::StopObjects(); Application::GetInstance()->OnShutdown(); @@ -342,7 +342,7 @@ pid_t Application::StartReloadProcess(void) Process::Ptr process = make_shared(Process::PrepareCommand(args)); process->SetTimeout(300); process->Run(&ReloadProcessCallback); - + return process->GetPID(); } @@ -1009,6 +1009,7 @@ void Application::MakeVariablesConstant(void) ScriptVariable::GetByName("PrefixDir")->SetConstant(true); ScriptVariable::GetByName("SysconfDir")->SetConstant(true); ScriptVariable::GetByName("LocalStateDir")->SetConstant(true); + ScriptVariable::GetByName("RunDir")->SetConstant(true); ScriptVariable::GetByName("PkgDataDir")->SetConstant(true); ScriptVariable::GetByName("StatePath")->SetConstant(false); ScriptVariable::GetByName("PidPath")->SetConstant(false); diff --git a/lib/icinga/apievents.cpp b/lib/icinga/apievents.cpp index bc0bd0f03..723f5ee80 100644 --- a/lib/icinga/apievents.cpp +++ b/lib/icinga/apievents.cpp @@ -898,6 +898,12 @@ void ApiEvents::RepositoryTimerHandler(void) } Endpoint::Ptr my_endpoint = Endpoint::GetLocalEndpoint(); + + if (!my_endpoint) { + Log(LogWarning, "ApiEvents", "No local endpoint defined. Bailing out."); + return; + } + Zone::Ptr my_zone = my_endpoint->GetZone(); Dictionary::Ptr params = make_shared(); @@ -965,14 +971,26 @@ Value ApiEvents::UpdateRepositoryAPIHandler(const MessageOrigin& origin, const D String ApiEvents::GetVirtualHostName(const Host::Ptr& host) { String host_name = host->GetName(); - if (host_name == "localhost") - host_name = Endpoint::GetLocalEndpoint()->GetName(); + if (host_name == "localhost") { + Endpoint::Ptr local = Endpoint::GetLocalEndpoint(); + + if (!local) + return Empty; + + host_name = local->GetName(); + } + return host_name; } Host::Ptr ApiEvents::FindHostByVirtualName(const String& hostName) { - if (hostName == Endpoint::GetLocalEndpoint()->GetName()) + Endpoint::Ptr local = Endpoint::GetLocalEndpoint(); + + if (!local) + return Host::Ptr(); + + if (hostName == local->GetName()) return Host::GetByName("localhost"); else return Host::GetByName(hostName); From 55c306796bbc807b488e094d1cab4d3a107dad38 Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Mon, 4 Aug 2014 16:26:17 +0200 Subject: [PATCH 2/5] Add Application::Exit() refs #6682 --- lib/base/application.cpp | 5 +++++ lib/base/application.hpp | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/base/application.cpp b/lib/base/application.cpp index 14ea9f6ff..1bcd86a82 100644 --- a/lib/base/application.cpp +++ b/lib/base/application.cpp @@ -112,6 +112,11 @@ Application::~Application(void) m_Instance = NULL; } +void Application::Exit(int code) +{ + _exit(code); +} + void Application::InitializeBase(void) { #ifndef _WIN32 diff --git a/lib/base/application.hpp b/lib/base/application.hpp index d6be2b1b5..a97208b45 100644 --- a/lib/base/application.hpp +++ b/lib/base/application.hpp @@ -46,6 +46,8 @@ public: static Application::Ptr GetInstance(void); + static void Exit(int code); + int Run(void); /** @@ -128,7 +130,7 @@ protected: pid_t StartReloadProcess(void); virtual void OnShutdown(void); - + private: static Application *m_Instance; /**< The application instance. */ From 51329f0b3cdd6f3fcc2a88fb551a1e9de6b7c9be Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Mon, 4 Aug 2014 16:34:17 +0200 Subject: [PATCH 3/5] Bail early if ApiListener cannot be started refs #6682 --- lib/remote/apilistener.cpp | 31 ++++++++++++++++++++++++------- lib/remote/apilistener.hpp | 2 +- lib/remote/authority.cpp | 3 +++ 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/lib/remote/apilistener.cpp b/lib/remote/apilistener.cpp index bdf387793..b404e1ef7 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -102,7 +102,10 @@ void ApiListener::Start(void) } /* create the primary JSON-RPC listener */ - AddListener(GetBindPort()); + if (!AddListener(GetBindPort())) { + Log(LogCritical, "ApiListener", "Cannot add listener for port '" + Convert::ToString(GetBindPort()) + "'."); + Application::Exit(EXIT_FAILURE); + } m_Timer = make_shared(); m_Timer->OnTimerExpired.connect(boost::bind(&ApiListener::ApiTimerHandler, this)); @@ -129,6 +132,10 @@ shared_ptr ApiListener::GetSSLContext(void) const Endpoint::Ptr ApiListener::GetMaster(void) const { Zone::Ptr zone = Zone::GetLocalZone(); + + if (!zone) + return Endpoint::Ptr(); + std::vector names; BOOST_FOREACH(const Endpoint::Ptr& endpoint, zone->GetEndpoints()) @@ -142,7 +149,12 @@ Endpoint::Ptr ApiListener::GetMaster(void) const bool ApiListener::IsMaster(void) const { - return GetMaster()->GetName() == GetIdentity(); + Endpoint::Ptr master = GetMaster(); + + if (!master) + return false; + + return master->GetName() == GetIdentity(); } /** @@ -150,7 +162,7 @@ bool ApiListener::IsMaster(void) const * * @param service The port to listen on. */ -void ApiListener::AddListener(const String& service) +bool ApiListener::AddListener(const String& service) { ObjectLock olock(this); @@ -158,7 +170,7 @@ void ApiListener::AddListener(const String& service) if (!sslContext) { Log(LogCritical, "ApiListener", "SSL context is required for AddListener()"); - return; + return false; } std::ostringstream s; @@ -171,13 +183,15 @@ void ApiListener::AddListener(const String& service) server->Bind(service, AF_UNSPEC); } catch(std::exception&) { Log(LogCritical, "ApiListener", "Cannot bind tcp socket on '" + service + "'."); - return; + return false; } boost::thread thread(boost::bind(&ApiListener::ListenerThreadProc, this, server)); thread.detach(); m_Servers.insert(server); + + return true; } void ApiListener::ListenerThreadProc(const Socket::Ptr& server) @@ -209,7 +223,7 @@ void ApiListener::AddConnection(const Endpoint::Ptr& endpoint) shared_ptr sslContext = m_SSLContext; if (!sslContext) { - Log(LogCritical, "ApiListener", "SSL context is required for AddListener()"); + Log(LogCritical, "ApiListener", "SSL context is required for AddConnection()"); return; } } @@ -398,7 +412,10 @@ void ApiListener::ApiTimerHandler(void) Utility::FormatDateTime("%Y/%m/%d %H:%M:%S", ts)); } - Log(LogNotice, "ApiListener", "Current zone master: " + GetMaster()->GetName()); + Endpoint::Ptr master = GetMaster(); + + if (master) + Log(LogNotice, "ApiListener", "Current zone master: " + master->GetName()); std::vector names; BOOST_FOREACH(const Endpoint::Ptr& endpoint, DynamicType::GetObjects()) diff --git a/lib/remote/apilistener.hpp b/lib/remote/apilistener.hpp index 8d95337d9..498f8af57 100644 --- a/lib/remote/apilistener.hpp +++ b/lib/remote/apilistener.hpp @@ -79,7 +79,7 @@ private: void ApiTimerHandler(void); - void AddListener(const String& service); + bool AddListener(const String& service); void AddConnection(const Endpoint::Ptr& endpoint); void NewClientHandler(const Socket::Ptr& client, ConnectionRole role); diff --git a/lib/remote/authority.cpp b/lib/remote/authority.cpp index a9aa49c5e..2c7159020 100644 --- a/lib/remote/authority.cpp +++ b/lib/remote/authority.cpp @@ -42,6 +42,9 @@ static void AuthorityTimerHandler(void) return; Zone::Ptr my_zone = Zone::GetLocalZone(); + if (!my_zone) + return; + Endpoint::Ptr my_endpoint = Endpoint::GetLocalEndpoint(); std::vector endpoints; From fd233ae901ac44d90f13496341b6d378b2dd0c61 Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Mon, 4 Aug 2014 16:43:34 +0200 Subject: [PATCH 4/5] Use Application::Exit() for main app termination refs #6682 --- icinga-app/icinga.cpp | 6 +++--- lib/base/application.cpp | 6 +++--- lib/base/application.hpp | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/icinga-app/icinga.cpp b/icinga-app/icinga.cpp index d7155b648..5cec83f91 100644 --- a/icinga-app/icinga.cpp +++ b/icinga-app/icinga.cpp @@ -595,7 +595,7 @@ int Main(void) int rc = Application::GetInstance()->Run(); #ifndef _DEBUG - _exit(rc); // Yay, our static destructors are pretty much beyond repair at this point. + Application::Exit(rc); #endif /* _DEBUG */ return rc; @@ -788,11 +788,11 @@ int main(int argc, char **argv) }; StartServiceCtrlDispatcher(dispatchTable); - _exit(1); + Application::Exit(1); } #endif /* _WIN32 */ int rc = Main(); - _exit(rc); + Application::Exit(rc); } diff --git a/lib/base/application.cpp b/lib/base/application.cpp index 1bcd86a82..f69d0194a 100644 --- a/lib/base/application.cpp +++ b/lib/base/application.cpp @@ -112,9 +112,9 @@ Application::~Application(void) m_Instance = NULL; } -void Application::Exit(int code) +void Application::Exit(int rc) { - _exit(code); + _exit(rc); // Yay, our static destructors are pretty much beyond repair at this point. } void Application::InitializeBase(void) @@ -711,7 +711,7 @@ void Application::UpdatePidFile(const String& filename, pid_t pid) if (fcntl(fd, F_SETLK, &lock) < 0) { Log(LogCritical, "Application", "Could not lock PID file. Make sure that only one instance of the application is running."); - _exit(EXIT_FAILURE); + Application::Exit(EXIT_FAILURE); } if (ftruncate(fd, 0) < 0) { diff --git a/lib/base/application.hpp b/lib/base/application.hpp index a97208b45..2213fc669 100644 --- a/lib/base/application.hpp +++ b/lib/base/application.hpp @@ -46,7 +46,7 @@ public: static Application::Ptr GetInstance(void); - static void Exit(int code); + static void Exit(int rc); int Run(void); From 9ae37bf1099e4cbae82fde40dca1d580a4b7c3af Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Mon, 4 Aug 2014 17:13:42 +0200 Subject: [PATCH 5/5] Add verbose SSL error messages refs #6682 --- lib/base/tlsutility.cpp | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/lib/base/tlsutility.cpp b/lib/base/tlsutility.cpp index ff3c10e1b..9234619a7 100644 --- a/lib/base/tlsutility.cpp +++ b/lib/base/tlsutility.cpp @@ -18,6 +18,10 @@ ******************************************************************************/ #include "base/tlsutility.hpp" +#include "base/convert.hpp" +#include "base/logger_fwd.hpp" +#include "base/context.hpp" + namespace icinga { @@ -72,6 +76,7 @@ static void InitializeOpenSSL(void) */ shared_ptr MakeSSLContext(const String& pubkey, const String& privkey, const String& cakey) { + std::ostringstream msgbuf; InitializeOpenSSL(); shared_ptr sslContext = shared_ptr(SSL_CTX_new(TLSv1_method()), SSL_CTX_free); @@ -79,6 +84,8 @@ shared_ptr MakeSSLContext(const String& pubkey, const String& privkey, SSL_CTX_set_mode(sslContext.get(), SSL_MODE_ENABLE_PARTIAL_WRITE | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); if (!SSL_CTX_use_certificate_chain_file(sslContext.get(), pubkey.CStr())) { + msgbuf << "Error with public key file '" << pubkey << "': " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), NULL) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("SSL_CTX_use_certificate_chain_file") << errinfo_openssl_error(ERR_get_error()) @@ -86,6 +93,8 @@ shared_ptr MakeSSLContext(const String& pubkey, const String& privkey, } if (!SSL_CTX_use_PrivateKey_file(sslContext.get(), privkey.CStr(), SSL_FILETYPE_PEM)) { + msgbuf << "Error with private key file '" << privkey << "': " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), NULL) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("SSL_CTX_use_PrivateKey_file") << errinfo_openssl_error(ERR_get_error()) @@ -93,12 +102,16 @@ shared_ptr MakeSSLContext(const String& pubkey, const String& privkey, } if (!SSL_CTX_check_private_key(sslContext.get())) { + msgbuf << "Error checking private key '" << privkey << "': " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), NULL) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("SSL_CTX_check_private_key") << errinfo_openssl_error(ERR_get_error())); } if (!SSL_CTX_load_verify_locations(sslContext.get(), cakey.CStr(), NULL)) { + msgbuf << "Error loading and verifying locations in ca key file '" << cakey << "': " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), NULL) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("SSL_CTX_load_verify_locations") << errinfo_openssl_error(ERR_get_error()) @@ -109,6 +122,8 @@ shared_ptr MakeSSLContext(const String& pubkey, const String& privkey, cert_names = SSL_load_client_CA_file(cakey.CStr()); if (cert_names == NULL) { + msgbuf << "Error loading client ca key file '" << cakey << "': " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), NULL) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("SSL_load_client_CA_file") << errinfo_openssl_error(ERR_get_error()) @@ -128,18 +143,23 @@ shared_ptr MakeSSLContext(const String& pubkey, const String& privkey, */ void AddCRLToSSLContext(const shared_ptr& context, const String& crlPath) { + std::ostringstream msgbuf; X509_STORE *x509_store = SSL_CTX_get_cert_store(context.get()); X509_LOOKUP *lookup; lookup = X509_STORE_add_lookup(x509_store, X509_LOOKUP_file()); if (!lookup) { + msgbuf << "Error adding X509 store lookup: " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), NULL) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("X509_STORE_add_lookup") << errinfo_openssl_error(ERR_get_error())); } if (X509_LOOKUP_load_file(lookup, crlPath.CStr(), X509_FILETYPE_PEM) != 0) { + msgbuf << "Error loading crl file '" << crlPath << "': " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), NULL) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("X509_LOOKUP_load_file") << errinfo_openssl_error(ERR_get_error()) @@ -160,12 +180,15 @@ void AddCRLToSSLContext(const shared_ptr& context, const String& crlPat */ String GetCertificateCN(const shared_ptr& certificate) { + std::ostringstream msgbuf; char buffer[256]; int rc = X509_NAME_get_text_by_NID(X509_get_subject_name(certificate.get()), NID_commonName, buffer, sizeof(buffer)); if (rc == -1) { + msgbuf << "Error with x509 NAME getting text by NID: " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), NULL) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("X509_NAME_get_text_by_NID") << errinfo_openssl_error(ERR_get_error())); @@ -182,16 +205,21 @@ String GetCertificateCN(const shared_ptr& certificate) */ shared_ptr GetX509Certificate(const String& pemfile) { + std::ostringstream msgbuf; X509 *cert; BIO *fpcert = BIO_new(BIO_s_file()); if (fpcert == NULL) { + msgbuf << "Error creating new BIO: " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), NULL) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("BIO_new") << errinfo_openssl_error(ERR_get_error())); } if (BIO_read_filename(fpcert, pemfile.CStr()) < 0) { + msgbuf << "Error reading pem file '" << pemfile << "': " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), NULL) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("BIO_read_filename") << errinfo_openssl_error(ERR_get_error()) @@ -200,6 +228,8 @@ shared_ptr GetX509Certificate(const String& pemfile) cert = PEM_read_bio_X509_AUX(fpcert, NULL, NULL, NULL); if (cert == NULL) { + msgbuf << "Error on bio X509 AUX reading pem file '" << pemfile << "': " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), NULL) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("PEM_read_bio_X509_AUX") << errinfo_openssl_error(ERR_get_error()) @@ -213,8 +243,8 @@ shared_ptr GetX509Certificate(const String& pemfile) int MakeX509CSR(const char *cn, const char *keyfile, const char *csrfile) { - InitializeOpenSSL(); - + InitializeOpenSSL(); + RSA *rsa = RSA_generate_key(4096, RSA_F4, NULL, NULL); BIO *bio = BIO_new(BIO_s_file()); @@ -251,22 +281,29 @@ int MakeX509CSR(const char *cn, const char *keyfile, const char *csrfile) String SHA256(const String& s) { + std::ostringstream msgbuf; SHA256_CTX context; unsigned char digest[SHA256_DIGEST_LENGTH]; if (!SHA256_Init(&context)) { + msgbuf << "Error on SHA256 Init: " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), NULL) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("SHA256_Init") << errinfo_openssl_error(ERR_get_error())); } if (!SHA256_Update(&context, (unsigned char*)s.CStr(), s.GetLength())) { + msgbuf << "Error on SHA256 Update: " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), NULL) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("SHA256_Update") << errinfo_openssl_error(ERR_get_error())); } if (!SHA256_Final(digest, &context)) { + msgbuf << "Error on SHA256 Final: " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), NULL) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("SHA256_Final") << errinfo_openssl_error(ERR_get_error()));