ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-09-26 23:19:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(telemetry): require https for metrics_endpoint under mTLS

Browse Source 
With tls_client_cert set, only traces_endpoint was checked for an https
scheme. Telemetry::makeMetricExporter() attaches the client certificate and
key to the metric exporter whenever use_tls=1, and metrics_endpoint defaults
to a plain http URL, so an operator who set up mTLS and overrode only
traces_endpoint exported every metric in the clear with the configured client
identity unused.

Check both endpoints, and state the requirement under metrics_endpoint and
tls_client_cert in the example config. Four config tests cover an explicit
http metrics endpoint, the omitted-key default, both endpoints on https, and
a one-way-TLS control that must stay accepted.
This commit is contained in:
Pratik Mankawde
2026-09-09 12:25:17 +01:00
parent b978b52028
commit bcf2d098d3
3 changed files with 101 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
cfg/xrpld-example.cfg
View File

@@ -1762,12 +1762,13 @@ validators.txt
# To enable mTLS, both tls_client_cert and tls_client_key must be
# specified. If only one is provided, xrpld will fail to start. Providing
# them while use_tls=0 also fails to start, rather than being ignored.
# traces_endpoint must be an https:// URL, because that scheme is what
# makes the exporter present the certificate at all. With use_tls=1 each
# path is opened at startup, so one that does not exist or cannot be read
# fails to start too, rather than failing later as an opaque TLS
# handshake error. All four checks apply only when enabled=1; with
# telemetry disabled these settings are read but never validated.
# traces_endpoint and metrics_endpoint must both be https:// URLs, because
# that scheme is what makes each exporter present the certificate at all.
# With use_tls=1 each path is opened at startup, so one that does not
# exist or cannot be read fails to start too, rather than failing later
# as an opaque TLS handshake error. All four checks apply only when
# enabled=1; with telemetry disabled these settings are read but never
# validated.
#
# tls_client_key=
#
@@ -1853,6 +1854,12 @@ validators.txt
# defaults.
# Default: http://localhost:4318/v1/metrics.
#
# The scheme decides encryption here exactly as it does for
# traces_endpoint, so setting tls_client_cert requires this URL to start
# with https:// as well — including leaving it at the default above.
# Overriding only traces_endpoint therefore makes xrpld fail to start,
# rather than export metrics without the client identity.
#
# metric_export_interval_ms=1000
#
# Gap in milliseconds between two metric exports. Must be greater than 0.

15
src/libxrpl/telemetry/TelemetryConfig.cpp
View File

@@ -387,13 +387,18 @@ makeTelemetrySetup(
}
// Still inside the enabled branch, and checked before the files are
// opened so a scheme problem is not hidden behind a path problem. The
// exporter reads TLS off the endpoint scheme, so a client certificate is
// only presented on an https endpoint. tls_ca_cert is left out of this
// check: it only names a trust store, while a client certificate is this
// node's own identity and has to reach the collector to mean anything.
// opened so a scheme problem is not hidden behind a path problem. Each
// exporter reads TLS off its own endpoint scheme, and both are handed
// the client certificate, so both endpoints have to be https. Checking
// only one leaves the other signal exporting in the clear without this
// node's identity. tls_ca_cert is left out of this check: it only names
// a trust store, while a client certificate is this node's own identity
// and has to reach the collector to mean anything.
if (!setup.tlsClientCertPath.empty())
{
requireHttpsEndpoint(setup.tracesEndpoint, key::tracesEndpoint);
requireHttpsEndpoint(setup.metricsEndpoint, key::metricsEndpoint);
}
// Still inside the enabled branch. The exporter opens these files only
// when TLS is on, so check them only then: a bad path behind use_tls=0

78
src/tests/libxrpl/telemetry/TelemetryConfig.cpp
View File

@@ -79,6 +79,19 @@ constexpr char const* httpsEndpoint = "https://collector:4318/v1/traces";
constexpr char const* defaultEndpoint = "http://localhost:4318/v1/traces";
constexpr char const* schemeError = "must start with 'https://'";
/**
* The same four values for the metric signal.
*
* The guard covers both endpoints, so every case that expects parsing to
* succeed has to set this key too. Spelled separately from the trace values so
* a case can put one signal on https and the other on http, which is the
* configuration that used to pass.
*/
constexpr char const* keyMetricsEndpoint = "metrics_endpoint";
constexpr char const* metricsHttpEndpoint = "http://collector:4318/v1/metrics";
constexpr char const* metricsHttpsEndpoint = "https://collector:4318/v1/metrics";
constexpr char const* defaultMetricsEndpoint = "http://localhost:4318/v1/metrics";
/**
* Build a [telemetry] section carrying only the `enabled` key.
*
@@ -366,6 +379,7 @@ TEST(TelemetryConfig, mtls_cert_and_key_both_set)
Section section = mtls::makeSection(true);
section.set("use_tls", "1");
section.set(mtls::keyEndpoint, mtls::httpsEndpoint);
section.set(mtls::keyMetricsEndpoint, mtls::metricsHttpsEndpoint);
section.set(mtls::keyClientCert, cert);
section.set(mtls::keyClientKey, key);
@@ -568,6 +582,7 @@ TEST(TelemetryConfig, tls_readable_files_are_accepted)
Section section = mtls::makeSection(true);
section.set("use_tls", "1");
section.set(mtls::keyEndpoint, mtls::httpsEndpoint);
section.set(mtls::keyMetricsEndpoint, mtls::metricsHttpsEndpoint);
section.set("tls_ca_cert", ca);
section.set(mtls::keyClientCert, cert);
section.set(mtls::keyClientKey, key);
@@ -666,22 +681,85 @@ TEST(TelemetryConfig, mtls_client_cert_on_an_https_endpoint_is_accepted)
{
// The same configuration as the two cases above with only the scheme
// changed, so nothing but the scheme can explain the different outcome.
// Both endpoints are https, which is the only shape the guard accepts.
TempDir const dir;
auto const cert = mtls::writeCertFile(dir.file("c.pem"));
auto const key = mtls::writeCertFile(dir.file("k.pem"));
Section section = mtls::makeSection(true);
section.set("use_tls", "1");
section.set(mtls::keyEndpoint, mtls::httpsEndpoint);
section.set(mtls::keyMetricsEndpoint, mtls::metricsHttpsEndpoint);
section.set(mtls::keyClientCert, cert);
section.set(mtls::keyClientKey, key);
telemetry::Telemetry::Setup setup;
ASSERT_NO_THROW(setup = mtls::parseSection(section));
EXPECT_EQ(setup.tracesEndpoint, mtls::httpsEndpoint);
EXPECT_EQ(setup.metricsEndpoint, mtls::metricsHttpsEndpoint);
EXPECT_EQ(setup.tlsClientCertPath, cert);
EXPECT_EQ(setup.tlsClientKeyPath, key);
}
TEST(TelemetryConfig, mtls_client_cert_on_a_plain_http_metrics_endpoint_throws)
{
// traces_endpoint is https and only metrics_endpoint is not, so the trace
// guard cannot be what fires. Before the metric endpoint was checked this
// configuration started the node and exported every metric in the clear,
// with the client certificate attached to the exporter and never used.
TempDir const dir;
Section section = mtls::makeSection(true);
section.set("use_tls", "1");
section.set(mtls::keyEndpoint, mtls::httpsEndpoint);
section.set(mtls::keyMetricsEndpoint, mtls::metricsHttpEndpoint);
section.set(mtls::keyClientCert, mtls::writeCertFile(dir.file("c.pem")));
section.set(mtls::keyClientKey, mtls::writeCertFile(dir.file("k.pem")));
EXPECT_THAT(
[&section] { mtls::parseSection(section); },
ThrowsMessage<std::runtime_error>(AllOf(
HasSubstr(mtls::schemeError),
HasSubstr(mtls::keyMetricsEndpoint),
HasSubstr(mtls::metricsHttpEndpoint))));
}
TEST(TelemetryConfig, mtls_client_cert_with_the_default_metrics_endpoint_throws)
{
// The key is absent, so the built-in default applies, and that default is a
// plain http URL. This is the shape an operator reaches by setting up mTLS
// and overriding only traces_endpoint, which makes it the case worth having.
TempDir const dir;
Section section = mtls::makeSection(true);
section.set("use_tls", "1");
section.set(mtls::keyEndpoint, mtls::httpsEndpoint);
section.set(mtls::keyClientCert, mtls::writeCertFile(dir.file("c.pem")));
section.set(mtls::keyClientKey, mtls::writeCertFile(dir.file("k.pem")));
EXPECT_THAT(
[&section] { mtls::parseSection(section); },
ThrowsMessage<std::runtime_error>(AllOf(
HasSubstr(mtls::schemeError),
HasSubstr(mtls::keyMetricsEndpoint),
HasSubstr(mtls::defaultMetricsEndpoint))));
}
TEST(TelemetryConfig, one_way_tls_on_a_plain_http_metrics_endpoint_is_accepted)
{
// The control for the metric guard's scope, matching the trace one below:
// same plain http metrics endpoint and use_tls=1, but no client identity to
// lose. Widen the guard to every use_tls=1 node and this case starts failing.
TempDir const dir;
auto const ca = mtls::writeCertFile(dir.file("ca.pem"));
Section section = mtls::makeSection(true);
section.set("use_tls", "1");
section.set(mtls::keyMetricsEndpoint, mtls::metricsHttpEndpoint);
section.set("tls_ca_cert", ca);
telemetry::Telemetry::Setup setup;
ASSERT_NO_THROW(setup = mtls::parseSection(section));
EXPECT_EQ(setup.metricsEndpoint, mtls::metricsHttpEndpoint);
EXPECT_TRUE(setup.tlsClientCertPath.empty());
}
TEST(TelemetryConfig, mtls_scheme_check_is_case_sensitive_like_the_exporter)
{
// The exporter compares the scheme byte for byte, so "HTTPS://" leaves it