From 9ac156a3176af7b725d36670a9c2347cdb352e52 Mon Sep 17 00:00:00 2001 From: Matthias Hensler Date: Thu, 12 May 2022 17:11:16 +0200 Subject: [PATCH] try to allow certificate-chains Currently the verification of certificates uses only the first pem in the ca-file and the first pem in the certificate-file. This breaks if an intermediate certificate is needed. A simple workaround is to put the full chain into the ca-file and give the ca-file instead of the X509-structure to the VerifyCertificate() methode. There we can just do the usual business but add the full ca-file again to OpenSSLs SSL_CTX_load_verify_locations(). While this seems a little bit hackish it should at least allow the proper verification of a certificate chain without introducing any security implications for setups with just a single root-ca. The only downside currently: while the CLI "pki verify" will correctly check the supplied parameters, it still only shows the topmost certificate from the ca-file (which I guess is fine for the moment). --- lib/base/tlsutility.cpp | 6 +++++- lib/base/tlsutility.hpp | 2 +- lib/cli/pkiverifycommand.cpp | 2 +- lib/remote/apilistener.cpp | 2 +- lib/remote/jsonrpcconnection-pki.cpp | 2 +- 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/base/tlsutility.cpp b/lib/base/tlsutility.cpp index 7e2193c8939..eb9fb759690 100644 --- a/lib/base/tlsutility.cpp +++ b/lib/base/tlsutility.cpp @@ -948,8 +948,10 @@ String BinaryToHex(const unsigned char* data, size_t length) { return output; } -bool VerifyCertificate(const std::shared_ptr &caCertificate, const std::shared_ptr &certificate, const String& crlFile) +bool VerifyCertificate(const String& caFile, const std::shared_ptr &certificate, const String& crlFile) { + std::shared_ptr caCertificate = GetX509Certificate(caFile); + X509_STORE *store = X509_STORE_new(); if (!store) @@ -961,6 +963,8 @@ bool VerifyCertificate(const std::shared_ptr &caCertificate, const std::sh AddCRLToSSLContext(store, crlFile); } + X509_STORE_load_locations(store, caFile.CStr(), NULL); /* ignore any errors for the moment, since this is just the convenient way to add full chain */ + X509_STORE_CTX *csc = X509_STORE_CTX_new(); X509_STORE_CTX_init(csc, store, certificate.get(), nullptr); diff --git a/lib/base/tlsutility.hpp b/lib/base/tlsutility.hpp index 3b8a89c5ecf..d25b25cbb2d 100644 --- a/lib/base/tlsutility.hpp +++ b/lib/base/tlsutility.hpp @@ -69,7 +69,7 @@ String SHA256(const String& s); String RandomString(int length); String BinaryToHex(const unsigned char* data, size_t length); -bool VerifyCertificate(const std::shared_ptr& caCertificate, const std::shared_ptr& certificate, const String& crlFile); +bool VerifyCertificate(const String& caFile, const std::shared_ptr& 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/lib/cli/pkiverifycommand.cpp b/lib/cli/pkiverifycommand.cpp index 963903a2114..67b8c1a6948 100644 --- a/lib/cli/pkiverifycommand.cpp +++ b/lib/cli/pkiverifycommand.cpp @@ -130,7 +130,7 @@ int PKIVerifyCommand::Run(const boost::program_options::variables_map& vm, const bool signedByCA; try { - signedByCA = VerifyCertificate(cacert, cert, crlFile); + signedByCA = VerifyCertificate(caCertFile, cert, crlFile); } catch (const std::exception& ex) { Log logmsg (LogCritical, "cli"); logmsg << "CRITICAL: Certificate with CN '" << certCN << "' is NOT signed by CA: "; diff --git a/lib/remote/apilistener.cpp b/lib/remote/apilistener.cpp index 2e1bb3a0e38..cba9ed06baa 100644 --- a/lib/remote/apilistener.cpp +++ b/lib/remote/apilistener.cpp @@ -184,7 +184,7 @@ std::shared_ptr ApiListener::RenewCert(const std::shared_ptr& cert) { std::shared_ptr pubkey (X509_get_pubkey(cert.get()), EVP_PKEY_free); auto subject (X509_get_subject_name(cert.get())); - auto cacert (GetX509Certificate(GetDefaultCaPath())); + auto cacert (GetDefaultCaPath()); auto newcert (CreateCertIcingaCA(pubkey.get(), subject)); /* verify that the new cert matches the CA we're using for the ApiListener; diff --git a/lib/remote/jsonrpcconnection-pki.cpp b/lib/remote/jsonrpcconnection-pki.cpp index 6188957eb9b..a94fef02a0a 100644 --- a/lib/remote/jsonrpcconnection-pki.cpp +++ b/lib/remote/jsonrpcconnection-pki.cpp @@ -60,7 +60,7 @@ Value RequestCertificateHandler(const MessageOrigin::Ptr& origin, const Dictiona logmsg << "Received certificate request for CN '" << cn << "'"; try { - signedByCA = VerifyCertificate(cacert, cert, listener->GetCrlPath()); + signedByCA = VerifyCertificate(listener->GetDefaultCaPath(), cert, listener->GetCrlPath()); if (!signedByCA) { logmsg << " not"; }