diff --git a/CHANGELOG.md b/CHANGELOG.md index ca463893e0b..e6a54bd7f3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/lib/base/tlsutility.cpp b/lib/base/tlsutility.cpp index f1ee06a32c8..c9cecb07b34 100644 --- a/lib/base/tlsutility.cpp +++ b/lib/base/tlsutility.cpp @@ -248,15 +248,26 @@ void SetTlsProtocolminToSSLContext(const Shared::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::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& caCertificate, const std::shared_ptr& certificate) +bool VerifyCertificate(const std::shared_ptr &caCertificate, const std::shared_ptr &certificate, const String& crlFile) { X509_STORE *store = X509_STORE_new(); @@ -842,6 +853,10 @@ bool VerifyCertificate(const std::shared_ptr& 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); diff --git a/lib/base/tlsutility.hpp b/lib/base/tlsutility.hpp index 665f4e7945b..73d032ed924 100644 --- a/lib/base/tlsutility.hpp +++ b/lib/base/tlsutility.hpp @@ -30,6 +30,7 @@ String GetOpenSSLVersion(); Shared::Ptr MakeAsioSslContext(const String& pubkey = String(), const String& privkey = String(), const String& cakey = String()); void AddCRLToSSLContext(const Shared::Ptr& context, const String& crlPath); +void AddCRLToSSLContext(X509_STORE *x509_store, const String& crlPath); void SetCipherListToSSLContext(const Shared::Ptr& context, const String& cipherList); void SetTlsProtocolminToSSLContext(const Shared::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& caCertificate, const std::shared_ptr& certificate); +bool VerifyCertificate(const std::shared_ptr& caCertificate, 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 d22d49d91e6..963903a2114 100644 --- a/lib/cli/pkiverifycommand.cpp +++ b/lib/cli/pkiverifycommand.cpp @@ -28,12 +28,13 @@ void PKIVerifyCommand::InitParameters(boost::program_options::options_descriptio visibleDesc.add_options() ("cn", po::value(), "Common Name (optional). Use with '--cert' to check the CN in the certificate.") ("cert", po::value(), "Certificate file path (optional). Standalone: print certificate. With '--cacert': Verify against CA.") - ("cacert", po::value(), "CA certificate file path (optional). If passed standalone, verifies whether this is a CA certificate"); + ("cacert", po::value(), "CA certificate file path (optional). If passed standalone, verifies whether this is a CA certificate") + ("crl", po::value(), "CRL file path (optional). Check the certificate against this revocation list when verifying against CA."); } std::vector 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 PKIVerifyCommand::GetArgumentSuggestions(const String& argum */ int PKIVerifyCommand::Run(const boost::program_options::variables_map& vm, const std::vector& ap) const { - String cn, certFile, caCertFile; + String cn, certFile, caCertFile, crlFile; if (vm.count("cn")) cn = vm["cn"].as(); @@ -57,6 +58,9 @@ int PKIVerifyCommand::Run(const boost::program_options::variables_map& vm, const if (vm.count("cacert")) caCertFile = vm["cacert"].as(); + if (vm.count("crl")) + crlFile = vm["crl"].as(); + /* Verify CN in certificate. */ if (!cn.IsEmpty() && !certFile.IsEmpty()) { std::shared_ptr 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(ex)) { + logmsg << X509_verify_cert_error_string(*openssl_code) << " (code " << *openssl_code << ")"; + } else { + logmsg << DiagnosticInformation(ex, false); + } return ServiceCritical; } diff --git a/lib/remote/jsonrpcconnection-pki.cpp b/lib/remote/jsonrpcconnection-pki.cpp index c3d7f9d8530..baa115d69d0 100644 --- a/lib/remote/jsonrpcconnection-pki.cpp +++ b/lib/remote/jsonrpcconnection-pki.cpp @@ -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(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.";