Merge pull request #9338 from Icinga/Al2Klimov-patch-3-212

Let new cluster certificates expire after 397 days, not 15 years

Author: julianbrost

lib/base/tlsutility.cpp

@@ -541,7 +541,7 @@ std::shared_ptr<X509> 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 ? ROOT_VALID_FOR : LEAF_VALID_FOR);
 	X509_set_pubkey(cert, pubkey);
 
 	X509_set_subject_name(cert, subject);
@@ -670,6 +670,20 @@ std::shared_ptr<X509> CreateCertIcingaCA(const std::shared_ptr<X509>& cert)
 	return CreateCertIcingaCA(pkey.get(), X509_get_subject_name(cert.get()));
 }
 
+bool IsCertUptodate(const std::shared_ptr<X509>& 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 + RENEW_THRESHOLD;
+
+	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<X509>& cert)
 {
 	BIO *mem = BIO_new(BIO_s_mem());

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();
@@ -45,6 +50,7 @@ String CertificateToString(const std::shared_ptr<X509>& cert);
 std::shared_ptr<X509> StringToCertificate(const String& cert);
 std::shared_ptr<X509> CreateCertIcingaCA(EVP_PKEY *pubkey, X509_NAME *subject);
 std::shared_ptr<X509> CreateCertIcingaCA(const std::shared_ptr<X509>& cert);
+bool IsCertUptodate(const std::shared_ptr<X509>& cert);
 
 String PBKDF2_SHA1(const String& password, const String& salt, int iterations);
 String PBKDF2_SHA256(const String& password, const String& salt, int iterations);

lib/remote/apilistener.cpp

@@ -30,6 +30,7 @@
 #include <boost/asio/ssl/context.hpp>
 #include <boost/date_time/posix_time/posix_time_duration.hpp>
 #include <boost/system/error_code.hpp>
+#include <boost/thread/locks.hpp>
 #include <climits>
 #include <cstdint>
 #include <fstream>
@@ -177,6 +178,30 @@ void ApiListener::OnConfigLoaded()
 	UpdateSSLContext();
 }
 
+std::shared_ptr<X509> ApiListener::RenewCert(const std::shared_ptr<X509>& cert)
+{
+	std::shared_ptr<EVP_PKEY> 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;
@@ -216,7 +241,11 @@ void ApiListener::UpdateSSLContext()
 		}
 	}
 
-	m_SSLContext = context;
+	{
+		boost::unique_lock<decltype(m_SSLContextMutex)> lock (m_SSLContextMutex);
+
+		m_SSLContext = context;
+	}
 
 	for (const Endpoint::Ptr& endpoint : ConfigType::GetObjectsByType<Endpoint>()) {
 		for (const JsonRpcConnection::Ptr& client : endpoint->GetClients()) {
@@ -247,6 +276,20 @@ void ApiListener::Start(bool runtimeCreated)
 
 	SyncLocalZoneDirs();
 
+	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<ApiListener>::Start(runtimeCreated);
 
 	{
@@ -296,6 +339,35 @@ void ApiListener::Start(bool runtimeCreated)
 	OnMasterChanged(true);
 }
 
+void ApiListener::RenewOwnCert()
+{
+	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<ApiListener>::Stop(runtimeDeleted);
@@ -417,23 +489,25 @@ 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<boost::asio::ip::tcp::acceptor>::Ptr& server, const Shared<boost::asio::ssl::context>::Ptr& sslContext)
+void ApiListener::ListenerCoroutineProc(boost::asio::yield_context yc, const Shared<boost::asio::ip::tcp::acceptor>::Ptr& server)
 {
 	namespace asio = boost::asio;
 
 	auto& io (IoEngine::Get().GetIoContext());
 
 	for (;;) {
 		try {
-			auto sslConn (Shared<AsioTlsStream>::Make(io, *sslContext));
+			boost::shared_lock<decltype(m_SSLContextMutex)> lock (m_SSLContextMutex);
+			auto sslConn (Shared<AsioTlsStream>::Make(io, *m_SSLContext));
 
+			lock.unlock();
 			server->async_accept(sslConn->lowest_layer(), yc);
 
 			auto strand (Shared<asio::io_context::strand>::Make(io));
@@ -486,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<decltype(m_SSLContextMutex)> lock (m_SSLContextMutex);
 			auto sslConn (Shared<AsioTlsStream>::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")
@@ -756,11 +833,9 @@ void ApiListener::SyncClient(const JsonRpcConnection::Ptr& aclient, const Endpoi
 		}
 
 		Zone::Ptr myZone = Zone::GetLocalZone();
+		auto parent (myZone->GetParent());
 
-		if (myZone->GetParent() == eZone) {
-			Log(LogInformation, "ApiListener")
-				<< "Requesting new certificate for this Icinga instance from endpoint '" << endpoint->GetName() << "'.";
-
+		if (parent == eZone || !parent && eZone == myZone) {
 			JsonRpcConnection::SendCertificateRequest(aclient, nullptr, String());
 
 			if (Utility::PathExists(ApiListener::GetCertificateRequestsDir()))

lib/remote/apilistener.hpp

@@ -21,6 +21,7 @@
 #include <boost/asio/ip/tcp.hpp>
 #include <boost/asio/spawn.hpp>
 #include <boost/asio/ssl/context.hpp>
+#include <boost/thread/shared_mutex.hpp>
 #include <mutex>
 #include <set>
 
@@ -59,6 +60,7 @@ class ApiListener final : public ObjectImpl<ApiListener>
 	static String GetCaDir();
 	static String GetCertificateRequestsDir();
 
+	std::shared_ptr<X509> RenewCert(const std::shared_ptr<X509>& cert);
 	void UpdateSSLContext();
 
 	static ApiListener::Ptr GetInstance();
@@ -129,6 +131,7 @@ class ApiListener final : public ObjectImpl<ApiListener>
 
 private:
 	Shared<boost::asio::ssl::context>::Ptr m_SSLContext;
+	boost::shared_mutex m_SSLContextMutex;
 
 	mutable boost::mutex m_AnonymousClientsLock;
 	mutable boost::mutex m_HttpClientsLock;
@@ -140,6 +143,7 @@ class ApiListener final : public ObjectImpl<ApiListener>
 	Timer::Ptr m_AuthorityTimer;
 	Timer::Ptr m_CleanupCertificateRequestsTimer;
 	Timer::Ptr m_ApiPackageIntegrityTimer;
+	Timer::Ptr m_RenewOwnCertTimer;
 
 	Endpoint::Ptr m_LocalEndpoint;
 
@@ -162,7 +166,7 @@ class ApiListener final : public ObjectImpl<ApiListener>
 		boost::asio::yield_context yc, const Shared<boost::asio::io_context::strand>::Ptr& strand,
 		const Shared<AsioTlsStream>::Ptr& client, const String& hostname, ConnectionRole role
 	);
-	void ListenerCoroutineProc(boost::asio::yield_context yc, const Shared<boost::asio::ip::tcp::acceptor>::Ptr& server, const Shared<boost::asio::ssl::context>::Ptr& sslContext);
+	void ListenerCoroutineProc(boost::asio::yield_context yc, const Shared<boost::asio::ip::tcp::acceptor>::Ptr& server);
 
 	WorkQueue m_RelayQueue;
 	WorkQueue m_SyncQueue{0, 4};
@@ -191,6 +195,7 @@ class ApiListener final : public ObjectImpl<ApiListener>
 
 	void SyncLocalZoneDirs() const;
 	void SyncLocalZoneDir(const Zone::Ptr& zone) const;
+	void RenewOwnCert();
 
 	void SendConfigUpdate(const JsonRpcConnection::Ptr& aclient);
 

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.";
@@ -154,8 +145,6 @@ Value RequestCertificateHandler(const MessageOrigin::Ptr& origin, const Dictiona
 	}
 
 	std::shared_ptr<X509> newcert;
-	std::shared_ptr<EVP_PKEY> pubkey;
-	X509_NAME *subject;
 	Dictionary::Ptr message;
 	String ticket;
 
@@ -206,23 +195,11 @@ Value RequestCertificateHandler(const MessageOrigin::Ptr& origin, const Dictiona
 		}
 	}
 
-	pubkey = std::shared_ptr<EVP_PKEY>(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")
@@ -288,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());