From 3504fc7ed688c10d86988e2029a65efc311393fe Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Wed, 16 Oct 2024 12:00:00 +0200 Subject: [PATCH 1/4] Security: fix TLS certificate validation bypass The previous validation in set_verify_callback() could be bypassed, tricking Icinga 2 into treating invalid certificates as valid. To fix this, the validation checks were moved into the IsVerifyOK() function. This is tracked as CVE-2024-49369, more details will be published at a later time. --- lib/base/tlsstream.cpp | 62 ++++++++++++++++++++++++++++++++---------- lib/base/tlsstream.hpp | 8 ++---- 2 files changed, 51 insertions(+), 19 deletions(-) diff --git a/lib/base/tlsstream.cpp b/lib/base/tlsstream.cpp index db54c919e..a71451a53 100644 --- a/lib/base/tlsstream.cpp +++ b/lib/base/tlsstream.cpp @@ -18,14 +18,48 @@ using namespace icinga; -bool UnbufferedAsioTlsStream::IsVerifyOK() const +/** + * Checks whether the TLS handshake was completed with a valid peer certificate. + * + * @return true if the peer presented a valid certificate, false otherwise + */ +bool UnbufferedAsioTlsStream::IsVerifyOK() { - return m_VerifyOK; + if (!SSL_is_init_finished(native_handle())) { + // handshake was not completed + return false; + } + + if (GetPeerCertificate() == nullptr) { + // no peer certificate was sent + return false; + } + + return SSL_get_verify_result(native_handle()) == X509_V_OK; } -String UnbufferedAsioTlsStream::GetVerifyError() const +/** + * Returns a human-readable error string for situations where IsVerifyOK() returns false. + * + * If the handshake was completed and a peer certificate was provided, + * the string additionally contains the OpenSSL verification error code. + * + * @return string containing the error message + */ +String UnbufferedAsioTlsStream::GetVerifyError() { - return m_VerifyError; + if (!SSL_is_init_finished(native_handle())) { + return "handshake not completed"; + } + + if (GetPeerCertificate() == nullptr) { + 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(); } std::shared_ptr UnbufferedAsioTlsStream::GetPeerCertificate() @@ -43,17 +77,17 @@ void UnbufferedAsioTlsStream::BeforeHandshake(handshake_type type) set_verify_mode(ssl::verify_peer | ssl::verify_client_once); - set_verify_callback([this](bool preverified, ssl::verify_context& ctx) { - if (!preverified) { - m_VerifyOK = false; - - std::ostringstream msgbuf; - int err = X509_STORE_CTX_get_error(ctx.native_handle()); - - msgbuf << "code " << err << ": " << X509_verify_cert_error_string(err); - m_VerifyError = msgbuf.str(); - } + set_verify_callback([](bool preverified, ssl::verify_context& ctx) { + (void) preverified; + (void) ctx; + /* Continue the handshake even if an invalid peer certificate was presented. The verification result has to be + * checked using the IsVerifyOK() method. + * + * Such connections are used for the initial enrollment of nodes where they use a self-signed certificate to + * send a certificate request and receive their valid certificate after approval (manually by the administrator + * or using a certificate ticket). + */ return true; }); diff --git a/lib/base/tlsstream.hpp b/lib/base/tlsstream.hpp index f6e52097e..9a6340baf 100644 --- a/lib/base/tlsstream.hpp +++ b/lib/base/tlsstream.hpp @@ -70,12 +70,12 @@ class UnbufferedAsioTlsStream : public AsioTcpTlsStream public: inline UnbufferedAsioTlsStream(UnbufferedAsioTlsStreamParams& init) - : AsioTcpTlsStream(init.IoContext, init.SslContext), m_VerifyOK(true), m_Hostname(init.Hostname) + : AsioTcpTlsStream(init.IoContext, init.SslContext), m_Hostname(init.Hostname) { } - bool IsVerifyOK() const; - String GetVerifyError() const; + bool IsVerifyOK(); + String GetVerifyError(); std::shared_ptr GetPeerCertificate(); template @@ -97,8 +97,6 @@ public: } private: - bool m_VerifyOK; - String m_VerifyError; String m_Hostname; void BeforeHandshake(handshake_type type); From 5ea8dcf471fcfa5d9a2c4b4ff3371a46b5aac228 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 16 Oct 2024 13:58:30 +0200 Subject: [PATCH 2/4] Bump OpenSSL shipped for Windows to v3.0.15 --- doc/win-dev.ps1 | 2 +- tools/win32/configure.ps1 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/win-dev.ps1 b/doc/win-dev.ps1 index 76d5122eb..65e698a81 100644 --- a/doc/win-dev.ps1 +++ b/doc/win-dev.ps1 @@ -14,7 +14,7 @@ function ThrowOnNativeFailure { $VsVersion = 2019 $MsvcVersion = '14.2' $BoostVersion = @(1, 83, 0) -$OpensslVersion = '3_0_12' +$OpensslVersion = '3_0_15' switch ($Env:BITS) { 32 { } diff --git a/tools/win32/configure.ps1 b/tools/win32/configure.ps1 index b6e903714..473ab768c 100644 --- a/tools/win32/configure.ps1 +++ b/tools/win32/configure.ps1 @@ -30,7 +30,7 @@ if (-not (Test-Path env:CMAKE_GENERATOR_PLATFORM)) { } } if (-not (Test-Path env:OPENSSL_ROOT_DIR)) { - $env:OPENSSL_ROOT_DIR = "c:\local\OpenSSL_3_0_12-Win${env:BITS}" + $env:OPENSSL_ROOT_DIR = "c:\local\OpenSSL_3_0_15-Win${env:BITS}" } if (-not (Test-Path env:BOOST_ROOT)) { $env:BOOST_ROOT = "c:\local\boost_1_83_0-Win${env:BITS}" From 0eb2949923d2483b919f2f0f19d2efe704842818 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Fri, 25 Oct 2024 16:39:51 +0200 Subject: [PATCH 3/4] Icinga 2.13.10 --- CHANGELOG.md | 9 +++++++++ ICINGA2_VERSION | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f46b9172..c5f5bb16d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ documentation before upgrading to a new release. Released closed milestones can be found on [GitHub](https://github.com/Icinga/icinga2/milestones?state=closed). +## 2.13.10 (2024-11-12) + +This security release fixes a TLS certificate validation bypass. +Given the severity of that issue, users are advised to upgrade all nodes immediately. + +* Security: fix TLS certificate validation bypass. CVE-2024-49369 +* Security: update OpenSSL shipped on Windows to v3.0.15. +* Windows: sign MSI packages with a certificate the OS trusts by default. + ## 2.13.9 (2023-12-21) Version 2.13.9 is a hotfix release for masters and satellites that mainly diff --git a/ICINGA2_VERSION b/ICINGA2_VERSION index 42da30970..27c840561 100644 --- a/ICINGA2_VERSION +++ b/ICINGA2_VERSION @@ -1,2 +1,2 @@ -Version: 2.13.9 +Version: 2.13.10 Revision: 1 From 6b362d6c93bece39507b4832660b1ce6d704e57d Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 22 Oct 2024 12:17:30 +0200 Subject: [PATCH 4/4] Resolve merge conflicts with support/2.13 This reverts commit 5ea8dcf471fcfa5d9a2c4b4ff3371a46b5aac228 which has already been cherry-picked there, but is also needed for v2.13.10. This has the same effect as `git merge support/2.13`, but involves no merge, no conflict resolution, less commits and a smaller diff. --- doc/win-dev.ps1 | 2 +- tools/win32/configure.ps1 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/win-dev.ps1 b/doc/win-dev.ps1 index 65e698a81..76d5122eb 100644 --- a/doc/win-dev.ps1 +++ b/doc/win-dev.ps1 @@ -14,7 +14,7 @@ function ThrowOnNativeFailure { $VsVersion = 2019 $MsvcVersion = '14.2' $BoostVersion = @(1, 83, 0) -$OpensslVersion = '3_0_15' +$OpensslVersion = '3_0_12' switch ($Env:BITS) { 32 { } diff --git a/tools/win32/configure.ps1 b/tools/win32/configure.ps1 index 473ab768c..b6e903714 100644 --- a/tools/win32/configure.ps1 +++ b/tools/win32/configure.ps1 @@ -30,7 +30,7 @@ if (-not (Test-Path env:CMAKE_GENERATOR_PLATFORM)) { } } if (-not (Test-Path env:OPENSSL_ROOT_DIR)) { - $env:OPENSSL_ROOT_DIR = "c:\local\OpenSSL_3_0_15-Win${env:BITS}" + $env:OPENSSL_ROOT_DIR = "c:\local\OpenSSL_3_0_12-Win${env:BITS}" } if (-not (Test-Path env:BOOST_ROOT)) { $env:BOOST_ROOT = "c:\local\boost_1_83_0-Win${env:BITS}"