From 6d3a8538a2b9f96045e60ff776341cd37d42cc2a Mon Sep 17 00:00:00 2001 From: Nicholas Dudfield Date: Fri, 18 Sep 2026 20:38:54 +0700 Subject: [PATCH] test(consensus): prove observer signature rejection from collector-commit trace B7 now waits for the honest encoding to be admitted on the observer, checks that observer's validated latch is still live, and sends one forged frame on the observer's peer. The observer extension journal must contain a single collector-commit record for that wire hash with signatureVerified=false and AdmitResult::invalid. Pre-commit leak finder hits protobuf TMExportShares::shares(); wire field name, not audited-hook vocabulary. --- .../consensus/SteppingExtensions_test.cpp | 307 +++++++++--------- 1 file changed, 146 insertions(+), 161 deletions(-) diff --git a/src/test/consensus/SteppingExtensions_test.cpp b/src/test/consensus/SteppingExtensions_test.cpp index fdcfa8bda2..87029bde48 100644 --- a/src/test/consensus/SteppingExtensions_test.cpp +++ b/src/test/consensus/SteppingExtensions_test.cpp @@ -1,12 +1,14 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -25,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -98,7 +101,6 @@ class SteppingExtensions_test : public beast::unit_test::suite std::string badShareBytes; Buffer badSignature; std::uint32_t badFramesReceived = 0; - std::uint32_t badShareJobs = 0; bool sawBadWhilePending = false; }; @@ -168,43 +170,6 @@ class SteppingExtensions_test : public beast::unit_test::suite return positions.size(); } - static bool - originSidecarHasSignature( - SHAMap const& map, - uint256 const& origin, - Slice signature) - { - bool found = false; - map.visitLeaves( - [&](boost::intrusive_ptr const& item) { - if (found) - return; - try - { - SerialIter sit{item->slice()}; - STObject const obj{sit, sfGeneric}; - if (obj.isFieldPresent(sfSidecarType) && - obj.getFieldU8(sfSidecarType) == sidecarExportSig && - obj.isFieldPresent(sfTransactionHash) && - obj.getFieldH256(sfTransactionHash) == origin && - obj.isFieldPresent(sfTxnSignature)) - { - auto const got = obj.getFieldVL(sfTxnSignature); - if (got.size() == signature.size() && - std::equal( - got.data(), - got.data() + got.size(), - signature.data())) - found = true; - } - } - catch (...) - { - } - }); - return found; - } - static bool authorizedAtEmission(Application& app, ExportShare const& share) { @@ -265,7 +230,8 @@ class SteppingExtensions_test : public beast::unit_test::suite directBatchId(batch)); for (auto const& blob : batch.shares()) { - if (stats->captureOrigin && !stats->honestShare) + if (id == observer && stats->captureOrigin && + !stats->honestShare) { auto const share = ExportShare::parse(makeSlice(blob)); @@ -273,7 +239,8 @@ class SteppingExtensions_test : public beast::unit_test::suite share->originTxn == *stats->captureOrigin) stats->honestShare = share; } - if (!stats->badShareBytes.empty() && + if (id == observer && + !stats->badShareBytes.empty() && blob == stats->badShareBytes) ++stats->badFramesReceived; } @@ -1453,8 +1420,7 @@ class SteppingExtensions_test : public beast::unit_test::suite if (!BEAST_EXPECT(net.minValidatedSeq() >= target)) return std::nullopt; if (net.controller().now() <= staleHorizon) - net.settle( - (staleHorizon + 1s) - net.controller().now(), 1'200'000); + net.settle((staleHorizon + 1s) - net.controller().now(), 1'200'000); BEAST_EXPECT(net.controller().now() > staleHorizon); BEAST_EXPECT(stats->newGenerationRan); BEAST_EXPECT(!stats->oldGenerationRan); @@ -1547,6 +1513,15 @@ class SteppingExtensions_test : public beast::unit_test::suite World world(net, false, true); if (!ready(world)) return std::nullopt; + + test::StreamSink sink{beast::severities::kTrace}; + auto& observerCE = net.node(observer).app().getConsensusExtensions(); + auto const previousJournal = observerCE.j_; + observerCE.j_ = beast::Journal{sink}; + scope_exit restoreJournal{[&observerCE, previousJournal]() { + observerCE.j_ = previousJournal; + }}; + auto const funding = world.submit( observer, jtx::pay(jtx::Account::master, world.owner, jtx::XRP(10'000)), @@ -1564,13 +1539,10 @@ class SteppingExtensions_test : public beast::unit_test::suite auto const origin = tx->getID(); stats->captureOrigin = origin; - auto collectorHasHonest = [&](std::uint32_t node) { + auto collectorHasHonest = [&]() { if (!stats->honestShare) return false; - auto const snap = net.node(node) - .app() - .getConsensusExtensions() - .postValidationExportSigCollector() + auto const snap = observerCE.postValidationExportSigCollector() .fullUnionSnapshot(); auto const it = snap.find(origin); if (it == snap.end()) @@ -1578,8 +1550,7 @@ class SteppingExtensions_test : public beast::unit_test::suite for (auto const& contrib : it->second) { if (contrib.signingKey == stats->honestShare->signingKey && - contrib.position == - stats->honestShare->committeePosition && + contrib.position == stats->honestShare->committeePosition && contrib.signature.size() == stats->honestShare->signature.size() && std::equal( @@ -1591,110 +1562,140 @@ class SteppingExtensions_test : public beast::unit_test::suite return false; }; - auto pendingOn = [&](std::uint32_t node) { - return net.node(node) - .app() - .getConsensusExtensions() - .hasEligiblePendingExports(); + auto observerOriginLive = [&]() { + if (!stats->honestShare) + return false; + auto const& share = *stats->honestShare; + if (net.validSeq(observer) < share.originLedgerSeq) + return false; + if (net.ledgerHash(observer, share.originLedgerSeq) != + share.originLedgerHash) + return false; + auto const ledger = net.ledger(observer, net.validSeq(observer)); + if (!ledger) + return false; + auto const latch = + ledger->read(keylet::exportLatch(share.owner, share.originTxn)); + return latch && latch->getType() == ltEXPORT_LATCH && + !latch->isFieldPresent(sfExportSignatureHash) && + latch->getFieldH256(sfTransactionHash) == share.originTxn && + ledger->seq() <= latch->getFieldU32(sfLastLedgerSequence); }; auto const junk = generateKeyPair( KeyType::secp256k1, generateSeed("dsf-b7-invalid-share")); + auto const invalidResult = std::to_string( + static_cast(ExportSigCollector::AdmitResult::invalid)); std::size_t sent = 0; bool injected = false; - bool honestKeptAfterBad = false; - bool conflictAfterBad = false; - net.controller().observeJobs( - [&](std::uint32_t id, JobType type, std::string const& name) { - if (injected) - { - if (id == observer && type == jtEXPORT_SHARES && - name == "recvExportShares") - ++stats->badShareJobs; - if (stats->badFramesReceived != 0) - { - if (collectorHasHonest(observer)) - honestKeptAfterBad = true; - auto const st = - net.node(observer) - .app() - .getConsensusExtensions() - .postValidationExportSigCollector() - .positionStatus( - origin, - stats->honestShare->committeePosition); - if (st == ExportSigCollector::PositionStatus::conflicted) - conflictAfterBad = true; - } - return; - } - if (!stats->honestShare) - return; - if (stats->honestShare->originLedgerSeq > net.validSeq(0)) - return; - if (!pendingOn(0)) - return; - auto bad = *stats->honestShare; - bad.signature = sign( - junk.first, junk.second, Slice{"dsf-b7-not-payload", 18}); - if (!bad.validShape()) - return; - stats->badSignature = bad.signature; - auto const framed = bad.serialize(); - stats->badShareBytes.assign( - reinterpret_cast(framed.data()), - framed.size()); - protocol::TMExportShares batch; - batch.add_shares(framed.data(), framed.size()); - auto const msg = std::make_shared( - batch, protocol::mtEXPORT_SHARES); - for (auto const& peer : - net.node(0).app().overlay().getActivePeers()) - { - peer->send(msg); - ++sent; - } - stats->sawBadWhilePending = pendingOn(0); - injected = true; - }); + std::size_t captureMark = 0; + uint256 badWire{}; + net.controller().observeJobs([&](std::uint32_t, + JobType, + std::string const&) { + if (injected) + return; + if (!stats->honestShare || !collectorHasHonest() || + !observerOriginLive()) + return; + auto bad = *stats->honestShare; + bad.signature = + sign(junk.first, junk.second, Slice{"dsf-b7-not-payload", 18}); + if (!bad.validShape()) + return; + stats->badSignature = bad.signature; + badWire = bad.wireHash(); + auto const framed = bad.serialize(); + stats->badShareBytes.assign( + reinterpret_cast(framed.data()), framed.size()); + protocol::TMExportShares batch; + batch.add_shares(framed.data(), framed.size()); + auto const msg = + std::make_shared(batch, protocol::mtEXPORT_SHARES); + auto const targetPub = + net.node(observer).app().nodeIdentity().first; + for (auto const& peer : + net.node(0).app().overlay().getActivePeers()) + { + if (peer->getNodePublic() != targetPub) + continue; + peer->send(msg); + ++sent; + break; + } + captureMark = sink.messages().str().size(); + stats->sawBadWhilePending = observerOriginLive(); + injected = true; + }); net.runTo(warmLedger + 8, SteppingNetwork::RunBudget{1600, 1'200'000}); net.controller().observeJobs({}); - if (!BEAST_EXPECT(injected && stats->honestShare && sent != 0)) + if (!BEAST_EXPECT(injected && stats->honestShare && sent == 1)) { - log << " no honest pending contribution before inject" + log << " no observer-admitted pending origin before inject" << " captured=" << static_cast(stats->honestShare) - << " pending0=" << pendingOn(0) - << " pendingObserver=" << pendingOn(observer) - << " injected=" << injected << " sent=" << sent << std::endl; + << " admitted=" << collectorHasHonest() + << " live=" << observerOriginLive() << " injected=" << injected + << " sent=" << sent << std::endl; + return std::nullopt; + } + if (!BEAST_EXPECT(stats->sawBadWhilePending)) + return std::nullopt; + if (!BEAST_EXPECT(stats->badFramesReceived == 1)) + { + log << " observer did not receive exactly one forged frame" + << " received=" << stats->badFramesReceived << " sent=" << sent + << std::endl; return std::nullopt; } - if (!BEAST_EXPECT(stats->badFramesReceived != 0)) + auto const captured = sink.messages().str().substr(captureMark); + auto const originText = "origin=" + to_string(origin); + auto const wireText = "wire=" + to_string(badWire); + auto const commitNeedle = std::string{"ExportShare: collector commit"}; + bool sawRejection = false; { - log << " bad share never arrived on the wire sent=" << sent - << std::endl; + std::istringstream in{captured}; + for (std::string line; std::getline(in, line);) + { + if (line.find(commitNeedle) == std::string::npos) + continue; + if (line.find(originText) == std::string::npos) + continue; + if (line.find(wireText) == std::string::npos) + continue; + if (line.find("signatureVerified=false") == std::string::npos) + continue; + if (line.find("result=" + invalidResult) == std::string::npos) + continue; + sawRejection = true; + break; + } + } + if (!BEAST_EXPECT(sawRejection)) + { + log << " no observer collector-commit rejection for forged wire" + << " " << originText << " " << wireText + << " result=" << invalidResult << std::endl; + std::istringstream in{captured}; + for (std::string line; std::getline(in, line);) + { + if (line.find("ExportShare:") == std::string::npos) + continue; + if (line.find(originText) == std::string::npos && + line.find(wireText) == std::string::npos) + continue; + log << " " << line << std::endl; + } return std::nullopt; } - if (!BEAST_EXPECT(stats->badShareJobs != 0)) - { - log << " recvExportShares did not run after inject jobs=" - << stats->badShareJobs << " " << net.jobDiagnostics() - << std::endl; - return std::nullopt; - } - BEAST_EXPECT(stats->sawBadWhilePending); - BEAST_EXPECT(honestKeptAfterBad); - BEAST_EXPECT(!conflictAfterBad); - BEAST_EXPECT(collectorHasHonest(observer)); - BEAST_EXPECT(collectorHasHonest(0)); - auto const& collector = - net.node(observer).app().getConsensusExtensions() - .postValidationExportSigCollector(); + + BEAST_EXPECT(collectorHasHonest()); BEAST_EXPECT( - collector.positionStatus( - origin, stats->honestShare->committeePosition) != - ExportSigCollector::PositionStatus::conflicted); - auto const snapAfter = collector.fullUnionSnapshot(); + observerCE.postValidationExportSigCollector().positionStatus( + origin, stats->honestShare->committeePosition) == + ExportSigCollector::PositionStatus::unique); + auto const snapAfter = + observerCE.postValidationExportSigCollector().fullUnionSnapshot(); auto const foundAfter = snapAfter.find(origin); if (!BEAST_EXPECT(foundAfter != snapAfter.end())) return std::nullopt; @@ -1705,23 +1706,6 @@ class SteppingExtensions_test : public beast::unit_test::suite contrib.signature.data(), contrib.signature.data() + contrib.signature.size(), stats->badSignature.data())); - if (auto const map = net.node(observer) - .app() - .getConsensusExtensions() - .exportSigSetMap_) - { - BEAST_EXPECT(!originSidecarHasSignature( - *map, - origin, - Slice{ - stats->badSignature.data(), stats->badSignature.size()})); - BEAST_EXPECT(originSidecarHasSignature( - *map, - origin, - Slice{ - stats->honestShare->signature.data(), - stats->honestShare->signature.size()})); - } auto const target = net.validSeq(0) + 4; net.runTo(target, SteppingNetwork::RunBudget{1600, 1'200'000}); @@ -1737,9 +1721,11 @@ class SteppingExtensions_test : public beast::unit_test::suite auto const seqW = witnessAt(net, origin, warmLedger); if (!BEAST_EXPECT(seqW != 0)) return std::nullopt; - BEAST_EXPECT(collectorHasHonest(0)); + BEAST_EXPECT(stats->builds[observer].contains( + {seqW, net.ledgerHash(0, seqW - 1), net.ledgerHash(0, seqW)})); + BEAST_EXPECT(collectorHasHonest()); BEAST_EXPECT( - collector.positionStatus( + observerCE.postValidationExportSigCollector().positionStatus( origin, stats->honestShare->committeePosition) != ExportSigCollector::PositionStatus::conflicted); @@ -1797,13 +1783,11 @@ class SteppingExtensions_test : public beast::unit_test::suite BEAST_EXPECT(unexpected.empty()); BEAST_EXPECT(hits[origin] == 1); log << " invalid-share: received=" << stats->badFramesReceived - << " jobs=" << stats->badShareJobs << " sent=" << sent + << " sent=" << sent << " pendingAtInject=" << stats->sawBadWhilePending - << " witness=" << seqW << std::endl; + << " rejected=" << sawRejection << " witness=" << seqW << std::endl; outcome.push_back(origin); - outcome.push_back(sha512Half( - static_cast(stats->badFramesReceived), - static_cast(stats->badShareJobs))); + outcome.push_back(badWire); return outcome; } @@ -1954,7 +1938,8 @@ public: return observerRestartAcrossExport(net); }); } - if (matches("invalid Export share does not poison an honest contribution")) + if (matches( + "invalid Export share does not poison an honest contribution")) { testcase( "invalid Export share does not poison an honest contribution");