mirror of
https://github.com/Icinga/icinga2.git
synced 2025-07-27 15:44:11 +02:00
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.
This commit is contained in:
parent
00864d1096
commit
4023128be4
@ -985,19 +985,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());
|
||||
|
||||
|
@ -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);
|
||||
|
@ -178,6 +178,7 @@ add_boost_test(base
|
||||
base_tlsutility/iscertuptodate_ok
|
||||
base_tlsutility/iscertuptodate_expiring
|
||||
base_tlsutility/iscertuptodate_old
|
||||
base_tlsutility/VerifyCertificate_revalidate
|
||||
base_utility/parse_version
|
||||
base_utility/compare_version
|
||||
base_utility/comparepasswords_works
|
||||
|
@ -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()
|
||||
|
Loading…
x
Reference in New Issue
Block a user