Skip to content

Commit 60f75f4

Browse files
julianbrostyhabteab
authored andcommitted
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.
1 parent 56f4abf commit 60f75f4

File tree

4 files changed

+47
-2
lines changed

4 files changed

+47
-2
lines changed

lib/base/tlsutility.cpp

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -975,19 +975,42 @@ String BinaryToHex(const unsigned char* data, size_t length) {
975975

976976
bool VerifyCertificate(const std::shared_ptr<X509> &caCertificate, const std::shared_ptr<X509> &certificate, const String& crlFile)
977977
{
978+
return VerifyCertificate(caCertificate.get(), certificate.get(), crlFile);
979+
}
980+
981+
bool VerifyCertificate(X509* caCertificate, X509* certificate, const String& crlFile)
982+
{
983+
#if OPENSSL_VERSION_NUMBER < 0x10100000L
984+
/*
985+
* OpenSSL older than version 1.1.0 stored a valid flag in the struct behind X509* which leads to certain validation
986+
* steps to be skipped on subsequent verification operations. If a certificate is verified multiple times with a
987+
* different configuration, for example with different trust anchors, this can result in the certificate
988+
* incorrectly being treated as valid.
989+
*
990+
* This issue is worked around by serializing and deserializing the certificate which creates a new struct instance
991+
* with the valid flag cleared, hence performing the full validation.
992+
*
993+
* The flag in question was removed in OpenSSL 1.1.0, so this extra step isn't necessary for more recent versions:
994+
* https://github.com/openssl/openssl/commit/0e76014e584ba78ef1d6ecb4572391ef61c4fb51
995+
*/
996+
std::shared_ptr<X509> copy = StringToCertificate(CertificateToString(certificate));
997+
VERIFY(copy.get() != certificate);
998+
certificate = copy.get();
999+
#endif
1000+
9781001
std::unique_ptr<X509_STORE, decltype(&X509_STORE_free)> store{X509_STORE_new(), &X509_STORE_free};
9791002

9801003
if (!store)
9811004
return false;
9821005

983-
X509_STORE_add_cert(store.get(), caCertificate.get());
1006+
X509_STORE_add_cert(store.get(), caCertificate);
9841007

9851008
if (!crlFile.IsEmpty()) {
9861009
AddCRLToSSLContext(store.get(), crlFile);
9871010
}
9881011

9891012
std::unique_ptr<X509_STORE_CTX, decltype(&X509_STORE_CTX_free)> csc{X509_STORE_CTX_new(), &X509_STORE_CTX_free};
990-
X509_STORE_CTX_init(csc.get(), store.get(), certificate.get(), nullptr);
1013+
X509_STORE_CTX_init(csc.get(), store.get(), certificate, nullptr);
9911014

9921015
int rc = X509_verify_cert(csc.get());
9931016

lib/base/tlsutility.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ String RandomString(int length);
7676
String BinaryToHex(const unsigned char* data, size_t length);
7777

7878
bool VerifyCertificate(const std::shared_ptr<X509>& caCertificate, const std::shared_ptr<X509>& certificate, const String& crlFile);
79+
bool VerifyCertificate(X509* caCertificate, X509* certificate, const String& crlFile);
7980
bool IsCa(const std::shared_ptr<X509>& cacert);
8081
int GetCertificateVersion(const std::shared_ptr<X509>& cert);
8182
String GetSignatureAlgorithm(const std::shared_ptr<X509>& cert);

test/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ add_boost_test(base
114114
base_tlsutility/iscertuptodate_ok
115115
base_tlsutility/iscertuptodate_expiring
116116
base_tlsutility/iscertuptodate_old
117+
base_tlsutility/VerifyCertificate_revalidate
117118
base_type/gettype
118119
base_type/assign
119120
base_type/byname

test/base-tlsutility.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,4 +132,24 @@ BOOST_AUTO_TEST_CASE(iscertuptodate_old)
132132
})));
133133
}
134134

135+
BOOST_AUTO_TEST_CASE(VerifyCertificate_revalidate)
136+
{
137+
X509_NAME *caSubject = X509_NAME_new();
138+
X509_NAME_add_entry_by_txt(caSubject, "CN", MBSTRING_ASC, (const unsigned char*)"Icinga CA", -1, -1, 0);
139+
140+
auto signingCaKey = GenKeypair();
141+
auto signingCaCert = CreateCert(signingCaKey, caSubject, caSubject, signingCaKey, true);
142+
143+
X509_NAME *leafSubject = X509_NAME_new();
144+
X509_NAME_add_entry_by_txt(leafSubject, "CN", MBSTRING_ASC, (const unsigned char*)"Leaf Certificate", -1, -1, 0);
145+
auto leafKey = GenKeypair();
146+
auto leafCert = CreateCert(leafKey, leafSubject, caSubject, signingCaKey, false);
147+
BOOST_CHECK(VerifyCertificate(signingCaCert, leafCert, ""));
148+
149+
// Create a second CA with a different key, the leaf certificate is supposed to fail validation against that CA.
150+
auto otherCaKey = GenKeypair();
151+
auto otherCaCert = CreateCert(otherCaKey, caSubject, caSubject, otherCaKey, true);
152+
BOOST_CHECK_THROW(VerifyCertificate(otherCaCert, leafCert, ""), openssl_error);
153+
}
154+
135155
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)