Merge pull request from GHSA-pcmr-2p2f-r7j6

Verify certificates against CRL before renewing them (2.13)
This commit is contained in:
Noah Hilverling 2020-12-15 12:30:19 +01:00 committed by GitHub
commit f7e368564f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 54 additions and 17 deletions

View File

@ -248,15 +248,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());
@ -833,7 +844,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();
@ -842,6 +853,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);

View File

@ -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);
@ -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);

View File

@ -28,12 +28,13 @@ void PKIVerifyCommand::InitParameters(boost::program_options::options_descriptio
visibleDesc.add_options()
("cn", po::value<std::string>(), "Common Name (optional). Use with '--cert' to check the CN in the certificate.")
("cert", po::value<std::string>(), "Certificate file path (optional). Standalone: print certificate. With '--cacert': Verify against CA.")
("cacert", po::value<std::string>(), "CA certificate file path (optional). If passed standalone, verifies whether this is a CA certificate");
("cacert", po::value<std::string>(), "CA certificate file path (optional). If passed standalone, verifies whether this is a CA certificate")
("crl", po::value<std::string>(), "CRL file path (optional). Check the certificate against this revocation list when verifying against CA.");
}
std::vector<String> PKIVerifyCommand::GetArgumentSuggestions(const String& argument, const String& word) const
{
if (argument == "cert" || argument == "cacert")
if (argument == "cert" || argument == "cacert" || argument == "crl")
return GetBashCompletionSuggestions("file", word);
else
return CLICommand::GetArgumentSuggestions(argument, word);
@ -46,7 +47,7 @@ std::vector<String> PKIVerifyCommand::GetArgumentSuggestions(const String& argum
*/
int PKIVerifyCommand::Run(const boost::program_options::variables_map& vm, const std::vector<std::string>& ap) const
{
String cn, certFile, caCertFile;
String cn, certFile, caCertFile, crlFile;
if (vm.count("cn"))
cn = vm["cn"].as<std::string>();
@ -57,6 +58,9 @@ int PKIVerifyCommand::Run(const boost::program_options::variables_map& vm, const
if (vm.count("cacert"))
caCertFile = vm["cacert"].as<std::string>();
if (vm.count("crl"))
crlFile = vm["crl"].as<std::string>();
/* Verify CN in certificate. */
if (!cn.IsEmpty() && !certFile.IsEmpty()) {
std::shared_ptr<X509> cert;
@ -126,10 +130,15 @@ int PKIVerifyCommand::Run(const boost::program_options::variables_map& vm, const
bool signedByCA;
try {
signedByCA = VerifyCertificate(cacert, cert);
signedByCA = VerifyCertificate(cacert, cert, crlFile);
} catch (const std::exception& ex) {
Log(LogCritical, "cli")
<< "CRITICAL: Certificate with CN '" << certCN << "' is NOT signed by CA: " << DiagnosticInformation(ex, false);
Log logmsg (LogCritical, "cli");
logmsg << "CRITICAL: Certificate with CN '" << certCN << "' is NOT signed by CA: ";
if (const unsigned long *openssl_code = boost::get_error_info<errinfo_openssl_error>(ex)) {
logmsg << X509_verify_cert_error_string(*openssl_code) << " (code " << *openssl_code << ")";
} else {
logmsg << DiagnosticInformation(ex, false);
}
return ServiceCritical;
}

View File

@ -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 logmsg(LogInformation, "JsonRpcConnection");
logmsg << "Received certificate request for CN '" << cn << "'";
Log(LogInformation, "JsonRpcConnection")
<< "Received certificate request for CN '" << cn << "'"
<< (signedByCA ? "" : " not") << " signed by our CA.";
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;
@ -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.";