diff --git a/src/test/app/Export_test.cpp b/src/test/app/Export_test.cpp index 28c56edf1b..ea4e31fd61 100644 --- a/src/test/app/Export_test.cpp +++ b/src/test/app/Export_test.cpp @@ -35,8 +35,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -348,6 +350,17 @@ struct Export_test : public beast::unit_test::suite dstEnv.close(); auto const feeDrops = dstEnv.current()->fees().base; + auto const unsignedB2M = dstEnv.jt( + unsignedCallbackImport(account, xpopJson), + sig(none), + fee(feeDrops * 10)); + BEAST_EXPECT( + checkValidity( + dstEnv.app().getHashRouter(), + *unsignedB2M.stx, + dstEnv.current()->rules(), + dstEnv.app().config()) + .first == Validity::SigBad); dstEnv( import::import(account, xpopJson), fee(feeDrops * 10), @@ -2646,6 +2659,13 @@ struct Export_test : public beast::unit_test::suite importVLSequence(env, callback.vlInfo->second) == 0); }; + env(unsignedCallbackImport(alice, callback.xpopJson), + sig(none), + fee(feeDrops), + ter(tefBAD_AUTH)); + BEAST_EXPECT(env.current()->exists(latchKey)); + BEAST_EXPECT(env.balance(alice) == STAmount{balance}); + BEAST_EXPECT(env.seq(alice) == sequence); reject(sig(dave), tefBAD_AUTH); if (mode == "master") { @@ -2680,6 +2700,14 @@ struct Export_test : public beast::unit_test::suite } } + static Json::Value + unsignedCallbackImport(jtx::Account const& owner, Json::Value const& proof) + { + auto tx = jtx::import::import(owner, proof); + tx[jss::SigningPubKey] = ""; + return tx; + } + void testExportCallbackFeeAllowance(FeatureBitset features) { @@ -2692,7 +2720,7 @@ struct Export_test : public beast::unit_test::suite Account const alice{"alice"}; Account const carol{"carol"}; Env env{*this, xpopCtx.makeEnvConfig(21337), features}; - env.fund(XRP(10000), alice, carol); + env.fund(XRP(10000), alice); env.close(); XRPAmount const callbackFee{ mode == "insufficient" ? 10'000'000'000LL : 1000}; @@ -2755,8 +2783,8 @@ struct Export_test : public beast::unit_test::suite if (mode == "insufficient") { - env(import::import(alice, callback.xpopJson), - sig(carol), + env(unsignedCallbackImport(alice, callback.xpopJson), + sig(none), fee(XRP(10000)), ter(terINSUF_FEE_B)); unchanged(); @@ -2765,8 +2793,8 @@ struct Export_test : public beast::unit_test::suite // signing authorization. That must not permit a partial // balance fee claim from an allowance-only callback either. auto const relay = env.jt( - import::import(alice, callback.xpopJson), - sig(carol), + unsignedCallbackImport(alice, callback.xpopJson), + sig(none), fee(XRP(10000))); auto next = std::make_shared( *env.app().getLedgerMaster().getClosedLedger(), @@ -2785,32 +2813,59 @@ struct Export_test : public beast::unit_test::suite continue; } - env(import::import(alice, callback.xpopJson), - sig(carol), + env(unsignedCallbackImport(alice, callback.xpopJson), + sig(none), fee(callbackFee + XRPAmount{1}), ter(tefBAD_AUTH)); unchanged(); - env(import::import(alice, callback.xpopJson), - sig(carol), + env(unsignedCallbackImport(alice, callback.xpopJson), + sig(none), fee(callbackFee - XRPAmount{1}), ter(tefBAD_AUTH)); unchanged(); + + // No carrier account or signature is needed. Conversely, adding + // an unrelated signature must not opt into the proof-only lane. + BEAST_EXPECT(!env.current()->exists(keylet::account(carol.id()))); + env(import::import(alice, callback.xpopJson), + sig(carol), + fee(callbackFee), + ter(tefBAD_AUTH)); + unchanged(); + + TestStopwatch stopwatch; + auto const rejectEnvelope = [&](JTx const& rejected) { + HashRouter router{stopwatch, std::chrono::seconds{300}}; + BEAST_EXPECT( + checkValidity( + router, + *rejected.stx, + env.current()->rules(), + env.app().config()) + .first == Validity::SigBad); + BEAST_EXPECT(!isTesSuccess(preflight( + env.app(), + env.current()->rules(), + *rejected.stx, + tapNONE, + env.journal) + .ter)); + unchanged(); + }; if (ownerTicket) { - env(import::import(alice, callback.xpopJson), - sig(carol), + rejectEnvelope(env.jt( + unsignedCallbackImport(alice, callback.xpopJson), + sig(none), fee(callbackFee), - ticket::use(*ownerTicket), - ter(tefBAD_AUTH)); - unchanged(); + ticket::use(*ownerTicket))); BEAST_EXPECT(env.current()->exists( keylet::ticket(alice.id(), *ownerTicket))); } - auto decorated = import::import(alice, callback.xpopJson); + auto decorated = unsignedCallbackImport(alice, callback.xpopJson); decorated[sfSourceTag.jsonName] = 1; - env(decorated, sig(carol), fee(callbackFee), ter(tefBAD_AUTH)); - unchanged(); + rejectEnvelope(env.jt(decorated, sig(none), fee(callbackFee))); decorated.removeMember(sfSourceTag.jsonName); Json::Value parameter; parameter[sfHookParameter.jsonName][sfHookParameterName.jsonName] = @@ -2818,21 +2873,63 @@ struct Export_test : public beast::unit_test::suite parameter[sfHookParameter.jsonName][sfHookParameterValue.jsonName] = "02"; decorated[sfHookParameters.jsonName].append(parameter); - env(decorated, sig(carol), fee(callbackFee), ter(tefBAD_AUTH)); - unchanged(); + rejectEnvelope(env.jt(decorated, sig(none), fee(callbackFee))); + + decorated = unsignedCallbackImport(alice, callback.xpopJson); + decorated[jss::TxnSignature] = ""; + rejectEnvelope(env.jt(decorated, sig(none), fee(callbackFee))); auto malformedProof = callback.xpopJson; malformedProof[jss::transaction][jss::blob] = "00"; - env(import::import(alice, malformedProof), - sig(carol), - fee(callbackFee), - ter(temMALFORMED)); - unchanged(); + auto const invalid = env.jt( + unsignedCallbackImport(alice, malformedProof), + sig(none), + fee(callbackFee)); + rejectEnvelope(invalid); + // A trusted outer-signature receipt must not skip XPOP checking. + HashRouter forced{stopwatch, std::chrono::seconds{300}}; + forceValidity( + forced, invalid.stx->getTransactionID(), Validity::Valid); + BEAST_EXPECT( + checkValidity( + forced, + *invalid.stx, + env.current()->rules(), + env.app().config()) + .first == Validity::SigBad); + + auto const candidate = env.jt( + unsignedCallbackImport(alice, callback.xpopJson), + sig(none), + fee(callbackFee)); + BEAST_EXPECT(candidate.stx->getSigningPubKey().empty()); + BEAST_EXPECT(!candidate.stx->isFieldPresent(sfTxnSignature)); + BEAST_EXPECT(!candidate.stx->isFieldPresent(sfSigners)); + HashRouter receipt{stopwatch, std::chrono::seconds{300}}; + for (int attempt = 0; attempt != 2; ++attempt) + BEAST_EXPECT( + checkValidity( + receipt, + *candidate.stx, + env.current()->rules(), + env.app().config()) + .first == Validity::Valid); + std::unordered_set> const noExport{ + featureImport}; + Rules const disabled{noExport}; + BEAST_EXPECT( + checkValidity( + receipt, *candidate.stx, disabled, env.app().config()) + .first == Validity::SigBad); + + STTx emitted{*candidate.stx}; + emitted.set(STObject{sfEmitDetails}); + BEAST_EXPECT(!Import::isUnsigned(emitted)); auto const paid = mode == "owner" ? callbackFee + XRPAmount{1} : callbackFee; - env(import::import(alice, callback.xpopJson), - sig(mode == "owner" ? alice : carol), + env(unsignedCallbackImport(alice, callback.xpopJson), + (mode == "owner" ? sig(alice) : sig(none)), fee(paid), ter(tesSUCCESS)); auto const after = env.current()->read(keylet::account(alice.id())); @@ -2849,8 +2946,8 @@ struct Export_test : public beast::unit_test::suite // The same valid proof cannot authorize another debit, whether // the latch was erased or retained awaiting its witness. - env(import::import(alice, callback.xpopJson), - sig(carol), + env(unsignedCallbackImport(alice, callback.xpopJson), + sig(none), fee(callbackFee), ter(tefBAD_AUTH)); auto const duplicate = @@ -2947,8 +3044,8 @@ struct Export_test : public beast::unit_test::suite auto const sequence = before->getFieldU32(sfSequence); for (int attempt = 0; attempt < 2; ++attempt) { - env(import::import(alice, callback.xpopJson), - sig(carol), + env(unsignedCallbackImport(alice, callback.xpopJson), + sig(none), fee(callbackFee), ter(tefBAD_AUTH)); auto const after = env.current()->read(keylet::account(alice.id())); @@ -2960,7 +3057,7 @@ struct Export_test : public beast::unit_test::suite importVLSequence(env, callback.vlInfo->second) == 0); } // An account-authorized transaction keeps ordinary fee-only semantics. - env(import::import(alice, callback.xpopJson), + env(unsignedCallbackImport(alice, callback.xpopJson), sig(alice), fee(callbackFee), ter(tecHOOK_REJECTED)); diff --git a/src/xrpld/app/consensus/ConsensusExtensionsDesign.md b/src/xrpld/app/consensus/ConsensusExtensionsDesign.md index fdee8a1bec..1646909a32 100644 --- a/src/xrpld/app/consensus/ConsensusExtensionsDesign.md +++ b/src/xrpld/app/consensus/ConsensusExtensionsDesign.md @@ -537,8 +537,10 @@ rounds and restarts. An intent may include the positive native `ExportCallbackFee` amount. The latch stores this optional authorization without reserving any balance. Without it, callback Imports require normal source-account authorization. -With it, a third party may deliver the exact valid callback with exactly that -outer fee; the owner's balance and sequence are used on successful apply. The +With it, anyone may assemble and deliver a signatureless Import containing the +exact valid callback with exactly that outer fee. No carrier account or key is +required: SigningPubKey is empty, TxnSignature and Signers are absent. The +owner's balance and sequence are used on successful apply. The allowance cannot authorize consuming a source-account Ticket. The owner must still have funds. Invalid, duplicate, or fee-only unsuccessful third-party attempts do not charge the owner. The final `uint64_t` argument to diff --git a/src/xrpld/app/consensus/ExportIntent.md b/src/xrpld/app/consensus/ExportIntent.md index 30d69a1154..3a5070940c 100644 --- a/src/xrpld/app/consensus/ExportIntent.md +++ b/src/xrpld/app/consensus/ExportIntent.md @@ -297,25 +297,36 @@ this amount on the outer intent; zero omits the field and retains the default policy. Zero is an API sentinel, not a valid serialized fee authorization. This seven-argument WASM import requires recompiling older six-argument callers. -A third-party-signed Import may use this allowance only after all ordinary -proof/latch/validator-list preclaim conditions pass, with its exact stored fee. +Anyone may assemble an unsigned Import and deliver its XPOP without a carrier +key, signature or ledger account. The outer SigningPubKey must be present and +empty; TxnSignature and Signers must be absent. A signed Import instead needs +normal account authority; an unrelated carrier signature cannot use the grant. +Unsigned delivery may use the allowance only after all ordinary proof/latch/ +validator-list conditions pass, with its exact stored fee. The proof's owner, source/target domains, origin, and target digest still bind the callback. An account-authorized Import may pay a higher fee because the account directly authorized it. If the fixed fee cannot meet current admission requirements, third-party delivery waits or the account authorizes a differently -priced Import. No funds are reserved in advance. Third-party delivery uses the owner's current +priced Import. No funds are reserved in advance. Unsigned delivery uses the owner's current sequence and debits the owner's balance; it cannot consume an unrelated source Ticket. Account-authorized Imports retain normal sequence/Ticket choice. Opting in permits delivery without another owner signature at callback time. -The third-party envelope is limited to TransactionType, Account, Sequence, Fee, -Blob, signing fields, Flags (only FullyCanonicalSig), NetworkID, +The unsigned envelope is limited to TransactionType, Account, Sequence, Fee, +Blob, empty SigningPubKey, Flags (only FullyCanonicalSig), NetworkID, LastLedgerSequence, and AccountTxnID. Other owner-side instructions such as Issuer, HookParameters, HookName, SourceTag, or Memos require normal account authorization; permission to deliver the proof does not authorize them. +Ingress checks the signatureless envelope and verifies the complete XPOP before +accepting it as cryptographically valid. Repeated identical transactions may +reuse a dedicated proof-verification receipt; an ordinary signature receipt +cannot bypass these checks. Account existence, exact fee permission, live +latch and validator-list eligibility remain current-ledger checks, never cached +authorization. This signatureless path does not admit B2M or create accounts. + **TODO(export-callback-canonicality):** Fixed Fee does not yet define a unique -third-party transaction. Relayer signatures, source sequence, optional envelope +third-party transaction. Source sequence, optional envelope fields and proof representations can still vary. Review the existing signature caches, transaction suppression and expensive-work ordering before treating the delivery mode as canonical. A latch-backed Sequence-0 lane is a design follow-up, diff --git a/src/xrpld/app/misc/HashRouter.h b/src/xrpld/app/misc/HashRouter.h index ced7f8d1d1..c684bd624d 100644 --- a/src/xrpld/app/misc/HashRouter.h +++ b/src/xrpld/app/misc/HashRouter.h @@ -43,6 +43,7 @@ namespace ripple { #define SF_PRIVATE4 0x0800 #define SF_PRIVATE5 0x1000 #define SF_PRIVATE6 0x2000 +#define SF_PRIVATE7 0x8000 #define SF_EMITTED 0x4000 diff --git a/src/xrpld/app/tx/detail/Import.cpp b/src/xrpld/app/tx/detail/Import.cpp index 62f664dd30..8891af2729 100644 --- a/src/xrpld/app/tx/detail/Import.cpp +++ b/src/xrpld/app/tx/detail/Import.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -49,6 +50,14 @@ namespace { enum class ImportPath { burnToMint, exportCallback }; +struct ImportProofContext +{ + STTx const& tx; + Rules const& rules; + std::uint32_t networkID; + beast::Journal j; +}; + ImportPath importPath(STTx const& inner) { @@ -112,6 +121,56 @@ Import::Import(ApplyContext& ctx) : Transactor(ctx) callbackAllowanceOnly_ = !isTesSuccess(checkAccountSign(signing)); } +bool +Import::isUnsigned(STTx const& tx) noexcept +{ + try + { + return tx.getTxnType() == ttIMPORT && + tx.isFieldPresent(sfSigningPubKey) && + tx.getSigningPubKey().empty() && !tx.isFieldPresent(sfSigners) && + !tx.isFieldPresent(sfEmitDetails); + } + catch (...) + { + return false; + } +} + +bool +Import::hasUnsignedCallbackShape(STTx const& tx) noexcept +{ + try + { + if (!isUnsigned(tx) || tx.isFieldPresent(sfTxnSignature) || + (tx.getFlags() & ~tfFullyCanonicalSig)) + return false; + + // One signatureless representation: empty SigningPubKey, absent + // TxnSignature and Signers. No source Ticket or extra instructions. + for (auto const& field : tx) + { + if (field.getSType() == STI_NOTPRESENT) + continue; + auto const& name = field.getFName(); + if (name != sfTransactionType && name != sfAccount && + name != sfSequence && name != sfFee && name != sfBlob && + name != sfSigningPubKey && name != sfFlags && + name != sfNetworkID && name != sfLastLedgerSequence && + name != sfAccountTxnID) + return false; + } + auto const& fee = tx.getFieldAmount(sfFee); + return tx.getAccountID(sfAccount) != beast::zero && + tx.getFieldU32(sfSequence) != 0 && tx.isFieldPresent(sfBlob) && + isXRP(fee) && fee > beast::zero && isLegalAmount(fee.xrp()); + } + catch (...) + { + return false; + } +} + NotTEC Import::checkImportSign(PreclaimContext const& ctx) { @@ -123,33 +182,16 @@ Import::checkImportSign(PreclaimContext const& ctx) if (importPath(*inner) != ImportPath::exportCallback) return tesSUCCESS; - auto const accountAuth = checkAccountSign(ctx); + auto const accountAuth = + isUnsigned(ctx.tx) ? NotTEC{tefBAD_AUTH} : checkAccountSign(ctx); if (isTesSuccess(accountAuth)) return accountAuth; - // The grant permits one callback using the owner's sequence. It does - // not authorize selecting and consuming an unrelated source Ticket. - if (ctx.tx.isFieldPresent(sfTicketSequence)) + // A signed Import must be account-authorized. The intent grant is for + // signatureless proof delivery, not signatures from unrelated accounts. + if (!hasUnsignedCallbackShape(ctx.tx)) return accountAuth; - // Permission to deliver a proof does not authorize additional owner-side - // instructions (Issuer, HookParameters/HookName, Memos, or other fields). - // Keep the relay envelope explicit so later common fields fail closed. - if (ctx.tx.getFlags() & ~tfFullyCanonicalSig) - return accountAuth; - for (auto const& field : ctx.tx) - { - if (field.getSType() == STI_NOTPRESENT) - continue; - auto const& name = field.getFName(); - if (name != sfTransactionType && name != sfAccount && - name != sfSequence && name != sfFee && name != sfBlob && - name != sfSigningPubKey && name != sfTxnSignature && - name != sfSigners && name != sfFlags && name != sfNetworkID && - name != sfLastLedgerSequence && name != sfAccountTxnID) - return accountAuth; - } - auto const owner = ctx.tx.getAccountID(sfAccount); if (inner->getAccountID(sfAccount) != owner) return accountAuth; @@ -289,6 +331,36 @@ Import::preflight(PreflightContext const& ctx) if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; + if (isUnsigned(ctx.tx)) + { + // Ingress and direct application use the same complete proof checks. + // Unlike preflight2's dry-run shortcut, simulation must verify XPOP. + auto const validity = checkValidity( + ctx.app.getHashRouter(), + ctx.tx, + ctx.rules, + ctx.app.config(), + ctx.flags); + if (validity.first != Validity::Valid) + return temINVALID; + return tesSUCCESS; + } + + if (auto const ret = + checkProof(ctx.tx, ctx.rules, ctx.app.config().NETWORK_ID, ctx.j); + !isTesSuccess(ret)) + return ret; + return preflight2(ctx); +} + +NotTEC +Import::checkProof( + STTx const& transaction, + Rules const& rules, + std::uint32_t sourceNetworkID, + beast::Journal j) +{ + ImportProofContext const ctx{transaction, rules, sourceNetworkID, j}; auto& tx = ctx.tx; if (!tx.isFieldPresent(sfBlob)) @@ -361,6 +433,10 @@ Import::preflight(PreflightContext const& ctx) auto const path = importPath(*stpTrans); bool const hasTicket = path == ImportPath::exportCallback; + // Signatureless delivery never admits the B2M/account-creation path. + if (isUnsigned(tx) && !hasTicket) + return temMALFORMED; + if (hasTicket && !ctx.rules.enabled(featureExport)) { JLOG(ctx.j.warn()) << "Import: cannot use TicketSequence XPOP."; @@ -440,8 +516,7 @@ Import::preflight(PreflightContext const& ctx) return temMALFORMED; } - if (stpTrans->getFieldU32(sfOperationLimit) != - ctx.app.config().NETWORK_ID) + if (stpTrans->getFieldU32(sfOperationLimit) != ctx.networkID) { JLOG(ctx.j.warn()) << "Import: Wrong network ID for OperationLimit in " @@ -1042,8 +1117,7 @@ Import::preflight(PreflightContext const& ctx) return temMALFORMED; } - JLOG(ctx.j.trace()) - << "Import: passed seq/fee/quorum checks, about to return preflight2"; + JLOG(ctx.j.trace()) << "Import: passed proof seq/fee/quorum checks"; if (stpTrans->getFieldAmount(sfFee) < beast::zero) { @@ -1052,7 +1126,7 @@ Import::preflight(PreflightContext const& ctx) return temBAD_FEE; } - return preflight2(ctx); + return tesSUCCESS; } TER diff --git a/src/xrpld/app/tx/detail/Import.h b/src/xrpld/app/tx/detail/Import.h index f05ecbece1..e8692b6118 100644 --- a/src/xrpld/app/tx/detail/Import.h +++ b/src/xrpld/app/tx/detail/Import.h @@ -82,6 +82,21 @@ public: explicit Import(ApplyContext& ctx); + // Candidate detection is not authorization. Full proof verification and + // current-ledger latch/fee checks are required before unsigned delivery. + static bool + isUnsigned(STTx const& tx) noexcept; + + static bool + hasUnsignedCallbackShape(STTx const& tx) noexcept; + + static NotTEC + checkProof( + STTx const& tx, + Rules const& rules, + std::uint32_t sourceNetworkID, + beast::Journal j); + static NotTEC checkImportSign(PreclaimContext const& ctx); diff --git a/src/xrpld/app/tx/detail/apply.cpp b/src/xrpld/app/tx/detail/apply.cpp index 9d26d4ada5..efeb39a7b9 100644 --- a/src/xrpld/app/tx/detail/apply.cpp +++ b/src/xrpld/app/tx/detail/apply.cpp @@ -21,16 +21,18 @@ #include #include #include +#include #include #include namespace ripple { // These are the same flags defined as SF_PRIVATE1-4 in HashRouter.h -#define SF_SIGBAD SF_PRIVATE1 // Signature is bad -#define SF_SIGGOOD SF_PRIVATE2 // Signature is good -#define SF_LOCALBAD SF_PRIVATE3 // Local checks failed -#define SF_LOCALGOOD SF_PRIVATE4 // Local checks passed +#define SF_SIGBAD SF_PRIVATE1 // Signature is bad +#define SF_SIGGOOD SF_PRIVATE2 // Signature is good +#define SF_LOCALBAD SF_PRIVATE3 // Local checks failed +#define SF_LOCALGOOD SF_PRIVATE4 // Local checks passed +#define SF_CALLBACK_PROOFGOOD SF_PRIVATE7 // Unsigned callback XPOP verified //------------------------------------------------------------------------------ @@ -97,11 +99,48 @@ checkValidity( return {Validity::Valid, ""}; } + bool const unsignedImport = Import::isUnsigned(tx); + if (unsignedImport) + { + // A carrier supplies proof, not an account signature. Shape is not + // authority: only this callback-specific, amendment-gated path may + // substitute complete XPOP verification for an outer signature. + if (!rules.enabled(featureImport) || !rules.enabled(featureExport) || + !Import::hasUnsignedCallbackShape(tx)) + return {Validity::SigBad, "Unsigned Import envelope is invalid."}; + + auto const networkID = tx[~sfNetworkID]; + if (requiresTxNetworkID(config.NETWORK_ID) + ? (!networkID || *networkID != config.NETWORK_ID) + : networkID.has_value()) + return {Validity::SigBad, "Unsigned Import network is invalid."}; + + if (flags & SF_SIGBAD) + return {Validity::SigBad, "Import proof is known bad."}; + + // forceValidity() can mark ordinary signatures trusted, but cannot + // stand in for proof verification. Use a distinct receipt for the + // exact immutable transaction; latch/fee authority is never cached. + if (!(flags & SF_CALLBACK_PROOFGOOD)) + { + if (!isTesSuccess(Import::checkProof( + tx, + rules, + config.NETWORK_ID, + beast::Journal{beast::Journal::getNullSink()}))) + { + router.setFlags(id, SF_SIGBAD); + return {Validity::SigBad, "Import proof is invalid."}; + } + router.setFlags(id, SF_CALLBACK_PROOFGOOD); + } + } + if (flags & SF_SIGBAD) // Signature is known bad return {Validity::SigBad, "Transaction has bad signature."}; - if (!(flags & SF_SIGGOOD)) + if (!unsignedImport && !(flags & SF_SIGGOOD)) { // Don't know signature state. Check it. auto const requireCanonicalSig =