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

fix: Report telemetry config errors instead of aborting at startup

Browse Source 
makeTelemetrySetup() rejects a contradictory [telemetry] mutual-TLS
setup by throwing, but it is called from ApplicationImp's
member-initializer list. A try/catch in the constructor body cannot
reach a throw from there, and nothing further up the stack caught it
either, so a config mistake reached std::terminate: the default handler
printed a terminate dump and raised SIGABRT, leaving a core file
instead of a startup error.

Catch std::exception around makeApplication() in run(), report the
reason on stderr and return -1, so the failure is a clean non-zero exit
with a message an operator can act on. Only the construction is
wrapped. setup() starts subsystems whose shutdown order is delicate and
is left outside deliberately, because unwinding a half-started
Application would skip the normal stop sequence.

Gate both validation guards on enabled. A node with telemetry switched
off previously refused to start over certificate paths that nothing
would read.

Document both throws on makeTelemetrySetup(), state in
cfg/xrpld-example.cfg and the configuration reference that a partial
mutual-TLS setup is fatal and that the checks apply only when
enabled=1, and add a runbook troubleshooting entry keyed on the two
error messages.

Tests cover both guards with the message asserted so the two are told
apart, both enabled=0 paths, and the default plaintext configuration.
This commit is contained in:
Pratik Mankawde
2026-08-20 16:14:56 +01:00
parent 45ad80c57a
commit 89b58da1e8
7 changed files with 273 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -108,24 +108,37 @@ makeTelemetrySetup(
setup.tlsClientCertPath = section.valueOr<std::string>(key::tlsClientCert, "");
setup.tlsClientKeyPath = section.valueOr<std::string>(key::tlsClientKey, "");
// Mutual TLS needs both the client certificate and its private key.
// Supplying only one fails later with a cryptic SSL handshake error, so
// reject the partial configuration here with an actionable message.
if (setup.tlsClientCertPath.empty() != setup.tlsClientKeyPath.empty())
// The mutual TLS (mTLS) checks below are fatal, so gate them on the one
// thing this parser can know: `enabled` is 1. With `enabled` 0 a leftover
// cert line must never stop the node from booting.
//
// The predicate is only that config switch, not whether an exporter can
// exist. This file has no preprocessor guard, so both checks also run in a
// -Dtelemetry=OFF build, where makeTelemetry() returns the null
// implementation whatever `enabled` says.
if (setup.enabled)
{
Throw<std::runtime_error>(
"[telemetry] tls_client_cert and tls_client_key must be set together "
"(set both for mutual TLS, or neither for one-way TLS).");
}
// mTLS needs both the client certificate and its private key.
// Supplying only one fails later with a cryptic SSL handshake error, so
// reject the partial configuration here with an actionable message.
if (setup.tlsClientCertPath.empty() != setup.tlsClientKeyPath.empty())
{
Throw<std::runtime_error>(
"[telemetry] tls_client_cert and tls_client_key must be set together "
"(set both for mutual TLS, or neither for one-way TLS).");
}
// Mutual TLS only takes effect when TLS is on. Certificate paths set with
// use_tls=0 would be silently ignored and the exporter would connect in
// plaintext, so reject that contradiction instead of failing open.
if (!setup.tlsClientCertPath.empty() && !setup.useTls)
{
Throw<std::runtime_error>(
"[telemetry] tls_client_cert/tls_client_key require use_tls=1 "
"(set use_tls=1 to enable mutual TLS, or remove the cert paths).");
// Still inside the enabled branch. mTLS only takes effect when TLS is
// on, so a client certificate set with use_tls=0 would be ignored and
// any exporter that did run would connect in plaintext. Reject that
// contradiction instead of failing open. tls_ca_cert is deliberately
// not checked this way.
if (!setup.tlsClientCertPath.empty() && !setup.useTls)
{
Throw<std::runtime_error>(
"[telemetry] tls_client_cert/tls_client_key require use_tls=1 "
"(set use_tls=1 to enable mutual TLS, or remove the cert paths).");
}
}
// Head sampling is intentionally fixed at 1.0 (sample everything) and is

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

@@ -2,12 +2,83 @@
#include <xrpl/config/BasicConfig.h>
#include <xrpl/telemetry/Telemetry.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <stdexcept>
using namespace xrpl;
using ::testing::HasSubstr;
using ::testing::ThrowsMessage;
namespace {
/**
* Shared inputs for the mutual TLS (mTLS) tests of makeTelemetrySetup().
*
* keyClientCert and keyClientKey are the config key names, named once so every
* test below spells them the same way, mirroring the `key::` constants the
* parser itself uses. A misspelling cannot hide here: the throwing case that
* names the misspelled key stops throwing, the use_tls case throws the pairing
* message instead and fails its matcher, and the value cases see an empty path
* or an unexpected throw. Tests that never set the key are unaffected. One
* source of truth still keeps the two files from drifting apart.
*
* clientCert and clientKey are the paths written to those keys. They are
* declared as `char const*` so they pass to Section::set() (which takes
* `std::string const&`) and compare against the parsed std::string members
* without an explicit conversion, exactly as a literal would.
*
* pairingError and useTlsError are message fragments. Both guards throw
* std::runtime_error, so the exception type alone cannot tell them apart.
* Each fragment occurs in exactly one of the two messages, so matching it
* proves which guard fired.
*/
namespace mtls {
constexpr char const* keyClientCert = "tls_client_cert";
constexpr char const* keyClientKey = "tls_client_key";
constexpr char const* clientCert = "/etc/ssl/client.pem";
constexpr char const* clientKey = "/etc/ssl/client.key";
constexpr char const* pairingError = "must be set together";
constexpr char const* useTlsError = "require use_tls=1";
/**
* Build a [telemetry] section carrying only the `enabled` key.
*
* Every mTLS test states `enabled` explicitly, because the validation
* guards run only when telemetry is on. Each test then adds the TLS keys its
* own case needs on top of the returned section.
*
* @param telemetryEnabled Value written to the `enabled` key.
* @return The section, ready for further set() calls.
*/
Section
makeSection(bool telemetryEnabled)
{
Section section;
section.set("enabled", telemetryEnabled ? "1" : "0");
return section;
}
/**
* Parse a [telemetry] section with a fixed placeholder node identity.
*
* Keeps the node key, version and network ID out of the individual cases,
* which vary only in their TLS keys.
*
* @param section The section to parse.
* @return The populated Setup struct.
*/
telemetry::Telemetry::Setup
parseSection(Section const& section)
{
return telemetry::makeTelemetrySetup(section, "nHUtest123", "2.0.0", 0);
}
} // namespace mtls
} // namespace
TEST(TelemetryConfig, setup_defaults)
{
telemetry::Telemetry::Setup const s;
@@ -87,39 +158,110 @@ TEST(TelemetryConfig, parse_full_section)
TEST(TelemetryConfig, mtls_cert_and_key_both_set)
{
Section section;
// Telemetry on and use_tls=1, so both guards run and neither may fire.
Section section = mtls::makeSection(true);
section.set("use_tls", "1");
section.set("tls_client_cert", "/etc/ssl/client.pem");
section.set("tls_client_key", "/etc/ssl/client.key");
section.set(mtls::keyClientCert, mtls::clientCert);
section.set(mtls::keyClientKey, mtls::clientKey);
auto setup = telemetry::makeTelemetrySetup(section, "nHUtest123", "2.0.0", 0);
EXPECT_EQ(setup.tlsClientCertPath, "/etc/ssl/client.pem");
EXPECT_EQ(setup.tlsClientKeyPath, "/etc/ssl/client.key");
auto const setup = mtls::parseSection(section);
EXPECT_TRUE(setup.enabled);
EXPECT_TRUE(setup.useTls);
EXPECT_EQ(setup.tlsClientCertPath, mtls::clientCert);
EXPECT_EQ(setup.tlsClientKeyPath, mtls::clientKey);
}
TEST(TelemetryConfig, mtls_cert_without_key_throws)
{
Section section;
section.set("tls_client_cert", "/etc/ssl/client.pem");
EXPECT_THROW(
telemetry::makeTelemetrySetup(section, "nHUtest123", "2.0.0", 0), std::runtime_error);
// Only the cert is set, so the pairing guard is the one that must fire.
Section section = mtls::makeSection(true);
section.set(mtls::keyClientCert, mtls::clientCert);
EXPECT_THAT(
[&section] { mtls::parseSection(section); },
ThrowsMessage<std::runtime_error>(HasSubstr(mtls::pairingError)));
}
TEST(TelemetryConfig, mtls_key_without_cert_throws)
{
Section section;
section.set("tls_client_key", "/etc/ssl/client.key");
EXPECT_THROW(
telemetry::makeTelemetrySetup(section, "nHUtest123", "2.0.0", 0), std::runtime_error);
// Only the key is set, the mirror image of the case above.
Section section = mtls::makeSection(true);
section.set(mtls::keyClientKey, mtls::clientKey);
EXPECT_THAT(
[&section] { mtls::parseSection(section); },
ThrowsMessage<std::runtime_error>(HasSubstr(mtls::pairingError)));
}
TEST(TelemetryConfig, mtls_cert_key_without_use_tls_throws)
{
// Both paths are set, so the pairing guard cannot fire; use_tls is absent
// and defaults to 0, so the use_tls guard is the only reachable throw.
Section section = mtls::makeSection(true);
section.set(mtls::keyClientCert, mtls::clientCert);
section.set(mtls::keyClientKey, mtls::clientKey);
EXPECT_THAT(
[&section] { mtls::parseSection(section); },
ThrowsMessage<std::runtime_error>(HasSubstr(mtls::useTlsError)));
}
TEST(TelemetryConfig, mtls_contradiction_ignored_when_telemetry_disabled)
{
// The use_tls contradiction with telemetry off: parsing must succeed so a
// stale cert line cannot stop the node from booting.
Section section = mtls::makeSection(false);
section.set(mtls::keyClientCert, mtls::clientCert);
section.set(mtls::keyClientKey, mtls::clientKey);
auto const setup = mtls::parseSection(section);
EXPECT_FALSE(setup.enabled);
EXPECT_FALSE(setup.useTls);
EXPECT_EQ(setup.tlsClientCertPath, mtls::clientCert);
EXPECT_EQ(setup.tlsClientKeyPath, mtls::clientKey);
}
TEST(TelemetryConfig, mtls_cert_without_key_ignored_when_telemetry_disabled)
{
// The pairing violation with telemetry off: also parsed, not rejected.
Section section = mtls::makeSection(false);
section.set(mtls::keyClientCert, mtls::clientCert);
auto const setup = mtls::parseSection(section);
EXPECT_FALSE(setup.enabled);
EXPECT_FALSE(setup.useTls);
EXPECT_EQ(setup.tlsClientCertPath, mtls::clientCert);
EXPECT_TRUE(setup.tlsClientKeyPath.empty());
}
TEST(TelemetryConfig, mtls_default_no_client_tls_is_accepted)
{
// The documented default with telemetry on: no client certificate, and
// use_tls absent so it defaults to 0. Both guards run and neither may
// fire. The use_tls guard tests the certificate path first; drop that
// conjunct and this config is rejected, so no default node could boot.
Section const section = mtls::makeSection(true);
telemetry::Telemetry::Setup setup;
ASSERT_NO_THROW(setup = mtls::parseSection(section));
EXPECT_TRUE(setup.enabled);
EXPECT_FALSE(setup.useTls);
EXPECT_TRUE(setup.tlsClientCertPath.empty());
EXPECT_TRUE(setup.tlsClientKeyPath.empty());
}
TEST(TelemetryConfig, mtls_neither_set_is_one_way_tls)
{
Section section;
// Telemetry is on so the guards run, and this config must pass both:
// one-way TLS with a CA bundle and no client certificate.
Section section = mtls::makeSection(true);
section.set("use_tls", "1");
section.set("tls_ca_cert", "/etc/ssl/ca.pem");
auto setup = telemetry::makeTelemetrySetup(section, "nHUtest123", "2.0.0", 0);
auto const setup = mtls::parseSection(section);
EXPECT_TRUE(setup.enabled);
EXPECT_TRUE(setup.useTls);
EXPECT_EQ(setup.tlsCertPath, "/etc/ssl/ca.pem");
EXPECT_TRUE(setup.tlsClientCertPath.empty());
EXPECT_TRUE(setup.tlsClientKeyPath.empty());
}

33
src/xrpld/app/main/Main.cpp
View File

@@ -12,6 +12,7 @@
#include <xrpl/beast/net/IPEndpoint.h>
#include <xrpl/beast/unit_test/suite_info.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/config/Constants.h>
#include <xrpl/core/StartUpType.h>
#include <xrpl/git/Git.h>
@@ -804,8 +805,36 @@ run(int argc, char** argv)
if (vm.contains("debug"))
setDebugLogSink(logs->makeSink("Debug", beast::Severity::Trace));
auto app =
makeApplication(std::move(config), std::move(logs), std::make_unique<TimeKeeper>());
// Application construction runs member initializers that validate
// config (for example the [telemetry] section) and can throw. A throw
// from a member-initializer list cannot be recovered inside the
// constructor, so catch it here. Left uncaught it reaches
// std::terminate, whose default handler prints a C++ terminate dump
// and raises SIGABRT, leaving a core file where the system allows one;
// the catch replaces that with two operator-readable lines on stderr
// and a non-zero exit status.
//
// Only the construction is covered. The [telemetry] section is parsed
// near the top of the member list, before the job queue and node store
// are built, so unwinding that throw destroys very little. setup() is
// left outside deliberately: it starts subsystems whose shutdown order
// is delicate, and only the normal stop sequence gets that order right.
std::unique_ptr<Application> app;
try
{
app =
makeApplication(std::move(config), std::move(logs), std::make_unique<TimeKeeper>());
}
catch (std::exception const& e)
{
std::cerr << "Unable to start " << systemName() << ": " << e.what() << std::endl;
std::cerr << "Fix the reported problem and start again." << std::endl;
return -1;
}
// Construction succeeded, so app holds an object: makeApplication never
// returns null and the catch above is the only other way out.
XRPL_ASSERT(app, "xrpl::run : non-null application");
if (!app->setup(vm))
return -1;