diff --git a/lib/base/tlsstream.cpp b/lib/base/tlsstream.cpp index 66514e0cf1..56c08d06b5 100644 --- a/lib/base/tlsstream.cpp +++ b/lib/base/tlsstream.cpp @@ -40,6 +40,24 @@ bool UnbufferedAsioTlsStream::IsVerifyOK() return SSL_get_verify_result(native_handle()) == X509_V_OK; } +/** + * Returns an error code for situations where IsVerifyOK() returns false. + * + * @return See SSL_get_verify_result(3) + */ +long UnbufferedAsioTlsStream::GetVerifyErrorCode() +{ + if (!SSL_is_init_finished(native_handle())) { + return HandshakeNotCompleted; + } + + if (GetPeerCertificate() == nullptr) { + return NoPeerCertificate; + } + + return SSL_get_verify_result(native_handle()); +} + /** * Returns a human-readable error string for situations where IsVerifyOK() returns false. * @@ -50,16 +68,16 @@ bool UnbufferedAsioTlsStream::IsVerifyOK() */ String UnbufferedAsioTlsStream::GetVerifyError() { - if (!SSL_is_init_finished(native_handle())) { - return "handshake not completed"; - } + auto err (GetVerifyErrorCode()); - if (GetPeerCertificate() == nullptr) { - return "no peer certificate provided"; + switch (err) { + case HandshakeNotCompleted: + return "handshake not completed"; + case NoPeerCertificate: + return "no peer certificate provided"; } std::ostringstream buf; - long err = SSL_get_verify_result(native_handle()); buf << "code " << err << ": " << X509_verify_cert_error_string(err); return buf.str(); } diff --git a/lib/base/tlsstream.hpp b/lib/base/tlsstream.hpp index 9eed8d3b11..5aaadaa711 100644 --- a/lib/base/tlsstream.hpp +++ b/lib/base/tlsstream.hpp @@ -68,6 +68,11 @@ typedef SeenStream> AsioT class UnbufferedAsioTlsStream : public AsioTcpTlsStream { public: + // SSL_get_verify_result() doesn't return negative values YET, but it might in the future. + // Better safe with a margin than sorry with a collision. + static constexpr long HandshakeNotCompleted = -42; + static constexpr long NoPeerCertificate = -43; + inline UnbufferedAsioTlsStream(UnbufferedAsioTlsStreamParams& init) : AsioTcpTlsStream(init.IoContext, init.SslContext), m_Hostname(init.Hostname) @@ -75,6 +80,7 @@ class UnbufferedAsioTlsStream : public AsioTcpTlsStream } bool IsVerifyOK(); + long GetVerifyErrorCode(); String GetVerifyError(); std::shared_ptr GetPeerCertificate(); diff --git a/lib/remote/jsonrpcconnection-pki.cpp b/lib/remote/jsonrpcconnection-pki.cpp index 340e12b301..de19ddd0e5 100644 --- a/lib/remote/jsonrpcconnection-pki.cpp +++ b/lib/remote/jsonrpcconnection-pki.cpp @@ -17,6 +17,8 @@ #include #include #include +#include +#include using namespace icinga; @@ -80,6 +82,8 @@ Value RequestCertificateHandler(const MessageOrigin::Ptr& origin, const Dictiona std::shared_ptr parsedRequestorCA; X509* requestorCA = nullptr; + constexpr long handshakeVerifyErrorUnset = X509_V_OK; + long handshakeVerifyError = handshakeVerifyErrorUnset; if (signedByCA) { bool uptodate = IsCertUptodate(cert); @@ -121,6 +125,35 @@ Value RequestCertificateHandler(const MessageOrigin::Ptr& origin, const Dictiona } } + if (uptodate) { + // There's a bug causing malformed certificates to be issued sometimes. + // The handshake chain verification fails with X509_V_ERR_CERT_SIGNATURE_FAILURE, but (luckily) + // the certificate sent via pki::RequestCertificate gets properly recognized by VerifyCertificate(). + // The latter allows to self-heal the cluster by issuing a new certificate, + // despite the old one being "valid and uptodate". + + if (cn == origin->FromClient->GetIdentity()) { + handshakeVerifyError = tlsConn.GetVerifyErrorCode(); + } else { + Value hve; + + if (params->Get("handshake_verify_error", &hve) && hve.IsNumber()) { + handshakeVerifyError = hve; + } + } + + if (handshakeVerifyError == X509_V_ERR_CERT_SIGNATURE_FAILURE) { + // In the hypothetic case of a permanent failure to issue sane certificates, + // don't issue a new one on every re-connect: + + time_t threshold = time(nullptr) - 10 * 60; + + if (X509_cmp_time(X509_get_notBefore(cert.get()), &threshold) < 0) { + uptodate = false; + } + } + } + if (uptodate) { Log(LogInformation, "JsonRpcConnection") << "The certificates for CN '" << cn << "' and its root CA are valid and uptodate. Skipping automated renewal."; @@ -276,6 +309,10 @@ Value RequestCertificateHandler(const MessageOrigin::Ptr& origin, const Dictiona request->Set("requestor_ca", CertificateToString(requestorCA)); } + if (handshakeVerifyError != handshakeVerifyErrorUnset) { + request->Set("handshake_verify_error", handshakeVerifyError); + } + Utility::SaveJsonFile(requestPath, 0600, request); JsonRpcConnection::SendCertificateRequest(nullptr, origin, requestPath);