From dcd5edaf0bcee7efb7eb3090db4fea9cb3abed3e Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Thu, 23 Jan 2025 20:43:24 +0000 Subject: [PATCH] Verify credentials in VaultDeposit --- src/xrpld/app/misc/CredentialHelpers.cpp | 64 +++++++++++++++++++++--- src/xrpld/app/misc/CredentialHelpers.h | 26 +++++++--- src/xrpld/app/tx/detail/VaultDeposit.cpp | 22 ++++++-- 3 files changed, 92 insertions(+), 20 deletions(-) diff --git a/src/xrpld/app/misc/CredentialHelpers.cpp b/src/xrpld/app/misc/CredentialHelpers.cpp index f117f7f8cec..67ce177f82f 100644 --- a/src/xrpld/app/misc/CredentialHelpers.cpp +++ b/src/xrpld/app/misc/CredentialHelpers.cpp @@ -42,12 +42,11 @@ checkExpired( } bool -removeExpired(ApplyView& view, STTx const& tx, beast::Journal const j) +removeExpired(ApplyView& view, STVector256 const& arr, beast::Journal const j) { auto const closeTime = view.info().parentCloseTime; bool foundExpired = false; - STVector256 const& arr(tx.getFieldV256(sfCredentialIDs)); for (auto const& h : arr) { // Credentials already checked in preclaim. Look only for expired here. @@ -189,10 +188,7 @@ valid(PreclaimContext const& ctx, AccountID const& src) } TER -authorizedDomain( - ReadView const& view, - uint256 domainID, - AccountID const& subject) +valid(ReadView const& view, uint256 domainID, AccountID const& subject) { auto const slePD = view.read(keylet::permissionedDomain(domainID)); if (!slePD || !slePD->isFieldPresent(sfAcceptedCredentials)) @@ -207,6 +203,7 @@ authorizedDomain( auto const type = makeSlice(h.getFieldVL(sfCredentialType)); auto const sleCredential = view.read(keylet::credential(subject, issuer, type)); + if (sleCredential && sleCredential->getFlags() & lsfAccepted) return tesSUCCESS; } @@ -301,6 +298,58 @@ checkArray(STArray const& credentials, unsigned maxSize, beast::Journal j) } // namespace credentials +TER +verifyDomain( + ApplyContext& ctx, + AccountID const& src, + AccountID const& dst, + std::shared_ptr const& object) +{ + if (!object->isFieldPresent(sfDomainID)) + return tesSUCCESS; + auto const domainID = object->getFieldH256(sfDomainID); + + auto& view = ctx.view(); + auto const slePD = view.read(keylet::permissionedDomain(domainID)); + if (!slePD || !slePD->isFieldPresent(sfAcceptedCredentials)) + return tefINTERNAL; + + // Collect all matching credentials on a side, so we can remove expired ones + // We may finish the loop with this collection empty, it's fine. + STVector256 credentials; + for (auto const& h : slePD->getFieldArray(sfAcceptedCredentials)) + { + if (!h.isFieldPresent(sfIssuer) || !h.isFieldPresent(sfCredentialType)) + return tefINTERNAL; + + auto const issuer = h.getAccountID(sfIssuer); + auto const type = makeSlice(h.getFieldVL(sfCredentialType)); + auto const keyletCredential = keylet::credential(dst, issuer, type); + if (view.exists(keyletCredential)) + credentials.push_back(keyletCredential.key); + } + + // Result intentionally ignored - only one good credential is enough. + [[maybe_unused]] bool _ = + credentials::removeExpired(view, credentials, ctx.journal); + + // Only do this check after we have removed expired credentials. + if (src == dst) + return tesSUCCESS; + + for (auto const& h : credentials) + { + auto sleCredential = view.read(keylet::credential(h)); + if (!sleCredential) + return tefINTERNAL; + + if (sleCredential->getFlags() & lsfAccepted) + return tesSUCCESS; + } + + return tecNO_PERMISSION; +} + TER verifyDepositPreauth( ApplyContext& ctx, @@ -317,7 +366,8 @@ verifyDepositPreauth( bool const credentialsPresent = ctx.tx.isFieldPresent(sfCredentialIDs); if (credentialsPresent && - credentials::removeExpired(ctx.view(), ctx.tx, ctx.journal)) + credentials::removeExpired( + ctx.view(), ctx.tx.getFieldV256(sfCredentialIDs), ctx.journal)) return tecEXPIRED; if (sleDst && (sleDst->getFlags() & lsfDepositAuth)) diff --git a/src/xrpld/app/misc/CredentialHelpers.h b/src/xrpld/app/misc/CredentialHelpers.h index fcfb3b28a08..476be32414a 100644 --- a/src/xrpld/app/misc/CredentialHelpers.h +++ b/src/xrpld/app/misc/CredentialHelpers.h @@ -34,9 +34,9 @@ checkExpired( std::shared_ptr const& sleCredential, NetClock::time_point const& closed); -// Return true if at least 1 expired credentials was found(and deleted) +// Return true if any expired credential was found in arr (and deleted) bool -removeExpired(ApplyView& view, STTx const& tx, beast::Journal const j); +removeExpired(ApplyView& view, STVector256 const& arr, beast::Journal const j); // Actually remove a credentials object from the ledger TER @@ -49,16 +49,17 @@ deleteSLE( NotTEC checkFields(PreflightContext const& ctx); -// Accessing the ledger to check if provided credentials are valid +// Accessing the ledger to check if provided credentials are valid. Do not use +// in doApply (only in preclaim) since it does not remove expired credentials. +// If you call it in prelaim, you also must call verifyDepositPreauth in doApply TER valid(PreclaimContext const& ctx, AccountID const& src); -// Check if subject has any credentials maching given credential domain +// Check if subject has any credential maching the given domain. Do not use in +// doApply (only in preclaim) since it does not remove expired credentials. If +// you call it in prelaim, you also must call verifyDomain in doApply TER -authorizedDomain( - ReadView const& view, - uint256 domainID, - AccountID const& subject); +valid(ReadView const& view, uint256 domainID, AccountID const& subject); // This function is only called when we about to return tecNO_PERMISSION // because all the checks for the DepositPreauth authorization failed. @@ -79,6 +80,15 @@ checkArray(STArray const& credentials, unsigned maxSize, beast::Journal j); } // namespace credentials +// Check expired credentials and for credentials maching DomainID of the ledger +// object +TER +verifyDomain( + ApplyContext& ctx, + AccountID const& src, + AccountID const& dst, + std::shared_ptr const& object); + // Check expired credentials and for existing DepositPreauth ledger object TER verifyDepositPreauth( diff --git a/src/xrpld/app/tx/detail/VaultDeposit.cpp b/src/xrpld/app/tx/detail/VaultDeposit.cpp index 6dafaac1cdf..9c6d95c1aac 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.cpp +++ b/src/xrpld/app/tx/detail/VaultDeposit.cpp @@ -56,11 +56,16 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) if (vault->getFlags() == tfVaultPrivate && ctx.tx[sfAccount] != vault->at(sfOwner)) { + // Similar to credential::valid call inside Payment::prelaim (different + // overload), if we do not see authorised credentials in preclaim, we do + // not progress to doApply. This means that any expired credentials are + // only deleted *if* we pass this check here in preclaim. if (auto const domain = vault->at(~sfVaultID)) { - if (credentials::authorizedDomain( - ctx.view, *domain, ctx.tx[sfAccount]) != tesSUCCESS) - return tecNO_PERMISSION; + if (auto const err = + credentials::valid(ctx.view, *domain, ctx.tx[sfAccount]); + !isTesSuccess(err)) + return err; } } @@ -74,9 +79,16 @@ VaultDeposit::doApply() if (!vault) return tecOBJECT_NOT_FOUND; - // TODO: Check credentials. + auto const dst = ctx_.tx[sfAccount]; + auto const src = vault->at(sfOwner); + if (vault->getFlags() & lsfVaultPrivate) - return tecNO_PERMISSION; + { + // TODO move DomainID from vault to MPTokenIssuance + if (auto const err = verifyDomain(ctx_, src, dst, vault); + !isTesSuccess(err)) + return err; + } auto const assets = ctx_.tx[sfAmount]; Asset const& asset = vault->at(sfAsset);