diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 5e9d47f8c4c7..b95ebcac2470 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -142,6 +142,7 @@ BITCOIN_TESTS =\ test/llmq_hash_tests.cpp \ test/llmq_invalid_type_tests.cpp \ test/llmq_params_tests.cpp \ + test/llmq_qgetdata_tests.cpp \ test/llmq_snapshot_tests.cpp \ test/llmq_utils_tests.cpp \ test/logging_tests.cpp \ diff --git a/src/llmq/net_quorum.cpp b/src/llmq/net_quorum.cpp index c5a1c3320886..2393f26f2a2c 100644 --- a/src/llmq/net_quorum.cpp +++ b/src/llmq/net_quorum.cpp @@ -79,6 +79,18 @@ void NetQuorum::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataS CQuorumDataRequest request; vRecv >> request; + // nError is a response-only field on QDATA. Honest QGETDATA senders never + // emit it (serialization skips UNDEFINED), so a request carrying one is not a + // request we could ever have produced -- score it in full rather than tolerating + // a run of them. Accepting a requester-supplied value let an attacker pick + // QUORUM_VERIFICATION_VECTOR_MISSING / ENCRYPTED_CONTRIBUTIONS_MISSING and + // suppress the rate-limit ban while still forcing the expensive response + // construction path. + if (request.GetError() != CQuorumDataRequest::Errors::UNDEFINED) { + m_peer_manager->PeerMisbehaving(pfrom.GetId(), 100, "qgetdata with error field"); + return; + } + auto sendQDATA = [&](CQuorumDataRequest::Errors nError, bool request_limit_exceeded, const CDataStream& body = CDataStream(SER_NETWORK, PROTOCOL_VERSION)) -> bool { @@ -104,25 +116,42 @@ void NetQuorum::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataS return misbehave; }; - const CQuorumDataRequestKey key(pfrom.GetVerifiedProRegTxHash(), false, request.GetQuorumHash(), request.GetLLMQType()); - const bool request_limit_exceeded = !m_qman.RegisterDataRequest(key, request, /*add_expiry_bias=*/false); - + // Validate cheap, attacker-controlled fields before registering a tracking entry so + // garbage llmqType / unknown quorumHash values cannot grow mapQuorumDataRequests + // (and so rate-limit keys only cover requests that can reach the expensive path). + // Neither reply below reports a rate-limit state: they are reached before any + // tracking entry exists, so there is none to report. Cost is a ~93-byte reply to a + // ~93-byte request, so there is no amplification to gate either. if (!Params().GetLLMQ(request.GetLLMQType()).has_value()) { - // Unlike the misses below, this one cannot be explained by the peer being ahead of + // Unlike the miss below, this one cannot be explained by the peer being ahead of // us: no quorum of an unregistered type can exist on this chain, so there is // nothing to ask about. Answer with the error anyway, then score in full. - sendQDATA(CQuorumDataRequest::Errors::QUORUM_TYPE_INVALID, request_limit_exceeded); + sendQDATA(CQuorumDataRequest::Errors::QUORUM_TYPE_INVALID, /*request_limit_exceeded=*/false); m_peer_manager->PeerMisbehaving(pfrom.GetId(), 100, "invalid llmqType in QGETDATA"); return; } const CBlockIndex* pQuorumBaseBlockIndex = WITH_LOCK(::cs_main, return m_chainman.m_blockman.LookupBlockIndex(request.GetQuorumHash())); if (pQuorumBaseBlockIndex == nullptr) { - if (sendQDATA(CQuorumDataRequest::Errors::QUORUM_BLOCK_NOT_FOUND, request_limit_exceeded)) { - m_peer_manager->PeerMisbehaving(pfrom.GetId(), 25, "request limit exceeded"); - } + // A block we have not synced yet is legitimate for an honest peer, so this one is + // not scored on its own. + sendQDATA(CQuorumDataRequest::Errors::QUORUM_BLOCK_NOT_FOUND, /*request_limit_exceeded=*/false); + return; + } + + const CQuorumDataRequestKey key(pfrom.GetVerifiedProRegTxHash(), false, request.GetQuorumHash(), request.GetLLMQType()); + const auto registered = m_qman.RegisterDataRequest(key, request, /*add_expiry_bias=*/false); + if (!registered.has_value()) { + // Per-identity tracking budget exhausted: this peer has too many live requests + // outstanding. Score it and drop without doing any of the response work below -- + // an honest peer never reaches this, and the rate limit alone cannot bound the + // map because a fresh quorumHash is always a fresh key. + LogPrint(BCLog::LLMQ, "NetQuorum::%s -- %s: inbound request budget exhausted, from peer=%d\n", + __func__, msg_type, pfrom.GetId()); + m_peer_manager->PeerMisbehaving(pfrom.GetId(), 25, "too many quorum data requests"); return; } + const bool request_limit_exceeded = !*registered; const auto pQuorum = m_qman.GetQuorum(request.GetLLMQType(), request.GetQuorumHash()); if (pQuorum == nullptr) { @@ -321,7 +350,8 @@ DataRequestStatus NetQuorum::RequestQuorumData(CNode& peer, const CQuorum& quoru quorum.m_quorum_base_block_index->GetBlockHash(), quorum.qc->llmqType); const CQuorumDataRequest request(quorum.qc->llmqType, quorum.m_quorum_base_block_index->GetBlockHash(), nDataMask, proTxHash); - if (!m_qman.RegisterDataRequest(key, request)) { + // We initiated this request, so the inbound budget cannot apply and the result is engaged. + if (m_qman.RegisterDataRequest(key, request) != std::optional{true}) { return m_qman.GetDataRequestStatus(peer.GetVerifiedProRegTxHash(), /*we_requested=*/true, quorum.m_quorum_base_block_index->GetBlockHash(), quorum.qc->llmqType); } diff --git a/src/llmq/quorumsman.cpp b/src/llmq/quorumsman.cpp index d340631f6636..e06a16f24ad9 100644 --- a/src/llmq/quorumsman.cpp +++ b/src/llmq/quorumsman.cpp @@ -308,6 +308,13 @@ void CQuorumManager::CleanupExpiredDataRequests() const auto it = mapQuorumDataRequests.begin(); while (it != mapQuorumDataRequests.end()) { if (it->second.IsExpired(/*add_bias=*/true)) { + if (!it->first.m_we_requested) { + // Release the entry's per-identity budget, else a peer stays locked out forever. + if (auto count_it = m_inbound_request_counts.find(it->first.proRegTx); + count_it != m_inbound_request_counts.end() && --count_it->second == 0) { + m_inbound_request_counts.erase(count_it); + } + } it = mapQuorumDataRequests.erase(it); } else { ++it; @@ -370,19 +377,32 @@ CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, gsl::not_nul return BuildQuorumFromCommitment(llmqType, pQuorumBaseBlockIndex, populate_cache); } -bool CQuorumManager::RegisterDataRequest(const CQuorumDataRequestKey& key, const CQuorumDataRequest& request, - bool add_expiry_bias) const +std::optional CQuorumManager::RegisterDataRequest(const CQuorumDataRequestKey& key, + const CQuorumDataRequest& request, + bool add_expiry_bias) const { LOCK(cs_data_requests); + // A peer-initiated request for an unseen key consumes tracking budget. The rate limit cannot + // bound the map on its own: a fresh quorumHash is always a fresh key, so it is never "already + // pending". Re-requests of an existing key fall through to the rate limit below instead. + if (!key.m_we_requested && !mapQuorumDataRequests.count(key)) { + if (auto it = m_inbound_request_counts.find(key.proRegTx); + it != m_inbound_request_counts.end() && it->second >= MAX_INBOUND_DATA_REQUESTS) { + return std::nullopt; + } + } auto [old_pair, inserted] = mapQuorumDataRequests.emplace(key, request); - if (!inserted) { - if (old_pair->second.IsExpired(add_expiry_bias)) { - old_pair->second = request; - return true; + if (inserted) { + if (!key.m_we_requested) { + ++m_inbound_request_counts[key.proRegTx]; } - return false; + return true; } - return true; + if (old_pair->second.IsExpired(add_expiry_bias)) { + old_pair->second = request; + return true; + } + return false; } CQuorumManager::DataResponseValidation CQuorumManager::ValidateDataResponse( diff --git a/src/llmq/quorumsman.h b/src/llmq/quorumsman.h index 1a35b1cf0ab0..17dfbaa84756 100644 --- a/src/llmq/quorumsman.h +++ b/src/llmq/quorumsman.h @@ -21,6 +21,7 @@ #include #include #include +#include #include class CBLSSignature; @@ -48,6 +49,18 @@ class CDKGSessionManager; class CQuorumBlockProcessor; class CQuorumSnapshotManager; +//! Per-identity budget for live peer-initiated QGETDATA tracking entries. +//! +//! Entries are keyed on the attacker-chosen quorumHash, so without a cap a peer that never +//! repeats a hash is never rate-limited and grows mapQuorumDataRequests unboundedly for the +//! 300s+bias expiry window (cleanup only runs per-block, and not at all during IBD). +//! The budget is per requesting identity rather than global so one peer cannot evict or +//! starve another; all qwatch peers share the null proRegTx identity and therefore one budget, +//! matching the existing rate-limit behaviour for that class of peer. +//! An honest peer requests at most vvec+contributions for a handful of quorums it is recovering, +//! so this is orders of magnitude above legitimate use. +static constexpr size_t MAX_INBOUND_DATA_REQUESTS{64}; + /** * The quorum manager maintains quorums which were mined on chain. When a quorum is requested from the manager, * it will lookup the commitment (through CQuorumBlockProcessor) and build a CQuorum object from it. @@ -70,6 +83,10 @@ class CQuorumManager final mutable Mutex cs_data_requests; mutable std::unordered_map mapQuorumDataRequests GUARDED_BY(cs_data_requests); + //! Live peer-initiated entries in mapQuorumDataRequests, counted per requesting identity so + //! the per-identity budget can be enforced without scanning the whole map. + mutable std::unordered_map m_inbound_request_counts + GUARDED_BY(cs_data_requests); mutable Mutex m_cs_maps; mutable std::map> mapQuorumsCache @@ -131,9 +148,12 @@ class CQuorumManager final bool IsMasternode() const; bool IsWatching() const; - //! Request tracking for QGETDATA/QDATA — used by NetQuorum and RPC - bool RegisterDataRequest(const CQuorumDataRequestKey& key, const CQuorumDataRequest& request, - bool add_expiry_bias = true) const + //! Request tracking for QGETDATA/QDATA — used by NetQuorum and RPC. + //! Returns nullopt when a peer-initiated request would exceed that identity's tracking + //! budget, true when the entry was created or refreshed, and false when an unexpired entry + //! already exists (the rate limit applies). + std::optional RegisterDataRequest(const CQuorumDataRequestKey& key, const CQuorumDataRequest& request, + bool add_expiry_bias = true) const EXCLUSIVE_LOCKS_REQUIRED(!cs_data_requests); enum class DataResponseValidation : uint8_t { OK, NotRequested, AlreadyReceived, Mismatch }; DataResponseValidation ValidateDataResponse(const CQuorumDataRequestKey& key, diff --git a/src/test/llmq_qgetdata_tests.cpp b/src/test/llmq_qgetdata_tests.cpp new file mode 100644 index 000000000000..6532abb7154c --- /dev/null +++ b/src/test/llmq_qgetdata_tests.cpp @@ -0,0 +1,250 @@ +// Copyright (c) 2026 The Dash Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include + +#include +#include +#include + +using namespace llmq; + +namespace { + +//! Minimal QuorumRole so NetQuorum accepts QGETDATA as if we were a masternode. +struct MockMasternodeRole final : public QuorumRole { + explicit MockMasternodeRole(CQuorumManager& qman) : QuorumRole(qman) {} + bool IsMasternode() const override { return true; } + bool IsWatching() const override { return false; } + bool SetQuorumSecretKeyShare(CQuorum& /*quorum*/, Span /*skContributions*/) const override + { + return false; + } +}; + +struct QGetDataSetup : public TestingSetup { + MockMasternodeRole m_role; + std::unique_ptr m_net_quorum; + + QGetDataSetup() : + TestingSetup{CBaseChainParams::REGTEST}, + m_role{*m_node.llmq_ctx->qman} + { + BOOST_REQUIRE(m_node.connman); + BOOST_REQUIRE(m_node.peerman); + BOOST_REQUIRE(m_node.dmnman); + BOOST_REQUIRE(m_node.llmq_ctx); + BOOST_REQUIRE(m_node.mn_sync); + BOOST_REQUIRE(m_node.sporkman); + BOOST_REQUIRE(m_node.chainman); + + // Mirror init.cpp: TestingSetup does not register NetQuorum, so install + // one with a masternode role so the QGETDATA gate opens for qwatch peers. + m_net_quorum = std::make_unique( + m_node.peerman.get(), *m_node.llmq_ctx->bls_worker, *m_node.connman, *m_node.dmnman, + *m_node.llmq_ctx->qman, *m_node.llmq_ctx->qsnapman, *m_node.chainman, *m_node.mn_sync, + *m_node.sporkman, &m_role, /*nodeman=*/nullptr, DEFAULT_WORKER_COUNT, QvvecSyncModeMap{}, + /*quorums_recovery=*/false); + } + + ~QGetDataSetup() + { + m_net_quorum.reset(); + } +}; + +std::unique_ptr MakePeer(NodeId id) +{ + in_addr peer_in_addr{}; + peer_in_addr.s_addr = htonl(0x0a000001 + static_cast(id)); + auto peer{std::make_unique(id, + /*sock=*/nullptr, + /*addrIn=*/CAddress{CService{peer_in_addr, 9999}, NODE_NETWORK}, + /*nKeyedNetGroupIn=*/0, + /*nLocalHostNonceIn=*/0, + /*addrBindIn=*/CAddress{}, + /*addrNameIn=*/std::string{}, + /*conn_type_in=*/ConnectionType::INBOUND, + /*inbound_onion=*/false)}; + peer->nVersion = PROTOCOL_VERSION; + peer->SetCommonVersion(PROTOCOL_VERSION); + peer->fSuccessfullyConnected = true; + // Unauthenticated QWATCH path: any peer can set this flag and then send QGETDATA. + peer->qwatch = true; + return peer; +} + +void AssertMisbehaviorScore(PeerManager& peerman, const CNode& peer, int expected) +{ + CNodeStateStats stats; + BOOST_REQUIRE(peerman.GetNodeStateStats(peer.GetId(), stats)); + BOOST_CHECK_EQUAL(stats.m_misbehavior_score, expected); +} + +//! Build a QGETDATA payload; when error_byte is set, append the wire nError field +//! that honest requesters never emit (only QDATA responses carry it). +CDataStream MakeQGetDataStream(Consensus::LLMQType llmq_type, const uint256& quorum_hash, uint16_t data_mask, + const uint256& protx_hash, std::optional error_byte = std::nullopt) +{ + CDataStream stream{SER_NETWORK, PROTOCOL_VERSION}; + stream << static_cast(llmq_type); + stream << quorum_hash; + stream << data_mask; + stream << protx_hash; + if (error_byte.has_value()) { + stream << *error_byte; + } + return stream; +} + +} // namespace + +BOOST_FIXTURE_TEST_SUITE(llmq_qgetdata_tests, QGetDataSetup) + +// An attacker-supplied nError on QGETDATA used to be accepted and then +// substituted into sendQDATA, selecting the *_MISSING branches that intentionally +// skip the rate-limit ban. A request must never carry an error code — reject it. +BOOST_AUTO_TEST_CASE(qgetdata_rejects_attacker_supplied_error) +{ + LOCK(NetEventsInterface::g_msgproc_mutex); + + auto peer{MakePeer(/*id=*/1)}; + m_node.peerman->InitializeNode(*peer, NODE_NETWORK); + AssertMisbehaviorScore(*m_node.peerman, *peer, 0); + + // Any known LLMQ type is fine; the nError check must fire before body work. + const uint256 quorum_hash{uint256S("0x11")}; + const uint256 protx_hash{uint256S("0x22")}; + auto stream = MakeQGetDataStream(Consensus::LLMQType::LLMQ_TEST, quorum_hash, + CQuorumDataRequest::QUORUM_VERIFICATION_VECTOR, protx_hash, + /*error_byte=*/static_cast( + CQuorumDataRequest::Errors::ENCRYPTED_CONTRIBUTIONS_MISSING)); + + m_net_quorum->ProcessMessage(*peer, NetMsgType::QGETDATA, stream); + + // Pre-fix: score stays 0 (nError steers the ban decision; no score applied). + // Post-fix: a request we could never have produced is scored in full. + AssertMisbehaviorScore(*m_node.peerman, *peer, 100); +} + +// Honest QGETDATA (no trailing error byte) must still be accepted by the gate +// and not scored solely for arriving over qwatch. +BOOST_AUTO_TEST_CASE(qgetdata_without_error_is_not_scored_for_malformed) +{ + LOCK(NetEventsInterface::g_msgproc_mutex); + + auto peer{MakePeer(/*id=*/2)}; + m_node.peerman->InitializeNode(*peer, NODE_NETWORK); + AssertMisbehaviorScore(*m_node.peerman, *peer, 0); + + const uint256 quorum_hash{uint256S("0x33")}; + const uint256 protx_hash{uint256S("0x44")}; + auto stream = MakeQGetDataStream(Consensus::LLMQType::LLMQ_TEST, quorum_hash, + CQuorumDataRequest::QUORUM_VERIFICATION_VECTOR, protx_hash); + + m_net_quorum->ProcessMessage(*peer, NetMsgType::QGETDATA, stream); + + // Missing block / missing quorum replies do not score unless the request + // limit is exceeded. First request must leave the score at zero. + AssertMisbehaviorScore(*m_node.peerman, *peer, 0); +} + +// Rate-limit still applies to repeated honest requests for the same real block +// key (even when no quorum was mined there — QUORUM_NOT_FOUND still rates). +BOOST_AUTO_TEST_CASE(qgetdata_rate_limit_scores_repeat_requests) +{ + LOCK(NetEventsInterface::g_msgproc_mutex); + + auto peer{MakePeer(/*id=*/3)}; + m_node.peerman->InitializeNode(*peer, NODE_NETWORK); + AssertMisbehaviorScore(*m_node.peerman, *peer, 0); + + // Use a block that exists so registration runs; no mined commitment means + // QUORUM_NOT_FOUND, which still participates in the rate-limit ban path. + const uint256 quorum_hash{ + WITH_LOCK(::cs_main, return m_node.chainman->ActiveChain().Genesis()->GetBlockHash())}; + const uint256 protx_hash{uint256S("0x66")}; + + auto stream1 = MakeQGetDataStream(Consensus::LLMQType::LLMQ_TEST, quorum_hash, + CQuorumDataRequest::QUORUM_VERIFICATION_VECTOR, protx_hash); + m_net_quorum->ProcessMessage(*peer, NetMsgType::QGETDATA, stream1); + AssertMisbehaviorScore(*m_node.peerman, *peer, 0); + + auto stream2 = MakeQGetDataStream(Consensus::LLMQType::LLMQ_TEST, quorum_hash, + CQuorumDataRequest::QUORUM_VERIFICATION_VECTOR, protx_hash); + m_net_quorum->ProcessMessage(*peer, NetMsgType::QGETDATA, stream2); + // Second request hits request_limit_exceeded on the QUORUM_NOT_FOUND path. + AssertMisbehaviorScore(*m_node.peerman, *peer, 25); +} + +// Every QGETDATA naming a *new* quorumHash creates a fresh tracking-map entry, +// and a fresh key is never "request limit exceeded", so the rate limit alone never fires. +// Only a budget on live entries bounds the map. Requests are made for a real block so they +// survive the cheap pre-checks and reach RegisterDataRequest. +BOOST_AUTO_TEST_CASE(qgetdata_caps_inbound_request_tracking_entries) +{ + LOCK(NetEventsInterface::g_msgproc_mutex); + + auto peer{MakePeer(/*id=*/4)}; + m_node.peerman->InitializeNode(*peer, NODE_NETWORK); + + const uint256 quorum_hash{ + WITH_LOCK(::cs_main, return m_node.chainman->ActiveChain().Genesis()->GetBlockHash())}; + + // Distinct llmqTypes are scarce, so vary the key via the (also attacker-chosen) quorumHash + // where possible; here a single valid block hash is reused, which means the first request + // registers and every later one is a repeat. Use the manager directly to prove the budget: + // distinct hashes are what an attacker actually varies. + const uint256 protx_hash{peer->GetVerifiedProRegTxHash()}; + for (size_t i = 0; i < MAX_INBOUND_DATA_REQUESTS; ++i) { + const CQuorumDataRequestKey key{protx_hash, /*we_requested=*/false, uint256{ArithToUint256(arith_uint256{i})}, + Consensus::LLMQType::LLMQ_TEST}; + const CQuorumDataRequest request{Consensus::LLMQType::LLMQ_TEST, key.quorumHash, + CQuorumDataRequest::QUORUM_VERIFICATION_VECTOR}; + // Each distinct hash is a brand-new key: always "not already pending", never rate-limited. + BOOST_CHECK(m_node.llmq_ctx->qman->RegisterDataRequest(key, request, /*add_expiry_bias=*/false) == + std::optional{true}); + } + + // Budget is now exhausted: a further *new* key is refused outright rather than tracked. + const CQuorumDataRequestKey over_key{protx_hash, /*we_requested=*/false, uint256S("0xdead"), + Consensus::LLMQType::LLMQ_TEST}; + const CQuorumDataRequest over_request{Consensus::LLMQType::LLMQ_TEST, over_key.quorumHash, + CQuorumDataRequest::QUORUM_VERIFICATION_VECTOR}; + BOOST_CHECK(m_node.llmq_ctx->qman->RegisterDataRequest(over_key, over_request, /*add_expiry_bias=*/false) == + std::nullopt); + + // ...and the QGETDATA handler turns that refusal into a misbehaviour score instead of + // doing the response work. + auto stream = MakeQGetDataStream(Consensus::LLMQType::LLMQ_TEST, quorum_hash, + CQuorumDataRequest::QUORUM_VERIFICATION_VECTOR, protx_hash); + m_net_quorum->ProcessMessage(*peer, NetMsgType::QGETDATA, stream); + AssertMisbehaviorScore(*m_node.peerman, *peer, 25); + + // A separate identity has its own budget, so one peer cannot starve another. + const CQuorumDataRequestKey other_key{uint256S("0xfeed"), /*we_requested=*/false, uint256S("0xdead"), + Consensus::LLMQType::LLMQ_TEST}; + BOOST_CHECK(m_node.llmq_ctx->qman->RegisterDataRequest(other_key, over_request, /*add_expiry_bias=*/false) == + std::optional{true}); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/test/functional/p2p_quorum_data.py b/test/functional/p2p_quorum_data.py index baa65f98267a..5e5d7e7ba207 100755 --- a/test/functional/p2p_quorum_data.py +++ b/test/functional/p2p_quorum_data.py @@ -376,6 +376,28 @@ def send_bad_qdata_expect_disconnect(bad_qdata): self.restart_mn(mn1) self.wait_for_quorum_data([mn1], 100, quorum_hash, recover=False) + # Attacker-supplied nError on QGETDATA must not suppress the rate-limit ban. + def test_attacker_error_cannot_bypass_rate_limit(): + self.log.info("Test attacker-supplied nError cannot bypass QGETDATA rate-limit ban") + force_request_expire() + p2p_mn = p2p_connection(mn2.get_node(self)) + id_p2p_mn = get_p2p_id(mn2.get_node(self)) + mnauth(mn2.get_node(self), id_p2p_mn, fake_mnauth_2[0], fake_mnauth_2[1]) + wait_for_banscore(mn2.get_node(self), id_p2p_mn, 0) + + # Honest first request establishes the tracking-map entry. + p2p_mn.test_qgetdata(qgetdata_vvec, 0, self.llmq_threshold, 0) + wait_for_banscore(mn2.get_node(self), id_p2p_mn, 0) + + # Second request with smuggled ENCRYPTED_CONTRIBUTIONS_MISSING (0x06) + # used to make sendQDATA skip misbehaviour. Post-fix a request carrying an + # error field is one we could never have produced, so the peer is scored in + # full and dropped on the first one instead of merely being slowed down. + poisoned = msg_qgetdata(quorum_hash_int, 100, 0x01, error=ENCRYPTED_CONTRIBUTIONS_MISSING) + p2p_mn.send_message(poisoned) + self.wait_until(lambda: not p2p_mn.is_connected, timeout=10) + mn2.get_node(self).disconnect_p2ps() + # Test request limiting / banscore increase def test_request_limit(): @@ -526,6 +548,7 @@ def test_qsigshares_inv_oom(): # Test with/without expired request cleanup for self.cleanup in [True, False]: test_basics() + test_attacker_error_cannot_bypass_rate_limit() test_request_limit() test_qwatch_connections() test_watchquorums() diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 5f66dc6af064..b2a47c74244c 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -2563,20 +2563,27 @@ def __repr__(self): class msg_qgetdata: - __slots__ = ("quorum_hash", "quorum_type", "data_mask", "protx_hash") + __slots__ = ("quorum_hash", "quorum_type", "data_mask", "protx_hash", "error") msgtype = b"qgetdata" - def __init__(self, quorum_hash=0, quorum_type=-1, data_mask=0, protx_hash=0): + def __init__(self, quorum_hash=0, quorum_type=-1, data_mask=0, protx_hash=0, error=None): self.quorum_hash = quorum_hash self.quorum_type = quorum_type self.data_mask = data_mask self.protx_hash = protx_hash + # error is response-only on the wire. Honest requesters leave it None so + # it is not serialized. Attackers can set it to smuggle a QDATA error + # code into a request (see CQuorumDataRequest SERIALIZE_METHODS). + self.error = error def deserialize(self, f): self.quorum_type = struct.unpack("