From e06b631f3a14f596b82a5c54ea7a0bd0af141519 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 7032c7a3a..e36cd95d0 100644 --- a/lib/base/tlsutility.cpp +++ b/lib/base/tlsutility.cpp @@ -623,7 +623,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 5f2e02139037a2f9ccab93b25b6a430b6419aa7a 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 7550b11c3..7cafffc46 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -771,8 +771,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 9be2eb8e5ec41eb68bebce17b0ac376feb55f35d 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 e36cd95d0..df0f872b4 100644 --- a/lib/base/tlsutility.cpp +++ b/lib/base/tlsutility.cpp @@ -752,6 +752,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 c62f5cfad..6ed84add5 100644 --- a/lib/base/tlsutility.hpp +++ b/lib/base/tlsutility.hpp @@ -55,6 +55,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 d955becb5..8d533b591 100644 --- a/lib/remote/jsonrpcconnection-pki.cpp +++ b/lib/remote/jsonrpcconnection-pki.cpp @@ -77,16 +77,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 6d470a3ca5b44596a5d2ff19b0391039afdf1e0c 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 7cafffc46..c9ff82072 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -179,6 +179,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() { m_SSLContext = SetupSslContext(GetDefaultCertPath(), GetDefaultKeyPath(), GetDefaultCaPath(), GetCrlPath(), GetCipherList(), GetTlsProtocolmin(), GetDebugInfo()); diff --git a/lib/remote/apilistener.hpp b/lib/remote/apilistener.hpp index 21c9eb9fb..e52a3ac5b 100644 --- a/lib/remote/apilistener.hpp +++ b/lib/remote/apilistener.hpp @@ -89,6 +89,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 8d533b591..8b3385c2a 100644 --- a/lib/remote/jsonrpcconnection-pki.cpp +++ b/lib/remote/jsonrpcconnection-pki.cpp @@ -146,8 +146,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; @@ -198,23 +196,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 3753f86c80e277c9764db448e5fbc2c0b3cc99c3 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 c9ff82072..a17d0fb17 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -235,6 +235,7 @@ void ApiListener::Start(bool runtimeCreated) << "'" << GetName() << "' started."; SyncLocalZoneDirs(); + RenewOwnCert(); ObjectImpl::Start(runtimeCreated); @@ -285,6 +286,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 e52a3ac5b..960f75431 100644 --- a/lib/remote/apilistener.hpp +++ b/lib/remote/apilistener.hpp @@ -222,6 +222,7 @@ private: void SyncLocalZoneDirs() const; void SyncLocalZoneDir(const Zone::Ptr& zone) const; + void RenewOwnCert(); void SendConfigUpdate(const JsonRpcConnection::Ptr& aclient); From e490883577f88176e479990141538e3b856d640c 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 df0f872b4..e022621e4 100644 --- a/lib/base/tlsutility.cpp +++ b/lib/base/tlsutility.cpp @@ -623,7 +623,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); @@ -761,7 +761,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 6ed84add5..3b8a89c5e 100644 --- a/lib/base/tlsutility.hpp +++ b/lib/base/tlsutility.hpp @@ -30,6 +30,11 @@ const char * const DEFAULT_TLS_CIPHERS = "ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RS const char * const DEFAULT_TLS_PROTOCOLMIN = "TLSv1.2"; const unsigned int DEFAULT_CONNECT_TIMEOUT = 15; +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 a17d0fb17..794b6e18d 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -235,7 +235,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); @@ -288,10 +301,6 @@ void ApiListener::Start(bool runtimeCreated) void ApiListener::RenewOwnCert() { - if (!Utility::PathExists(GetIcingaCADir() + "/ca.key")) { - return; - } - auto certPath (GetDefaultCertPath()); auto cert (GetX509Certificate(certPath)); @@ -832,9 +841,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 960f75431..d5e6ffe8c 100644 --- a/lib/remote/apilistener.hpp +++ b/lib/remote/apilistener.hpp @@ -171,6 +171,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 8b3385c2a..2a2cd26c3 100644 --- a/lib/remote/jsonrpcconnection-pki.cpp +++ b/lib/remote/jsonrpcconnection-pki.cpp @@ -266,6 +266,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 c9e4c016e058bb33e315d383ecbf52545a1e0a3c 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 | 21 +++++++++++++++++---- lib/remote/apilistener.hpp | 4 +++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/remote/apilistener.cpp b/lib/remote/apilistener.cpp index 794b6e18d..2e1bb3a0e 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -205,7 +206,13 @@ std::shared_ptr ApiListener::RenewCert(const std::shared_ptr& cert) void ApiListener::UpdateSSLContext() { - m_SSLContext = SetupSslContext(GetDefaultCertPath(), GetDefaultKeyPath(), GetDefaultCaPath(), GetCrlPath(), GetCipherList(), GetTlsProtocolmin(), GetDebugInfo()); + auto ctx (SetupSslContext(GetDefaultCertPath(), GetDefaultKeyPath(), GetDefaultCaPath(), GetCrlPath(), GetCipherList(), GetTlsProtocolmin(), GetDebugInfo())); + + { + boost::unique_lock lock (m_SSLContextMutex); + + m_SSLContext = std::move(ctx); + } for (const Endpoint::Ptr& endpoint : ConfigType::GetObjectsByType()) { for (const JsonRpcConnection::Ptr& client : endpoint->GetClients()) { @@ -449,14 +456,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; @@ -485,7 +492,10 @@ void ApiListener::ListenerCoroutineProc(boost::asio::yield_context yc, const Sha } } - auto sslConn (Shared::Make(io, *sslContext)); + boost::shared_lock lock (m_SSLContextMutex); + auto sslConn (Shared::Make(io, *m_SSLContext)); + + lock.unlock(); sslConn->lowest_layer() = std::move(socket); auto strand (Shared::Make(io)); @@ -538,8 +548,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 d5e6ffe8c..1be29522f 100644 --- a/lib/remote/apilistener.hpp +++ b/lib/remote/apilistener.hpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -160,6 +161,7 @@ protected: private: Shared::Ptr m_SSLContext; + boost::shared_mutex m_SSLContextMutex; mutable std::mutex m_AnonymousClientsLock; mutable std::mutex m_HttpClientsLock; @@ -194,7 +196,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};