From 19ecb241f5d42956c3cf5565d1eebd25a34a1635 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 22 Mar 2022 10:58:46 +0100 Subject: [PATCH 1/7] Let new cluster certificates expire after 397 days, not 15 years https://cabforum.org/wp-content/uploads/CA-Browser-Forum-BR-1.7.3.pdf, section 6.3.2: "Subscriber Certificates issued on or after 1 September 2020 SHOULD NOT have a Validity Period greater than 397 days and MUST NOT have a Validity Period greater than 398 days." --- lib/base/tlsutility.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/base/tlsutility.cpp b/lib/base/tlsutility.cpp index c9cecb07b..32c141e7e 100644 --- a/lib/base/tlsutility.cpp +++ b/lib/base/tlsutility.cpp @@ -541,7 +541,7 @@ std::shared_ptr CreateCert(EVP_PKEY *pubkey, X509_NAME *subject, X509_NAME X509 *cert = X509_new(); X509_set_version(cert, 2); X509_gmtime_adj(X509_get_notBefore(cert), 0); - X509_gmtime_adj(X509_get_notAfter(cert), 365 * 24 * 60 * 60 * 15); + X509_gmtime_adj(X509_get_notAfter(cert), (ca ? 15 * 365 : 397) * 24 * 60 * 60); X509_set_pubkey(cert, pubkey); X509_set_subject_name(cert, subject); From 01422dfdf772322b2aae3c82bf7badb51eaad6da Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 22 Mar 2022 13:59:48 +0100 Subject: [PATCH 2/7] Request certificate renewal also master2->master1 not only sat->master to prevent master2's certificate from expiring. --- lib/remote/apilistener.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/remote/apilistener.cpp b/lib/remote/apilistener.cpp index e798390cd..2bac6f11d 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -756,8 +756,9 @@ void ApiListener::SyncClient(const JsonRpcConnection::Ptr& aclient, const Endpoi } Zone::Ptr myZone = Zone::GetLocalZone(); + auto parent (myZone->GetParent()); - if (myZone->GetParent() == eZone) { + if (parent == eZone || !parent && eZone == myZone) { Log(LogInformation, "ApiListener") << "Requesting new certificate for this Icinga instance from endpoint '" << endpoint->GetName() << "'."; From 913373fc382352e0ec51cdc3db83956fcc1a45d1 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 29 Mar 2022 15:47:16 +0200 Subject: [PATCH 3/7] Introduce IsCertUptodate() --- lib/base/tlsutility.cpp | 14 ++++++++++++++ lib/base/tlsutility.hpp | 1 + lib/remote/jsonrpcconnection-pki.cpp | 11 +---------- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/lib/base/tlsutility.cpp b/lib/base/tlsutility.cpp index 32c141e7e..66882e1da 100644 --- a/lib/base/tlsutility.cpp +++ b/lib/base/tlsutility.cpp @@ -670,6 +670,20 @@ std::shared_ptr CreateCertIcingaCA(const std::shared_ptr& cert) return CreateCertIcingaCA(pkey.get(), X509_get_subject_name(cert.get())); } +bool IsCertUptodate(const std::shared_ptr& cert) +{ + time_t now; + time(&now); + + /* auto-renew all certificates which were created before 2017 to force an update of the CA, + * because Icinga versions older than 2.4 sometimes create certificates with an invalid + * serial number. */ + time_t forceRenewalEnd = 1483228800; /* January 1st, 2017 */ + time_t renewalStart = now + 30 * 24 * 60 * 60; + + return X509_cmp_time(X509_get_notBefore(cert.get()), &forceRenewalEnd) != -1 && X509_cmp_time(X509_get_notAfter(cert.get()), &renewalStart) != -1; +} + String CertificateToString(const std::shared_ptr& cert) { BIO *mem = BIO_new(BIO_s_mem()); diff --git a/lib/base/tlsutility.hpp b/lib/base/tlsutility.hpp index 73d032ed9..54bf702ef 100644 --- a/lib/base/tlsutility.hpp +++ b/lib/base/tlsutility.hpp @@ -45,6 +45,7 @@ String CertificateToString(const std::shared_ptr& cert); std::shared_ptr StringToCertificate(const String& cert); std::shared_ptr CreateCertIcingaCA(EVP_PKEY *pubkey, X509_NAME *subject); std::shared_ptr CreateCertIcingaCA(const std::shared_ptr& cert); +bool IsCertUptodate(const std::shared_ptr& cert); String PBKDF2_SHA1(const String& password, const String& salt, int iterations); String PBKDF2_SHA256(const String& password, const String& salt, int iterations); diff --git a/lib/remote/jsonrpcconnection-pki.cpp b/lib/remote/jsonrpcconnection-pki.cpp index baa115d69..c38c96d0b 100644 --- a/lib/remote/jsonrpcconnection-pki.cpp +++ b/lib/remote/jsonrpcconnection-pki.cpp @@ -76,16 +76,7 @@ Value RequestCertificateHandler(const MessageOrigin::Ptr& origin, const Dictiona } if (signedByCA) { - time_t now; - time(&now); - - /* auto-renew all certificates which were created before 2017 to force an update of the CA, - * because Icinga versions older than 2.4 sometimes create certificates with an invalid - * serial number. */ - time_t forceRenewalEnd = 1483228800; /* January 1st, 2017 */ - time_t renewalStart = now + 30 * 24 * 60 * 60; - - if (X509_cmp_time(X509_get_notBefore(cert.get()), &forceRenewalEnd) != -1 && X509_cmp_time(X509_get_notAfter(cert.get()), &renewalStart) != -1) { + if (IsCertUptodate(cert)) { Log(LogInformation, "JsonRpcConnection") << "The certificate for CN '" << cn << "' is valid and uptodate. Skipping automated renewal."; From 1492bffcccea7b527c61bf8606eaebf5a32eebe9 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 29 Mar 2022 16:36:13 +0200 Subject: [PATCH 4/7] Introduce ApiListener#RenewCert() --- lib/remote/apilistener.cpp | 24 ++++++++++++++++++++++++ lib/remote/apilistener.hpp | 1 + lib/remote/jsonrpcconnection-pki.cpp | 22 ++++------------------ 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/lib/remote/apilistener.cpp b/lib/remote/apilistener.cpp index 2bac6f11d..ef7af9842 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -177,6 +177,30 @@ void ApiListener::OnConfigLoaded() UpdateSSLContext(); } +std::shared_ptr ApiListener::RenewCert(const std::shared_ptr& cert) +{ + std::shared_ptr pubkey (X509_get_pubkey(cert.get()), EVP_PKEY_free); + auto subject (X509_get_subject_name(cert.get())); + auto cacert (GetX509Certificate(GetDefaultCaPath())); + auto newcert (CreateCertIcingaCA(pubkey.get(), subject)); + + /* verify that the new cert matches the CA we're using for the ApiListener; + * this ensures that the CA we have in /var/lib/icinga2/ca matches the one + * we're using for cluster connections (there's no point in sending a client + * a certificate it wouldn't be able to use to connect to us anyway) */ + try { + if (!VerifyCertificate(cacert, newcert, GetCrlPath())) { + Log(LogWarning, "ApiListener") + << "The CA in '" << GetDefaultCaPath() << "' does not match the CA which Icinga uses " + << "for its own cluster connections. This is most likely a configuration problem."; + + return nullptr; + } + } catch (const std::exception&) { } /* Swallow the exception on purpose, cacert will never be a non-CA certificate. */ + + return newcert; +} + void ApiListener::UpdateSSLContext() { namespace ssl = boost::asio::ssl; diff --git a/lib/remote/apilistener.hpp b/lib/remote/apilistener.hpp index c808581e3..93de7ed14 100644 --- a/lib/remote/apilistener.hpp +++ b/lib/remote/apilistener.hpp @@ -59,6 +59,7 @@ public: static String GetCaDir(); static String GetCertificateRequestsDir(); + std::shared_ptr RenewCert(const std::shared_ptr& cert); void UpdateSSLContext(); static ApiListener::Ptr GetInstance(); diff --git a/lib/remote/jsonrpcconnection-pki.cpp b/lib/remote/jsonrpcconnection-pki.cpp index c38c96d0b..7b7c63f3b 100644 --- a/lib/remote/jsonrpcconnection-pki.cpp +++ b/lib/remote/jsonrpcconnection-pki.cpp @@ -145,8 +145,6 @@ Value RequestCertificateHandler(const MessageOrigin::Ptr& origin, const Dictiona } std::shared_ptr newcert; - std::shared_ptr pubkey; - X509_NAME *subject; Dictionary::Ptr message; String ticket; @@ -197,23 +195,11 @@ Value RequestCertificateHandler(const MessageOrigin::Ptr& origin, const Dictiona } } - pubkey = std::shared_ptr(X509_get_pubkey(cert.get()), EVP_PKEY_free); - subject = X509_get_subject_name(cert.get()); + newcert = listener->RenewCert(cert); - newcert = CreateCertIcingaCA(pubkey.get(), subject); - - /* verify that the new cert matches the CA we're using for the ApiListener; - * this ensures that the CA we have in /var/lib/icinga2/ca matches the one - * we're using for cluster connections (there's no point in sending a client - * a certificate it wouldn't be able to use to connect to us anyway) */ - try { - if (!VerifyCertificate(cacert, newcert, listener->GetCrlPath())) { - Log(LogWarning, "JsonRpcConnection") - << "The CA in '" << listener->GetDefaultCaPath() << "' does not match the CA which Icinga uses " - << "for its own cluster connections. This is most likely a configuration problem."; - goto delayed_request; - } - } catch (const std::exception&) { } /* Swallow the exception on purpose, cacert will never be a non-CA certificate. */ + if (!newcert) { + goto delayed_request; + } /* Send the signed certificate update. */ Log(LogInformation, "JsonRpcConnection") From ff6219597a4cef54e4b901b60e6dc0a7c20e0a20 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 29 Mar 2022 16:45:18 +0200 Subject: [PATCH 5/7] ApiListener#Start(): auto-renew own cert if CA owner otherwise that particular cert would expire. --- lib/remote/apilistener.cpp | 34 ++++++++++++++++++++++++++++++++++ lib/remote/apilistener.hpp | 1 + 2 files changed, 35 insertions(+) diff --git a/lib/remote/apilistener.cpp b/lib/remote/apilistener.cpp index ef7af9842..2f3ce4139 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -270,6 +270,7 @@ void ApiListener::Start(bool runtimeCreated) << "'" << GetName() << "' started."; SyncLocalZoneDirs(); + RenewOwnCert(); ObjectImpl::Start(runtimeCreated); @@ -320,6 +321,39 @@ void ApiListener::Start(bool runtimeCreated) OnMasterChanged(true); } +void ApiListener::RenewOwnCert() +{ + if (!Utility::PathExists(GetIcingaCADir() + "/ca.key")) { + return; + } + + auto certPath (GetDefaultCertPath()); + auto cert (GetX509Certificate(certPath)); + + if (IsCertUptodate(cert)) { + return; + } + + Log(LogInformation, "ApiListener") + << "Our certificate will expire soon, but we own the CA. Renewing."; + + cert = RenewCert(cert); + + if (!cert) { + return; + } + + std::fstream certfp; + auto tempCertPath (Utility::CreateTempFile(certPath + ".XXXXXX", 0644, certfp)); + + certfp.exceptions(std::ofstream::failbit | std::ofstream::badbit); + certfp << CertificateToString(cert); + certfp.close(); + + Utility::RenameFile(tempCertPath, certPath); + UpdateSSLContext(); +} + void ApiListener::Stop(bool runtimeDeleted) { ObjectImpl::Stop(runtimeDeleted); diff --git a/lib/remote/apilistener.hpp b/lib/remote/apilistener.hpp index 93de7ed14..2c99ac35f 100644 --- a/lib/remote/apilistener.hpp +++ b/lib/remote/apilistener.hpp @@ -192,6 +192,7 @@ private: void SyncLocalZoneDirs() const; void SyncLocalZoneDir(const Zone::Ptr& zone) const; + void RenewOwnCert(); void SendConfigUpdate(const JsonRpcConnection::Ptr& aclient); From 97dce396990b3a009224a609a10aa79d80114497 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 30 Mar 2022 18:38:57 +0200 Subject: [PATCH 6/7] Renew certificates also periodically --- lib/base/tlsutility.cpp | 4 ++-- lib/base/tlsutility.hpp | 5 +++++ lib/remote/apilistener.cpp | 22 ++++++++++++++-------- lib/remote/apilistener.hpp | 1 + lib/remote/jsonrpcconnection-pki.cpp | 11 +++++++++++ 5 files changed, 33 insertions(+), 10 deletions(-) diff --git a/lib/base/tlsutility.cpp b/lib/base/tlsutility.cpp index 66882e1da..803eecb7e 100644 --- a/lib/base/tlsutility.cpp +++ b/lib/base/tlsutility.cpp @@ -541,7 +541,7 @@ std::shared_ptr CreateCert(EVP_PKEY *pubkey, X509_NAME *subject, X509_NAME X509 *cert = X509_new(); X509_set_version(cert, 2); X509_gmtime_adj(X509_get_notBefore(cert), 0); - X509_gmtime_adj(X509_get_notAfter(cert), (ca ? 15 * 365 : 397) * 24 * 60 * 60); + X509_gmtime_adj(X509_get_notAfter(cert), ca ? ROOT_VALID_FOR : LEAF_VALID_FOR); X509_set_pubkey(cert, pubkey); X509_set_subject_name(cert, subject); @@ -679,7 +679,7 @@ bool IsCertUptodate(const std::shared_ptr& cert) * because Icinga versions older than 2.4 sometimes create certificates with an invalid * serial number. */ time_t forceRenewalEnd = 1483228800; /* January 1st, 2017 */ - time_t renewalStart = now + 30 * 24 * 60 * 60; + time_t renewalStart = now + RENEW_THRESHOLD; return X509_cmp_time(X509_get_notBefore(cert.get()), &forceRenewalEnd) != -1 && X509_cmp_time(X509_get_notAfter(cert.get()), &renewalStart) != -1; } diff --git a/lib/base/tlsutility.hpp b/lib/base/tlsutility.hpp index 54bf702ef..b0ab17c44 100644 --- a/lib/base/tlsutility.hpp +++ b/lib/base/tlsutility.hpp @@ -24,6 +24,11 @@ namespace icinga { +const auto ROOT_VALID_FOR = 60 * 60 * 24 * 365 * 15; +const auto LEAF_VALID_FOR = 60 * 60 * 24 * 397; +const auto RENEW_THRESHOLD = 60 * 60 * 24 * 30; +const auto RENEW_INTERVAL = 60 * 60 * 24; + void InitializeOpenSSL(); String GetOpenSSLVersion(); diff --git a/lib/remote/apilistener.cpp b/lib/remote/apilistener.cpp index 2f3ce4139..cb9798876 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -270,7 +270,20 @@ void ApiListener::Start(bool runtimeCreated) << "'" << GetName() << "' started."; SyncLocalZoneDirs(); - RenewOwnCert(); + + m_RenewOwnCertTimer = new Timer(); + + if (Utility::PathExists(GetIcingaCADir() + "/ca.key")) { + RenewOwnCert(); + m_RenewOwnCertTimer->OnTimerExpired.connect([this](const Timer * const&) { RenewOwnCert(); }); + } else { + m_RenewOwnCertTimer->OnTimerExpired.connect([this](const Timer * const&) { + JsonRpcConnection::SendCertificateRequest(nullptr, nullptr, String()); + }); + } + + m_RenewOwnCertTimer->SetInterval(RENEW_INTERVAL); + m_RenewOwnCertTimer->Start(); ObjectImpl::Start(runtimeCreated); @@ -323,10 +336,6 @@ void ApiListener::Start(bool runtimeCreated) void ApiListener::RenewOwnCert() { - if (!Utility::PathExists(GetIcingaCADir() + "/ca.key")) { - return; - } - auto certPath (GetDefaultCertPath()); auto cert (GetX509Certificate(certPath)); @@ -817,9 +826,6 @@ void ApiListener::SyncClient(const JsonRpcConnection::Ptr& aclient, const Endpoi auto parent (myZone->GetParent()); if (parent == eZone || !parent && eZone == myZone) { - Log(LogInformation, "ApiListener") - << "Requesting new certificate for this Icinga instance from endpoint '" << endpoint->GetName() << "'."; - JsonRpcConnection::SendCertificateRequest(aclient, nullptr, String()); if (Utility::PathExists(ApiListener::GetCertificateRequestsDir())) diff --git a/lib/remote/apilistener.hpp b/lib/remote/apilistener.hpp index 2c99ac35f..f08e643ab 100644 --- a/lib/remote/apilistener.hpp +++ b/lib/remote/apilistener.hpp @@ -141,6 +141,7 @@ private: Timer::Ptr m_AuthorityTimer; Timer::Ptr m_CleanupCertificateRequestsTimer; Timer::Ptr m_ApiPackageIntegrityTimer; + Timer::Ptr m_RenewOwnCertTimer; Endpoint::Ptr m_LocalEndpoint; diff --git a/lib/remote/jsonrpcconnection-pki.cpp b/lib/remote/jsonrpcconnection-pki.cpp index 7b7c63f3b..8ab58f416 100644 --- a/lib/remote/jsonrpcconnection-pki.cpp +++ b/lib/remote/jsonrpcconnection-pki.cpp @@ -265,6 +265,17 @@ void JsonRpcConnection::SendCertificateRequest(const JsonRpcConnection::Ptr& acl /* Path is empty if this is our own request. */ if (path.IsEmpty()) { + { + Log msg (LogInformation, "JsonRpcConnection"); + msg << "Requesting new certificate for this Icinga instance"; + + if (aclient) { + msg << " from endpoint '" << aclient->GetIdentity() << "'"; + } + + msg << "."; + } + String ticketPath = ApiListener::GetCertsDir() + "/ticket"; std::ifstream fp(ticketPath.CStr()); From a2817aefc7ba97623c29d37372984d4812f01274 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Mon, 4 Apr 2022 13:26:55 +0200 Subject: [PATCH 7/7] Protect ApiListener#m_SSLContext with a mutex --- lib/remote/apilistener.cpp | 18 ++++++++++++++---- lib/remote/apilistener.hpp | 4 +++- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/lib/remote/apilistener.cpp b/lib/remote/apilistener.cpp index cb9798876..618591358 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -240,7 +241,11 @@ void ApiListener::UpdateSSLContext() } } - m_SSLContext = context; + { + boost::unique_lock lock (m_SSLContextMutex); + + m_SSLContext = context; + } for (const Endpoint::Ptr& endpoint : ConfigType::GetObjectsByType()) { for (const JsonRpcConnection::Ptr& client : endpoint->GetClients()) { @@ -484,14 +489,14 @@ bool ApiListener::AddListener(const String& node, const String& service) Log(LogInformation, "ApiListener") << "Started new listener on '[" << localEndpoint.address() << "]:" << localEndpoint.port() << "'"; - IoEngine::SpawnCoroutine(io, [this, acceptor](asio::yield_context yc) { ListenerCoroutineProc(yc, acceptor, m_SSLContext); }); + IoEngine::SpawnCoroutine(io, [this, acceptor](asio::yield_context yc) { ListenerCoroutineProc(yc, acceptor); }); UpdateStatusFile(localEndpoint); return true; } -void ApiListener::ListenerCoroutineProc(boost::asio::yield_context yc, const Shared::Ptr& server, const Shared::Ptr& sslContext) +void ApiListener::ListenerCoroutineProc(boost::asio::yield_context yc, const Shared::Ptr& server) { namespace asio = boost::asio; @@ -499,8 +504,10 @@ void ApiListener::ListenerCoroutineProc(boost::asio::yield_context yc, const Sha for (;;) { try { - auto sslConn (Shared::Make(io, *sslContext)); + boost::shared_lock lock (m_SSLContextMutex); + auto sslConn (Shared::Make(io, *m_SSLContext)); + lock.unlock(); server->async_accept(sslConn->lowest_layer(), yc); auto strand (Shared::Make(io)); @@ -553,8 +560,11 @@ void ApiListener::AddConnection(const Endpoint::Ptr& endpoint) << "Reconnecting to endpoint '" << endpoint->GetName() << "' via host '" << host << "' and port '" << port << "'"; try { + boost::shared_lock lock (m_SSLContextMutex); auto sslConn (Shared::Make(io, *m_SSLContext, endpoint->GetName())); + lock.unlock(); + Timeout::Ptr timeout(new Timeout(strand->context(), *strand, boost::posix_time::microseconds(int64_t(GetConnectTimeout() * 1e6)), [sslConn, endpoint, host, port](asio::yield_context yc) { Log(LogCritical, "ApiListener") diff --git a/lib/remote/apilistener.hpp b/lib/remote/apilistener.hpp index f08e643ab..939ccf6a7 100644 --- a/lib/remote/apilistener.hpp +++ b/lib/remote/apilistener.hpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -130,6 +131,7 @@ protected: private: Shared::Ptr m_SSLContext; + boost::shared_mutex m_SSLContextMutex; mutable boost::mutex m_AnonymousClientsLock; mutable boost::mutex m_HttpClientsLock; @@ -164,7 +166,7 @@ private: boost::asio::yield_context yc, const Shared::Ptr& strand, const Shared::Ptr& client, const String& hostname, ConnectionRole role ); - void ListenerCoroutineProc(boost::asio::yield_context yc, const Shared::Ptr& server, const Shared::Ptr& sslContext); + void ListenerCoroutineProc(boost::asio::yield_context yc, const Shared::Ptr& server); WorkQueue m_RelayQueue; WorkQueue m_SyncQueue{0, 4};