tlsutility: normalize cert valid for ts on 32-bit systems

Author: yhabteab

lib/base/tlsutility.cpp

@@ -478,7 +478,7 @@ std::shared_ptr<X509> GetX509Certificate(const String& pemfile)
 	return std::shared_ptr<X509>(cert, X509_free);
 }
 
-int MakeX509CSR(const String& cn, const String& keyfile, const String& csrfile, const String& certfile, int validFor, bool ca)
+int MakeX509CSR(const String& cn, const String& keyfile, const String& csrfile, const String& certfile, long validFor, bool ca)
 {
 	char errbuf[256];
 
@@ -640,7 +640,7 @@ int MakeX509CSR(const String& cn, const String& keyfile, const String& csrfile,
 	return 1;
 }
 
-std::shared_ptr<X509> CreateCert(EVP_PKEY *pubkey, X509_NAME *subject, X509_NAME *issuer, EVP_PKEY *cakey, int validFor, bool ca)
+std::shared_ptr<X509> CreateCert(EVP_PKEY *pubkey, X509_NAME *subject, X509_NAME *issuer, EVP_PKEY *cakey, long validFor, bool ca)
 {
 	X509 *cert = X509_new();
 	X509_set_version(cert, 2);
@@ -649,7 +649,7 @@ std::shared_ptr<X509> CreateCert(EVP_PKEY *pubkey, X509_NAME *subject, X509_NAME
 		notBefore = validFor - validFor / 4; // Add 25% of the validity period to the past
 	}
 	X509_gmtime_adj(X509_get_notBefore(cert), notBefore);
-	X509_gmtime_adj(X509_get_notAfter(cert), validFor);
+	X509_gmtime_adj(X509_get_notAfter(cert), NormalizeCertValidFor(validFor));
 	X509_set_pubkey(cert, pubkey);
 
 	X509_set_subject_name(cert, subject);
@@ -732,7 +732,7 @@ String GetIcingaCADir()
 	return Configuration::DataDir + "/ca";
 }
 
-std::shared_ptr<X509> CreateCertIcingaCA(EVP_PKEY *pubkey, X509_NAME *subject, int validFor, bool ca)
+std::shared_ptr<X509> CreateCertIcingaCA(EVP_PKEY *pubkey, X509_NAME *subject, long validFor, bool ca)
 {
 	char errbuf[256];
 
@@ -779,7 +779,7 @@ std::shared_ptr<X509> CreateCertIcingaCA(EVP_PKEY *pubkey, X509_NAME *subject, i
  * @param validFor The validity period in seconds. Defaults to LEAF_VALID_FOR.
  * @returns The new X509 certificate or an empty shared_ptr on error.
  */
-std::shared_ptr<X509> CreateCertIcingaCA(const std::shared_ptr<X509>& cert, int validFor)
+std::shared_ptr<X509> CreateCertIcingaCA(const std::shared_ptr<X509>& cert, long validFor)
 {
 	std::shared_ptr<EVP_PKEY> pkey = std::shared_ptr<EVP_PKEY>(X509_get_pubkey(cert.get()), EVP_PKEY_free);
 	return CreateCertIcingaCA(pkey.get(), X509_get_subject_name(cert.get()), validFor);
@@ -812,6 +812,30 @@ bool IsCaUptodate(X509* cert)
 	return !CertExpiresWithin(cert, LEAF_VALID_FOR);
 }
 
+/**
+ * Normalizes the specified certificate validity period in seconds to avoid
+ * issues with 32-bit time_t wrap-around in 2038.
+ *
+ * On 64-bit systems this function simply returns the specified value. On 32-bit systems it ensures that
+ * the current time plus the specified value never overflows the maximum value of time_t. Otherwise, the
+ * returned value is adjusted so that the sum is exactly the maximum value of time_t.
+ *
+ * @param seconds The desired validity period in seconds.
+ * @returns The normalized validity period in seconds.
+ */
+long NormalizeCertValidFor(long seconds)
+{
+	if constexpr (sizeof(time_t) > sizeof(int32_t)) {
+		return seconds;
+	}
+
+	constexpr time_t maxTimeT = static_cast<time_t>(std::numeric_limits<int32_t>::max());
+	if (auto rem = maxTimeT - time(nullptr); rem < seconds) {
+		return rem;
+	}
+	return seconds;
+}
+
 String CertificateToString(X509* cert)
 {
 	BIO *mem = BIO_new(BIO_s_mem());

lib/base/tlsutility.hpp

@@ -54,8 +54,8 @@ Shared<boost::asio::ssl::context>::Ptr SetupSslContext(String certPath, String k
 
 String GetCertificateCN(const std::shared_ptr<X509>& certificate);
 std::shared_ptr<X509> GetX509Certificate(const String& pemfile);
-int MakeX509CSR(const String& cn, const String& keyfile, const String& csrfile = String(), const String& certfile = String(), int validFor = LEAF_VALID_FOR, bool ca = false);
-std::shared_ptr<X509> CreateCert(EVP_PKEY *pubkey, X509_NAME *subject, X509_NAME *issuer, EVP_PKEY *cakey, int validFor, bool ca);
+int MakeX509CSR(const String& cn, const String& keyfile, const String& csrfile = String(), const String& certfile = String(), long validFor = LEAF_VALID_FOR, bool ca = false);
+std::shared_ptr<X509> CreateCert(EVP_PKEY *pubkey, X509_NAME *subject, X509_NAME *issuer, EVP_PKEY *cakey, long validFor, bool ca);
 
 String GetIcingaCADir();
 String CertificateToString(X509* cert);
@@ -66,10 +66,11 @@ inline 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, int validFor, bool ca = false);
-std::shared_ptr<X509> CreateCertIcingaCA(const std::shared_ptr<X509>& cert, int validFor = LEAF_VALID_FOR);
+std::shared_ptr<X509> CreateCertIcingaCA(EVP_PKEY *pubkey, X509_NAME *subject, long validFor, bool ca = false);
+std::shared_ptr<X509> CreateCertIcingaCA(const std::shared_ptr<X509>& cert, long validFor = LEAF_VALID_FOR);
 bool IsCertUptodate(const std::shared_ptr<X509>& cert);
 bool IsCaUptodate(X509* cert);
+long NormalizeCertValidFor(long seconds);
 
 String PBKDF2_SHA1(const String& password, const String& salt, int iterations);
 String PBKDF2_SHA256(const String& password, const String& salt, int iterations);

test/base-tlsutility.cpp

@@ -70,26 +70,6 @@ static std::shared_ptr<EVP_PKEY> GetEVP_PKEY(const String& keyfile)
 	return std::shared_ptr<EVP_PKEY>(pkey, EVP_PKEY_free);
 }
 
-/**
- * Returns a time_t value that is the current time plus the specified number of seconds,
- * but capped to max time_t on 32-bit systems to avoid the 2038 problem.
- *
- * @param timestamp The number of seconds to add to the current time.
- * @return The resulting time_t value, capped to max time_t on 32-bit systems.
- */
-time_t ValidCertValidity(long timestamp)
-{
-	time_t t = time(nullptr);
-	if constexpr (sizeof(t) > sizeof(int32_t)) {
-		t = t + timestamp;
-	} else {
-		// On 32-bit systems time_t is 32-bit and thus limited to 2038.
-		// So, make sure t+timestamp is never greater than max time_t.
-		t = std::min(t + timestamp, std::numeric_limits<time_t>::max());
-	}
-	return t;
-}
-
 /**
  * Creates a new X509 certificate signed by the Icinga CA based on an existing CA certificate.
  *
@@ -118,7 +98,7 @@ BOOST_FIXTURE_TEST_CASE(create_verify_ca, CertificateFixture)
 	BOOST_CHECK_EQUAL(true, VerifyCertificate(cacert, cacert, String())); // Self-signed CA!
 	BOOST_CHECK_EQUAL(true, IsCaUptodate(cacert.get())); // Is CA up-to-date after its creation?
 
-	auto caValidUntil = ValidCertValidity(ROOT_VALID_FOR);
+	auto caValidUntil = NormalizeCertValidFor(ROOT_VALID_FOR);
 	// Due to processing time the expiration date might be off by a few seconds,
 	// so we just check whether it is less than the expected value and not exactly equal.
 	BOOST_CHECK_EQUAL(-1, X509_cmp_time(X509_get_notAfter(cacert.get()), &caValidUntil));
@@ -158,7 +138,7 @@ BOOST_FIXTURE_TEST_CASE(create_verify_leaf_certs, CertificateFixture)
 	BOOST_CHECK_EQUAL(true, IsCertUptodate(cert)); // Is leaf up-to-date after its creation?
 	BOOST_CHECK_EQUAL(true, VerifyCertificate(cacert, cert, String())); // Signed by our CA?
 
-	auto certValidUntil = ValidCertValidity(LEAF_VALID_FOR);
+	auto certValidUntil = NormalizeCertValidFor(LEAF_VALID_FOR);
 	// Due to processing time the expiration date might be off by a few seconds,
 	// so we just check whether it is less than the expected value and not exactly equal.
 	BOOST_CHECK_EQUAL(-1, X509_cmp_time(X509_get_notAfter(cert.get()), &certValidUntil));
@@ -170,7 +150,7 @@ BOOST_FIXTURE_TEST_CASE(create_verify_leaf_certs, CertificateFixture)
 	// Check whether expired certificates are correctly detected and verification fails.
 	PkiUtility::SignCsr(certInfo.csrFile, certInfo.crtFile, 60*60*24*-10); // Expire 10 days ago!
 	cert = GetX509Certificate(certInfo.crtFile);
-	certValidUntil = ValidCertValidity(60*60*24*-10);
+	certValidUntil = NormalizeCertValidFor(60*60*24*-10);
 	BOOST_CHECK_EQUAL(-1, X509_cmp_time(X509_get_notAfter(cert.get()), &certValidUntil)); // Is certificate indeed expired?
 	BOOST_CHECK_EQUAL(false, IsCertUptodate(cert)); // It's already expired, so definitely not up-to-date.
 	BOOST_CHECK_THROW(VerifyCertificate(cacert, cert, String()), openssl_error);