Skip to content

Commit

Permalink
try to allow certificate-chains
Browse files Browse the repository at this point in the history
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).

also process certificate chains presented by the client

To make this work the icinga2-master only holds to root-ca in its local
ca.crt, while the icinga2-agent has the intermediate-cert in its local
ca.crt (or the intermediate together with the root in the ca.crt / or
the intermediate in the cert.pem - doesn't matter).
  • Loading branch information
sircubbi committed Jun 20, 2023
1 parent a3dabde commit 9932a61
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 7 deletions.
5 changes: 5 additions & 0 deletions lib/base/tlsstream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ std::shared_ptr<X509> UnbufferedAsioTlsStream::GetPeerCertificate()
return std::shared_ptr<X509>(SSL_get_peer_certificate(native_handle()), X509_free);
}

STACK_OF(X509) *UnbufferedAsioTlsStream::GetPeerCertificateChain()
{
return SSL_get_peer_cert_chain(native_handle());
}

void UnbufferedAsioTlsStream::BeforeHandshake(handshake_type type)
{
namespace ssl = boost::asio::ssl;
Expand Down
1 change: 1 addition & 0 deletions lib/base/tlsstream.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ class UnbufferedAsioTlsStream : public AsioTcpTlsStream
bool IsVerifyOK() const;
String GetVerifyError() const;
std::shared_ptr<X509> GetPeerCertificate();
STACK_OF(X509) *GetPeerCertificateChain();

template<class... Args>
inline
Expand Down
8 changes: 6 additions & 2 deletions lib/base/tlsutility.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -948,8 +948,10 @@ String BinaryToHex(const unsigned char* data, size_t length) {
return output;
}

bool VerifyCertificate(const std::shared_ptr<X509> &caCertificate, const std::shared_ptr<X509> &certificate, const String& crlFile)
bool VerifyCertificate(const String& caFile, const std::shared_ptr<X509> &certificate, const String& crlFile, STACK_OF(X509) *chain)
{
std::shared_ptr<X509> caCertificate = GetX509Certificate(caFile);

X509_STORE *store = X509_STORE_new();

if (!store)
Expand All @@ -961,8 +963,10 @@ bool VerifyCertificate(const std::shared_ptr<X509> &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);
X509_STORE_CTX_init(csc, store, certificate.get(), chain);

int rc = X509_verify_cert(csc);

Expand Down
2 changes: 1 addition & 1 deletion lib/base/tlsutility.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<X509>& caCertificate, const std::shared_ptr<X509>& certificate, const String& crlFile);
bool VerifyCertificate(const String& caFile, const std::shared_ptr<X509>& certificate, const String& crlFile, STACK_OF(X509) *chain);
bool IsCa(const std::shared_ptr<X509>& cacert);
int GetCertificateVersion(const std::shared_ptr<X509>& cert);
String GetSignatureAlgorithm(const std::shared_ptr<X509>& cert);
Expand Down
2 changes: 1 addition & 1 deletion lib/cli/pkiverifycommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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, nullptr);
} catch (const std::exception& ex) {
Log logmsg (LogCritical, "cli");
logmsg << "CRITICAL: Certificate with CN '" << certCN << "' is NOT signed by CA: ";
Expand Down
4 changes: 2 additions & 2 deletions lib/remote/apilistener.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,15 @@ std::shared_ptr<X509> ApiListener::RenewCert(const std::shared_ptr<X509>& cert)
{
std::shared_ptr<EVP_PKEY> 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;
* this ensures that the CA we have in /var/lib/icinga2/ca matches the one
* we're using for cluster connections (there's no point in sending a client
* a certificate it wouldn't be able to use to connect to us anyway) */
try {
if (!VerifyCertificate(cacert, newcert, GetCrlPath())) {
if (!VerifyCertificate(cacert, newcert, GetCrlPath(), nullptr)) {
Log(LogWarning, "ApiListener")
<< "The CA in '" << GetDefaultCaPath() << "' does not match the CA which Icinga uses "
<< "for its own cluster connections. This is most likely a configuration problem.";
Expand Down
5 changes: 4 additions & 1 deletion lib/remote/jsonrpcconnection-pki.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,18 @@ Value RequestCertificateHandler(const MessageOrigin::Ptr& origin, const Dictiona
String certText = params->Get("cert_request");

std::shared_ptr<X509> cert;
STACK_OF(X509) *chain;

Dictionary::Ptr result = new Dictionary();

/* Use the presented client certificate if not provided. */
if (certText.IsEmpty()) {
auto stream (origin->FromClient->GetStream());
cert = stream->next_layer().GetPeerCertificate();
chain = stream->next_layer().GetPeerCertificateChain();
} else {
cert = StringToCertificate(certText);
chain = nullptr;
}

if (!cert) {
Expand All @@ -61,7 +64,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(), chain);
if (!signedByCA) {
logmsg << " not";
}
Expand Down

0 comments on commit 9932a61

Please sign in to comment.