From 041f874d4cedc7bb9c3bb5e4a64d30db180c3682 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Thu, 18 Sep 2014 14:16:48 -0700 Subject: [PATCH 1/4] Improve transaction security * Check signatures of every transaction on every validator * Remove obsolete code * Check transaction status in submit/sign RPC handler --- src/ripple/module/rpc/handlers/Submit.cpp | 10 +++++++++- src/ripple/overlay/impl/PeerImp.cpp | 10 +++++++++- src/ripple/overlay/impl/handshake_analyzer.h | 5 ----- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/ripple/module/rpc/handlers/Submit.cpp b/src/ripple/module/rpc/handlers/Submit.cpp index e1c54aa3c..066b267f3 100644 --- a/src/ripple/module/rpc/handlers/Submit.cpp +++ b/src/ripple/module/rpc/handlers/Submit.cpp @@ -64,7 +64,7 @@ Json::Value doSubmit (RPC::Context& context) try { - tpTrans = std::make_shared (stpTrans, false); + tpTrans = std::make_shared (stpTrans, true); } catch (std::exception& e) { @@ -74,6 +74,14 @@ Json::Value doSubmit (RPC::Context& context) return jvResult; } + if (tpTrans->getStatus() != NEW) + { + jvResult[jss::error] = "invalidTransactions"; + jvResult["error_exception"] = "fails local checks"; + + return jvResult; + } + try { (void) context.netOps_.processTransaction ( diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 193a82e23..c6ad63d0a 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -942,7 +942,15 @@ PeerImp::on_message (std::shared_ptr const& m) m_journal.debug << "Got transaction from peer " << *this << ": " << txID; if (m_clusterNode) - flags |= SF_TRUSTED | SF_SIGGOOD; + { + flags |= SF_TRUSTED; + if (! getConfig().VALIDATION_PRIV.isSet()) + { + // For now, be paranoid and have each validator + // check each transaction, regardless of source + flags |= SF_SIGGOOD; + } + } if (getApp().getJobQueue().getJobCount(jtTRANSACTION) > 100) m_journal.info << "Transaction queue is full"; diff --git a/src/ripple/overlay/impl/handshake_analyzer.h b/src/ripple/overlay/impl/handshake_analyzer.h index 49dd45818..cb090a83e 100644 --- a/src/ripple/overlay/impl/handshake_analyzer.h +++ b/src/ripple/overlay/impl/handshake_analyzer.h @@ -1078,10 +1078,8 @@ private: static void checkTransaction (Job&, int flags, SerializedTransaction::pointer stx, std::weak_ptr peer) { - #ifndef TRUST_NETWORK try { - #endif if (stx->isFieldPresent(sfLastLedgerSequence) && (stx->getFieldU32 (sfLastLedgerSequence) < @@ -1108,15 +1106,12 @@ private: bool const trusted (flags & SF_TRUSTED); getApp().getOPs ().processTransaction (tx, trusted, false, false); - #ifndef TRUST_NETWORK } catch (...) { getApp().getHashRouter ().setFlag (stx->getTransactionID (), SF_BAD); charge (peer, Resource::feeInvalidRequest); } - - #endif } // Called from our JobQueue From 02d9c774020a43ccdf666dccca11f8330a4f8245 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 19 Sep 2014 11:57:22 -0700 Subject: [PATCH 2/4] Set version to 0.26.3-sp2 --- Builds/rpm/rippled.spec | 2 +- src/ripple/module/data/protocol/BuildInfo.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Builds/rpm/rippled.spec b/Builds/rpm/rippled.spec index 54c2069ee..34943d0be 100644 --- a/Builds/rpm/rippled.spec +++ b/Builds/rpm/rippled.spec @@ -1,5 +1,5 @@ Name: rippled -Version: 0.26.3-sp1 +Version: 0.26.3-sp2 Release: 1%{?dist} Summary: Ripple peer-to-peer network daemon diff --git a/src/ripple/module/data/protocol/BuildInfo.cpp b/src/ripple/module/data/protocol/BuildInfo.cpp index 07942efc4..e78a5c224 100644 --- a/src/ripple/module/data/protocol/BuildInfo.cpp +++ b/src/ripple/module/data/protocol/BuildInfo.cpp @@ -31,7 +31,7 @@ char const* BuildInfo::getRawVersionString () // // The build version number (edit this for each release) // - "0.26.3-sp1" + "0.26.3-sp2" // // Must follow the format described here: // From 590c3b876b9ec2645e988faa42c45b83f6bb279b Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Tue, 30 Sep 2014 18:00:40 -0700 Subject: [PATCH 3/4] Use trusted validators median fee --- src/ripple/module/app/ledger/LedgerMaster.cpp | 27 +++++++++----- src/ripple/module/app/misc/Validations.cpp | 36 ++++++++----------- src/ripple/module/app/misc/Validations.h | 6 ++-- 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/src/ripple/module/app/ledger/LedgerMaster.cpp b/src/ripple/module/app/ledger/LedgerMaster.cpp index e370d7d59..1a39ca788 100644 --- a/src/ripple/module/app/ledger/LedgerMaster.cpp +++ b/src/ripple/module/app/ledger/LedgerMaster.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include // @@ -756,15 +757,25 @@ public: getApp().getOrderBookDB().setup(ledger); } - std::uint64_t fee, fee2, ref; - ref = getApp().getFeeTrack().getLoadBase(); - int count = getApp().getValidations().getFeeAverage(ledger->getHash(), ref, fee); - int count2 = getApp().getValidations().getFeeAverage(ledger->getParentHash(), ref, fee2); - - if ((count + count2) == 0) - getApp().getFeeTrack().setRemoteFee(ref); + std::uint64_t const base = getApp().getFeeTrack().getLoadBase(); + auto fees = getApp().getValidations().fees (ledger->getHash(), base); + { + auto fees2 = getApp().getValidations().fees (ledger->getParentHash(), base); + fees.reserve (fees.size() + fees2.size()); + std::copy (fees2.begin(), fees2.end(), std::back_inserter(fees)); + } + std::uint64_t fee; + if (! fees.empty()) + { + std::sort (fees.begin(), fees.end()); + fee = fees[fees.size() / 2]; // median + } else - getApp().getFeeTrack().setRemoteFee(((fee * count) + (fee2 * count2)) / (count + count2)); + { + fee = base; + } + + getApp().getFeeTrack().setRemoteFee(fee); tryAdvance (); } diff --git a/src/ripple/module/app/misc/Validations.cpp b/src/ripple/module/app/misc/Validations.cpp index 2fe375d97..ccf7917e7 100644 --- a/src/ripple/module/app/misc/Validations.cpp +++ b/src/ripple/module/app/misc/Validations.cpp @@ -17,6 +17,7 @@ */ //============================================================================== +#include #include namespace ripple { @@ -24,10 +25,10 @@ namespace ripple { class ValidationsImp : public Validations { private: - typedef RippleMutex LockType; + using LockType = std::mutex; typedef std::lock_guard ScopedLockType; typedef beast::GenericScopedUnlock ScopedUnlockType; - LockType mLock; + std::mutex mutable mLock; TaggedCache mValidations; ValidationSet mCurrentValidations; @@ -231,34 +232,27 @@ private: return trusted; } - int getFeeAverage (uint256 const& ledger, std::uint64_t ref, std::uint64_t& fee) + std::vector + fees (uint256 const& ledger, std::uint64_t base) override { - int trusted = 0; - fee = 0; - - ScopedLockType sl (mLock); - auto set = findSet (ledger); - + std::vector result; + std::lock_guard lock (mLock); + auto const set = findSet (ledger); if (set) { - for (auto& it: *set) + for (auto const& v : *set) { - if (it.second->isTrusted ()) + if (v.second->isTrusted()) { - ++trusted; - if (it.second->isFieldPresent(sfLoadFee)) - fee += it.second->getFieldU32(sfLoadFee); + if (v.second->isFieldPresent(sfLoadFee)) + result.push_back(v.second->getFieldU32(sfLoadFee)); else - fee += ref; - } + result.push_back(base); + } } } - if (trusted == 0) - fee = ref; - else - fee /= trusted; - return trusted; + return result; } int getNodesAfter (uint256 const& ledger) diff --git a/src/ripple/module/app/misc/Validations.h b/src/ripple/module/app/misc/Validations.h index 7f2eed13d..a1320e9ae 100644 --- a/src/ripple/module/app/misc/Validations.h +++ b/src/ripple/module/app/misc/Validations.h @@ -50,8 +50,10 @@ public: virtual int getTrustedValidationCount (uint256 const& ledger) = 0; - virtual int getFeeAverage( - uint256 const& ledger, std::uint64_t ref, std::uint64_t& fee) = 0; + /** Returns fees reported by trusted validators in the given ledger. */ + virtual + std::vector + fees (uint256 const& ledger, std::uint64_t base) = 0; virtual int getNodesAfter (uint256 const& ledger) = 0; virtual int getLoadRatio (bool overLoaded) = 0; From a8296f73018a9dd6d2a8ee451e2892c190708b4e Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Mon, 29 Sep 2014 10:51:33 -0700 Subject: [PATCH 4/4] Set version to 0.26.3-sp4 --- Builds/rpm/rippled.spec | 2 +- src/ripple/module/data/protocol/BuildInfo.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Builds/rpm/rippled.spec b/Builds/rpm/rippled.spec index 34943d0be..359a4c9a7 100644 --- a/Builds/rpm/rippled.spec +++ b/Builds/rpm/rippled.spec @@ -1,5 +1,5 @@ Name: rippled -Version: 0.26.3-sp2 +Version: 0.26.3-sp4 Release: 1%{?dist} Summary: Ripple peer-to-peer network daemon diff --git a/src/ripple/module/data/protocol/BuildInfo.cpp b/src/ripple/module/data/protocol/BuildInfo.cpp index e78a5c224..c38c3cbc7 100644 --- a/src/ripple/module/data/protocol/BuildInfo.cpp +++ b/src/ripple/module/data/protocol/BuildInfo.cpp @@ -31,7 +31,7 @@ char const* BuildInfo::getRawVersionString () // // The build version number (edit this for each release) // - "0.26.3-sp2" + "0.26.3-sp4" // // Must follow the format described here: //