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

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

View File

@ -9,9 +9,25 @@ Released closed milestones can be found on [GitHub](https://github.com/Icinga/ic
## 2.12.3 (2020-12-15)
Version 2.12.3 mainly focuses on resolving issues with high load on Windows regarding the config sync
Version 2.12.3 resolves a security vulnerability with revoked certificates being
renewed automatically ignoring the CRL.
This version also resolves issues with high load on Windows regarding the config sync
and not being able to disable/enable Icinga 2 features over the API.
### Security
* Fix that revoked certificates due for renewal will automatically be renewed ignoring the CRL (CVE-2020-29663)
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).
Because Icinga 2 currently (v2.12.3 and earlier) uses a validity duration of 15 years,
this only affects setups with external certificate signing and revoked certificates
that expire in less then 30 days.
### Bugfixes
* Improve config sync locking - resolves high load issues on Windows #8511

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.";