From 9c8caddc5a197bdd642556f8beb14f06d53cdfd3 Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Mon, 29 Mar 2021 09:17:56 -0700 Subject: [PATCH 01/10] Support HTTP health check in reporting mode. --- src/ripple/app/ledger/impl/LedgerMaster.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/ripple/app/ledger/impl/LedgerMaster.cpp b/src/ripple/app/ledger/impl/LedgerMaster.cpp index 586023737..6a517aaab 100644 --- a/src/ripple/app/ledger/impl/LedgerMaster.cpp +++ b/src/ripple/app/ledger/impl/LedgerMaster.cpp @@ -304,6 +304,25 @@ bool LedgerMaster::isCaughtUp(std::string& reason) { using namespace std::chrono_literals; + +#ifdef RIPPLED_REPORTING + if (app_.config().reporting()) + { + auto age = PgQuery(app_.getPgPool())("SELECT age()"); + if (!age || age.isNull()) + { + reason = "No ledgers in database"; + return false; + } + if (std::chrono::seconds{age.asInt()} > 3min) + { + reason = "No recently-published ledger"; + return false; + } + return true; + } +#endif + if (getPublishedLedgerAge() > 3min) { reason = "No recently-published ledger"; From 8579eb0c191005022dcb20641444ab471e277f67 Mon Sep 17 00:00:00 2001 From: CJ Cobb Date: Mon, 29 Mar 2021 16:51:41 -0400 Subject: [PATCH 02/10] Maintain compatibility for forwarded RPC responses: Typically, an RPC response contains a `result` field, which contains details about the operation performed. For ease of parsing, forwarded responses must look like a non-forwarded response. In some instances the response was incorrectly composed, so that the actual `result` object would be encapsulated by an outer `result` object, breaking existing code. This commit, addresses this issue and correctly "folds" the `result` field, ensuring a consistent schema for responses. --- src/ripple/rpc/impl/ServerHandlerImp.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/ripple/rpc/impl/ServerHandlerImp.cpp b/src/ripple/rpc/impl/ServerHandlerImp.cpp index fd709507d..51e1a5ef1 100644 --- a/src/ripple/rpc/impl/ServerHandlerImp.cpp +++ b/src/ripple/rpc/impl/ServerHandlerImp.cpp @@ -904,6 +904,17 @@ ServerHandlerImp::processRequest( reply.append(std::move(r)); else reply = std::move(r); + + if (reply.isMember(jss::result) && + reply[jss::result].isMember(jss::result)) + { + reply = reply[jss::result]; + if (reply.isMember(jss::status)) + { + reply[jss::result][jss::status] = reply[jss::status]; + reply.removeMember(jss::status); + } + } } auto response = to_string(reply); From 73116297aa94c4acbfc74c2593d1aa2323b4cc52 Mon Sep 17 00:00:00 2001 From: CJ Cobb Date: Wed, 10 Mar 2021 16:46:19 -0500 Subject: [PATCH 03/10] Properly encode results from the `tx` RPC command: The `tx` command supports output in both "text" and "binary" modes, controlled by the binary flag. For more details on the command and the possible arguments, please see: https://xrpl.org/tx.html. The existing handler would incorrectly deal with metadata when in binary mode. This commit corrects this issue, ensuring that the metadata is properly encoded, depending on the mode. --- src/ripple/rpc/handlers/Tx.cpp | 9 ++++++++- src/test/rpc/Transaction_test.cpp | 13 ++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/ripple/rpc/handlers/Tx.cpp b/src/ripple/rpc/handlers/Tx.cpp index 110def657..049ed8193 100644 --- a/src/ripple/rpc/handlers/Tx.cpp +++ b/src/ripple/rpc/handlers/Tx.cpp @@ -236,7 +236,14 @@ doTxHelp(RPC::Context& context, TxArgs const& args) if (ledger && meta) { - result.meta = meta; + if (args.binary) + { + result.meta = meta->getAsObject().getSerializer().getData(); + } + else + { + result.meta = meta; + } result.validated = isValidated( context.ledgerMaster, ledger->info().seq, ledger->info().hash); } diff --git a/src/test/rpc/Transaction_test.cpp b/src/test/rpc/Transaction_test.cpp index c58cd8022..33cadf746 100644 --- a/src/test/rpc/Transaction_test.cpp +++ b/src/test/rpc/Transaction_test.cpp @@ -49,18 +49,23 @@ class Transaction_test : public beast::unit_test::suite env.close(); std::vector> txns; + std::vector> metas; auto const startLegSeq = env.current()->info().seq; for (int i = 0; i < 750; ++i) { env(noop(alice)); txns.emplace_back(env.tx()); env.close(); + metas.emplace_back( + env.closed()->txRead(env.tx()->getTransactionID()).second); } auto const endLegSeq = env.closed()->info().seq; // Find the existing transactions - for (auto&& tx : txns) + for (size_t i = 0; i < txns.size(); ++i) { + auto const& tx = txns[i]; + auto const& meta = metas[i]; auto const result = env.rpc( COMMAND, to_string(tx->getTransactionID()), @@ -69,6 +74,12 @@ class Transaction_test : public beast::unit_test::suite to_string(endLegSeq)); BEAST_EXPECT(result[jss::result][jss::status] == jss::success); + BEAST_EXPECT( + result[jss::result][jss::tx] == + strHex(tx->getSerializer().getData())); + BEAST_EXPECT( + result[jss::result][jss::meta] == + strHex(meta->getSerializer().getData())); } auto const tx = env.jt(noop(alice), seq(env.seq(alice))).stx; From 79e69da3647019840dca49622621c3d88bc3883f Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Tue, 30 Mar 2021 02:40:00 -0700 Subject: [PATCH 04/10] Adjust OpenSSL defaults and mitigate CVE-2021-3499: In order to effectively mitigate CVE-2021-3499 even when compiling against versions of OpenSSL prior to 1.1.1k, this commit: 1) requires use of TLS 1.2 or later. Note that both TLS 1.0 and TLS 1.1 have been officially deprecated for over a year. 2) disables renegotiation support for TLS 1.2 connections. Lastly, this commit also changes the default list of ciphers that the server offers, limiting it only to ciphers that are part of TLS 1.2. --- src/ripple/basics/impl/make_SSLContext.cpp | 98 ++++++---------------- 1 file changed, 24 insertions(+), 74 deletions(-) diff --git a/src/ripple/basics/impl/make_SSLContext.cpp b/src/ripple/basics/impl/make_SSLContext.cpp index 80dc698b7..67beb79d3 100644 --- a/src/ripple/basics/impl/make_SSLContext.cpp +++ b/src/ripple/basics/impl/make_SSLContext.cpp @@ -29,13 +29,21 @@ namespace ripple { namespace openssl { namespace detail { -// We limit the ciphers we request and allow to ensure that weak -// ciphers aren't used. While this isn't strictly necessary for -// the rippled server-server use case, where we only need MITM -// detection/prevention, we also have websocket and rpc scenarios -// and want to ensure weak ciphers can't be used. -std::string const defaultCipherList = - "HIGH:MEDIUM:!aNULL:!MD5:!DSS:!3DES:!RC4:!EXPORT"; +/** The default list of ciphers we accept over TLS. + + Generally we include cipher suites that are part of TLS v1.2, but + we specifically exclude: + + - the DSS cipher suites (!DSS); + - cipher suites using pre-shared keys (!PSK); + - cipher suites that don't offer encryption (!eNULL); and + - cipher suites that don't offer authentication (!aNULL). + + @note Server administrators can override this default list, on either a + global or per-port basis, using the `ssl_ciphers` directive in the + config file. + */ +std::string const defaultCipherList = "TLSv1.2:!DSS:!PSK:!eNULL:!aNULL"; template struct custom_delete; @@ -194,69 +202,6 @@ ssl_ctx_use_privatekey(SSL_CTX* const ctx, evp_pkey_ptr key) LogicError("SSL_CTX_use_PrivateKey failed"); } -#ifdef SSL_FLAGS_NO_RENEGOTIATE_CIPHERS -static bool -disallowRenegotiation(SSL const* ssl, bool isNew) -{ - // Track when SSL connections have last negotiated and - // do not allow a connection to renegotiate more than - // once every 4 minutes - struct StaticData - { - std::mutex lock; - beast::aged_unordered_set set; - - StaticData() : set(ripple::stopwatch()) - { - } - }; - - static StaticData sd; - std::lock_guard lock(sd.lock); - auto const expired(sd.set.clock().now() - std::chrono::minutes(4)); - - // Remove expired entries - for (auto iter(sd.set.chronological.begin()); - (iter != sd.set.chronological.end()) && (iter.when() <= expired); - iter = sd.set.chronological.begin()) - { - sd.set.erase(iter); - } - - auto iter = sd.set.find(ssl); - if (iter != sd.set.end()) - { - if (!isNew) - { - // This is a renegotiation and the last negotiation was recent - return true; - } - - sd.set.touch(iter); - } - else - { - sd.set.emplace(ssl); - } - - return false; -} - -static void -info_handler(SSL const* ssl, int event, int) -{ -#if OPENSSL_VERSION_NUMBER < 0x10100000L - if ((ssl->s3) && (event & SSL_CB_HANDSHAKE_START)) - { - if (disallowRenegotiation(ssl, SSL_in_before(ssl))) - ssl->s3->flags |= SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS; - } -#else - // empty, flag removed in OpenSSL 1.1 -#endif -} -#endif - static std::string error_message(std::string const& what, boost::system::error_code const& ec) { @@ -388,7 +333,10 @@ get_context(std::string const& cipherList) boost::asio::ssl::context::default_workarounds | boost::asio::ssl::context::no_sslv2 | boost::asio::ssl::context::no_sslv3 | - boost::asio::ssl::context::single_dh_use); + boost::asio::ssl::context::no_tlsv1 | + boost::asio::ssl::context::no_tlsv1_1 | + boost::asio::ssl::context::single_dh_use | + boost::asio::ssl::context::no_compression); { auto const& l = !cipherList.empty() ? cipherList : defaultCipherList; @@ -435,9 +383,11 @@ get_context(std::string const& cipherList) SSL_CTX_set_tmp_dh(c->native_handle(), dh.get()); -#ifdef SSL_FLAGS_NO_RENEGOTIATE_CIPHERS - SSL_CTX_set_info_callback(c->native_handle(), info_handler); -#endif + // Disable all renegotiation support in TLS v1.2. This can help prevent + // exploitation of the bug described in CVE-2021-3499 (for details see + // https://www.openssl.org/news/secadv/20210325.txt) when linking against + // OpenSSL versions prior to 1.1.1k. + SSL_CTX_set_options(c->native_handle(), SSL_OP_NO_RENEGOTIATION); return c; } From 9106a06579a97ac2127ab73a8a93b015098f5305 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Thu, 1 Apr 2021 11:02:48 -0700 Subject: [PATCH 05/10] Set version to 1.7.1 --- src/ripple/protocol/impl/BuildInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/protocol/impl/BuildInfo.cpp b/src/ripple/protocol/impl/BuildInfo.cpp index d13ec8906..6b20a189d 100644 --- a/src/ripple/protocol/impl/BuildInfo.cpp +++ b/src/ripple/protocol/impl/BuildInfo.cpp @@ -33,7 +33,7 @@ namespace BuildInfo { // and follow the format described at http://semver.org/ //------------------------------------------------------------------------------ // clang-format off -char const* const versionString = "1.7.0" +char const* const versionString = "1.7.1" // clang-format on #if defined(DEBUG) || defined(SANITIZER) From 430802c1cf6d4179f2249a30bfab9eff8e1fa748 Mon Sep 17 00:00:00 2001 From: CJ Cobb Date: Fri, 9 Apr 2021 14:56:13 -0400 Subject: [PATCH 06/10] Add load_factor to server_info in reporting mode * load_factor was missing from server_info when the server was running in reporting mode. Now, the reporting mode server calls server_info on the p2p node, and propagates the load_factor back to the client. --- src/ripple/rpc/handlers/ServerInfo.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/ripple/rpc/handlers/ServerInfo.cpp b/src/ripple/rpc/handlers/ServerInfo.cpp index 174315c08..80fde471e 100644 --- a/src/ripple/rpc/handlers/ServerInfo.cpp +++ b/src/ripple/rpc/handlers/ServerInfo.cpp @@ -17,7 +17,9 @@ */ //============================================================================== +#include #include +#include #include #include #include @@ -38,6 +40,12 @@ doServerInfo(RPC::JsonContext& context) context.params.isMember(jss::counters) && context.params[jss::counters].asBool()); + if (context.app.config().reporting()) + { + Json::Value const proxied = forwardToP2p(context); + auto const lf = proxied[jss::result][jss::info][jss::load_factor]; + ret[jss::info][jss::load_factor] = lf.isNull() ? 1 : lf; + } return ret; } From 1bb99e5d3ceb95446a6abd0700d833660e853f31 Mon Sep 17 00:00:00 2001 From: seelabs Date: Mon, 19 Apr 2021 21:49:15 -0400 Subject: [PATCH 07/10] Rm some offers where the quality is reduced: Substantial reductions in an offer's effective quality from its initial quality may clog offer books. --- src/ripple/app/tx/impl/OfferStream.cpp | 130 ++++++++++ src/ripple/app/tx/impl/OfferStream.h | 4 + src/ripple/basics/IOUAmount.h | 3 + src/ripple/basics/XRPAmount.h | 6 + src/ripple/basics/impl/IOUAmount.cpp | 8 +- src/ripple/protocol/AmountConversions.h | 28 +- src/ripple/protocol/Feature.h | 2 + src/ripple/protocol/Quality.h | 7 + src/ripple/protocol/impl/Feature.cpp | 4 +- src/test/app/Offer_test.cpp | 332 +++++++++++++++++++++++- 10 files changed, 510 insertions(+), 14 deletions(-) diff --git a/src/ripple/app/tx/impl/OfferStream.cpp b/src/ripple/app/tx/impl/OfferStream.cpp index 9b2f3beed..184f24849 100644 --- a/src/ripple/app/tx/impl/OfferStream.cpp +++ b/src/ripple/app/tx/impl/OfferStream.cpp @@ -19,6 +19,7 @@ #include #include +#include namespace ripple { @@ -132,6 +133,73 @@ accountFundsHelper( view, id, issue.currency, issue.account, freezeHandling, j)); } +template +template +bool +TOfferStreamBase::shouldRmSmallIncreasedQOffer() const +{ + static_assert( + std::is_same_v || + std::is_same_v, + "STAmount is not supported"); + + static_assert( + std::is_same_v || + std::is_same_v, + "STAmount is not supported"); + + static_assert( + !std::is_same_v || + !std::is_same_v, + "Cannot have XRP/XRP offers"); + + if (!view_.rules().enabled(fixRmSmallIncreasedQOffers)) + return false; + + // Consider removing the offer if: + // o `TakerPays` is XRP (because of XRP drops granularity) or + // o `TakerPays` and `TakerGets` are both IOU and `TakerPays`<`TakerGets` + constexpr bool const inIsXRP = std::is_same_v; + constexpr bool const outIsXRP = std::is_same_v; + + if constexpr (outIsXRP) + { + // If `TakerGets` is XRP, the worst this offer's quality can change is + // to about 10^-81 `TakerPays` and 1 drop `TakerGets`. This will be + // remarkably good quality for any realistic asset, so these offers + // don't need this extra check. + return false; + } + + TAmounts const ofrAmts{ + toAmount(offer_.amount().in), + toAmount(offer_.amount().out)}; + + if constexpr (!inIsXRP && !outIsXRP) + { + if (ofrAmts.in >= ofrAmts.out) + return false; + } + + TTakerGets const ownerFunds = toAmount(*ownerFunds_); + + auto const effectiveAmounts = [&] { + if (offer_.owner() != offer_.issueOut().account && + ownerFunds < ofrAmts.out) + { + // adjust the amounts by owner funds + return offer_.quality().ceil_out(ofrAmts, ownerFunds); + } + return ofrAmts; + }(); + + if (effectiveAmounts.in > TTakerPays::minPositiveAmount()) + return false; + + Quality const effectiveQuality{effectiveAmounts}; + return effectiveQuality < offer_.quality(); +} + template bool TOfferStreamBase::step() @@ -222,6 +290,68 @@ TOfferStreamBase::step() << "Removing became unfunded offer " << entry->key(); } offer_ = TOffer{}; + // See comment at top of loop for how the offer is removed + continue; + } + + bool const rmSmallIncreasedQOffer = [&] { + bool const inIsXRP = isXRP(offer_.issueIn()); + bool const outIsXRP = isXRP(offer_.issueOut()); + if (inIsXRP && !outIsXRP) + { + // Without the `if constexpr`, the + // `shouldRmSmallIncreasedQOffer` template will be instantiated + // even if it is never used. This can cause compiler errors in + // some cases, hence the `if constexpr` guard. + // Note that TIn can be XRPAmount or STAmount, and TOut can be + // IOUAmount or STAmount. + if constexpr (!(std::is_same_v || + std::is_same_v)) + return shouldRmSmallIncreasedQOffer(); + } + if (!inIsXRP && outIsXRP) + { + // See comment above for `if constexpr` rationale + if constexpr (!(std::is_same_v || + std::is_same_v)) + return shouldRmSmallIncreasedQOffer(); + } + if (!inIsXRP && !outIsXRP) + { + // See comment above for `if constexpr` rationale + if constexpr (!(std::is_same_v || + std::is_same_v)) + return shouldRmSmallIncreasedQOffer(); + } + assert(0); // xrp/xrp offer!?! should never happen + return false; + }(); + + if (rmSmallIncreasedQOffer) + { + auto const original_funds = accountFundsHelper( + cancelView_, + offer_.owner(), + amount.out, + offer_.issueOut(), + fhZERO_IF_FROZEN, + j_); + + if (original_funds == *ownerFunds_) + { + permRmOffer(entry->key()); + JLOG(j_.trace()) + << "Removing tiny offer due to reduced quality " + << entry->key(); + } + else + { + JLOG(j_.trace()) << "Removing tiny offer that became tiny due " + "to reduced quality " + << entry->key(); + } + offer_ = TOffer{}; + // See comment at top of loop for how the offer is removed continue; } diff --git a/src/ripple/app/tx/impl/OfferStream.h b/src/ripple/app/tx/impl/OfferStream.h index d8c739b36..8cd443cff 100644 --- a/src/ripple/app/tx/impl/OfferStream.h +++ b/src/ripple/app/tx/impl/OfferStream.h @@ -85,6 +85,10 @@ protected: virtual void permRmOffer(uint256 const& offerIndex) = 0; + template + bool + shouldRmSmallIncreasedQOffer() const; + public: TOfferStreamBase( ApplyView& view, diff --git a/src/ripple/basics/IOUAmount.h b/src/ripple/basics/IOUAmount.h index 46a37d23a..7e9e50d7e 100644 --- a/src/ripple/basics/IOUAmount.h +++ b/src/ripple/basics/IOUAmount.h @@ -129,6 +129,9 @@ public: { return mantissa_; } + + static IOUAmount + minPositiveAmount(); }; std::string diff --git a/src/ripple/basics/XRPAmount.h b/src/ripple/basics/XRPAmount.h index e8f26266e..133c1f466 100644 --- a/src/ripple/basics/XRPAmount.h +++ b/src/ripple/basics/XRPAmount.h @@ -238,6 +238,12 @@ public: s >> val.drops_; return s; } + + static XRPAmount + minPositiveAmount() + { + return XRPAmount{1}; + } }; /** Number of drops per 1 XRP */ diff --git a/src/ripple/basics/impl/IOUAmount.cpp b/src/ripple/basics/impl/IOUAmount.cpp index bb3c39ef3..272546623 100644 --- a/src/ripple/basics/impl/IOUAmount.cpp +++ b/src/ripple/basics/impl/IOUAmount.cpp @@ -34,6 +34,12 @@ static std::int64_t const maxMantissa = 9999999999999999ull; static int const minExponent = -96; static int const maxExponent = 80; +IOUAmount +IOUAmount::minPositiveAmount() +{ + return IOUAmount(minMantissa, minExponent); +} + void IOUAmount::normalize() { @@ -353,7 +359,7 @@ mulRatio( { if (!result) { - return IOUAmount(minMantissa, minExponent); + return IOUAmount::minPositiveAmount(); } // This addition cannot overflow because the mantissa is already // normalized diff --git a/src/ripple/protocol/AmountConversions.h b/src/ripple/protocol/AmountConversions.h index 899d313fb..f60b976d5 100644 --- a/src/ripple/protocol/AmountConversions.h +++ b/src/ripple/protocol/AmountConversions.h @@ -63,11 +63,7 @@ toSTAmount(XRPAmount const& xrp, Issue const& iss) template T -toAmount(STAmount const& amt) -{ - static_assert(sizeof(T) == -1, "Must use specialized function"); - return T(0); -} +toAmount(STAmount const& amt) = delete; template <> inline STAmount @@ -102,6 +98,28 @@ toAmount(STAmount const& amt) return XRPAmount(sMant); } +template +T +toAmount(IOUAmount const& amt) = delete; + +template <> +inline IOUAmount +toAmount(IOUAmount const& amt) +{ + return amt; +} + +template +T +toAmount(XRPAmount const& amt) = delete; + +template <> +inline XRPAmount +toAmount(XRPAmount const& amt) +{ + return amt; +} + } // namespace ripple #endif diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 05506614f..e28bbdb62 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -116,6 +116,7 @@ class FeatureCollections "TicketBatch", "FlowSortStrands", "fixSTAmountCanonicalize", + "fixRmSmallIncreasedQOffers", }; std::vector features; @@ -376,6 +377,7 @@ extern uint256 const featureNegativeUNL; extern uint256 const featureTicketBatch; extern uint256 const featureFlowSortStrands; extern uint256 const fixSTAmountCanonicalize; +extern uint256 const fixRmSmallIncreasedQOffers; } // namespace ripple diff --git a/src/ripple/protocol/Quality.h b/src/ripple/protocol/Quality.h index 27026b674..6324a8836 100644 --- a/src/ripple/protocol/Quality.h +++ b/src/ripple/protocol/Quality.h @@ -132,6 +132,13 @@ public: /** Create a quality from the ratio of two amounts. */ explicit Quality(Amounts const& amount); + /** Create a quality from the ratio of two amounts. */ + template + Quality(TAmounts const& amount) + : Quality(Amounts(toSTAmount(amount.in), toSTAmount(amount.out))) + { + } + /** Create a quality from the ratio of two amounts. */ template Quality(Out const& out, In const& in) diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index eaf1cbf71..97a1d01d1 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -135,6 +135,7 @@ detail::supportedAmendments() "TicketBatch", "FlowSortStrands", "fixSTAmountCanonicalize", + "fixRmSmallIncreasedQOffers", }; return supported; } @@ -190,7 +191,8 @@ uint256 const featureNegativeUNL = *getRegisteredFeature("NegativeUNL"), featureTicketBatch = *getRegisteredFeature("TicketBatch"), featureFlowSortStrands = *getRegisteredFeature("FlowSortStrands"), - fixSTAmountCanonicalize = *getRegisteredFeature("fixSTAmountCanonicalize"); + fixSTAmountCanonicalize = *getRegisteredFeature("fixSTAmountCanonicalize"), + fixRmSmallIncreasedQOffers = *getRegisteredFeature("fixRmSmallIncreasedQOffers"); // The following amendments have been active for at least two years. Their // pre-amendment code has been removed and the identifiers are deprecated. diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index f30fc19e1..e0d2de4e3 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -320,17 +320,20 @@ public: Env env{*this, features}; env.fund(XRP(10000), alice, bob, carol, dan, erin, gw); + env.close(); env.trust(USD(1000), alice, bob, carol, dan, erin); + env.close(); env(pay(gw, carol, USD(0.99999))); env(pay(gw, dan, USD(1))); env(pay(gw, erin, USD(1))); + env.close(); // Carol doesn't quite have enough funds for this offer // The amount left after this offer is taken will cause // STAmount to incorrectly round to zero when the next offer - // (at a good quality) is considered. (when the - // stAmountCalcSwitchover2 patch is inactive) - env(offer(carol, drops(1), USD(1))); + // (at a good quality) is considered. (when the now removed + // stAmountCalcSwitchover2 patch was inactive) + env(offer(carol, drops(1), USD(0.99999))); // Offer at a quality poor enough so when the input xrp is // calculated in the reverse pass, the amount is not zero. env(offer(dan, XRP(100), USD(1))); @@ -340,9 +343,9 @@ public: // It is considered after the offer from carol, which leaves a // tiny amount left to pay. When calculating the amount of xrp // needed for this offer, it will incorrectly compute zero in both - // the forward and reverse passes (when the stAmountCalcSwitchover2 - // is inactive.) - env(offer(erin, drops(2), USD(2))); + // the forward and reverse passes (when the now removed + // stAmountCalcSwitchover2 was inactive.) + env(offer(erin, drops(2), USD(1))); env(pay(alice, bob, USD(1)), path(~USD), @@ -356,6 +359,317 @@ public: env.require(balance(erin, USD(0.99999)), offers(erin, 1)); } + void + testRmSmallIncreasedQOffersXRP(FeatureBitset features) + { + testcase("Rm small increased q offers XRP"); + + // Carol places an offer, but cannot fully fund the offer. When her + // funding is taken into account, the offer's quality drops below its + // initial quality and has an input amount of 1 drop. This is removed as + // an offer that may block offer books. + + using namespace jtx; + using namespace std::chrono_literals; + auto const alice = Account{"alice"}; + auto const bob = Account{"bob"}; + auto const carol = Account{"carol"}; + auto const gw = Account{"gw"}; + + auto const USD = gw["USD"]; + + // Test offer crossing + for (auto crossBothOffers : {false, true}) + { + Env env{*this, features}; + + env.fund(XRP(10000), alice, bob, carol, gw); + env.close(); + env.trust(USD(1000), alice, bob, carol); + // underfund carol's offer + auto initialCarolUSD = USD(0.499); + env(pay(gw, carol, initialCarolUSD)); + env(pay(gw, bob, USD(100))); + env.close(); + // This offer is underfunded + env(offer(carol, drops(1), USD(1))); + env.close(); + // offer at a lower quality + env(offer(bob, drops(2), USD(1), tfPassive)); + env.close(); + env.require(offers(bob, 1), offers(carol, 1)); + + // alice places an offer that crosses carol's; depending on + // "crossBothOffers" it may cross bob's as well + auto aliceTakerGets = crossBothOffers ? drops(2) : drops(1); + env(offer(alice, USD(1), aliceTakerGets)); + env.close(); + + if (features[fixRmSmallIncreasedQOffers]) + { + env.require( + offers(carol, 0), + balance( + carol, + initialCarolUSD)); // offer is removed but not taken + if (crossBothOffers) + { + env.require( + offers(alice, 0), + balance(alice, USD(1))); // alice's offer is crossed + } + else + { + env.require( + offers(alice, 1), + balance( + alice, USD(0))); // alice's offer is not crossed + } + } + else + { + env.require( + offers(alice, 1), + offers(bob, 1), + offers(carol, 1), + balance(alice, USD(0)), + balance( + carol, + initialCarolUSD)); // offer is not crossed at all + } + } + + // Test payments + for (auto partialPayment : {false, true}) + { + Env env{*this, features}; + + env.fund(XRP(10000), alice, bob, carol, gw); + env.close(); + env.trust(USD(1000), alice, bob, carol); + env.close(); + auto const initialCarolUSD = USD(0.999); + env(pay(gw, carol, initialCarolUSD)); + env.close(); + env(pay(gw, bob, USD(100))); + env.close(); + env(offer(carol, drops(1), USD(1))); + env.close(); + env(offer(bob, drops(2), USD(2), tfPassive)); + env.close(); + env.require(offers(bob, 1), offers(carol, 1)); + + std::uint32_t const flags = partialPayment + ? (tfNoRippleDirect | tfPartialPayment) + : tfNoRippleDirect; + + TER const expectedTer = + partialPayment ? TER{tesSUCCESS} : TER{tecPATH_PARTIAL}; + + env(pay(alice, bob, USD(5)), + path(~USD), + sendmax(XRP(1)), + txflags(flags), + ter(expectedTer)); + env.close(); + + if (features[fixRmSmallIncreasedQOffers]) + { + if (expectedTer == tesSUCCESS) + { + env.require(offers(carol, 0)); + env.require(balance( + carol, + initialCarolUSD)); // offer is removed but not taken + } + else + { + // TODO: Offers are not removed when payments fail + // If that is addressed, the test should show that carol's + // offer is removed but not taken, as in the other branch of + // this if statement + } + } + else + { + if (partialPayment) + { + env.require(offers(carol, 0)); + env.require( + balance(carol, USD(0))); // offer is removed and taken + } + else + { + // offer is not removed or taken + BEAST_EXPECT(isOffer(env, carol, drops(1), USD(1))); + } + } + } + } + + void + testRmSmallIncreasedQOffersIOU(FeatureBitset features) + { + testcase("Rm small increased q offers IOU"); + + // Carol places an offer, but cannot fully fund the offer. When her + // funding is taken into account, the offer's quality drops below its + // initial quality and has an input amount of 1 drop. This is removed as + // an offer that may block offer books. + + using namespace jtx; + using namespace std::chrono_literals; + auto const alice = Account{"alice"}; + auto const bob = Account{"bob"}; + auto const carol = Account{"carol"}; + auto const gw = Account{"gw"}; + + auto const USD = gw["USD"]; + auto const EUR = gw["EUR"]; + + auto tinyAmount = [&](IOU const& iou) -> PrettyAmount { + STAmount amt( + iou.issue(), + /*mantissa*/ 1, + /*exponent*/ -81); + return PrettyAmount(amt, iou.account.name()); + }; + + // Test offer crossing + for (auto crossBothOffers : {false, true}) + { + Env env{*this, features}; + + env.fund(XRP(10000), alice, bob, carol, gw); + env.close(); + env.trust(USD(1000), alice, bob, carol); + env.trust(EUR(1000), alice, bob, carol); + // underfund carol's offer + auto initialCarolUSD = tinyAmount(USD); + env(pay(gw, carol, initialCarolUSD)); + env(pay(gw, bob, USD(100))); + env(pay(gw, alice, EUR(100))); + env.close(); + // This offer is underfunded + env(offer(carol, EUR(1), USD(10))); + env.close(); + // offer at a lower quality + env(offer(bob, EUR(1), USD(5), tfPassive)); + env.close(); + env.require(offers(bob, 1), offers(carol, 1)); + + // alice places an offer that crosses carol's; depending on + // "crossBothOffers" it may cross bob's as well + // Whatever + auto aliceTakerGets = crossBothOffers ? EUR(0.2) : EUR(0.1); + env(offer(alice, USD(1), aliceTakerGets)); + env.close(); + + if (features[fixRmSmallIncreasedQOffers]) + { + env.require( + offers(carol, 0), + balance( + carol, + initialCarolUSD)); // offer is removed but not taken + if (crossBothOffers) + { + env.require( + offers(alice, 0), + balance(alice, USD(1))); // alice's offer is crossed + } + else + { + env.require( + offers(alice, 1), + balance( + alice, USD(0))); // alice's offer is not crossed + } + } + else + { + env.require( + offers(alice, 1), + offers(bob, 1), + offers(carol, 1), + balance(alice, USD(0)), + balance( + carol, + initialCarolUSD)); // offer is not crossed at all + } + } + + // Test payments + for (auto partialPayment : {false, true}) + { + Env env{*this, features}; + + env.fund(XRP(10000), alice, bob, carol, gw); + env.close(); + env.trust(USD(1000), alice, bob, carol); + env.trust(EUR(1000), alice, bob, carol); + env.close(); + // underfund carol's offer + auto const initialCarolUSD = tinyAmount(USD); + env(pay(gw, carol, initialCarolUSD)); + env(pay(gw, bob, USD(100))); + env(pay(gw, alice, EUR(100))); + env.close(); + // This offer is underfunded + env(offer(carol, EUR(1), USD(2))); + env.close(); + env(offer(bob, EUR(2), USD(4), tfPassive)); + env.close(); + env.require(offers(bob, 1), offers(carol, 1)); + + std::uint32_t const flags = partialPayment + ? (tfNoRippleDirect | tfPartialPayment) + : tfNoRippleDirect; + + TER const expectedTer = + partialPayment ? TER{tesSUCCESS} : TER{tecPATH_PARTIAL}; + + env(pay(alice, bob, USD(5)), + path(~USD), + sendmax(EUR(10)), + txflags(flags), + ter(expectedTer)); + env.close(); + + if (features[fixRmSmallIncreasedQOffers]) + { + if (expectedTer == tesSUCCESS) + { + env.require(offers(carol, 0)); + env.require(balance( + carol, + initialCarolUSD)); // offer is removed but not taken + } + else + { + // TODO: Offers are not removed when payments fail + // If that is addressed, the test should show that carol's + // offer is removed but not taken, as in the other branch of + // this if statement + } + } + else + { + if (partialPayment) + { + env.require(offers(carol, 0)); + env.require( + balance(carol, USD(0))); // offer is removed and taken + } + else + { + // offer is not removed or taken + BEAST_EXPECT(isOffer(env, carol, EUR(1), USD(2))); + } + } + } + } + void testEnforceNoRipple(FeatureBitset features) { @@ -5387,7 +5701,7 @@ public: { // An assert was falsely triggering when computing rates for offers. // This unit test would trigger that assert (which has been removed). - testcase("false assert"); + testcase("incorrect assert fixed"); using namespace jtx; Env env{*this}; @@ -5459,6 +5773,8 @@ public: testTickSize(features); testTicketOffer(features); testTicketCancelOffer(features); + testRmSmallIncreasedQOffersXRP(features); + testRmSmallIncreasedQOffersIOU(features); } void @@ -5468,10 +5784,12 @@ public: FeatureBitset const all{supported_amendments()}; FeatureBitset const flowCross{featureFlowCross}; FeatureBitset const takerDryOffer{fixTakerDryOfferRemoval}; + FeatureBitset const rmSmallIncreasedQOffers{fixRmSmallIncreasedQOffers}; testAll(all - takerDryOffer); testAll(all - flowCross - takerDryOffer); testAll(all - flowCross); + testAll(all - rmSmallIncreasedQOffers); testAll(all); testFalseAssert(); } From 36fe1966c3cd37f668693b5d9910fab59c3f8b1f Mon Sep 17 00:00:00 2001 From: Edward Hennis Date: Mon, 10 May 2021 13:27:12 -0400 Subject: [PATCH 08/10] Update version of vcpkg for Windows builds --- bin/sh/install-vcpkg.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/sh/install-vcpkg.sh b/bin/sh/install-vcpkg.sh index b77982987..8cf8f2d08 100755 --- a/bin/sh/install-vcpkg.sh +++ b/bin/sh/install-vcpkg.sh @@ -20,7 +20,7 @@ else if [[ -d "${VCPKG_DIR}" ]] ; then rm -rf "${VCPKG_DIR}" fi - git clone --branch 2019.12 https://github.com/Microsoft/vcpkg.git ${VCPKG_DIR} + git clone --branch 2021.04.30 https://github.com/Microsoft/vcpkg.git ${VCPKG_DIR} pushd ${VCPKG_DIR} BSARGS=() if [[ "$(uname)" == "Darwin" ]] ; then From 30fd45890b1d3d5f372a2091d1397b1e8e29d2ca Mon Sep 17 00:00:00 2001 From: Edward Hennis Date: Fri, 5 Mar 2021 13:29:17 -0500 Subject: [PATCH 09/10] Create comparator wrapper classes for MSVC 2019 build issues: * The std::less and std::equal_to comparators have [[nodiscard]], which conflicts with boost::bimap validations. --- src/ripple/basics/comparators.h | 76 ++++++++++++++++++++++++++ src/ripple/peerfinder/impl/Bootcache.h | 8 ++- src/test/csf/ledgers.h | 5 +- 3 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 src/ripple/basics/comparators.h diff --git a/src/ripple/basics/comparators.h b/src/ripple/basics/comparators.h new file mode 100644 index 000000000..ce782dcd3 --- /dev/null +++ b/src/ripple/basics/comparators.h @@ -0,0 +1,76 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2012, 2013 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef RIPPLE_BASICS_COMPARATORS_H_INCLUDED +#define RIPPLE_BASICS_COMPARATORS_H_INCLUDED + +#include + +namespace ripple { + +#ifdef _MSC_VER + +/* + * MSVC 2019 version 16.9.0 added [[nodiscard]] to the std comparison + * operator() functions. boost::bimap checks that the comparitor is a + * BinaryFunction, in part by calling the function and ignoring the value. + * These two things don't play well together. These wrapper classes simply + * strip [[nodiscard]] from operator() for use in boost::bimap. + * + * See also: + * https://www.boost.org/doc/libs/1_75_0/libs/bimap/doc/html/boost_bimap/the_tutorial/controlling_collection_types.html + */ + +template +struct less +{ + using result_type = bool; + + constexpr bool + operator()(const T& left, const T& right) const + { + return std::less()(left, right); + } +}; + +template +struct equal_to +{ + using result_type = bool; + + constexpr bool + operator()(const T& left, const T& right) const + { + return std::equal_to()(left, right); + } +}; + +#else + +template +using less = std::less; + +template +using equal_to = std::equal_to; + +#endif + +} // namespace ripple + +#endif diff --git a/src/ripple/peerfinder/impl/Bootcache.h b/src/ripple/peerfinder/impl/Bootcache.h index bdcdffd43..eb6455879 100644 --- a/src/ripple/peerfinder/impl/Bootcache.h +++ b/src/ripple/peerfinder/impl/Bootcache.h @@ -20,6 +20,7 @@ #ifndef RIPPLE_PEERFINDER_BOOTCACHE_H_INCLUDED #define RIPPLE_PEERFINDER_BOOTCACHE_H_INCLUDED +#include #include #include #include @@ -81,8 +82,11 @@ private: int m_valence; }; - using left_t = boost::bimaps::unordered_set_of; - using right_t = boost::bimaps::multiset_of; + using left_t = boost::bimaps::unordered_set_of< + beast::IP::Endpoint, + boost::hash, + ripple::equal_to>; + using right_t = boost::bimaps::multiset_of>; using map_type = boost::bimap; using value_type = map_type::value_type; diff --git a/src/test/csf/ledgers.h b/src/test/csf/ledgers.h index 60aa34f74..27e122a8b 100644 --- a/src/test/csf/ledgers.h +++ b/src/test/csf/ledgers.h @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -242,7 +243,9 @@ private: */ class LedgerOracle { - using InstanceMap = boost::bimaps::bimap; + using InstanceMap = boost::bimaps::bimap< + boost::bimaps::set_of>, + boost::bimaps::set_of>>; using InstanceEntry = InstanceMap::value_type; // Set of all known ledgers; note this is never pruned From 34ee4ca0cb59037e840f7d454114701b534f0afa Mon Sep 17 00:00:00 2001 From: manojsdoshi Date: Fri, 7 May 2021 15:03:49 -0700 Subject: [PATCH 10/10] Set version to 1.7.2 --- RELEASENOTES.md | 23 ++++++++++++++++++++++- src/ripple/protocol/impl/BuildInfo.cpp | 2 +- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index f1710cf97..856fa7886 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -2,13 +2,34 @@ ![XRP](docs/images/xrp-text-mark-black-small@2x.png) -This document contains the release notes for `rippled`, the reference server implementation of the Ripple protocol. To learn more about how to build, run or update a `rippled` server, visit https://xrpl.org/install-rippled.html +This document contains the release notes for `rippled`, the reference server implementation of the XRP Ledger protocol. To learn more about how to build, run or update a `rippled` server, visit https://xrpl.org/install-rippled.html Have new ideas? Need help with setting up your node? Come visit us [here](https://github.com/ripple/rippled/issues/new/choose) # Releases +## Version 1.7.2 + +This the 1.7.2 release of rippled, the reference server implementation of the XRP Ledger protocol. This release protects against the security issue [CVE-2021-3499](https://www.openssl.org/news/secadv/20210325.txt) affecting OpenSSL, adds an amendment to fix an issue with small offers not being properly removed from order books in some cases, and includes various other minor fixes. +Version 1.7.2 supersedes version 1.7.1 and adds fixes for more issues that were discovered during the release cycle. + +## Action Required + +This release introduces a new amendment to the XRP Ledger protocol: `fixRmSmallIncreasedQOffers`. This amendments is now open for voting according to the XRP Ledger's amendment process, which enables protocol changes following two weeks of >80% support from trusted validators. +If you operate an XRP Ledger server, then you should upgrade to version 1.7.2 within two weeks, to ensure service continuity. The exact time that protocol changes take effect depends on the voting decisions of the decentralized network. +If you operate an XRP Ledger validator, please learn more about this amendment so you can make informed decisions about how your validator votes. If you take no action, your validator begins voting in favor of any new amendments as soon as it has been upgraded. + +### Bug Fixes + +- **fixRmSmallIncreasedQOffers Amendment:** This amendment fixes an issue where certain small offers can be left at the tip of an order book without being consumed or removed when appropriate and causes some payments and Offers to fail when they should have succeeded [(#3827)](https://github.com/ripple/rippled/pull/3827). +- **Adjust OpenSSL defaults and mitigate CVE-2021-3499:** Prior to this fix, servers compiled against a vulnerable version of OpenSSL could have a crash triggered by a malicious network connection. This fix disables renegotiation support in OpenSSL so that the rippled server is not vulnerable to this bug regardless of the OpenSSL version used to compile the server. This also removes support for deprecated TLS versions 1.0 and 1.1 and ciphers that are not part of TLS 1.2 [(#79e69da)](https://github.com/ripple/rippled/pull/3843/commits/79e69da3647019840dca49622621c3d88bc3883f). +- **Support HTTP health check in reporting mode:** Enables the Health Check special method when running the server in the new Reporting Mode introduced in 1.7.0 [(9c8cadd)](https://github.com/ripple/rippled/pull/3843/commits/9c8caddc5a197bdd642556f8beb14f06d53cdfd3). +- **Maintain compatibility for forwarded RPC responses:** Fixes a case in API responses from servers in Reporting Mode, where requests that were forwarded to a P2P-mode server would have the result field nested inside another result field [(8579eb0)](https://github.com/ripple/rippled/pull/3843/commits/8579eb0c191005022dcb20641444ab471e277f67). +- **Add load_factor in reporting mode:** Adds a load_factor value to the server info method response when running the server in Reporting Mode so that the response is compatible with the format returned by servers in P2P mode (the default) [(430802c)](https://github.com/ripple/rippled/pull/3843/commits/430802c1cf6d4179f2249a30bfab9eff8e1fa748). +- **Properly encode metadata from tx RPC command:** Fixes a problem where transaction metadata in the tx API method response would be in JSON format even when binary was requested [(7311629)](https://github.com/ripple/rippled/pull/3843/commits/73116297aa94c4acbfc74c2593d1aa2323b4cc52). +- **Updates to Windows builds:** When building on Windows, use vcpkg 2021 by default and add compatibility with MSVC 2019 [(36fe196)](https://github.com/ripple/rippled/pull/3843/commits/36fe1966c3cd37f668693b5d9910fab59c3f8b1f), [(30fd458)](https://github.com/ripple/rippled/pull/3843/commits/30fd45890b1d3d5f372a2091d1397b1e8e29d2ca). + ## Version 1.7.0 Ripple has released version 1.7.0 of `rippled`, the reference server implementation of the XRP Ledger protocol. diff --git a/src/ripple/protocol/impl/BuildInfo.cpp b/src/ripple/protocol/impl/BuildInfo.cpp index 6b20a189d..732c74f1a 100644 --- a/src/ripple/protocol/impl/BuildInfo.cpp +++ b/src/ripple/protocol/impl/BuildInfo.cpp @@ -33,7 +33,7 @@ namespace BuildInfo { // and follow the format described at http://semver.org/ //------------------------------------------------------------------------------ // clang-format off -char const* const versionString = "1.7.1" +char const* const versionString = "1.7.2" // clang-format on #if defined(DEBUG) || defined(SANITIZER)