Skip to content

Commit

Permalink
Merge pull request from GHSA-cxfm-8j5v-5qr2
Browse files Browse the repository at this point in the history
Add TLS server certificate validation to ElasticsearchWriter, GelfWriter and InfluxdbWriter (v2)
  • Loading branch information
N-o-X authored Aug 19, 2021
2 parents 236e10d + 08e2d9f commit f6e21d2
Show file tree
Hide file tree
Showing 10 changed files with 68 additions and 5 deletions.
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,19 @@ documentation before upgrading to a new release.

Released closed milestones can be found on [GitHub](https://github.com/Icinga/icinga2/milestones?state=closed).

## 2.11.11 (2021-08-19)

The main focus of these versions is a security vulnerability in the TLS certificate verification of our metrics writers ElasticsearchWriter, GelfWriter and InfluxdbWriter.

### Security

* Add TLS server certificate validation to ElasticsearchWriter, GelfWriter and InfluxdbWriter

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

## 2.11.10 (2021-07-15)

Version 2.11.10 fixes two security vulnerabilities that may lead to privilege
Expand Down
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
Version: 2.11.10
Version: 2.11.11
Revision: 1
3 changes: 3 additions & 0 deletions doc/09-object-types.md
Original file line number Diff line number Diff line change
Expand Up @@ -1229,6 +1229,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`.
Expand Down Expand Up @@ -1316,6 +1317,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`.
Expand Down Expand Up @@ -1611,6 +1613,7 @@ Configuration Attributes:
username | String | **Optional.** InfluxDB user name. Defaults to `none`.
password | String | **Optional.** InfluxDB user password. Defaults to `none`.
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.
Expand Down
4 changes: 4 additions & 0 deletions lib/base/tlsstream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
12 changes: 12 additions & 0 deletions lib/perfdata/elasticsearchwriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,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);
Expand Down
3 changes: 3 additions & 0 deletions lib/perfdata/elasticsearchwriter.ti
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
18 changes: 14 additions & 4 deletions lib/perfdata/gelfwriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,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();
}
Expand Down Expand Up @@ -196,6 +194,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);
Expand Down
3 changes: 3 additions & 0 deletions lib/perfdata/gelfwriter.ti
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 12 additions & 0 deletions lib/perfdata/influxdbwriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,18 @@ OptionalTlsStream InfluxdbWriter::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);
Expand Down
3 changes: 3 additions & 0 deletions lib/perfdata/influxdbwriter.ti
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ class InfluxdbWriter : ConfigObject
[config] bool ssl_enable {
default {{{ return false; }}}
};
[config] bool ssl_insecure_noverify {
default {{{ return false; }}}
};
[config] String ssl_ca_cert {
default {{{ return ""; }}}
};
Expand Down

0 comments on commit f6e21d2

Please sign in to comment.