diff --git a/lib/base/tlsutility.cpp b/lib/base/tlsutility.cpp index 1f074e236..cc9d2a3de 100644 --- a/lib/base/tlsutility.cpp +++ b/lib/base/tlsutility.cpp @@ -860,19 +860,42 @@ String RandomString(int length) bool VerifyCertificate(const std::shared_ptr &caCertificate, const std::shared_ptr &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 copy = StringToCertificate(CertificateToString(certificate)); + VERIFY(copy.get() != certificate); + certificate = copy.get(); +#endif + std::unique_ptr 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 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()); diff --git a/lib/base/tlsutility.hpp b/lib/base/tlsutility.hpp index 5bc5671b8..96641b3aa 100644 --- a/lib/base/tlsutility.hpp +++ b/lib/base/tlsutility.hpp @@ -64,6 +64,7 @@ String SHA256(const String& s); String RandomString(int length); bool VerifyCertificate(const std::shared_ptr& caCertificate, const std::shared_ptr& certificate, const String& crlFile); +bool VerifyCertificate(X509* caCertificate, X509* certificate, const String& crlFile); bool IsCa(const std::shared_ptr& cacert); int GetCertificateVersion(const std::shared_ptr& cert); String GetSignatureAlgorithm(const std::shared_ptr& cert); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 3c8c2bbaf..5e94caa4d 100644 --- a/test/CMakeLists.txt +++ b/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 diff --git a/test/base-tlsutility.cpp b/test/base-tlsutility.cpp new file mode 100644 index 000000000..9f06949be --- /dev/null +++ b/test/base-tlsutility.cpp @@ -0,0 +1,53 @@ +/* Icinga 2 | (c) 2021 Icinga GmbH | GPLv2+ */ + +#include "base/tlsutility.hpp" +#include +#include +#include +#include + +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()