mirror of
https://github.com/XRPLF/rippled.git
synced 2026-08-21 14:20:56 +00:00
fix: Cap untrusted manifests per message and drop oversized ones
Bound the number of manifests carried in a single TMManifests message (kMaxManifestsPerMessage). Trusted manifests are always included and processed; untrusted gossip is capped per message on both send and receive, and the sender is charged only when untrusted entries are actually skipped. Oversized TMManifests messages are dropped without penalty at the protocol layer so an unpatched peer is not disconnected. Complements the cache bound from #276/#323.
This commit is contained in:
committed by
Ed Hennis
parent
0cce5a06d9
commit
4bd1d1ca2f
@@ -54,16 +54,16 @@ encodedSize(std::size_t const nBytes)
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the maximum number of bytes a base64 string of @p nChars characters
|
||||
* Returns the maximum number of bytes a base64 string of @p numChars characters
|
||||
* decodes to.
|
||||
*
|
||||
* @param nChars Number of base64 characters.
|
||||
* @param numChars Number of base64 characters.
|
||||
* @return Upper bound on the number of decoded bytes.
|
||||
*/
|
||||
constexpr std::size_t
|
||||
decodedSize(std::size_t const nChars)
|
||||
decodedSize(std::size_t const numChars)
|
||||
{
|
||||
return ((nChars / 4) * 3) + 2;
|
||||
return ((numChars / 4) * 3) + 2;
|
||||
}
|
||||
|
||||
} // namespace base64
|
||||
|
||||
@@ -47,8 +47,8 @@ namespace xrpl {
|
||||
entry (which contains the manifest for this validator) is decoded and
|
||||
added to the manifest cache. Other manifests are added as "gossip"
|
||||
received from xrpld peers, including ones for validators this node does not
|
||||
list. Manifests for unlisted validators are capped (kMaxUntrustedCount)
|
||||
so peer gossip cannot grow the cache without bound; listed validators are
|
||||
trust. Manifests for untrusted validators are capped (kMaxUntrustedCount)
|
||||
so peer gossip cannot grow the cache without bound; trusted validators are
|
||||
not capped. Entries are never evicted, so a stored revocation is permanent.
|
||||
|
||||
When an ephemeral key is compromised, a new signing key pair is created,
|
||||
@@ -149,7 +149,7 @@ to_string(Manifest const& m);
|
||||
* field body. Taking every field at its largest gives the maximum below, so
|
||||
* anything larger cannot be a valid manifest.
|
||||
*
|
||||
* Field header + length + body = bytes
|
||||
* Field header + length + body = bytes
|
||||
* sfVersion (U16) 2 0 2 4
|
||||
* sfSequence (U32) 1 0 4 5
|
||||
* sfPublicKey (33) 1 1 33 35
|
||||
@@ -172,16 +172,31 @@ constexpr std::size_t kMaxManifestBytes = 358;
|
||||
*/
|
||||
constexpr std::size_t kMaxManifestBase64 = base64::encodedSize(kMaxManifestBytes);
|
||||
|
||||
/**
|
||||
* Constructs Manifest from serialized string
|
||||
*
|
||||
* @param s Serialized manifest string
|
||||
*
|
||||
* @return `std::nullopt` if string is invalid
|
||||
*
|
||||
* @note This does not verify manifest signatures.
|
||||
* `Manifest::verify` should be called after constructing manifest.
|
||||
*/
|
||||
/** Maximum number of manifests carried in a single TMManifests message.
|
||||
|
||||
Outbound, the TMManifests message sent to a peer includes every trusted
|
||||
manifest and fills the rest of this budget with untrusted gossip, so it
|
||||
never exceeds this size. Inbound, trusted manifests are always processed
|
||||
and untrusted ones are processed up to this many, so a peer sending its
|
||||
whole cache cannot force unbounded work.
|
||||
|
||||
The trusted set is tiny relative to this bound, so trusted manifests are
|
||||
not dropped in practice. This is a transitional per-message cap; the cache
|
||||
already bounds untrusted manifests (see kMaxUntrustedCount), so it is no
|
||||
longer needed once the network has upgraded past nodes that send their
|
||||
whole cache in one message.
|
||||
*/
|
||||
constexpr std::size_t kMaxManifestsPerMessage = 200;
|
||||
|
||||
/** Constructs Manifest from serialized string
|
||||
|
||||
@param s Serialized manifest string
|
||||
|
||||
@return `std::nullopt` if string is invalid
|
||||
|
||||
@note This does not verify manifest signatures.
|
||||
`Manifest::verify` should be called after constructing manifest.
|
||||
*/
|
||||
/** @{ */
|
||||
std::optional<Manifest>
|
||||
deserializeManifest(Slice s, beast::Journal journal);
|
||||
@@ -282,7 +297,7 @@ to_string(ManifestDisposition m)
|
||||
* must choose. `Capped` is the safe, flood-resistant value; only listed or
|
||||
* configured keys should use `Uncapped`.
|
||||
*/
|
||||
enum class ManifestRateLimitCap : std::uint8_t {
|
||||
enum class ManifestRateLimitCapPolicy : std::uint8_t {
|
||||
Capped, ///< Subject to the untrusted cap (unlisted peer gossip)
|
||||
Uncapped ///< Bypasses the cap (listed/trusted or config manifests)
|
||||
};
|
||||
@@ -433,7 +448,7 @@ public:
|
||||
May be called concurrently
|
||||
*/
|
||||
ManifestDisposition
|
||||
applyManifest(Manifest m, ManifestRateLimitCap cap);
|
||||
applyManifest(Manifest m, ManifestRateLimitCapPolicy cap);
|
||||
|
||||
/**
|
||||
* Stop counting a master key against the untrusted cap.
|
||||
|
||||
@@ -382,9 +382,9 @@ ManifestCache::revoked(PublicKey const& pk) const
|
||||
}
|
||||
|
||||
ManifestDisposition
|
||||
ManifestCache::applyManifest(Manifest m, ManifestRateLimitCap const cap)
|
||||
ManifestCache::applyManifest(Manifest m, ManifestRateLimitCapPolicy const cap)
|
||||
{
|
||||
bool const uncapped = cap == ManifestRateLimitCap::Uncapped;
|
||||
bool const uncapped = cap == ManifestRateLimitCapPolicy::Uncapped;
|
||||
|
||||
// The signature is checked only on the first `prewriteCheck` run (under the
|
||||
// read lock). It is expensive, so `checkSignature` is cleared the first
|
||||
@@ -634,7 +634,7 @@ ManifestCache::load(
|
||||
JLOG(j_.warn()) << "Configured manifest revokes public key";
|
||||
}
|
||||
|
||||
if (applyManifest(std::move(*mo), ManifestRateLimitCap::Uncapped) ==
|
||||
if (applyManifest(std::move(*mo), ManifestRateLimitCapPolicy::Uncapped) ==
|
||||
ManifestDisposition::Invalid)
|
||||
{
|
||||
JLOG(j_.error()) << "Manifest in config was rejected";
|
||||
@@ -658,7 +658,7 @@ ManifestCache::load(
|
||||
auto mo = deserializeManifest(base64Decode(revocationStr));
|
||||
|
||||
if (!mo || !mo->revoked() ||
|
||||
applyManifest(std::move(*mo), ManifestRateLimitCap::Uncapped) ==
|
||||
applyManifest(std::move(*mo), ManifestRateLimitCapPolicy::Uncapped) ==
|
||||
ManifestDisposition::Invalid)
|
||||
{
|
||||
JLOG(j_.error()) << "Invalid validator key revocation in config";
|
||||
|
||||
@@ -78,7 +78,7 @@ getManifests(
|
||||
|
||||
// Only trusted manifests are persisted (see saveManifests), so
|
||||
// anything loaded from the DB bypasses the untrusted cap.
|
||||
cache.applyManifest(std::move(*mo), ManifestRateLimitCap::Uncapped);
|
||||
cache.applyManifest(std::move(*mo), ManifestRateLimitCapPolicy::Uncapped);
|
||||
}
|
||||
else
|
||||
{
|
||||
|
||||
@@ -400,7 +400,7 @@ public:
|
||||
ManifestDisposition::Accepted ==
|
||||
cache.applyManifest(
|
||||
makeManifest(sk, KeyType::Ed25519, kp0.second, KeyType::Secp256k1, 0),
|
||||
ManifestRateLimitCap::Capped));
|
||||
ManifestRateLimitCapPolicy::Capped));
|
||||
BEAST_EXPECT(cache.getSigningKey(pk) == kp0.first);
|
||||
BEAST_EXPECT(cache.getMasterKey(kp0.first) == pk);
|
||||
|
||||
@@ -413,7 +413,7 @@ public:
|
||||
ManifestDisposition::Accepted ==
|
||||
cache.applyManifest(
|
||||
makeManifest(sk, KeyType::Ed25519, kp1.second, KeyType::Secp256k1, 1),
|
||||
ManifestRateLimitCap::Capped));
|
||||
ManifestRateLimitCapPolicy::Capped));
|
||||
BEAST_EXPECT(cache.getSigningKey(pk) == kp1.first);
|
||||
BEAST_EXPECT(cache.getMasterKey(kp1.first) == pk);
|
||||
BEAST_EXPECT(cache.getMasterKey(kp0.first) == kp0.first);
|
||||
@@ -424,7 +424,7 @@ public:
|
||||
ManifestDisposition::BadEphemeralKey ==
|
||||
cache.applyManifest(
|
||||
makeManifest(sk, KeyType::Ed25519, kp1.second, KeyType::Secp256k1, 2),
|
||||
ManifestRateLimitCap::Capped));
|
||||
ManifestRateLimitCapPolicy::Capped));
|
||||
BEAST_EXPECT(cache.getSigningKey(pk) == kp1.first);
|
||||
BEAST_EXPECT(cache.getMasterKey(kp1.first) == pk);
|
||||
BEAST_EXPECT(cache.getMasterKey(kp0.first) == kp0.first);
|
||||
@@ -435,7 +435,7 @@ public:
|
||||
BEAST_EXPECT(
|
||||
ManifestDisposition::Accepted ==
|
||||
cache.applyManifest(
|
||||
makeRevocation(sk, KeyType::Ed25519), ManifestRateLimitCap::Capped));
|
||||
makeRevocation(sk, KeyType::Ed25519), ManifestRateLimitCapPolicy::Capped));
|
||||
BEAST_EXPECT(cache.revoked(pk));
|
||||
BEAST_EXPECT(cache.getSigningKey(pk) == pk);
|
||||
BEAST_EXPECT(cache.getMasterKey(kp0.first) == kp0.first);
|
||||
@@ -907,28 +907,28 @@ public:
|
||||
// higher sequence numbers
|
||||
auto const seq0 = cache.sequence();
|
||||
BEAST_EXPECT(
|
||||
cache.applyManifest(clone(sA0), ManifestRateLimitCap::Capped) ==
|
||||
cache.applyManifest(clone(sA0), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::Accepted);
|
||||
BEAST_EXPECT(cache.sequence() > seq0);
|
||||
|
||||
auto const seq1 = cache.sequence();
|
||||
BEAST_EXPECT(
|
||||
cache.applyManifest(clone(sA0), ManifestRateLimitCap::Capped) ==
|
||||
cache.applyManifest(clone(sA0), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::Stale);
|
||||
BEAST_EXPECT(cache.sequence() == seq1);
|
||||
|
||||
BEAST_EXPECT(
|
||||
cache.applyManifest(clone(sA1), ManifestRateLimitCap::Capped) ==
|
||||
cache.applyManifest(clone(sA1), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::Accepted);
|
||||
BEAST_EXPECT(
|
||||
cache.applyManifest(clone(sA1), ManifestRateLimitCap::Capped) ==
|
||||
cache.applyManifest(clone(sA1), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::Stale);
|
||||
BEAST_EXPECT(
|
||||
cache.applyManifest(clone(sA0), ManifestRateLimitCap::Capped) ==
|
||||
cache.applyManifest(clone(sA0), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::Stale);
|
||||
|
||||
BEAST_EXPECT(
|
||||
cache.applyManifest(clone(sA2), ManifestRateLimitCap::Capped) ==
|
||||
cache.applyManifest(clone(sA2), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::BadEphemeralKey);
|
||||
|
||||
// applyManifest should accept manifests with max sequence numbers
|
||||
@@ -936,38 +936,38 @@ public:
|
||||
BEAST_EXPECT(!cache.revoked(pkA));
|
||||
BEAST_EXPECT(sAMax.revoked());
|
||||
BEAST_EXPECT(
|
||||
cache.applyManifest(clone(sAMax), ManifestRateLimitCap::Capped) ==
|
||||
cache.applyManifest(clone(sAMax), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::Accepted);
|
||||
BEAST_EXPECT(
|
||||
cache.applyManifest(clone(sAMax), ManifestRateLimitCap::Capped) ==
|
||||
cache.applyManifest(clone(sAMax), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::Stale);
|
||||
BEAST_EXPECT(
|
||||
cache.applyManifest(clone(sA1), ManifestRateLimitCap::Capped) ==
|
||||
cache.applyManifest(clone(sA1), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::Stale);
|
||||
BEAST_EXPECT(
|
||||
cache.applyManifest(clone(sA0), ManifestRateLimitCap::Capped) ==
|
||||
cache.applyManifest(clone(sA0), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::Stale);
|
||||
BEAST_EXPECT(cache.revoked(pkA));
|
||||
|
||||
// applyManifest should reject manifests with invalid signatures
|
||||
BEAST_EXPECT(
|
||||
cache.applyManifest(clone(sB0), ManifestRateLimitCap::Capped) ==
|
||||
cache.applyManifest(clone(sB0), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::Accepted);
|
||||
BEAST_EXPECT(
|
||||
cache.applyManifest(clone(sB0), ManifestRateLimitCap::Capped) ==
|
||||
cache.applyManifest(clone(sB0), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::Stale);
|
||||
BEAST_EXPECT(!deserializeManifest(fake));
|
||||
BEAST_EXPECT(
|
||||
cache.applyManifest(clone(sB1), ManifestRateLimitCap::Capped) ==
|
||||
cache.applyManifest(clone(sB1), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::Invalid);
|
||||
BEAST_EXPECT(
|
||||
cache.applyManifest(clone(sB2), ManifestRateLimitCap::Capped) ==
|
||||
cache.applyManifest(clone(sB2), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::Accepted);
|
||||
|
||||
auto const sC0 = makeManifest(
|
||||
kpB2.second, KeyType::Ed25519, randomSecretKey(), KeyType::Ed25519, 47);
|
||||
BEAST_EXPECT(
|
||||
cache.applyManifest(clone(sC0), ManifestRateLimitCap::Capped) ==
|
||||
cache.applyManifest(clone(sC0), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::BadMasterKey);
|
||||
}
|
||||
|
||||
|
||||
@@ -279,7 +279,7 @@ private:
|
||||
|
||||
// NOLINTBEGIN(bugprone-unchecked-optional-access)
|
||||
manifests.applyManifest(
|
||||
*deserializeManifest(cfgManifest), ManifestRateLimitCap::Capped);
|
||||
*deserializeManifest(cfgManifest), ManifestRateLimitCapPolicy::Capped);
|
||||
// NOLINTEND(bugprone-unchecked-optional-access)
|
||||
BEAST_EXPECT(
|
||||
trustedKeys->load(localSigningPublicOuter, emptyCfgKeys, emptyCfgPublishers));
|
||||
@@ -372,7 +372,7 @@ private:
|
||||
|
||||
// NOLINTBEGIN(bugprone-unchecked-optional-access)
|
||||
manifests.applyManifest(
|
||||
*deserializeManifest(cfgManifest), ManifestRateLimitCap::Capped);
|
||||
*deserializeManifest(cfgManifest), ManifestRateLimitCapPolicy::Capped);
|
||||
// NOLINTEND(bugprone-unchecked-optional-access)
|
||||
|
||||
BEAST_EXPECT(trustedKeys->load(localSigningPublicOuter, cfgKeys, emptyCfgPublishers));
|
||||
@@ -466,7 +466,7 @@ private:
|
||||
pubRevokedSigning.first,
|
||||
pubRevokedSigning.second,
|
||||
std::numeric_limits<std::uint32_t>::max())),
|
||||
ManifestRateLimitCap::Capped);
|
||||
ManifestRateLimitCapPolicy::Capped);
|
||||
// NOLINTEND(bugprone-unchecked-optional-access)
|
||||
|
||||
// these two are not revoked (and not in the manifest cache at all.)
|
||||
@@ -508,7 +508,7 @@ private:
|
||||
pubRevokedSigning.first,
|
||||
pubRevokedSigning.second,
|
||||
std::numeric_limits<std::uint32_t>::max())),
|
||||
ManifestRateLimitCap::Capped);
|
||||
ManifestRateLimitCapPolicy::Capped);
|
||||
// NOLINTEND(bugprone-unchecked-optional-access)
|
||||
|
||||
// this one is not revoked (and not in the manifest cache at all.)
|
||||
@@ -1174,7 +1174,7 @@ private:
|
||||
|
||||
BEAST_EXPECT(
|
||||
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
|
||||
manifestsOuter.applyManifest(std::move(*m1), ManifestRateLimitCap::Capped) ==
|
||||
manifestsOuter.applyManifest(std::move(*m1), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::Accepted);
|
||||
BEAST_EXPECT(trustedKeysOuter->listed(masterPublic));
|
||||
BEAST_EXPECT(trustedKeysOuter->trusted(masterPublic));
|
||||
@@ -1189,7 +1189,7 @@ private:
|
||||
masterPublic, masterPrivate, signingPublic2, signingKeys2.second, 2));
|
||||
BEAST_EXPECT(
|
||||
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
|
||||
manifestsOuter.applyManifest(std::move(*m2), ManifestRateLimitCap::Capped) ==
|
||||
manifestsOuter.applyManifest(std::move(*m2), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::Accepted);
|
||||
BEAST_EXPECT(trustedKeysOuter->listed(masterPublic));
|
||||
BEAST_EXPECT(trustedKeysOuter->trusted(masterPublic));
|
||||
@@ -1207,7 +1207,7 @@ private:
|
||||
// NOLINTBEGIN(bugprone-unchecked-optional-access)
|
||||
BEAST_EXPECT(max->revoked());
|
||||
BEAST_EXPECT(
|
||||
manifestsOuter.applyManifest(std::move(*max), ManifestRateLimitCap::Capped) ==
|
||||
manifestsOuter.applyManifest(std::move(*max), ManifestRateLimitCapPolicy::Capped) ==
|
||||
ManifestDisposition::Accepted);
|
||||
// NOLINTEND(bugprone-unchecked-optional-access)
|
||||
|
||||
@@ -2627,7 +2627,7 @@ private:
|
||||
{
|
||||
valManifests.applyManifest(
|
||||
*deserializeManifest(base64Decode(self->manifest)),
|
||||
ManifestRateLimitCap::Capped);
|
||||
ManifestRateLimitCapPolicy::Capped);
|
||||
BEAST_EXPECT(
|
||||
result->load(self->signingPublic, emptyCfgKeys, cfgPublishers, threshold));
|
||||
}
|
||||
|
||||
@@ -1105,8 +1105,8 @@ ValidatorList::updatePublisherList(
|
||||
continue;
|
||||
}
|
||||
|
||||
if (auto const r =
|
||||
validatorManifests_.applyManifest(std::move(*m), ManifestRateLimitCap::Uncapped);
|
||||
if (auto const r = validatorManifests_.applyManifest(
|
||||
std::move(*m), ManifestRateLimitCapPolicy::Uncapped);
|
||||
r == ManifestDisposition::Invalid)
|
||||
{
|
||||
JLOG(j_.warn()) << "List for " << strHex(pubKey)
|
||||
@@ -1362,8 +1362,8 @@ ValidatorList::verify(
|
||||
|
||||
// Publisher keys are configured/trusted (checked above), so bypass the
|
||||
// untrusted cap.
|
||||
auto const result =
|
||||
publisherManifests_.applyManifest(std::move(manifest), ManifestRateLimitCap::Uncapped);
|
||||
auto const result = publisherManifests_.applyManifest(
|
||||
std::move(manifest), ManifestRateLimitCapPolicy::Uncapped);
|
||||
|
||||
if (revoked && result == ManifestDisposition::Accepted)
|
||||
{
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
#include <xrpl/basics/ByteUtilities.h>
|
||||
#include <xrpl/protocol/PublicKey.h>
|
||||
#include <xrpl/protocol/messages.h>
|
||||
#include <xrpl/server/Manifest.h>
|
||||
|
||||
#include <algorithm>
|
||||
#include <cstdint>
|
||||
@@ -13,6 +14,13 @@ namespace xrpl {
|
||||
|
||||
constexpr std::size_t kMaximumMessageSize = megabytes(64);
|
||||
|
||||
// Upper bound on the wire size of a TMManifests message: kMaxManifestsPerMessage entries
|
||||
// of at most kMaxManifestBytes each, plus a small allowance for protobuf
|
||||
// framing per entry.
|
||||
constexpr std::size_t kManifestFramingBytes = 8;
|
||||
constexpr std::size_t kMaximumManifestsMessageSize =
|
||||
kMaxManifestsPerMessage * (kMaxManifestBytes + kManifestFramingBytes);
|
||||
|
||||
// VFALCO NOTE If we forward declare Message and write out shared_ptr
|
||||
// instead of using the in-class type alias, we can remove the
|
||||
// entire ripple.pb.h from the main headers.
|
||||
|
||||
@@ -43,6 +43,7 @@
|
||||
#include <xrpl/protocol/Serializer.h>
|
||||
#include <xrpl/protocol/SystemParameters.h>
|
||||
#include <xrpl/protocol/jss.h>
|
||||
#include <xrpl/resource/Fees.h>
|
||||
#include <xrpl/resource/ResourceManager.h>
|
||||
#include <xrpl/server/Handoff.h>
|
||||
#include <xrpl/server/Manifest.h>
|
||||
@@ -661,12 +662,17 @@ OverlayImpl::onManifests(
|
||||
std::shared_ptr<protocol::TMManifests> const& m,
|
||||
std::shared_ptr<PeerImp> const& from)
|
||||
{
|
||||
auto const n = m->list_size();
|
||||
auto const& journal = from->pJournal();
|
||||
|
||||
// Process every trusted manifest, but stop processing untrusted ones once
|
||||
// kMaxManifestsPerMessage of them have been handled, so the work stays bounded.
|
||||
auto const total = static_cast<std::size_t>(m->list_size());
|
||||
std::size_t untrusted = 0;
|
||||
bool skippedUntrusted = false;
|
||||
|
||||
protocol::TMManifests relay;
|
||||
|
||||
for (std::size_t i = 0; i < n; ++i)
|
||||
for (std::size_t i = 0; i < total; ++i)
|
||||
{
|
||||
auto& s = m->list().Get(i).stobject();
|
||||
|
||||
@@ -677,6 +683,19 @@ OverlayImpl::onManifests(
|
||||
// lock: listed() takes the validator-list lock, so ordering it
|
||||
// first avoids holding the two locks in opposite orders.
|
||||
bool const isTrusted = app_.getValidators().listed(mo->masterKey);
|
||||
|
||||
// Bound untrusted work: process at most kMaxManifestsPerMessage
|
||||
// untrusted manifests, but never skip a trusted one. Trusted
|
||||
// manifests are not counted against the cap.
|
||||
if (!isTrusted)
|
||||
{
|
||||
if (untrusted >= kMaxManifestsPerMessage)
|
||||
{
|
||||
skippedUntrusted = true;
|
||||
continue;
|
||||
}
|
||||
++untrusted;
|
||||
}
|
||||
// Updates to a known key are relayed even when untrusted. Use
|
||||
// getSequence, not getManifest, to avoid copying the cached payload
|
||||
// on this hot path.
|
||||
@@ -685,7 +704,8 @@ OverlayImpl::onManifests(
|
||||
|
||||
auto const result = app_.getValidatorManifests().applyManifest(
|
||||
std::move(*mo),
|
||||
isTrusted ? ManifestRateLimitCap::Uncapped : ManifestRateLimitCap::Capped);
|
||||
isTrusted ? ManifestRateLimitCapPolicy::Uncapped
|
||||
: ManifestRateLimitCapPolicy::Capped);
|
||||
|
||||
if (result == ManifestDisposition::Accepted)
|
||||
{
|
||||
@@ -724,6 +744,18 @@ OverlayImpl::onManifests(
|
||||
}
|
||||
}
|
||||
|
||||
if (skippedUntrusted)
|
||||
{
|
||||
// The sender exceeded the untrusted per-message cap. Charge it (once,
|
||||
// here) so a flood of untrusted manifests is penalized, while an honest
|
||||
// message of trusted manifests never is.
|
||||
from->charge(Resource::kFeeMalformedRequest, "too many untrusted manifests");
|
||||
|
||||
JLOG(journal.warn()) << "Manifests: message had " << total
|
||||
<< " entries; processed all trusted plus the first "
|
||||
<< kMaxManifestsPerMessage << " untrusted";
|
||||
}
|
||||
|
||||
if (!relay.list().empty())
|
||||
{
|
||||
forEach([m2 = std::make_shared<Message>(relay, protocol::mtMANIFESTS)](
|
||||
@@ -1225,15 +1257,60 @@ OverlayImpl::getManifestsMessage()
|
||||
|
||||
if (auto seq = app_.getValidatorManifests().sequence(); seq != manifestListSeq_)
|
||||
{
|
||||
protocol::TMManifests tm;
|
||||
|
||||
// Phase 1: snapshot the cache under its own lock. Do not call
|
||||
// Validators::listed() here — that takes the validator-list lock, and
|
||||
// forEachManifest holds the manifest-cache lock, so consulting trust
|
||||
// inside the callback would invert the lock order used elsewhere
|
||||
// (see onManifests) and risk deadlock. Capture the manifest hash now,
|
||||
// while we have the Manifest object, for the suppression key.
|
||||
struct CachedManifest
|
||||
{
|
||||
PublicKey masterKey;
|
||||
std::string serialized;
|
||||
uint256 hash;
|
||||
};
|
||||
std::vector<CachedManifest> cached;
|
||||
app_.getValidatorManifests().forEachManifest(
|
||||
[&tm](std::size_t s) { tm.mutable_list()->Reserve(s); },
|
||||
[&tm, &hr = app_.getHashRouter()](Manifest const& manifest) {
|
||||
tm.add_list()->set_stobject(manifest.serialized.data(), manifest.serialized.size());
|
||||
hr.addSuppression(manifest.hash());
|
||||
[&cached](std::size_t s) { cached.reserve(s); },
|
||||
[&cached](Manifest const& manifest) {
|
||||
cached.push_back({manifest.masterKey, manifest.serialized, manifest.hash()});
|
||||
});
|
||||
|
||||
// Phase 2: no cache lock held, so trust checks are safe. Include every
|
||||
// trusted manifest, then fill any remaining headroom up to
|
||||
// kMaxManifestsPerMessage with untrusted gossip, so the whole message
|
||||
// stays within the per-message cap the receiver enforces (trusted
|
||||
// count is tiny in practice, so this effectively never drops trusted).
|
||||
std::vector<CachedManifest const*> selected;
|
||||
std::vector<CachedManifest const*> untrusted;
|
||||
for (auto const& e : cached)
|
||||
{
|
||||
if (app_.getValidators().listed(e.masterKey))
|
||||
{
|
||||
selected.push_back(&e);
|
||||
}
|
||||
else
|
||||
{
|
||||
untrusted.push_back(&e);
|
||||
}
|
||||
}
|
||||
|
||||
// Cap untrusted only; trusted manifests are all included above.
|
||||
auto const take = std::min(kMaxManifestsPerMessage, untrusted.size());
|
||||
selected.insert(selected.end(), untrusted.begin(), untrusted.begin() + take);
|
||||
|
||||
// Shuffle the order. Cryptographic randomness is not needed here.
|
||||
std::shuffle(selected.begin(), selected.end(), defaultPrng());
|
||||
|
||||
protocol::TMManifests tm;
|
||||
auto& hr = app_.getHashRouter();
|
||||
tm.mutable_list()->Reserve(static_cast<int>(selected.size()));
|
||||
for (auto const* e : selected)
|
||||
{
|
||||
tm.add_list()->set_stobject(e->serialized.data(), e->serialized.size());
|
||||
hr.addSuppression(e->hash);
|
||||
}
|
||||
|
||||
manifestMessage_.reset();
|
||||
|
||||
if (tm.list_size() != 0)
|
||||
|
||||
@@ -61,6 +61,7 @@
|
||||
#include <xrpl/resource/Gossip.h>
|
||||
#include <xrpl/server/Handoff.h>
|
||||
#include <xrpl/server/LoadFeeTrack.h>
|
||||
#include <xrpl/server/Manifest.h>
|
||||
#include <xrpl/server/NetworkOPs.h>
|
||||
#include <xrpl/shamap/SHAMapNodeID.h>
|
||||
#include <xrpl/tx/apply.h>
|
||||
@@ -1131,6 +1132,9 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMManifests> const& m)
|
||||
if (s > 100)
|
||||
fee_.update(Resource::kFeeModerateBurdenPeer, "oversize");
|
||||
|
||||
// OverlayImpl::onManifests bounds the untrusted work and charges the fee
|
||||
// if the untrusted count exceeds the per-message cap; trusted manifests
|
||||
// are always processed and not counted against it.
|
||||
app_.getJobQueue().addJob(JtManifest, "RcvManifests", [this, that = shared_from_this(), m]() {
|
||||
overlay_.onManifests(m, that);
|
||||
});
|
||||
|
||||
@@ -351,6 +351,16 @@ invokeProtocolMessage(Buffers const& buffers, Handler& handler, std::size_t& hin
|
||||
return result;
|
||||
}
|
||||
|
||||
// Drop an oversized TMManifests without penalty: consume the bytes and
|
||||
// return no error, so the connection is preserved.
|
||||
if (header->messageType == protocol::mtMANIFESTS &&
|
||||
(header->payloadWireSize > kMaximumManifestsMessageSize ||
|
||||
header->uncompressedSize > kMaximumManifestsMessageSize))
|
||||
{
|
||||
result.first = header->totalWireSize;
|
||||
return result;
|
||||
}
|
||||
|
||||
bool success = false;
|
||||
|
||||
switch (header->messageType)
|
||||
|
||||
Reference in New Issue
Block a user