Skip to content

Commit

Permalink
Verify certificates against CRL before renewing them
Browse files Browse the repository at this point in the history
When a CRL is specified in the ApiListener configuration, Icinga 2 only
used it when connections were established so far, but not when a
certificate is requested. This allows a node to automatically renew a
revoked certificate if it meets the other conditions for auto-renewal
(issued before 2017 or expires in less than 30 days).
  • Loading branch information
julianbrost authored and N-o-X committed Dec 15, 2020
1 parent 3b862fe commit cae22a8
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 13 deletions.
21 changes: 18 additions & 3 deletions lib/base/tlsutility.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -242,15 +242,26 @@ void SetTlsProtocolminToSSLContext(const Shared<boost::asio::ssl::context>::Ptr&
}

/**
* Loads a CRL and appends its certificates to the specified SSL context.
* Loads a CRL and appends its certificates to the specified Boost SSL context.
*
* @param context The SSL context.
* @param crlPath The path to the CRL file.
*/
void AddCRLToSSLContext(const Shared<boost::asio::ssl::context>::Ptr& context, const String& crlPath)
{
char errbuf[256];
X509_STORE *x509_store = SSL_CTX_get_cert_store(context->native_handle());
AddCRLToSSLContext(x509_store, crlPath);
}

/**
* Loads a CRL and appends its certificates to the specified OpenSSL X509 store.
*
* @param context The SSL context.
* @param crlPath The path to the CRL file.
*/
void AddCRLToSSLContext(X509_STORE *x509_store, const String& crlPath)
{
char errbuf[256];

X509_LOOKUP *lookup;
lookup = X509_STORE_add_lookup(x509_store, X509_LOOKUP_file());
Expand Down Expand Up @@ -801,7 +812,7 @@ String RandomString(int length)
return result;
}

bool VerifyCertificate(const std::shared_ptr<X509>& caCertificate, const std::shared_ptr<X509>& certificate)
bool VerifyCertificate(const std::shared_ptr<X509> &caCertificate, const std::shared_ptr<X509> &certificate, const String& crlFile)
{
X509_STORE *store = X509_STORE_new();

Expand All @@ -810,6 +821,10 @@ bool VerifyCertificate(const std::shared_ptr<X509>& caCertificate, const std::sh

X509_STORE_add_cert(store, caCertificate.get());

if (!crlFile.IsEmpty()) {
AddCRLToSSLContext(store, crlFile);
}

X509_STORE_CTX *csc = X509_STORE_CTX_new();
X509_STORE_CTX_init(csc, store, certificate.get(), nullptr);

Expand Down
3 changes: 2 additions & 1 deletion lib/base/tlsutility.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ String GetOpenSSLVersion();

Shared<boost::asio::ssl::context>::Ptr MakeAsioSslContext(const String& pubkey = String(), const String& privkey = String(), const String& cakey = String());
void AddCRLToSSLContext(const Shared<boost::asio::ssl::context>::Ptr& context, const String& crlPath);
void AddCRLToSSLContext(X509_STORE *x509_store, const String& crlPath);
void SetCipherListToSSLContext(const Shared<boost::asio::ssl::context>::Ptr& context, const String& cipherList);
void SetTlsProtocolminToSSLContext(const Shared<boost::asio::ssl::context>::Ptr& context, const String& tlsProtocolmin);

Expand All @@ -51,7 +52,7 @@ String SHA1(const String& s, bool binary = false);
String SHA256(const String& s);
String RandomString(int length);

bool VerifyCertificate(const std::shared_ptr<X509>& caCertificate, const std::shared_ptr<X509>& certificate);
bool VerifyCertificate(const std::shared_ptr<X509>& caCertificate, const std::shared_ptr<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);
Expand Down
2 changes: 1 addition & 1 deletion lib/cli/pkiverifycommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ int PKIVerifyCommand::Run(const boost::program_options::variables_map& vm, const
bool signedByCA;

try {
signedByCA = VerifyCertificate(cacert, cert);
signedByCA = VerifyCertificate(cacert, cert, String());
} catch (const std::exception& ex) {
Log(LogCritical, "cli")
<< "CRITICAL: Certificate with CN '" << certCN << "' is NOT signed by CA: " << DiagnosticInformation(ex, false);
Expand Down
28 changes: 20 additions & 8 deletions lib/remote/jsonrpcconnection-pki.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,25 @@ Value RequestCertificateHandler(const MessageOrigin::Ptr& origin, const Dictiona

bool signedByCA = false;

try {
signedByCA = VerifyCertificate(cacert, cert);
} catch (const std::exception&) { } /* Swallow the exception on purpose, cacert will never be a non-CA certificate. */

Log(LogInformation, "JsonRpcConnection")
<< "Received certificate request for CN '" << cn << "'"
<< (signedByCA ? "" : " not") << " signed by our CA.";
{
Log logmsg(LogInformation, "JsonRpcConnection");
logmsg << "Received certificate request for CN '" << cn << "'";

try {
signedByCA = VerifyCertificate(cacert, cert, listener->GetCrlPath());
if (!signedByCA) {
logmsg << " not";
}
logmsg << " signed by our CA.";
} catch (const std::exception &ex) {
logmsg << " not signed by our CA";
if (const unsigned long *openssl_code = boost::get_error_info<errinfo_openssl_error>(ex)) {
logmsg << ": " << X509_verify_cert_error_string(long(*openssl_code)) << " (code " << *openssl_code << ")";
} else {
logmsg << ".";
}
}
}

if (signedByCA) {
time_t now;
Expand Down Expand Up @@ -204,7 +216,7 @@ Value RequestCertificateHandler(const MessageOrigin::Ptr& origin, const Dictiona
* 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)) {
if (!VerifyCertificate(cacert, newcert, listener->GetCrlPath())) {
Log(LogWarning, "JsonRpcConnection")
<< "The CA in '" << listener->GetDefaultCaPath() << "' does not match the CA which Icinga uses "
<< "for its own cluster connections. This is most likely a configuration problem.";
Expand Down

0 comments on commit cae22a8

Please sign in to comment.