Verify credentials in VaultDeposit

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<SLE> 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))

src/xrpld/app/misc/CredentialHelpers.h

@@ -34,9 +34,9 @@ checkExpired(
     std::shared_ptr<SLE const> 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<SLE> const& object);
+
 // Check expired credentials and for existing DepositPreauth ledger object
 TER
 verifyDepositPreauth(

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);