diff --git a/include/xrpl/protocol/Firewall.h b/include/xrpl/protocol/Firewall.h deleted file mode 100644 index 2661ded3126..00000000000 --- a/include/xrpl/protocol/Firewall.h +++ /dev/null @@ -1,98 +0,0 @@ -//------------------------------------------------------------------------------ -/* - 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_PROTOCOL_FIREWALL_H_INCLUDED -#define RIPPLE_PROTOCOL_FIREWALL_H_INCLUDED - -#include -#include -#include -#include - -namespace ripple { - -/** - * @brief Serializes firewall authorization data into a message. - * - * This function serializes the given account and preauthorize account IDs - * into the provided Serializer object. It adds a firewall hash prefix, - * followed by the account and preauthorize account IDs. - * - * @param msg The Serializer object to serialize the data into. - * @param account The account ID to be serialized. - * @param preauthorize The preauthorize account ID to be serialized. - */ -inline void -serializeFirewallAuthorization( - Serializer& msg, - AccountID const& account, - AccountID const& preauthorize) -{ - msg.add32(HashPrefix::firewall); - msg.addBitString(account); - msg.addBitString(preauthorize); -} - -/** - * @brief Serializes firewall authorization data into a message. - * - * This function serializes the given account ID and amount into the provided - * Serializer object. It adds a firewall hash prefix, followed by the account - * ID and the amount's mantissa. - * - * @param msg The Serializer object to serialize the data into. - * @param account The account ID to be serialized. - * @param amount The amount to be serialized. - */ -inline void -serializeFirewallAuthorization( - Serializer& msg, - AccountID const& account, - STAmount const& amount) -{ - msg.add32(HashPrefix::firewall); - msg.addBitString(account); - msg.add64(amount.mantissa()); -} - -/** - * @brief Serializes firewall authorization data into a message. - * - * This function serializes the given account ID and public key into the - * provided Serializer object. It adds a firewall hash prefix, followed by - * the account ID and the raw bytes of the public key. - * - * @param msg The Serializer object to serialize the data into. - * @param account The account ID to be serialized. - * @param pk The public key to be serialized. - */ -inline void -serializeFirewallAuthorization( - Serializer& msg, - AccountID const& account, - PublicKey const& pk) -{ - msg.add32(HashPrefix::firewall); - msg.addBitString(account); - msg.addRaw(pk.slice()); -} - -} // namespace ripple - -#endif \ No newline at end of file diff --git a/include/xrpl/protocol/HashPrefix.h b/include/xrpl/protocol/HashPrefix.h index 3def1c5b2c8..0b6ddda4921 100644 --- a/include/xrpl/protocol/HashPrefix.h +++ b/include/xrpl/protocol/HashPrefix.h @@ -87,9 +87,6 @@ enum class HashPrefix : std::uint32_t { /** Credentials signature */ credential = detail::make_hash_prefix('C', 'R', 'D'), - - /** Firewall signature */ - firewall = detail::make_hash_prefix('F', 'I', 'R'), }; template diff --git a/include/xrpl/protocol/detail/sfields.macro b/include/xrpl/protocol/detail/sfields.macro index c2c48e30f22..08e4553c9b9 100644 --- a/include/xrpl/protocol/detail/sfields.macro +++ b/include/xrpl/protocol/detail/sfields.macro @@ -379,4 +379,4 @@ UNTYPED_SFIELD(sfPriceDataSeries, ARRAY, 24) UNTYPED_SFIELD(sfAuthAccounts, ARRAY, 25) UNTYPED_SFIELD(sfAuthorizeCredentials, ARRAY, 26) UNTYPED_SFIELD(sfUnauthorizeCredentials, ARRAY, 27) -UNTYPED_SFIELD(sfFirewallSigners, ARRAY, 28) +UNTYPED_SFIELD(sfFirewallSigners, ARRAY, 28, SField::sMD_Default, SField::notSigning) diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index 27ab8672d74..52876f0966c 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -461,8 +461,7 @@ TRANSACTION(ttFIREWALL_SET, 62, FirewallSet, ({ {sfAuthorize, soeOPTIONAL}, {sfAmount, soeOPTIONAL}, {sfTimePeriod, soeOPTIONAL}, - {sfPublicKey, soeOPTIONAL}, - {sfSignature, soeOPTIONAL}, + {sfFirewallSigners, soeOPTIONAL}, })) // /** This transaction type deletes an Firewall instance */ diff --git a/src/libxrpl/protocol/InnerObjectFormats.cpp b/src/libxrpl/protocol/InnerObjectFormats.cpp index 87c42a8085f..69a81132ab4 100644 --- a/src/libxrpl/protocol/InnerObjectFormats.cpp +++ b/src/libxrpl/protocol/InnerObjectFormats.cpp @@ -154,6 +154,14 @@ InnerObjectFormats::InnerObjectFormats() {sfIssuer, soeREQUIRED}, {sfCredentialType, soeREQUIRED}, }); + + add(sfFirewallSigner.jsonName.c_str(), + sfFirewallSigner.getCode(), + { + {sfAccount, soeREQUIRED}, + {sfSigningPubKey, soeREQUIRED}, + {sfTxnSignature, soeREQUIRED}, + }); } InnerObjectFormats const& diff --git a/src/libxrpl/protocol/STTx.cpp b/src/libxrpl/protocol/STTx.cpp index ec6fb7cc87a..c8dde3ade94 100644 --- a/src/libxrpl/protocol/STTx.cpp +++ b/src/libxrpl/protocol/STTx.cpp @@ -239,9 +239,10 @@ STTx::checkFirewallSign( for (auto const& signer : signers) { Blob const& signingPubKey = signer.getFieldVL(sfSigningPubKey); - auto const result = signingPubKey.empty() - ? checkMultiSign(signer, requireCanonicalSig, rules) - : checkSingleSign(signer, requireCanonicalSig); + auto const result = checkSingleSign(signer, requireCanonicalSig); + // auto const result = signingPubKey.empty() + // ? checkMultiSign(signer, requireCanonicalSig, rules) + // : checkSingleSign(signer, requireCanonicalSig); if (!result) return result; diff --git a/src/test/app/Firewall_test.cpp b/src/test/app/Firewall_test.cpp index 37c50483f13..6bbf91c3d38 100644 --- a/src/test/app/Firewall_test.cpp +++ b/src/test/app/Firewall_test.cpp @@ -20,7 +20,6 @@ #include #include #include -#include #include namespace ripple { @@ -34,30 +33,6 @@ struct Firewall_test : public beast::unit_test::suite return std::distance(ownerDir.begin(), ownerDir.end()); }; - static Buffer - sigFirewallAuthAmount( - PublicKey const& pk, - SecretKey const& sk, - AccountID const& account, - STAmount const& amount) - { - Serializer msg; - serializeFirewallAuthorization(msg, account, amount); - return sign(pk, sk, msg.slice()); - } - - static Buffer - sigFirewallAuthPK( - PublicKey const& pk, - SecretKey const& sk, - AccountID const& account, - PublicKey const& _pk) - { - Serializer msg; - serializeFirewallAuthorization(msg, account, _pk); - return sign(pk, sk, msg.slice()); - } - static std::pair> firewallKeyAndSle(ReadView const& view, jtx::Account const& account) { @@ -79,13 +54,25 @@ struct Firewall_test : public beast::unit_test::suite BEAST_EXPECT((*sle)[sfOwner] == account.id()); BEAST_EXPECT((*sle)[sfIssuer] == issuer.id()); if (amount) + { + std::cout << "amount: " << *amount << std::endl; BEAST_EXPECT((*sle)[sfAmount] == *amount); + } if (timePeriod) + { + std::cout << "timePeriod: " << *timePeriod << std::endl; BEAST_EXPECT((*sle)[sfTimePeriod] == *timePeriod); + } if (timeStart) + { + std::cout << "timeStart: " << *timeStart << std::endl; BEAST_EXPECT((*sle)[sfTimePeriodStart] == *timeStart); + } if (totalOut) + { + std::cout << "totalOut: " << *totalOut << std::endl; BEAST_EXPECT((*sle)[sfTotalOut] == *totalOut); + } } void @@ -113,7 +100,9 @@ struct Firewall_test : public beast::unit_test::suite withFirewall ? ter(tesSUCCESS) : ter(temDISABLED); auto const dirCount = withFirewall ? 2 : 0; - env(firewall::set(alice), + auto const seq = env.seq(alice); + auto const fee = env.current()->fees().base; + env(firewall::set(alice, seq, fee), firewall::auth(bob), firewall::issuer(carol), txResult); @@ -210,8 +199,10 @@ struct Firewall_test : public beast::unit_test::suite env.fund(XRP(1000), alice, bob, carol); env.close(); - env(firewall::set(alice), - firewall::auth(carol), + auto const seq = env.seq(alice); + auto const fee = env.current()->fees().base; + env(firewall::set(alice, seq, fee), + firewall::auth(bob), firewall::issuer(carol), ter(tesSUCCESS)); env.close(); @@ -225,8 +216,10 @@ struct Firewall_test : public beast::unit_test::suite env.fund(XRP(1000), alice, bob, carol); env.close(); - env(firewall::set(alice), - firewall::auth(carol), + auto const seq = env.seq(alice); + auto const fee = env.current()->fees().base; + env(firewall::set(alice, seq, fee), + firewall::auth(bob), firewall::amt(XRP(10)), firewall::issuer(carol), ter(tesSUCCESS)); @@ -243,8 +236,10 @@ struct Firewall_test : public beast::unit_test::suite env.close(); auto const timeStart = env.now(); - env(firewall::set(alice), - firewall::auth(carol), + auto const seq = env.seq(alice); + auto const fee = env.current()->fees().base; + env(firewall::set(alice, seq, fee), + firewall::auth(bob), firewall::amt(XRP(10)), firewall::time_period(3600), firewall::issuer(carol), @@ -273,149 +268,200 @@ struct Firewall_test : public beast::unit_test::suite Account const alice = Account("alice"); Account const bob = Account("bob"); Account const carol = Account("carol"); + Account const dave = Account("dave"); { Env env{*this, features}; - env.fund(XRP(1000), alice, bob, carol); + env.fund(XRP(1000), alice, bob, carol, dave); env.close(); - env(firewall::set(alice), - firewall::auth(carol), + auto const seq = env.seq(alice); + auto const fee = env.current()->fees().base; + env(firewall::set(alice, seq, fee), + firewall::auth(bob), firewall::amt(XRP(10)), firewall::issuer(carol), ter(tesSUCCESS)); env.close(); - env(pay(alice, bob, XRP(100)), ter(tecFIREWALL_BLOCK)); + { + Json::Value params; + params[jss::ledger_index] = env.current()->seq() - 1; + params[jss::transactions] = true; + params[jss::expand] = true; + auto const jrr = env.rpc("json", "ledger", to_string(params)); + std::cout << "jrr: " << jrr << "\n"; + } + + env(pay(alice, dave, XRP(100)), ter(tecFIREWALL_BLOCK)); env.close(); + + { + Json::Value params; + params[jss::ledger_index] = env.current()->seq() - 1; + params[jss::transactions] = true; + params[jss::expand] = true; + auto const jrr = env.rpc("json", "ledger", to_string(params)); + std::cout << "jrr: " << jrr << "\n"; + } } } void - testUpdateAmount(FeatureBitset features) + testFirewallSetUpdate(FeatureBitset features) { - testcase("update amount"); + testcase("set update"); using namespace jtx; using namespace std::literals::chrono_literals; Account const alice = Account("alice"); Account const bob = Account("bob"); Account const carol = Account("carol"); + Account const dave = Account("dave"); + Account const elsa = Account("elsa"); + // Update Amount w/out time limit { Env env{*this, features}; - env.fund(XRP(1000), alice, bob, carol); + auto const baseFee = env.current()->fees().base; + env.fund(XRP(1000), alice, bob, carol, dave); env.close(); - env(firewall::set(alice), - firewall::auth(carol), + env(firewall::set(alice, env.seq(alice), baseFee), + firewall::auth(bob), firewall::amt(XRP(10)), firewall::issuer(carol), ter(tesSUCCESS)); env.close(); - env(pay(alice, bob, XRP(100)), ter(tecFIREWALL_BLOCK)); + env(pay(alice, dave, XRP(100)), ter(tecFIREWALL_BLOCK)); env.close(); - // auto const sig = sigFirewallAuthAmount( - // carol.pk(), carol.sk(), alice.id(), XRP(100)); - // env(firewall::set(alice), - // firewall::amt(XRP(100)), - // firewall::sig(sig), - // ter(tesSUCCESS)); - // env.close(); - - // // verifyFirewall(*env.current(), alice, XRP(100), carol.pk()); + env(firewall::set(alice, env.seq(alice), baseFee), + firewall::amt(XRP(101)), + firewall::sig(carol), + ter(tesSUCCESS)); + env.close(); - // env(pay(alice, bob, XRP(100)), ter(tesSUCCESS)); - // env.close(); + verifyFirewallSle(*env.current(), alice, carol, XRP(101)); + env(pay(alice, dave, XRP(100)), ter(tesSUCCESS)); + env.close(); } - } - - void - testUpdatePK(FeatureBitset features) - { - testcase("update pk"); - using namespace jtx; - using namespace std::literals::chrono_literals; - - Account const alice = Account("alice"); - Account const bob = Account("bob"); - Account const carol = Account("carol"); - Account const dave = Account("dave"); + // Update Amount w/ time limit { Env env{*this, features}; + auto const baseFee = env.current()->fees().base; env.fund(XRP(1000), alice, bob, carol, dave); env.close(); - env(firewall::set(alice), - firewall::auth(carol), + env(firewall::set(alice, env.seq(alice), baseFee), + firewall::auth(bob), firewall::amt(XRP(10)), + firewall::time_period(300), firewall::issuer(carol), ter(tesSUCCESS)); env.close(); - // verifyFirewall(*env.current(), alice, XRP(10), carol.pk()); + verifyFirewallSle( + *env.current(), + alice, + carol, + XRP(10), + 300, + env.now().time_since_epoch().count(), + STAmount(0)); - env(pay(alice, bob, XRP(100)), ter(tecFIREWALL_BLOCK)); + env(pay(alice, dave, XRP(100)), ter(tecFIREWALL_BLOCK)); env.close(); - // auto const sig1 = sigFirewallAuthPK( - // carol.pk(), carol.sk(), alice.id(), dave.pk()); - // env(firewall::set(alice), - // firewall::issuer(dave.pk()), - // firewall::sig(sig1), - // ter(tesSUCCESS)); - // env.close(); - - // verifyFirewall(*env.current(), alice, XRP(10), dave.pk()); - - // auto const sig2 = sigFirewallAuthAmount( - // dave.pk(), dave.sk(), alice.id(), XRP(100)); - // env(firewall::set(alice), - // firewall::amt(XRP(100)), - // firewall::sig(sig2), - // ter(tesSUCCESS)); - // env.close(); + env(firewall::set(alice, env.seq(alice), baseFee), + firewall::amt(XRP(101)), + firewall::time_period(3600), + firewall::sig(carol), + ter(tesSUCCESS)); + env.close(); - // verifyFirewall(*env.current(), alice, XRP(100), dave.pk()); + verifyFirewallSle( + *env.current(), + alice, + carol, + XRP(101), + 3600, + env.now().time_since_epoch().count(), + STAmount(0)); - env(pay(alice, bob, XRP(100)), ter(tesSUCCESS)); + env(pay(alice, dave, XRP(100)), ter(tesSUCCESS)); env.close(); } + + // // Update Issuer + // { + // Env env{*this, features}; + // auto const baseFee = env.current()->fees().base; + // env.fund(XRP(1000), alice, bob, carol, dave); + // env.close(); + + // env(firewall::set(alice, env.seq(alice), baseFee), + // firewall::auth(bob), + // firewall::issuer(carol), + // ter(tesSUCCESS)); + // env.close(); + + // verifyFirewallSle( + // *env.current(), + // alice, + // carol); + + // env(pay(alice, bob, XRP(100)), ter(tecFIREWALL_BLOCK)); + // env.close(); + + // env(firewall::set(alice, env.seq(alice), baseFee), + // firewall::issuer(dave), + // firewall::sig(carol), + // ter(tesSUCCESS)); + // env.close(); + + // verifyFirewallSle( + // *env.current(), + // alice, + // dave); + + // env(pay(alice, bob, XRP(100)), ter(tesSUCCESS)); + // env.close(); + // } } - void - testMasterDisable(FeatureBitset features) - { - testcase("master disable"); - using namespace jtx; - using namespace std::literals::chrono_literals; + // void + // testMasterDisable(FeatureBitset features) + // { + // testcase("master disable"); + // using namespace jtx; + // using namespace std::literals::chrono_literals; - Account const alice = Account("alice"); - Account const bob = Account("bob"); - Account const carol = Account("carol"); - Account const dave = Account("dave"); + // Account const alice = Account("alice"); + // Account const bob = Account("bob"); + // Account const carol = Account("carol"); + // Account const dave = Account("dave"); - { - Env env{*this, features}; - env.fund(XRP(1000), alice, bob, carol, dave); - env.close(); + // { + // Env env{*this, features}; + // env.fund(XRP(1000), alice, bob, carol, dave); + // env.close(); - env(firewall::set(alice), - firewall::auth(carol), - firewall::amt(XRP(10)), - firewall::issuer(carol), - ter(tesSUCCESS)); - env.close(); + // env(firewall::set(alice), + // firewall::auth(carol), + // firewall::amt(XRP(10)), + // firewall::issuer(carol), + // ter(tesSUCCESS)); + // env.close(); - // verifyFirewall(*env.current(), alice, XRP(10), carol.pk()); + // // verifyFirewall(*env.current(), alice, XRP(10), carol.pk()); - env(fset(alice, asfDisableMaster), ter(tecNO_PERMISSION)); - env.close(); - } - } + // env(fset(alice, asfDisableMaster), ter(tecNO_PERMISSION)); + // env.close(); + // } + // } // void // testTransactionTypes(FeatureBitset features) @@ -443,9 +489,9 @@ struct Firewall_test : public beast::unit_test::suite // testPreclaim(features); // testDoApply(features); // testFirewallSet(features); - testFirewallBlock(features); + // testFirewallBlock(features); // testFirewallDelete(features); - // testUpdateAmount(features); + testFirewallSetUpdate(features); // testUpdatePK(features); // testMasterDisable(features); // testTransactionTypes(features); @@ -470,3 +516,8 @@ struct Firewall_test : public beast::unit_test::suite BEAST_DEFINE_TESTSUITE(Firewall, app, ripple); } // namespace test } // namespace ripple + + +1. Blocking all transaction types with an amount makes this extremely difficult because there are multiple amounts on a txn type and blocking by amount for payment wont work because of pathing. The way we handle it in Firewall is by writing something similar to the invariant checker using before/after. +2. The whole entire premise of firewall is that you are protected. If you're keys are compromised then the attacker can just remove your guard with this proposal. Incoming settings are fine but this is pointless to block outgoing txns. +3. If you're going to do it like this add outgoing/incoming as the current way only allows you to guard one side of the transaction. diff --git a/src/test/jtx/firewall.h b/src/test/jtx/firewall.h index 728e886f8b1..5ee1fadd03a 100644 --- a/src/test/jtx/firewall.h +++ b/src/test/jtx/firewall.h @@ -32,7 +32,7 @@ namespace firewall { /** Set/Update a firewall. */ Json::Value -set(Account const& account); +set(Account const& account, std::uint32_t const& seq, STAmount const& fee); /** Sets the optional TimePeriod on a JTx. */ class time_period diff --git a/src/test/jtx/impl/firewall.cpp b/src/test/jtx/impl/firewall.cpp index b5c3d4a59a8..87d44639a52 100644 --- a/src/test/jtx/impl/firewall.cpp +++ b/src/test/jtx/impl/firewall.cpp @@ -33,11 +33,14 @@ namespace jtx { namespace firewall { Json::Value -set(Account const& account) +set(Account const& account, std::uint32_t const& seq, STAmount const& fee) { Json::Value jv; jv[jss::Account] = account.human(); jv[jss::TransactionType] = jss::FirewallSet; + jv[jss::Sequence] = seq; + jv[jss::Fee] = fee.getJson(JsonOptions::none); + jv[jss::SigningPubKey] = strHex(account.pk().slice()); return jv; } @@ -79,7 +82,6 @@ sig::sig(std::vector signers_) : signers(std::move(signers_)) void sig::operator()(Env& env, JTx& jt) const { - auto const mySigners = signers; std::optional st; try { @@ -90,6 +92,7 @@ sig::operator()(Env& env, JTx& jt) const env.test.log << pretty(jt.jv) << std::endl; Rethrow(); } + auto const mySigners = signers; auto& js = jt[sfFirewallSigners.getJsonName()]; for (std::size_t i = 0; i < mySigners.size(); ++i) { @@ -98,12 +101,11 @@ sig::operator()(Env& env, JTx& jt) const jo[jss::Account] = e.acct.human(); jo[jss::SigningPubKey] = strHex(e.sig.pk().slice()); - Serializer msg; - // serializeBatch(msg, st->getFlags(), st->getFieldV256(sfTxIDs)); - // auto const sig = ripple::sign( - // *publicKeyType(e.sig.pk().slice()), e.sig.sk(), msg.slice()); - // jo[sfTxnSignature.getJsonName()] = - // strHex(Slice{sig.data(), sig.size()}); + Serializer ss; + ss.add32(HashPrefix::txSign); + st->addWithoutSigningFields(ss); + auto const sig = ripple::sign(*publicKeyType(e.sig.pk().slice()), e.sig.sk(), ss.slice()); + jo[jss::TxnSignature] = strHex(Slice{sig.data(), sig.size()}); } } diff --git a/src/xrpld/app/tx/detail/Firewall.cpp b/src/xrpld/app/tx/detail/Firewall.cpp index 877f487a58f..281e572346d 100644 --- a/src/xrpld/app/tx/detail/Firewall.cpp +++ b/src/xrpld/app/tx/detail/Firewall.cpp @@ -22,7 +22,6 @@ #include #include #include -#include #include #include #include @@ -30,6 +29,18 @@ namespace ripple { +XRPAmount +FirewallSet::calculateBaseFee(ReadView const& view, STTx const& tx) +{ + // Calculate the FirewallSigners Fees + // std::int32_t signerCount = tx.isFieldPresent(sfFirewallSigners) + // ? tx.getFieldArray(sfFirewallSigners).size() + // : 0; + + // return ((signerCount + 2) * view.fees().base); + return view.fees().base; +} + NotTEC FirewallSet::preflight(PreflightContext const& ctx) { @@ -44,13 +55,13 @@ FirewallSet::preflight(PreflightContext const& ctx) // if (amount.issue() == amount2.issue()) // { // JLOG(ctx.j.debug()) - // << "Firewall: tokens can not have the same currency/issuer."; + // << "FirewallSet: tokens can not have the same currency/issuer."; // return temBAD_AMM_TOKENS; // } // if (auto const err = invalidAmount(amount)) // { - // JLOG(ctx.j.debug()) << "Firewall: invalid asset1 amount."; + // JLOG(ctx.j.debug()) << "FirewallSet: invalid asset1 amount."; // return err; // } @@ -88,79 +99,84 @@ FirewallSet::preclaim(PreclaimContext const& ctx) if (!sleFirewall) { - if (ctx.tx.isFieldPresent(sfSignature)) + if (ctx.tx.isFieldPresent(sfFirewallSigners)) { JLOG(ctx.j.debug()) - << "Firewall: Set must not contain a sfSignature"; + << "FirewallSet: Set must not contain a sfFirewallSigners"; return temMALFORMED; } if (!ctx.tx.isFieldPresent(sfAuthorize)) { - JLOG(ctx.j.debug()) << "Firewall: Set must contain a sfAuthorize"; + JLOG(ctx.j.debug()) << "FirewallSet: Set must contain a sfAuthorize"; return temMALFORMED; } if (!ctx.tx.isFieldPresent(sfIssuer)) { - JLOG(ctx.j.debug()) << "Firewall: Set must contain a sfIssuer"; + JLOG(ctx.j.debug()) << "FirewallSet: Set must contain a sfIssuer"; return temMALFORMED; } } else { - if (!ctx.tx.isFieldPresent(sfSignature)) + if (ctx.tx.isFieldPresent(sfAuthorize)) { JLOG(ctx.j.debug()) - << "Firewall: Update must contain a sfSignature"; + << "FirewallSet: Update cannot contain a sfAuthorize"; return temMALFORMED; } - if (ctx.tx.isFieldPresent(sfAuthorize)) + if (!ctx.tx.isFieldPresent(sfFirewallSigners)) { - JLOG(ctx.j.debug()) - << "Firewall: Update cannot contain a sfAuthorize"; + JLOG(ctx.j.debug()) << "FirewallSet: Update must contain sfFirewallSigners"; return temMALFORMED; } - if (ctx.tx.isFieldPresent(sfIssuer) && - ctx.tx.isFieldPresent(sfAmount)) + std::set firewallSignersSet; + if (ctx.tx.isFieldPresent(sfFirewallSigners)) { - JLOG(ctx.j.debug()) << "Firewall: Update cannot contain both " - "sfIssuer & sfAmount"; - return temMALFORMED; + STArray const signers = ctx.tx.getFieldArray(sfFirewallSigners); + + // Check that the firewall signers array is not too large. + if (signers.size() > 8) + { + JLOG(ctx.j.trace()) << "FirewallSet: signers array exceeds 8 entries."; + return temARRAY_TOO_LARGE; + } + + // Add the batch signers to the set. + for (auto const& signer : signers) + { + AccountID const innerAccount = signer.getAccountID(sfAccount); + if (!firewallSignersSet.insert(innerAccount).second) + { + JLOG(ctx.j.trace()) + << "FirewallSet: Duplicate signer found: " << innerAccount; + return temBAD_SIGNER; + } + } + + // Check the batch signers signatures. + auto const requireCanonicalSig = + ctx.view.rules().enabled(featureRequireFullyCanonicalSig) + ? STTx::RequireFullyCanonicalSig::yes + : STTx::RequireFullyCanonicalSig::no; + auto const sigResult = + ctx.tx.checkFirewallSign(requireCanonicalSig, ctx.view.rules()); + + if (!sigResult) + { + JLOG(ctx.j.trace()) << "FirewallSet: invalid batch txn signature."; + return temBAD_SIGNATURE; + } } - // if (ctx.tx.isFieldPresent(sfSignature) && - // ctx.tx.isFieldPresent(sfPublicKey)) - // { - // PublicKey const txPK(makeSlice(ctx.tx.getFieldVL(sfPublicKey))); - // auto const sig = ctx.tx.getFieldVL(sfSignature); - // PublicKey const fPK( - // makeSlice(sleFirewall->getFieldVL(sfPublicKey))); - // Serializer msg; - // serializeFirewallAuthorization(msg, accountID, txPK); - // if (!verify(fPK, msg.slice(), makeSlice(sig), /*canonical*/ true)) - // { - // JLOG(ctx.j.debug()) - // << "Firewall: Bad Signature for update sfPublicKey"; - // return temBAD_SIGNATURE; - // } - // } - - // if (ctx.tx.isFieldPresent(sfSignature) && - // ctx.tx.isFieldPresent(sfAmount)) - // { - // auto const amount = ctx.tx.getFieldAmount(sfAmount); - // auto const sig = ctx.tx.getFieldVL(sfSignature); - // PublicKey const pk(makeSlice(sleFirewall->getFieldVL(sfPublicKey))); - // Serializer msg; - // serializeFirewallAuthorization(msg, accountID, amount); - // if (!verify(pk, msg.slice(), makeSlice(sig), /*canonical*/ true)) - // { - // JLOG(ctx.j.debug()) - // << "Firewall: Bad Signature for update sfAmount"; - // return temBAD_SIGNATURE; - // } - // } + if (ctx.tx.isFieldPresent(sfFirewallSigners) && + firewallSignersSet.size() != ctx.tx.getFieldArray(sfFirewallSigners).size()) + { + JLOG(ctx.j.trace()) + << "FirewallSet: unique signers does not match firewall signers."; + return temBAD_SIGNER; + } } return tesSUCCESS; @@ -174,7 +190,7 @@ FirewallSet::doApply() auto const sleOwner = sb.peek(keylet::account(account_)); if (!sleOwner) { - JLOG(j_.debug()) << "Firewall: Owner account not found"; + JLOG(j_.debug()) << "FirewallSet: Owner account not found"; return tefINTERNAL; } @@ -205,7 +221,7 @@ FirewallSet::doApply() } else { - JLOG(j_.debug()) << "Firewall: failed to insert owner dir"; + JLOG(j_.debug()) << "FirewallSet: failed to insert owner dir"; return tecDIR_FULL; } sb.insert(sleFirewall); @@ -235,7 +251,7 @@ FirewallSet::doApply() } else { - JLOG(j_.debug()) << "Firewall: failed to insert owner dir"; + JLOG(j_.debug()) << "FirewallSet: failed to insert owner dir"; return tecDIR_FULL; } sb.insert(slePreauth); diff --git a/src/xrpld/app/tx/detail/Firewall.h b/src/xrpld/app/tx/detail/Firewall.h index 87d156580e4..aef58b1d2dc 100644 --- a/src/xrpld/app/tx/detail/Firewall.h +++ b/src/xrpld/app/tx/detail/Firewall.h @@ -36,6 +36,9 @@ class FirewallSet : public Transactor { } + static XRPAmount + calculateBaseFee(ReadView const& view, STTx const& tx); + static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 68ea1fd38ed..3243c47e1a7 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -645,21 +645,49 @@ Transactor::apply() return doApply(); } +NotTEC +Transactor::checkFirewallSign(PreclaimContext const& ctx) +{ + auto const account = ctx.tx.getAccountID(sfAccount); + + // Check if the firewall is enabled. + auto const sleFirewall = ctx.view.read(keylet::firewall(account)); + if (!sleFirewall) + return tesSUCCESS; + + // Check if the firewall has signers. + auto const issuer = sleFirewall->getAccountID(sfIssuer); + STArray const& signers(ctx.tx.getFieldArray(sfFirewallSigners)); + auto const sleAccountSigners = ctx.view.read(keylet::signers(issuer)); + if (sleAccountSigners) + return checkMultiSign(ctx, account, signers); + + // Check if the firewall has a single signer. + Blob const pkSigner = signers[0].getFieldVL(sfSigningPubKey); + AccountID const idAccount = signers[0].getAccountID(sfAccount); + return checkSingleSign(ctx, idAccount, pkSigner); +} + NotTEC Transactor::checkSign(PreclaimContext const& ctx) { // If the pk is empty, then we must be multi-signing. if (ctx.tx.getSigningPubKey().empty()) - return checkMultiSign(ctx); + { + auto const account = ctx.tx.getAccountID(sfAccount); + STArray const& signers(ctx.tx.getFieldArray(sfSigners)); + return checkMultiSign(ctx, account, signers); + } - return checkSingleSign(ctx); + Blob const pkSigner = ctx.tx.getSigningPubKey(); + AccountID const idAccount = ctx.tx.getAccountID(sfAccount); + return checkSingleSign(ctx, idAccount, pkSigner); } NotTEC -Transactor::checkSingleSign(PreclaimContext const& ctx) +Transactor::checkSingleSign(PreclaimContext const& ctx, AccountID const& idAccount, Blob const& pkSigner) { // Check that the value in the signing key slot is a public key. - auto const pkSigner = ctx.tx.getSigningPubKey(); if (!publicKeyType(makeSlice(pkSigner))) { JLOG(ctx.j.trace()) @@ -669,7 +697,6 @@ Transactor::checkSingleSign(PreclaimContext const& ctx) // Look up the account. auto const idSigner = calcAccountID(PublicKey(makeSlice(pkSigner))); - auto const idAccount = ctx.tx.getAccountID(sfAccount); auto const sleAccount = ctx.view.read(keylet::account(idAccount)); if (!sleAccount) return terNO_ACCOUNT; @@ -730,9 +757,11 @@ Transactor::checkSingleSign(PreclaimContext const& ctx) } NotTEC -Transactor::checkMultiSign(PreclaimContext const& ctx) +Transactor::checkMultiSign( + PreclaimContext const& ctx, + AccountID const& id, + STArray const& txSigners) { - auto const id = ctx.tx.getAccountID(sfAccount); // Get mTxnAccountID's SignerList and Quorum. std::shared_ptr sleAccountSigners = ctx.view.read(keylet::signers(id)); @@ -754,9 +783,6 @@ Transactor::checkMultiSign(PreclaimContext const& ctx) if (!accountSigners) return accountSigners.error(); - // Get the array of transaction signers. - STArray const& txSigners(ctx.tx.getFieldArray(sfSigners)); - // Walk the accountSigners performing a variety of checks and see if // the quorum is met. diff --git a/src/xrpld/app/tx/detail/Transactor.h b/src/xrpld/app/tx/detail/Transactor.h index 6f04578103a..c203af3b842 100644 --- a/src/xrpld/app/tx/detail/Transactor.h +++ b/src/xrpld/app/tx/detail/Transactor.h @@ -137,6 +137,9 @@ class Transactor static NotTEC checkSign(PreclaimContext const& ctx); + static NotTEC + checkFirewallSign(PreclaimContext const& ctx); + // Returns the fee in fee units, not scaled for load. static XRPAmount calculateBaseFee(ReadView const& view, STTx const& tx); @@ -198,9 +201,15 @@ class Transactor TER payFee(); static NotTEC - checkSingleSign(PreclaimContext const& ctx); + checkSingleSign( + PreclaimContext const& ctx, + AccountID const& idAccount, + Blob const& pkSigner); static NotTEC - checkMultiSign(PreclaimContext const& ctx); + checkMultiSign( + PreclaimContext const& ctx, + AccountID const& id, + STArray const& txSigners); void trapTransaction(uint256) const; }; diff --git a/src/xrpld/app/tx/detail/WithdrawPreauth.cpp b/src/xrpld/app/tx/detail/WithdrawPreauth.cpp index 43cc25033c7..f56ccd57854 100644 --- a/src/xrpld/app/tx/detail/WithdrawPreauth.cpp +++ b/src/xrpld/app/tx/detail/WithdrawPreauth.cpp @@ -21,7 +21,6 @@ #include #include #include -#include #include #include #include @@ -84,31 +83,31 @@ WithdrawPreauth::preclaim(PreclaimContext const& ctx) Serializer msg; AccountID const accountID = ctx.tx[sfAccount]; - // Determine which operation we're performing: authorizing or unauthorizing. - if (ctx.tx.isFieldPresent(sfAuthorize)) - { - // Verify that the Authorize account is present in the ledger. - AccountID const auth{ctx.tx[sfAuthorize]}; - if (!ctx.view.exists(keylet::account(auth))) - return tecNO_TARGET; - - // Verify that the Preauth entry they asked to add is not already - // in the ledger. - if (ctx.view.exists(keylet::withdrawPreauth(ctx.tx[sfAccount], auth))) - return tecDUPLICATE; + // // Determine which operation we're performing: authorizing or unauthorizing. + // if (ctx.tx.isFieldPresent(sfAuthorize)) + // { + // // Verify that the Authorize account is present in the ledger. + // AccountID const auth{ctx.tx[sfAuthorize]}; + // if (!ctx.view.exists(keylet::account(auth))) + // return tecNO_TARGET; + + // // Verify that the Preauth entry they asked to add is not already + // // in the ledger. + // if (ctx.view.exists(keylet::withdrawPreauth(ctx.tx[sfAccount], auth))) + // return tecDUPLICATE; - serializeFirewallAuthorization(msg, accountID, auth); - } - else - { - // Verify that the Preauth entry they asked to remove is in the ledger. - AccountID const unauth{ctx.tx[sfUnauthorize]}; - if (!ctx.view.exists( - keylet::withdrawPreauth(ctx.tx[sfAccount], unauth))) - return tecNO_ENTRY; + // serializeFirewallAuthorization(msg, accountID, auth); + // } + // else + // { + // // Verify that the Preauth entry they asked to remove is in the ledger. + // AccountID const unauth{ctx.tx[sfUnauthorize]}; + // if (!ctx.view.exists( + // keylet::withdrawPreauth(ctx.tx[sfAccount], unauth))) + // return tecNO_ENTRY; - serializeFirewallAuthorization(msg, accountID, unauth); - } + // serializeFirewallAuthorization(msg, accountID, unauth); + // } // Validate Signature ripple::Keylet const firewallKeylet = keylet::firewall(accountID); diff --git a/src/xrpld/app/tx/detail/applySteps.cpp b/src/xrpld/app/tx/detail/applySteps.cpp index a7bffe88fe0..7fb49699ba9 100644 --- a/src/xrpld/app/tx/detail/applySteps.cpp +++ b/src/xrpld/app/tx/detail/applySteps.cpp @@ -197,6 +197,14 @@ invoke_preclaim(PreclaimContext const& ctx) if (result != tesSUCCESS) return result; + + if (ctx.tx.getTxnType() == ttFIREWALL_SET || + ctx.tx.getTxnType() == ttWITHDRAW_PREAUTH) + { + result = T::checkFirewallSign(ctx); + if (result != tesSUCCESS) + return result; + } } return T::preclaim(ctx);