VerifyCertificate: Work around issue in OpenSSL < 1.1.0 causing invalid certifcates being treated as valid

Old versions of OpenSSL stored a valid flag in the certificate (see inline code
comment for details) that if already set, causes parts of the verification to
be skipped and return that the certificate is valid, even if it's not actually
signed by the CA in the trust store.

This issue was assigned CVE-2025-48057.

lib/base/tlsutility.cpp

@@ -860,19 +860,42 @@ String RandomString(int length)
 
 bool VerifyCertificate(const std::shared_ptr<X509> &caCertificate, const std::shared_ptr<X509> &certificate, const String& crlFile)
 {
+	return VerifyCertificate(caCertificate.get(), certificate.get(), crlFile);
+}
+
+bool VerifyCertificate(X509* caCertificate, X509* certificate, const String& crlFile)
+{
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
+	/*
+	 * OpenSSL older than version 1.1.0 stored a valid flag in the struct behind X509* which leads to certain validation
+	 * steps to be skipped on subsequent verification operations. If a certificate is verified multiple times with a
+	 * different configuration, for example with different trust anchors, this can result in the certificate
+	 * incorrectly being treated as valid.
+	 *
+	 * This issue is worked around by serializing and deserializing the certificate which creates a new struct instance
+	 * with the valid flag cleared, hence performing the full validation.
+	 *
+	 * The flag in question was removed in OpenSSL 1.1.0, so this extra step isn't necessary for more recent versions:
+	 * https://github.com/openssl/openssl/commit/0e76014e584ba78ef1d6ecb4572391ef61c4fb51
+	 */
+	std::shared_ptr<X509> copy = StringToCertificate(CertificateToString(certificate));
+	VERIFY(copy.get() != certificate);
+	certificate = copy.get();
+#endif
+
 	std::unique_ptr<X509_STORE, decltype(&X509_STORE_free)> store{X509_STORE_new(), &X509_STORE_free};
 
 	if (!store)
 		return false;
 
-	X509_STORE_add_cert(store.get(), caCertificate.get());
+	X509_STORE_add_cert(store.get(), caCertificate);
 
 	if (!crlFile.IsEmpty()) {
 		AddCRLToSSLContext(store.get(), crlFile);
 	}
 
 	std::unique_ptr<X509_STORE_CTX, decltype(&X509_STORE_CTX_free)> csc{X509_STORE_CTX_new(), &X509_STORE_CTX_free};
-	X509_STORE_CTX_init(csc.get(), store.get(), certificate.get(), nullptr);
+	X509_STORE_CTX_init(csc.get(), store.get(), certificate, nullptr);
 
 	int rc = X509_verify_cert(csc.get());
 

lib/base/tlsutility.hpp

@@ -64,6 +64,7 @@ String SHA256(const String& s);
 String RandomString(int length);
 
 bool VerifyCertificate(const std::shared_ptr<X509>& caCertificate, const std::shared_ptr<X509>& certificate, const String& crlFile);
+bool VerifyCertificate(X509* caCertificate, X509* certificate, const String& crlFile);
 bool IsCa(const std::shared_ptr<X509>& cacert);
 int GetCertificateVersion(const std::shared_ptr<X509>& cert);
 String GetSignatureAlgorithm(const std::shared_ptr<X509>& cert);

test/CMakeLists.txt

@@ -21,6 +21,7 @@ set(base_test_SOURCES
   base-string.cpp
   base-timer.cpp
   base-type.cpp
+  base-tlsutility.cpp
   base-utility.cpp
   base-value.cpp
   config-ops.cpp
@@ -109,6 +110,7 @@ add_boost_test(base
     base_type/assign
     base_type/byname
     base_type/instantiate
+    base_tlsutility/VerifyCertificate_revalidate
     base_utility/parse_version
     base_utility/compare_version
     base_utility/comparepasswords_works

test/base-tlsutility.cpp

@@ -0,0 +1,53 @@
+/* Icinga 2 | (c) 2021 Icinga GmbH | GPLv2+ */
+
+#include "base/tlsutility.hpp"
+#include <BoostTestTargetConfig.h>
+#include <openssl/evp.h>
+#include <openssl/rsa.h>
+#include <openssl/x509.h>
+
+using namespace icinga;
+
+static EVP_PKEY* GenKeypair()
+{
+	InitializeOpenSSL();
+
+	auto e (BN_new());
+	BOOST_REQUIRE(e);
+
+	auto rsa (RSA_new());
+	BOOST_REQUIRE(rsa);
+
+	auto key (EVP_PKEY_new());
+	BOOST_REQUIRE(key);
+
+	BOOST_REQUIRE(BN_set_word(e, RSA_F4));
+	BOOST_REQUIRE(RSA_generate_key_ex(rsa, 4096, e, nullptr));
+	BOOST_REQUIRE(EVP_PKEY_assign_RSA(key, rsa));
+
+	return key;
+}
+
+BOOST_AUTO_TEST_SUITE(base_tlsutility)
+
+BOOST_AUTO_TEST_CASE(VerifyCertificate_revalidate)
+{
+	X509_NAME *caSubject = X509_NAME_new();
+	X509_NAME_add_entry_by_txt(caSubject, "CN", MBSTRING_ASC, (const unsigned char*)"Icinga CA", -1, -1, 0);
+
+	auto signingCaKey = GenKeypair();
+	auto signingCaCert = CreateCert(signingCaKey, caSubject, caSubject, signingCaKey, true);
+
+	X509_NAME *leafSubject = X509_NAME_new();
+	X509_NAME_add_entry_by_txt(leafSubject, "CN", MBSTRING_ASC, (const unsigned char*)"Leaf Certificate", -1, -1, 0);
+	auto leafKey = GenKeypair();
+	auto leafCert = CreateCert(leafKey, leafSubject, caSubject, signingCaKey, false);
+	BOOST_CHECK(VerifyCertificate(signingCaCert, leafCert, ""));
+
+	// Create a second CA with a different key, the leaf certificate is supposed to fail validation against that CA.
+	auto otherCaKey = GenKeypair();
+	auto otherCaCert = CreateCert(otherCaKey, caSubject, caSubject, otherCaKey, true);
+	BOOST_CHECK_THROW(VerifyCertificate(otherCaCert, leafCert, ""), openssl_error);
+}
+
+BOOST_AUTO_TEST_SUITE_END()