From ad218c9a123519f363e2e7aa6f2c98bec7ff6f97 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Wed, 8 Jun 2022 17:14:09 +0200 Subject: [PATCH] Icinga DB: initialize environment ID during config validation IcingaDB may receive callbacks from Boost signals before being fully started. This resulted in situations where m_EnvironmentId was used before it was initialized properly. This is fixed by initializing it earlier (during the config validation stage). However, at this stage, it should not yet write to disk, therefore, persisting the environment ID to disk is delayed until later in the startup process. Initializing at this stage has an extra benefit: if there is an error for some reason (possibly corrupt icingadb.env file), this now shows up as a nice error during config validation. Additionally, this replaces the use of std::call_once with std::mutex due to bug in libstdc++ (see inline comment for reference). --- lib/icingadb/icingadb.cpp | 103 +++++++++++++++++++++++++------------- lib/icingadb/icingadb.hpp | 8 ++- 2 files changed, 76 insertions(+), 35 deletions(-) diff --git a/lib/icingadb/icingadb.cpp b/lib/icingadb/icingadb.cpp index 1adcaee84..21702f3a8 100644 --- a/lib/icingadb/icingadb.cpp +++ b/lib/icingadb/icingadb.cpp @@ -25,7 +25,7 @@ using namespace icinga; using Prio = RedisConnection::QueryPriority; String IcingaDB::m_EnvironmentId; -std::once_flag IcingaDB::m_EnvironmentIdOnce; +std::mutex IcingaDB::m_EnvironmentIdInitMutex; REGISTER_TYPE(IcingaDB); @@ -74,6 +74,13 @@ void IcingaDB::Validate(int types, const ValidationUtils& utils) if (GetEnableTls() && GetCertPath().IsEmpty() != GetKeyPath().IsEmpty()) { BOOST_THROW_EXCEPTION(ValidationError(this, std::vector(), "Validation failed: Either both a client certificate (cert_path) and its private key (key_path) or none of them must be given.")); } + + try { + InitEnvironmentId(); + } catch (const std::exception& e) { + BOOST_THROW_EXCEPTION(ValidationError(this, std::vector(), + String("Validation failed: ") + e.what())); + } } /** @@ -83,39 +90,8 @@ void IcingaDB::Start(bool runtimeCreated) { ObjectImpl::Start(runtimeCreated); - std::call_once(m_EnvironmentIdOnce, []() { - String path = Configuration::DataDir + "/icingadb.env"; - - if (Utility::PathExists(path)) { - m_EnvironmentId = Utility::LoadJsonFile(path); - - if (m_EnvironmentId.GetLength() != 2*SHA_DIGEST_LENGTH) { - throw std::runtime_error("Wrong length of stored Icinga DB environment"); - } - - for (unsigned char c : m_EnvironmentId) { - if (!std::isxdigit(c)) { - throw std::runtime_error("Stored Icinga DB environment is not a hex string"); - } - } - } else { - std::shared_ptr cert = GetX509Certificate(ApiListener::GetDefaultCaPath()); - - unsigned int n; - unsigned char digest[EVP_MAX_MD_SIZE]; - if (X509_pubkey_digest(cert.get(), EVP_sha1(), digest, &n) != 1) { - BOOST_THROW_EXCEPTION(openssl_error() - << boost::errinfo_api_function("X509_pubkey_digest") - << errinfo_openssl_error(ERR_peek_error())); - } - - m_EnvironmentId = BinaryToHex(digest, n); - - Utility::SaveJsonFile(path, 0600, m_EnvironmentId); - } - - m_EnvironmentId = m_EnvironmentId.ToLower(); - }); + VERIFY(!m_EnvironmentId.IsEmpty()); + PersistEnvironmentId(); Log(LogInformation, "IcingaDB") << "'" << GetName() << "' started."; @@ -288,3 +264,62 @@ bool IcingaDB::DumpedGlobals::IsNew(const String& id) std::lock_guard l (m_Mutex); return m_Ids.emplace(id).second; } + +/** + * Initializes the m_EnvironmentId attribute or throws an exception on failure to do so. Can be called concurrently. + */ +void IcingaDB::InitEnvironmentId() +{ + // Initialize m_EnvironmentId once across all IcingaDB objects. In theory, this could be done using + // std::call_once, however, due to a bug in libstdc++ (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66146), + // this can result in a deadlock when an exception is thrown (which is explicitly allowed by the standard). + std::unique_lock lock (m_EnvironmentIdInitMutex); + + if (m_EnvironmentId.IsEmpty()) { + String path = Configuration::DataDir + "/icingadb.env"; + String envId; + + if (Utility::PathExists(path)) { + envId = Utility::LoadJsonFile(path); + + if (envId.GetLength() != 2*SHA_DIGEST_LENGTH) { + throw std::runtime_error("environment ID stored at " + path + " is corrupt: wrong length."); + } + + for (unsigned char c : envId) { + if (!std::isxdigit(c)) { + throw std::runtime_error("environment ID stored at " + path + " is corrupt: invalid hex string."); + } + } + } else { + std::shared_ptr cert = GetX509Certificate(ApiListener::GetDefaultCaPath()); + + unsigned int n; + unsigned char digest[EVP_MAX_MD_SIZE]; + if (X509_pubkey_digest(cert.get(), EVP_sha1(), digest, &n) != 1) { + BOOST_THROW_EXCEPTION(openssl_error() + << boost::errinfo_api_function("X509_pubkey_digest") + << errinfo_openssl_error(ERR_peek_error())); + } + + envId = BinaryToHex(digest, n); + } + + m_EnvironmentId = envId.ToLower(); + } +} + +/** + * Ensures that the environment ID is persisted on disk or throws an exception on failure to do so. + * Can be called concurrently. + */ +void IcingaDB::PersistEnvironmentId() +{ + String path = Configuration::DataDir + "/icingadb.env"; + + std::unique_lock lock (m_EnvironmentIdInitMutex); + + if (!Utility::PathExists(path)) { + Utility::SaveJsonFile(path, 0600, m_EnvironmentId); + } +} diff --git a/lib/icingadb/icingadb.hpp b/lib/icingadb/icingadb.hpp index 785cd01ba..ef4483a7a 100644 --- a/lib/icingadb/icingadb.hpp +++ b/lib/icingadb/icingadb.hpp @@ -179,6 +179,9 @@ private: static std::vector GetTypes(); + static void InitEnvironmentId(); + static void PersistEnvironmentId(); + Timer::Ptr m_StatsTimer; WorkQueue m_WorkQueue{0, 1, LogNotice}; @@ -199,8 +202,11 @@ private: DumpedGlobals CustomVar, ActionUrl, NotesUrl, IconImage; } m_DumpedGlobals; + // m_EnvironmentId is shared across all IcingaDB objects (typically there is at most one, but it is perfectly fine + // to have multiple ones). It is initialized once (synchronized using m_EnvironmentIdInitMutex). After successful + // initialization, the value is read-only and can be accessed without further synchronization. static String m_EnvironmentId; - static std::once_flag m_EnvironmentIdOnce; + static std::mutex m_EnvironmentIdInitMutex; }; }