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.

Author: julianbrost

lib/base/tlsutility.cpp

@@ -983,19 +983,42 @@ String BinaryToHex(const unsigned char* data, size_t 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

@@ -79,6 +79,7 @@ String RandomString(int length);
 String BinaryToHex(const unsigned char* data, size_t 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

@@ -123,6 +123,7 @@ add_boost_test(base
     base_tlsutility/iscertuptodate_ok
     base_tlsutility/iscertuptodate_expiring
     base_tlsutility/iscertuptodate_old
+    base_tlsutility/VerifyCertificate_revalidate
     base_type/gettype
     base_type/assign
     base_type/byname

test/base-tlsutility.cpp

@@ -132,4 +132,24 @@ BOOST_AUTO_TEST_CASE(iscertuptodate_old)
 	})));
 }
 
+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()