From 51c5f2bfc9db56a18d8ede1b3d9f9fde75a9f62d Mon Sep 17 00:00:00 2001
From: Bronek Kozicki <brok@incorrekt.com>
Date: Thu, 25 Sep 2025 15:14:29 +0100
Subject: [PATCH] Improve ValidatorList invalid UNL manifest logging (#5804)

This change raises logging severity from `INFO` to `WARN` when handling UNL manifest signed with an unexpected / invalid key. It also changes the internal error code for an invalid format of UNL manifest to `invalid` (from `untrusted`).

This is a follow up to problems experienced by an UNL node due to old manifest key configured in `validators.txt`, which would be easier to diagnose with improved logging.

It also replaces a log line with `UNREACHABLE` for an impossible situation when we match UNL manifest key against a configured key which has an invalid type (we cannot configure such a key because of checks when loading configured keys).
---
 src/test/app/ValidatorList_test.cpp         | 18 ++++++++++
 src/xrpld/app/misc/ValidatorList.h          |  2 +-
 src/xrpld/app/misc/detail/ValidatorList.cpp | 40 +++++++++++++--------
 3 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/src/test/app/ValidatorList_test.cpp b/src/test/app/ValidatorList_test.cpp
index a3b62bd4f7..2b004c3b52 100644
--- a/src/test/app/ValidatorList_test.cpp
+++ b/src/test/app/ValidatorList_test.cpp
@@ -768,6 +768,24 @@ private:
         expectUntrusted(lists.at(7));
         expectTrusted(lists.at(2));
 
+        // try empty or mangled manifest
+        checkResult(
+            trustedKeys->applyLists(
+                "", version, {{blob7, sig7, {}}, {blob6, sig6, {}}}, siteUri),
+            publisherPublic,
+            ListDisposition::invalid,
+            ListDisposition::invalid);
+
+        checkResult(
+            trustedKeys->applyLists(
+                base64_encode("not a manifest"),
+                version,
+                {{blob7, sig7, {}}, {blob6, sig6, {}}},
+                siteUri),
+            publisherPublic,
+            ListDisposition::invalid,
+            ListDisposition::invalid);
+
         // do not use list from untrusted publisher
         auto const untrustedManifest = base64_encode(makeManifestString(
             randomMasterKey(),
diff --git a/src/xrpld/app/misc/ValidatorList.h b/src/xrpld/app/misc/ValidatorList.h
index 1f5d728824..9a2018cbd4 100644
--- a/src/xrpld/app/misc/ValidatorList.h
+++ b/src/xrpld/app/misc/ValidatorList.h
@@ -877,7 +877,7 @@ private:
     verify(
         lock_guard const&,
         Json::Value& list,
-        std::string const& manifest,
+        Manifest manifest,
         std::string const& blob,
         std::string const& signature);
 
diff --git a/src/xrpld/app/misc/detail/ValidatorList.cpp b/src/xrpld/app/misc/detail/ValidatorList.cpp
index 1ddb51c9dd..2b45cec3be 100644
--- a/src/xrpld/app/misc/detail/ValidatorList.cpp
+++ b/src/xrpld/app/misc/detail/ValidatorList.cpp
@@ -1149,21 +1149,33 @@ ValidatorList::applyList(
 
     Json::Value list;
     auto const& manifest = localManifest ? *localManifest : globalManifest;
-    auto [result, pubKeyOpt] = verify(lock, list, manifest, blob, signature);
+    auto m = deserializeManifest(base64_decode(manifest));
+    if (!m)
+    {
+        JLOG(j_.warn()) << "UNL manifest cannot be deserialized";
+        return PublisherListStats{ListDisposition::invalid};
+    }
+
+    auto [result, pubKeyOpt] =
+        verify(lock, list, std::move(*m), blob, signature);
 
     if (!pubKeyOpt)
     {
-        JLOG(j_.info()) << "ValidatorList::applyList unable to retrieve the "
-                           "master public key from the verify function\n";
+        JLOG(j_.warn())
+            << "UNL manifest is signed with an unrecognized master public key";
         return PublisherListStats{result};
     }
 
     if (!publicKeyType(*pubKeyOpt))
-    {
-        JLOG(j_.info()) << "ValidatorList::applyList Invalid Public Key type"
-                           " retrieved from the verify function\n ";
+    {  // LCOV_EXCL_START
+       // This is an impossible situation because we will never load an
+       // invalid public key type (see checks in `ValidatorList::load`) however
+       // we can only arrive here if the key used by the manifest matched one of
+       // the loaded keys
+        UNREACHABLE(
+            "ripple::ValidatorList::applyList : invalid public key type");
         return PublisherListStats{result};
-    }
+    }  // LCOV_EXCL_STOP
 
     PublicKey pubKey = *pubKeyOpt;
     if (result > ListDisposition::pending)
@@ -1356,19 +1368,17 @@ std::pair<ListDisposition, std::optional<PublicKey>>
 ValidatorList::verify(
     ValidatorList::lock_guard const& lock,
     Json::Value& list,
-    std::string const& manifest,
+    Manifest manifest,
     std::string const& blob,
     std::string const& signature)
 {
-    auto m = deserializeManifest(base64_decode(manifest));
-
-    if (!m || !publisherLists_.count(m->masterKey))
+    if (!publisherLists_.count(manifest.masterKey))
         return {ListDisposition::untrusted, {}};
 
-    PublicKey masterPubKey = m->masterKey;
-    auto const revoked = m->revoked();
+    PublicKey masterPubKey = manifest.masterKey;
+    auto const revoked = manifest.revoked();
 
-    auto const result = publisherManifests_.applyManifest(std::move(*m));
+    auto const result = publisherManifests_.applyManifest(std::move(manifest));
 
     if (revoked && result == ManifestDisposition::accepted)
     {
@@ -1796,7 +1806,7 @@ ValidatorList::getAvailable(
 
     if (!keyBlob || !publicKeyType(makeSlice(*keyBlob)))
     {
-        JLOG(j_.info()) << "Invalid requested validator list publisher key: "
+        JLOG(j_.warn()) << "Invalid requested validator list publisher key: "
                         << pubKey;
         return {};
     }