From 6080538223c29ca83317943f8ec4998d1cdf1853 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Fri, 13 Aug 2021 09:28:57 +0200 Subject: [PATCH 1/7] Enable hostname verification in UnbufferedAsioTlsStream --- lib/base/tlsstream.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/base/tlsstream.cpp b/lib/base/tlsstream.cpp index b72a88030..db54c919e 100644 --- a/lib/base/tlsstream.cpp +++ b/lib/base/tlsstream.cpp @@ -37,6 +37,10 @@ void UnbufferedAsioTlsStream::BeforeHandshake(handshake_type type) { namespace ssl = boost::asio::ssl; + if (!m_Hostname.IsEmpty()) { + X509_VERIFY_PARAM_set1_host(SSL_get0_param(native_handle()), m_Hostname.CStr(), m_Hostname.GetLength()); + } + set_verify_mode(ssl::verify_peer | ssl::verify_client_once); set_verify_callback([this](bool preverified, ssl::verify_context& ctx) { From 055cdf22a83ee0e0ae7b57511c569e45c2434d5f Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Thu, 12 Aug 2021 16:42:23 +0200 Subject: [PATCH 2/7] ElasticsearchWriter: actually verify TLS server certificates And add a new option insecure_noverify to explicitly disable it if desired. --- doc/09-object-types.md | 1 + lib/perfdata/elasticsearchwriter.cpp | 12 ++++++++++++ lib/perfdata/elasticsearchwriter.ti | 3 +++ 3 files changed, 16 insertions(+) diff --git a/doc/09-object-types.md b/doc/09-object-types.md index 554b8f794..4adbe7aea 100644 --- a/doc/09-object-types.md +++ b/doc/09-object-types.md @@ -1238,6 +1238,7 @@ Configuration Attributes: username | String | **Optional.** Basic auth username if Elasticsearch is hidden behind an HTTP proxy. password | String | **Optional.** Basic auth password if Elasticsearch is hidden behind an HTTP proxy. enable\_tls | Boolean | **Optional.** Whether to use a TLS stream. Defaults to `false`. Requires an HTTP proxy. + insecure\_noverify | Boolean | **Optional.** Disable TLS peer verification. ca\_path | String | **Optional.** Path to CA certificate to validate the remote host. Requires `enable_tls` set to `true`. cert\_path | String | **Optional.** Path to host certificate to present to the remote host for mutual verification. Requires `enable_tls` set to `true`. key\_path | String | **Optional.** Path to host key to accompany the cert\_path. Requires `enable_tls` set to `true`. diff --git a/lib/perfdata/elasticsearchwriter.cpp b/lib/perfdata/elasticsearchwriter.cpp index e313e8110..477616b23 100644 --- a/lib/perfdata/elasticsearchwriter.cpp +++ b/lib/perfdata/elasticsearchwriter.cpp @@ -632,6 +632,18 @@ OptionalTlsStream ElasticsearchWriter::Connect() << "TLS handshake with host '" << GetHost() << "' on port " << GetPort() << " failed."; throw; } + + if (!GetInsecureNoverify()) { + if (!tlsStream.GetPeerCertificate()) { + BOOST_THROW_EXCEPTION(std::runtime_error("Elasticsearch didn't present any TLS certificate.")); + } + + if (!tlsStream.IsVerifyOK()) { + BOOST_THROW_EXCEPTION(std::runtime_error( + "TLS certificate validation failed: " + std::string(tlsStream.GetVerifyError()) + )); + } + } } return std::move(stream); diff --git a/lib/perfdata/elasticsearchwriter.ti b/lib/perfdata/elasticsearchwriter.ti index a072220de..e3b8e27f5 100644 --- a/lib/perfdata/elasticsearchwriter.ti +++ b/lib/perfdata/elasticsearchwriter.ti @@ -29,6 +29,9 @@ class ElasticsearchWriter : ConfigObject [config] bool enable_tls { default {{{ return false; }}} }; + [config] bool insecure_noverify { + default {{{ return false; }}} + }; [config] String ca_path; [config] String cert_path; [config] String key_path; From 4855cbc23253e3c994f9aae6ce0e63e9ee46d8a7 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Thu, 12 Aug 2021 16:43:29 +0200 Subject: [PATCH 3/7] GelfWriter: actually verify TLS server certificates And add a new option insecure_noverify to explicitly disable it if desired. --- doc/09-object-types.md | 1 + lib/perfdata/gelfwriter.cpp | 12 ++++++++++++ lib/perfdata/gelfwriter.ti | 3 +++ 3 files changed, 16 insertions(+) diff --git a/doc/09-object-types.md b/doc/09-object-types.md index 4adbe7aea..454ecce3c 100644 --- a/doc/09-object-types.md +++ b/doc/09-object-types.md @@ -1326,6 +1326,7 @@ Configuration Attributes: enable\_send\_perfdata | Boolean | **Optional.** Enable performance data for 'CHECK RESULT' events. enable\_ha | Boolean | **Optional.** Enable the high availability functionality. Only valid in a [cluster setup](06-distributed-monitoring.md#distributed-monitoring-high-availability-features). Defaults to `false`. enable\_tls | Boolean | **Optional.** Whether to use a TLS stream. Defaults to `false`. + insecure\_noverify | Boolean | **Optional.** Disable TLS peer verification. ca\_path | String | **Optional.** Path to CA certificate to validate the remote host. Requires `enable_tls` set to `true`. cert\_path | String | **Optional.** Path to host certificate to present to the remote host for mutual verification. Requires `enable_tls` set to `true`. key\_path | String | **Optional.** Path to host key to accompany the cert\_path. Requires `enable_tls` set to `true`. diff --git a/lib/perfdata/gelfwriter.cpp b/lib/perfdata/gelfwriter.cpp index 78b2bdcef..dc9f34f96 100644 --- a/lib/perfdata/gelfwriter.cpp +++ b/lib/perfdata/gelfwriter.cpp @@ -206,6 +206,18 @@ void GelfWriter::ReconnectInternal() << "TLS handshake with host '" << GetHost() << " failed.'"; throw; } + + if (!GetInsecureNoverify()) { + if (!tlsStream.GetPeerCertificate()) { + BOOST_THROW_EXCEPTION(std::runtime_error("Graylog Gelf didn't present any TLS certificate.")); + } + + if (!tlsStream.IsVerifyOK()) { + BOOST_THROW_EXCEPTION(std::runtime_error( + "TLS certificate validation failed: " + std::string(tlsStream.GetVerifyError()) + )); + } + } } SetConnected(true); diff --git a/lib/perfdata/gelfwriter.ti b/lib/perfdata/gelfwriter.ti index 2176fd877..387ee1487 100644 --- a/lib/perfdata/gelfwriter.ti +++ b/lib/perfdata/gelfwriter.ti @@ -34,6 +34,9 @@ class GelfWriter : ConfigObject [config] bool enable_tls { default {{{ return false; }}} }; + [config] bool insecure_noverify { + default {{{ return false; }}} + }; [config] String ca_path; [config] String cert_path; [config] String key_path; From 50a798070997459d7005168d2fe2af87b8000c15 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Thu, 12 Aug 2021 16:43:55 +0200 Subject: [PATCH 4/7] InfluxdbCommonWriter: actually verify TLS server certificates And add a new option ssl_insecure_noverify to explicitly disable it if desired. --- doc/09-object-types.md | 2 ++ lib/perfdata/influxdbcommonwriter.cpp | 12 ++++++++++++ lib/perfdata/influxdbcommonwriter.ti | 3 +++ 3 files changed, 17 insertions(+) diff --git a/doc/09-object-types.md b/doc/09-object-types.md index 454ecce3c..e9562b5df 100644 --- a/doc/09-object-types.md +++ b/doc/09-object-types.md @@ -1664,6 +1664,7 @@ Configuration Attributes: password | String | **Optional.** InfluxDB user password. Defaults to `none`. basic\_auth | Dictionary | **Optional.** Username and password for HTTP basic authentication. ssl\_enable | Boolean | **Optional.** Whether to use a TLS stream. Defaults to `false`. + ssl\_insecure\_noverify | Boolean | **Optional.** Disable TLS peer verification. ssl\_ca\_cert | String | **Optional.** Path to CA certificate to validate the remote host. ssl\_cert | String | **Optional.** Path to host certificate to present to the remote host for mutual verification. ssl\_key | String | **Optional.** Path to host key to accompany the ssl\_cert. @@ -1726,6 +1727,7 @@ Configuration Attributes: bucket | String | **Required.** InfluxDB bucket name. auth\_token | String | **Required.** InfluxDB authentication token. ssl\_enable | Boolean | **Optional.** Whether to use a TLS stream. Defaults to `false`. + ssl\_insecure\_noverify | Boolean | **Optional.** Disable TLS peer verification. ssl\_ca\_cert | String | **Optional.** Path to CA certificate to validate the remote host. ssl\_cert | String | **Optional.** Path to host certificate to present to the remote host for mutual verification. ssl\_key | String | **Optional.** Path to host key to accompany the ssl\_cert. diff --git a/lib/perfdata/influxdbcommonwriter.cpp b/lib/perfdata/influxdbcommonwriter.cpp index d85426f44..1aafffced 100644 --- a/lib/perfdata/influxdbcommonwriter.cpp +++ b/lib/perfdata/influxdbcommonwriter.cpp @@ -187,6 +187,18 @@ OptionalTlsStream InfluxdbCommonWriter::Connect() << "TLS handshake with host '" << GetHost() << "' failed."; throw; } + + if (!GetSslInsecureNoverify()) { + if (!tlsStream.GetPeerCertificate()) { + BOOST_THROW_EXCEPTION(std::runtime_error("InfluxDB didn't present any TLS certificate.")); + } + + if (!tlsStream.IsVerifyOK()) { + BOOST_THROW_EXCEPTION(std::runtime_error( + "TLS certificate validation failed: " + std::string(tlsStream.GetVerifyError()) + )); + } + } } return std::move(stream); diff --git a/lib/perfdata/influxdbcommonwriter.ti b/lib/perfdata/influxdbcommonwriter.ti index 7eb26dac9..5cfe83f1a 100644 --- a/lib/perfdata/influxdbcommonwriter.ti +++ b/lib/perfdata/influxdbcommonwriter.ti @@ -18,6 +18,9 @@ abstract class InfluxdbCommonWriter : ConfigObject [config] bool ssl_enable { default {{{ return false; }}} }; + [config] bool ssl_insecure_noverify { + default {{{ return false; }}} + }; [config] String ssl_ca_cert { default {{{ return ""; }}} }; From f0b1195b4a89a8c1d90ec1c747837d4bc972a824 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Thu, 12 Aug 2021 17:01:49 +0200 Subject: [PATCH 5/7] GelfWriter: show error message of exceptions --- lib/perfdata/gelfwriter.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/perfdata/gelfwriter.cpp b/lib/perfdata/gelfwriter.cpp index dc9f34f96..f8ebb61d3 100644 --- a/lib/perfdata/gelfwriter.cpp +++ b/lib/perfdata/gelfwriter.cpp @@ -135,10 +135,8 @@ void GelfWriter::AssertOnWorkQueue() void GelfWriter::ExceptionHandler(boost::exception_ptr exp) { - Log(LogCritical, "GelfWriter", "Exception during Graylog Gelf operation: Verify that your backend is operational!"); - - Log(LogDebug, "GelfWriter") - << "Exception during Graylog Gelf operation: " << DiagnosticInformation(std::move(exp)); + Log(LogCritical, "GelfWriter") << "Exception during Graylog Gelf operation: " << DiagnosticInformation(exp, false); + Log(LogDebug, "GelfWriter") << "Exception during Graylog Gelf operation: " << DiagnosticInformation(exp, true); DisconnectInternal(); } From b98f939c7f7ad56391a19331266e7c7db0f518f2 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Fri, 13 Aug 2021 11:15:14 +0200 Subject: [PATCH 6/7] RedisConnection: remove now redundant setting of TLS verification parameters This is now done in UnbufferedAsioTlsStream. --- lib/icingadb/redisconnection.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/lib/icingadb/redisconnection.cpp b/lib/icingadb/redisconnection.cpp index bfd04e933..1c1fff080 100644 --- a/lib/icingadb/redisconnection.cpp +++ b/lib/icingadb/redisconnection.cpp @@ -274,13 +274,6 @@ void RedisConnection::Connect(asio::yield_context& yc) auto connectTimeout (MakeTimeout(conn)); Defer cancelTimeout ([&connectTimeout]() { connectTimeout->Cancel(); }); - if (!m_Insecure) { - auto native (tlsConn.native_handle()); - - X509_VERIFY_PARAM_set1_host(SSL_get0_param(native), m_Host.CStr(), 0); - SSL_set_verify(native, SSL_VERIFY_PEER, NULL); - } - icinga::Connect(conn->lowest_layer(), m_Host, Convert::ToString(m_Port), yc); tlsConn.async_handshake(tlsConn.client, yc); From 4951cdf5610f6ab37ac86263143d99a9c0c8beb3 Mon Sep 17 00:00:00 2001 From: Noah Hilverling Date: Thu, 19 Aug 2021 11:17:45 +0200 Subject: [PATCH 7/7] Add security changelog for 2.13.1 --- CHANGELOG.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41b15915a..f23820835 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,18 @@ Released closed milestones can be found on [GitHub](https://github.com/Icinga/ic ## 2.13.1 (2021-08-19) -Version 2.13.1 fixes two issues indroduced with the 2.13.0 release. +The main focus of this version is a security vulnerability in the TLS certificate verification of our metrics writers ElasticsearchWriter, GelfWriter, InfluxdbWriter and Influxdb2Writer. + +Version 2.13.1 also fixes two issues indroduced with the 2.13.0 release. + +### Security + +* Add TLS server certificate validation to ElasticsearchWriter, GelfWriter, InfluxdbWriter and Influxdb2Writer ([GHSA-cxfm-8j5v-5qr2](https://github.com/Icinga/icinga2/security/advisories/GHSA-cxfm-8j5v-5qr2)) + +Depending on your setup, manual intervention beyond installing the new versions +may be required, so please read the more detailed information in the +[release blog post](https://icinga.com/blog/2021/08/19/icinga-2-13-1-security-release//) +carefully ### Bugfixes