Add [validator_list_threshold] to validators.txt to improve UNL security (#5112)

API-CHANGELOG.md

@@ -87,9 +87,10 @@ The `network_id` field was added in the `server_info` response in version 1.5.0
 
 As of 2025-01-23, version 2.4.0 is in development. You can use a pre-release version by building from source or [using the `nightly` package](https://xrpl.org/docs/infrastructure/installation/install-rippled-on-ubuntu).
 
-### Addition in 2.4
+### Additions and bugfixes in 2.4.0
 
 - `ledger_entry`: `state` is added an alias for `ripple_state`.
+- `validators`: Added new field `validator_list_threshold` in response.
 
 ## XRP Ledger server version 2.3.0
 

cfg/validators-example.txt

@@ -86,3 +86,21 @@ ED2677ABFFD1B33AC6FBC3062B71F1E8397C1505E1C42C64D11AD1B28FF73F4734
 #
 # [import_vl_keys]
 # ED264807102805220DA0F312E71FC2C69E1552C9C5790F6C25E3729DEB573D5860
+
+
+# [validator_list_threshold]
+#
+#   Minimum number of validator lists on which a validator must be listed in
+#   order to be used.
+#
+#   This can be set explicitly to any positive integer number not greater than
+#   the size of [validator_list_keys]. If it is not set, or set to 0, the
+#   value will be calculated at startup from the size of [validator_list_keys],
+#   where the calculation is:
+#
+#     threshold = size(validator_list_keys) < 3
+#       ? 1
+#       : floor(size(validator_list_keys) / 2) + 1
+
+[validator_list_threshold]
+0

include/xrpl/protocol/jss.h

@@ -746,6 +746,7 @@ JSS(validation_quorum);       // out: NetworkOPs
 JSS(validation_seed);         // out: ValidationCreate, ValidationSeed
 JSS(validation);
 JSS(validations);      // out: AmendmentTableImpl
+JSS(validator_list_threshold);  // out: ValidatorList
 JSS(validator_sites);  // out: ValidatorSites
 JSS(value);            // out: STAmount
 JSS(version);          // out: RPCVersion

src/test/app/Regression_test.cpp

@@ -322,8 +322,9 @@ struct Regression_test : public beast::unit_test::suite
                     beforeCounts.at("CachedView::hit"s) ==
                     afterCounts.at("CachedView::hit"s));
                 BEAST_EXPECT(
-                    beforeCounts.at("CachedView::hitExpired"s) + 6 /* original:  1 */ ==
+                    beforeCounts.at("CachedView::hitExpired"s) +
-                    afterCounts.at("CachedView::hitExpired"s));
+                        6 /* original:  1 */
+                    == afterCounts.at("CachedView::hitExpired"s));
                 BEAST_EXPECT(
                     beforeCounts.at("CachedView::miss"s) ==
                     afterCounts.at("CachedView::miss"s));

src/test/core/Config_test.cpp

@@ -22,6 +22,7 @@
 #include <xrpld/core/Config.h>
 #include <xrpld/core/ConfigSections.h>
 #include <xrpl/basics/contract.h>
+#include <xrpl/beast/unit_test/suite.h>
 #include <xrpl/server/Port.h>
 #include <boost/filesystem.hpp>
 #include <boost/format.hpp>
@@ -222,6 +223,9 @@ moreripplevalidators.net
 [validator_list_keys]
 03E74EE14CB525AFBB9F1B7D86CD58ECC4B91452294B42AB4E78F260BD905C091D
 030775A669685BD6ABCEBD80385921C7851783D991A8055FD21D2F3966C96F1B56
+
+[validator_list_threshold]
+2
 )rippleConfig");
     return configContents;
 }
@@ -538,6 +542,7 @@ nHBu9PTL9dn2GuZtdW4U2WzBwffyX9qsQCd9CNU4Z5YG3PQfViM8
             c.loadFromString(toLoad);
             BEAST_EXPECT(c.legacy("validators_file").empty());
             BEAST_EXPECT(c.section(SECTION_VALIDATORS).values().size() == 5);
+            BEAST_EXPECT(c.VALIDATOR_LIST_THRESHOLD == std::nullopt);
         }
         {
             // load validator list sites and keys from config
@@ -549,6 +554,9 @@ trustthesevalidators.gov
 
 [validator_list_keys]
 021A99A537FDEBC34E4FCA03B39BEADD04299BB19E85097EC92B15A3518801E566
+
+[validator_list_threshold]
+1
 )rippleConfig");
             c.loadFromString(toLoad);
             BEAST_EXPECT(
@@ -565,6 +573,133 @@ trustthesevalidators.gov
                 c.section(SECTION_VALIDATOR_LIST_KEYS).values()[0] ==
                 "021A99A537FDEBC34E4FCA03B39BEADD04299BB19E85097EC92B15A3518801"
                 "E566");
+            BEAST_EXPECT(
+                c.section(SECTION_VALIDATOR_LIST_THRESHOLD).values().size() ==
+                1);
+            BEAST_EXPECT(
+                c.section(SECTION_VALIDATOR_LIST_THRESHOLD).values()[0] == "1");
+            BEAST_EXPECT(c.VALIDATOR_LIST_THRESHOLD == std::size_t(1));
+        }
+        {
+            // load validator list sites and keys from config
+            Config c;
+            std::string toLoad(R"rippleConfig(
+[validator_list_sites]
+ripplevalidators.com
+trustthesevalidators.gov
+
+[validator_list_keys]
+021A99A537FDEBC34E4FCA03B39BEADD04299BB19E85097EC92B15A3518801E566
+
+[validator_list_threshold]
+0
+)rippleConfig");
+            c.loadFromString(toLoad);
+            BEAST_EXPECT(
+                c.section(SECTION_VALIDATOR_LIST_SITES).values().size() == 2);
+            BEAST_EXPECT(
+                c.section(SECTION_VALIDATOR_LIST_SITES).values()[0] ==
+                "ripplevalidators.com");
+            BEAST_EXPECT(
+                c.section(SECTION_VALIDATOR_LIST_SITES).values()[1] ==
+                "trustthesevalidators.gov");
+            BEAST_EXPECT(
+                c.section(SECTION_VALIDATOR_LIST_KEYS).values().size() == 1);
+            BEAST_EXPECT(
+                c.section(SECTION_VALIDATOR_LIST_KEYS).values()[0] ==
+                "021A99A537FDEBC34E4FCA03B39BEADD04299BB19E85097EC92B15A3518801"
+                "E566");
+            BEAST_EXPECT(
+                c.section(SECTION_VALIDATOR_LIST_THRESHOLD).values().size() ==
+                1);
+            BEAST_EXPECT(
+                c.section(SECTION_VALIDATOR_LIST_THRESHOLD).values()[0] == "0");
+            BEAST_EXPECT(c.VALIDATOR_LIST_THRESHOLD == std::nullopt);
+        }
+        {
+            // load should throw if [validator_list_threshold] is greater than
+            // the number of [validator_list_keys]
+            Config c;
+            std::string toLoad(R"rippleConfig(
+[validator_list_sites]
+ripplevalidators.com
+trustthesevalidators.gov
+
+[validator_list_keys]
+021A99A537FDEBC34E4FCA03B39BEADD04299BB19E85097EC92B15A3518801E566
+
+[validator_list_threshold]
+2
+)rippleConfig");
+            std::string error;
+            auto const expectedError =
+                "Value in config section [validator_list_threshold] exceeds "
+                "the number of configured list keys";
+            try
+            {
+                c.loadFromString(toLoad);
+                fail();
+            }
+            catch (std::runtime_error& e)
+            {
+                error = e.what();
+            }
+            BEAST_EXPECT(error == expectedError);
+        }
+        {
+            // load should throw if [validator_list_threshold] is malformed
+            Config c;
+            std::string toLoad(R"rippleConfig(
+[validator_list_sites]
+ripplevalidators.com
+trustthesevalidators.gov
+
+[validator_list_keys]
+021A99A537FDEBC34E4FCA03B39BEADD04299BB19E85097EC92B15A3518801E566
+
+[validator_list_threshold]
+value = 2
+)rippleConfig");
+            std::string error;
+            auto const expectedError =
+                "Config section [validator_list_threshold] should contain "
+                "single value only";
+            try
+            {
+                c.loadFromString(toLoad);
+                fail();
+            }
+            catch (std::runtime_error& e)
+            {
+                error = e.what();
+            }
+            BEAST_EXPECT(error == expectedError);
+        }
+        {
+            // load should throw if [validator_list_threshold] is negative
+            Config c;
+            std::string toLoad(R"rippleConfig(
+[validator_list_sites]
+ripplevalidators.com
+trustthesevalidators.gov
+
+[validator_list_keys]
+021A99A537FDEBC34E4FCA03B39BEADD04299BB19E85097EC92B15A3518801E566
+
+[validator_list_threshold]
+-1
+)rippleConfig");
+            bool error = false;
+            try
+            {
+                c.loadFromString(toLoad);
+                fail();
+            }
+            catch (std::bad_cast& e)
+            {
+                error = true;
+            }
+            BEAST_EXPECT(error);
         }
         {
             // load should throw if [validator_list_sites] is configured but
@@ -581,6 +716,7 @@ trustthesevalidators.gov
             try
             {
                 c.loadFromString(toLoad);
+                fail();
             }
             catch (std::runtime_error& e)
             {
@@ -602,6 +738,10 @@ trustthesevalidators.gov
                 c.section(SECTION_VALIDATOR_LIST_SITES).values().size() == 2);
             BEAST_EXPECT(
                 c.section(SECTION_VALIDATOR_LIST_KEYS).values().size() == 2);
+            BEAST_EXPECT(
+                c.section(SECTION_VALIDATOR_LIST_THRESHOLD).values().size() ==
+                1);
+            BEAST_EXPECT(c.VALIDATOR_LIST_THRESHOLD == 2);
         }
         {
             // load from specified [validators_file] file name
@@ -620,6 +760,10 @@ trustthesevalidators.gov
                 c.section(SECTION_VALIDATOR_LIST_SITES).values().size() == 2);
             BEAST_EXPECT(
                 c.section(SECTION_VALIDATOR_LIST_KEYS).values().size() == 2);
+            BEAST_EXPECT(
+                c.section(SECTION_VALIDATOR_LIST_THRESHOLD).values().size() ==
+                1);
+            BEAST_EXPECT(c.VALIDATOR_LIST_THRESHOLD == 2);
         }
         {
             // load from specified [validators_file] relative path
@@ -639,6 +783,10 @@ trustthesevalidators.gov
                 c.section(SECTION_VALIDATOR_LIST_SITES).values().size() == 2);
             BEAST_EXPECT(
                 c.section(SECTION_VALIDATOR_LIST_KEYS).values().size() == 2);
+            BEAST_EXPECT(
+                c.section(SECTION_VALIDATOR_LIST_THRESHOLD).values().size() ==
+                1);
+            BEAST_EXPECT(c.VALIDATOR_LIST_THRESHOLD == 2);
         }
         {
             // load from validators file in default location
@@ -655,6 +803,10 @@ trustthesevalidators.gov
                 c.section(SECTION_VALIDATOR_LIST_SITES).values().size() == 2);
             BEAST_EXPECT(
                 c.section(SECTION_VALIDATOR_LIST_KEYS).values().size() == 2);
+            BEAST_EXPECT(
+                c.section(SECTION_VALIDATOR_LIST_THRESHOLD).values().size() ==
+                1);
+            BEAST_EXPECT(c.VALIDATOR_LIST_THRESHOLD == 2);
         }
         {
             // load from specified [validators_file] instead
@@ -675,6 +827,10 @@ trustthesevalidators.gov
                 c.section(SECTION_VALIDATOR_LIST_SITES).values().size() == 2);
             BEAST_EXPECT(
                 c.section(SECTION_VALIDATOR_LIST_KEYS).values().size() == 2);
+            BEAST_EXPECT(
+                c.section(SECTION_VALIDATOR_LIST_THRESHOLD).values().size() ==
+                1);
+            BEAST_EXPECT(c.VALIDATOR_LIST_THRESHOLD == 2);
         }
 
         {
@@ -712,6 +868,39 @@ trustthesevalidators.gov
                 c.section(SECTION_VALIDATOR_LIST_SITES).values().size() == 4);
             BEAST_EXPECT(
                 c.section(SECTION_VALIDATOR_LIST_KEYS).values().size() == 3);
+            BEAST_EXPECT(
+                c.section(SECTION_VALIDATOR_LIST_THRESHOLD).values().size() ==
+                1);
+            BEAST_EXPECT(c.VALIDATOR_LIST_THRESHOLD == 2);
+        }
+        {
+            // load should throw if [validator_list_threshold] is present both
+            // in rippled cfg and validators file
+            boost::format cc(R"rippleConfig(
+[validators_file]
+%1%
+
+[validator_list_threshold]
+1
+)rippleConfig");
+            std::string error;
+            detail::ValidatorsTxtGuard const vtg(
+                *this, "test_cfg", "validators.cfg");
+            BEAST_EXPECT(vtg.validatorsFileExists());
+            auto const expectedError =
+                "Config section [validator_list_threshold] should contain "
+                "single value only";
+            try
+            {
+                Config c;
+                c.loadFromString(boost::str(cc % vtg.validatorsFile()));
+                fail();
+            }
+            catch (std::runtime_error& e)
+            {
+                error = e.what();
+            }
+            BEAST_EXPECT(error == expectedError);
         }
         {
             // load should throw if [validators], [validator_keys] and

src/xrpld/app/main/Application.cpp

@@ -1368,7 +1368,8 @@ ApplicationImp::setup(boost::program_options::variables_map const& cmdline)
         if (!validators_->load(
                 localSigningKey,
                 config().section(SECTION_VALIDATORS).values(),
-                config().section(SECTION_VALIDATOR_LIST_KEYS).values()))
+                config().section(SECTION_VALIDATOR_LIST_KEYS).values(),
+                config().VALIDATOR_LIST_THRESHOLD))
         {
             JLOG(m_journal.fatal())
                 << "Invalid entry in validator configuration.";

src/xrpld/app/misc/ValidatorList.h

@@ -242,6 +242,9 @@ class ValidatorList
     // The current list of trusted master keys
     hash_set<PublicKey> trustedMasterKeys_;
 
+    // Minimum number of lists on which a trusted validator must appear on
+    std::size_t listThreshold_;
+
     // The current list of trusted signing keys. For those validators using
     // a manifest, the signing key is the ephemeral key. For the ones using
     // a seed, the signing key is the same as the master key.
@@ -343,7 +346,8 @@ public:
     load(
         std::optional<PublicKey> const& localSigningKey,
         std::vector<std::string> const& configKeys,
-        std::vector<std::string> const& publisherKeys);
+        std::vector<std::string> const& publisherKeys,
+        std::optional<std::size_t> listThreshold = {});
 
     /** Pull the blob/signature/manifest information out of the appropriate Json
         body fields depending on the version.
@@ -679,6 +683,13 @@ public:
     hash_set<PublicKey>
     getTrustedMasterKeys() const;
 
+    /**
+     * get the validator list threshold
+     * @return the threshold
+     */
+    std::size_t
+    getListThreshold() const;
+
     /**
      * get the master public keys of Negative UNL validators
      * @return the master public keys

src/xrpld/app/misc/detail/ValidatorList.cpp

@@ -129,6 +129,7 @@ ValidatorList::ValidatorList(
     , j_(j)
     , quorum_(minimumQuorum.value_or(1))  // Genesis ledger quorum
     , minimumQuorum_(minimumQuorum)
+    , listThreshold_(1)
 {
 }
 
@@ -136,7 +137,8 @@ bool
 ValidatorList::load(
     std::optional<PublicKey> const& localSigningKey,
     std::vector<std::string> const& configKeys,
-    std::vector<std::string> const& publisherKeys)
+    std::vector<std::string> const& publisherKeys,
+    std::optional<std::size_t> listThreshold)
 {
     static boost::regex const re(
         "[[:space:]]*"       // skip leading whitespace
@@ -190,6 +192,26 @@ ValidatorList::load(
         ++count;
     }
 
+    if (listThreshold)
+    {
+        listThreshold_ = *listThreshold;
+        // This should be enforced by Config class
+        XRPL_ASSERT(
+            listThreshold_ > 0 && listThreshold_ <= publisherLists_.size(),
+            "ripple::ValidatorList::load : list threshold inside range");
+        JLOG(j_.debug()) << "Validator list threshold set in configuration to "
+                         << listThreshold_;
+    }
+    else
+    {
+        // Want truncated result when dividing an odd integer
+        listThreshold_ = (publisherLists_.size() < 3)
+            ? 1  //
+            : publisherLists_.size() / 2 + 1;
+        JLOG(j_.debug()) << "Validator list threshold computed as "
+                         << listThreshold_;
+    }
+
     JLOG(j_.debug()) << "Loaded " << count << " keys";
 
     if (localSigningKey)
@@ -197,7 +219,17 @@ ValidatorList::load(
 
     // Treat local validator key as though it was listed in the config
     if (localPubKey_)
-        keyListings_.insert({*localPubKey_, 1});
+    {
+        // The local validator must meet listThreshold_ so the validator does
+        // not ignore itself.
+        auto const [_, inserted] =
+            keyListings_.insert({*localPubKey_, listThreshold_});
+        if (inserted)
+        {
+            JLOG(j_.debug()) << "Added own master key "
+                             << toBase58(TokenType::NodePublic, *localPubKey_);
+        }
+    }
 
     JLOG(j_.debug()) << "Loading configured validator keys";
 
@@ -227,7 +259,7 @@ ValidatorList::load(
         if (*id == localPubKey_ || *id == localSigningKey)
             continue;
 
-        auto ret = keyListings_.insert({*id, 1});
+        auto ret = keyListings_.insert({*id, listThreshold_});
         if (!ret.second)
         {
             JLOG(j_.warn()) << "Duplicate node identity: " << match[1];
@@ -1615,6 +1647,8 @@ ValidatorList::getJson() const
             x[jss::status] = "unknown";
             x[jss::expiration] = "unknown";
         }
+
+        x[jss::validator_list_threshold] = Json::UInt(listThreshold_);
     }
 
     // Validator keys listed in the local config file
@@ -1794,11 +1828,42 @@ ValidatorList::calculateQuorum(
         return *minimumQuorum_;
     }
 
-    // Do not use achievable quorum until lists from all configured
+    if (!publisherLists_.empty())
-    // publishers are available
-    for (auto const& list : publisherLists_)
     {
-        if (list.second.status != PublisherStatus::available)
+        // Do not use achievable quorum until lists from a sufficient number of
+        // configured publishers are available
+        std::size_t unavailable = 0;
+        for (auto const& list : publisherLists_)
+        {
+            if (list.second.status != PublisherStatus::available)
+                unavailable += 1;
+        }
+        // There are two, subtly different, sides to list threshold:
+        //
+        // 1. The minimum required intersection between lists listThreshold_
+        //    for a validator to be included in trustedMasterKeys_.
+        //    If this many (or more) publishers are unavailable, we are likely
+        //    to NOT include a validator which otherwise would have been used.
+        //    We disable quorum if this happens.
+        // 2. The minimum number of publishers which, when unavailable, will
+        //    prevent us from hitting the above threshold on ANY validator.
+        //    This is calculated as:
+        //      N - M + 1
+        //    where
+        //      N: number of publishers i.e. publisherLists_.size()
+        //      M: minimum required intersection i.e. listThreshold_
+        //    If this happens, we still have this local validator and we do not
+        //    want it to form a quorum of 1, so we disable quorum as well.
+        //
+        // We disable quorum if the number of unavailable publishers exceeds
+        // either of the above thresholds
+        auto const errorThreshold = std::min(
+            listThreshold_,  //
+            publisherLists_.size() - listThreshold_ + 1);
+        XRPL_ASSERT(
+            errorThreshold > 0,
+            "ripple::ValidatorList::calculateQuorum : nonzero error threshold");
+        if (unavailable >= errorThreshold)
             return std::numeric_limits<std::size_t>::max();
     }
 
@@ -1943,20 +2008,27 @@ ValidatorList::updateTrusted(
     auto it = trustedMasterKeys_.cbegin();
     while (it != trustedMasterKeys_.cend())
     {
-        if (!keyListings_.count(*it) || validatorManifests_.revoked(*it))
+        auto const kit = keyListings_.find(*it);
+        if (kit == keyListings_.end() ||     //
+            kit->second < listThreshold_ ||  //
+            validatorManifests_.revoked(*it))
         {
             trustChanges.removed.insert(calcNodeID(*it));
             it = trustedMasterKeys_.erase(it);
         }
         else
         {
+            XRPL_ASSERT(
+                kit->second >= listThreshold_,
+                "ripple::ValidatorList::updateTrusted : count meets threshold");
             ++it;
         }
     }
 
     for (auto const& val : keyListings_)
     {
-        if (!validatorManifests_.revoked(val.first) &&
+        if (val.second >= listThreshold_ &&
+            !validatorManifests_.revoked(val.first) &&
             trustedMasterKeys_.emplace(val.first).second)
             trustChanges.added.insert(calcNodeID(val.first));
     }
@@ -2036,6 +2108,13 @@ ValidatorList::getTrustedMasterKeys() const
     return trustedMasterKeys_;
 }
 
+std::size_t
+ValidatorList::getListThreshold() const
+{
+    std::shared_lock read_lock{mutex_};
+    return listThreshold_;
+}
+
 hash_set<PublicKey>
 ValidatorList::getNegativeUNL() const
 {

src/xrpld/core/Config.h

@@ -320,6 +320,8 @@ public:
     std::optional<std::pair<std::uint32_t, std::uint32_t>>
         FORCED_LEDGER_RANGE_PRESENT;
 
+    std::optional<std::size_t> VALIDATOR_LIST_THRESHOLD;
+
 public:
     Config();
 

src/xrpld/core/ConfigSections.h

@@ -93,6 +93,7 @@ struct ConfigSection
 #define SECTION_VALIDATOR_KEY_REVOCATION "validator_key_revocation"
 #define SECTION_VALIDATOR_LIST_KEYS "validator_list_keys"
 #define SECTION_VALIDATOR_LIST_SITES "validator_list_sites"
+#define SECTION_VALIDATOR_LIST_THRESHOLD "validator_list_threshold"
 #define SECTION_VALIDATORS "validators"
 #define SECTION_VALIDATOR_TOKEN "validator_token"
 #define SECTION_VETO_AMENDMENTS "veto_amendments"

src/xrpld/core/detail/Config.cpp

@@ -942,6 +942,13 @@ Config::loadFromString(std::string const& fileContents)
             if (valListKeys)
                 section(SECTION_VALIDATOR_LIST_KEYS).append(*valListKeys);
 
+            auto valListThreshold =
+                getIniFileSection(iniFile, SECTION_VALIDATOR_LIST_THRESHOLD);
+
+            if (valListThreshold)
+                section(SECTION_VALIDATOR_LIST_THRESHOLD)
+                    .append(*valListThreshold);
+
             if (!entries && !valKeyEntries && !valListKeys)
                 Throw<std::runtime_error>(
                     "The file specified in [" SECTION_VALIDATORS_FILE
@@ -956,6 +963,38 @@ Config::loadFromString(std::string const& fileContents)
                     validatorsFile.string());
         }
 
+        VALIDATOR_LIST_THRESHOLD = [&]() -> std::optional<std::size_t> {
+            auto const& listThreshold =
+                section(SECTION_VALIDATOR_LIST_THRESHOLD);
+            if (listThreshold.lines().empty())
+                return std::nullopt;
+            else if (listThreshold.values().size() == 1)
+            {
+                auto strTemp = listThreshold.values()[0];
+                auto const listThreshold =
+                    beast::lexicalCastThrow<std::size_t>(strTemp);
+                if (listThreshold == 0)
+                    return std::nullopt;  // NOTE: Explicitly ask for computed
+                else if (
+                    listThreshold >
+                    section(SECTION_VALIDATOR_LIST_KEYS).values().size())
+                {
+                    Throw<std::runtime_error>(
+                        "Value in config section "
+                        "[" SECTION_VALIDATOR_LIST_THRESHOLD
+                        "] exceeds the number of configured list keys");
+                }
+                return listThreshold;
+            }
+            else
+            {
+                Throw<std::runtime_error>(
+                    "Config section "
+                    "[" SECTION_VALIDATOR_LIST_THRESHOLD
+                    "] should contain single value only");
+            }
+        }();
+
         // Consolidate [validator_keys] and [validators]
         section(SECTION_VALIDATORS)
             .append(section(SECTION_VALIDATOR_KEYS).lines());