ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-08-22 14:50:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix bugs related to perm. domain checks, add unit test

Browse Source
This commit is contained in:
Bronek Kozicki
2025-03-11 22:58:02 +00:00
parent 90fef02164
commit 3715d7e2e4
8 changed files with 225 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
src/test/app/Credentials_test.cpp
View File

@@ -42,16 +42,6 @@ checkVL(
return strHex(expected) == strHex(sle->getFieldVL(field));
}
static inline Keylet
credentialKeylet(
test::jtx::Account const& subject,
test::jtx::Account const& issuer,
std::string_view credType)
{
return keylet::credential(
subject.id(), issuer.id(), Slice(credType.data(), credType.size()));
}
struct Credentials_test : public beast::unit_test::suite
{
void
@@ -71,7 +61,7 @@ struct Credentials_test : public beast::unit_test::suite
{
testcase("Create for subject.");
auto const credKey = credentialKeylet(subject, issuer, credType);
auto const credKey = credentials::keylet(subject, issuer, credType);
env.fund(XRP(5000), subject, issuer, other);
env.close();
@@ -149,7 +139,7 @@ struct Credentials_test : public beast::unit_test::suite
{
testcase("Create for themself.");
auto const credKey = credentialKeylet(issuer, issuer, credType);
auto const credKey = credentials::keylet(issuer, issuer, credType);
env(credentials::create(issuer, issuer, credType),
credentials::uri(uri));
@@ -223,7 +213,7 @@ struct Credentials_test : public beast::unit_test::suite
{
testcase("Delete issuer before accept");
auto const credKey = credentialKeylet(subject, issuer, credType);
auto const credKey = credentials::keylet(subject, issuer, credType);
env(credentials::create(subject, issuer, credType));
env.close();
@@ -259,7 +249,7 @@ struct Credentials_test : public beast::unit_test::suite
{
testcase("Delete issuer after accept");
auto const credKey = credentialKeylet(subject, issuer, credType);
auto const credKey = credentials::keylet(subject, issuer, credType);
env(credentials::create(subject, issuer, credType));
env.close();
env(credentials::accept(subject, issuer, credType));
@@ -297,7 +287,7 @@ struct Credentials_test : public beast::unit_test::suite
{
testcase("Delete subject before accept");
auto const credKey = credentialKeylet(subject, issuer, credType);
auto const credKey = credentials::keylet(subject, issuer, credType);
env(credentials::create(subject, issuer, credType));
env.close();
@@ -333,7 +323,7 @@ struct Credentials_test : public beast::unit_test::suite
{
testcase("Delete subject after accept");
auto const credKey = credentialKeylet(subject, issuer, credType);
auto const credKey = credentials::keylet(subject, issuer, credType);
env(credentials::create(subject, issuer, credType));
env.close();
env(credentials::accept(subject, issuer, credType));
@@ -371,7 +361,7 @@ struct Credentials_test : public beast::unit_test::suite
{
testcase("Delete by other");
auto const credKey = credentialKeylet(subject, issuer, credType);
auto const credKey = credentials::keylet(subject, issuer, credType);
auto jv = credentials::create(subject, issuer, credType);
uint32_t const t = env.current()
->info()
@@ -416,7 +406,7 @@ struct Credentials_test : public beast::unit_test::suite
env.close();
{
auto const credKey =
credentialKeylet(subject, issuer, credType);
credentials::keylet(subject, issuer, credType);
BEAST_EXPECT(!env.le(credKey));
BEAST_EXPECT(!ownerCount(env, subject));
BEAST_EXPECT(!ownerCount(env, issuer));
@@ -438,7 +428,7 @@ struct Credentials_test : public beast::unit_test::suite
env.close();
{
auto const credKey =
credentialKeylet(subject, issuer, credType);
credentials::keylet(subject, issuer, credType);
BEAST_EXPECT(!env.le(credKey));
BEAST_EXPECT(!ownerCount(env, subject));
BEAST_EXPECT(!ownerCount(env, issuer));

142
src/test/app/Vault_test.cpp
View File

@@ -19,8 +19,10 @@
#include <test/jtx/Account.h>
#include <test/jtx/Env.h>
#include <test/jtx/credentials.h>
#include <test/jtx/fee.h>
#include <test/jtx/mpt.h>
#include <test/jtx/permissioned_domains.h>
#include <test/jtx/utility.h>
#include <test/jtx/vault.h>
#include <xrpl/basics/base_uint.h>
@@ -605,6 +607,145 @@ class Vault_test : public beast::unit_test::suite
});
}
void
testWithDomainCheck()
{
testcase("private vault");
Env env{*this};
Account issuer{"issuer"};
Account owner{"owner"};
Account depositor{"depositor"};
Account pdOwner{"pdOwner"};
Account credIssuer1{"credIssuer1"};
Account credIssuer2{"credIssuer2"};
std::string const credType = "credential";
auto vault = env.vault();
env.fund(
XRP(1000),
issuer,
owner,
depositor,
pdOwner,
credIssuer1,
credIssuer2);
env.close();
env(fset(issuer, asfAllowTrustLineClawback));
env.close();
env.require(flags(issuer, asfAllowTrustLineClawback));
PrettyAsset asset{xrpIssue(), 1'000'000};
auto [tx, keylet] = vault.create(
{.owner = owner, .asset = asset, .flags = tfVaultPrivate});
env(tx);
env.close();
BEAST_EXPECT(env.le(keylet));
{
testcase("private vault owner can deposit");
auto tx = vault.deposit(
{.depositor = owner, .id = keylet.key, .amount = asset(50)});
env(tx);
}
{
testcase("private vault depositor not authorized yet");
auto tx = vault.deposit(
{.depositor = depositor,
.id = keylet.key,
.amount = asset(50)});
env(tx, ter{tecNO_AUTH});
}
{
testcase("private vault set domainId");
pdomain::Credentials const credentials{
{.issuer = credIssuer1, .credType = credType},
{.issuer = credIssuer2, .credType = credType}};
env(pdomain::setTx(pdOwner, credentials));
auto const domainId = [&]() {
auto tx = env.tx()->getJson(JsonOptions::none);
return pdomain::getNewDomain(env.meta());
}();
auto tx = vault.set({.owner = owner, .id = keylet.key});
tx[sfDomainID] = to_string(domainId);
env(tx);
env.close();
}
{
testcase("private vault depositor still not authorized");
auto tx = vault.deposit(
{.depositor = depositor,
.id = keylet.key,
.amount = asset(50)});
env(tx, ter{tecNO_AUTH});
env.close();
}
auto const credKeylet =
credentials::keylet(depositor, credIssuer1, credType);
{
testcase("private vault depositor now authorized");
env(credentials::create(depositor, credIssuer1, credType));
env(credentials::accept(depositor, credIssuer1, credType));
env.close();
auto credSle = env.le(credKeylet);
BEAST_EXPECT(credSle != nullptr);
auto tx = vault.deposit(
{.depositor = depositor,
.id = keylet.key,
.amount = asset(50)});
env(tx);
env.close();
}
{
testcase("private vault depositor lost authorization");
env(credentials::deleteCred(
credIssuer1, depositor, credIssuer1, credType));
env.close();
auto credSle = env.le(credKeylet);
BEAST_EXPECT(credSle == nullptr);
auto tx = vault.deposit(
{.depositor = depositor,
.id = keylet.key,
.amount = asset(50)});
env(tx, ter{tecNO_AUTH});
}
{
testcase("private vault depositor new authorization");
env(credentials::create(depositor, credIssuer2, credType));
env(credentials::accept(depositor, credIssuer2, credType));
env.close();
auto tx = vault.deposit(
{.depositor = depositor,
.id = keylet.key,
.amount = asset(50)});
env(tx);
}
{
testcase("private vault no authorization needed to withdraw");
env(credentials::deleteCred(
depositor, depositor, credIssuer2, credType));
env.close();
auto tx = vault.withdraw(
{.depositor = depositor,
.id = keylet.key,
.amount = asset(100)});
env(tx);
}
}
public:
void
run() override
@@ -614,6 +755,7 @@ public:
testCreateFailIOU();
testCreateFailMPT();
testWithMPT();
testWithDomainCheck();
}
};

10
src/test/jtx/credentials.h
View File

@@ -29,6 +29,16 @@ namespace jtx {
namespace credentials {
inline Keylet
keylet(
test::jtx::Account const& subject,
test::jtx::Account const& issuer,
std::string_view credType)
{
return keylet::credential(
subject.id(), issuer.id(), Slice(credType.data(), credType.size()));
}
// Sets the optional URI.
class uri
{

14
src/xrpld/app/misc/CredentialHelpers.cpp
View File

@@ -205,9 +205,10 @@ validDomain(ReadView const& view, uint256 domainID, AccountID const& subject)
return tefINTERNAL;
auto const issuer = h.getAccountID(sfIssuer);
auto const type = makeSlice(h.getFieldVL(sfCredentialType));
auto const sleCredential =
view.read(keylet::credential(subject, issuer, type));
auto const type = h.getFieldVL(sfCredentialType);
auto const keyletCredential =
keylet::credential(subject, issuer, makeSlice(type));
auto const sleCredential = view.read(keyletCredential);
// We cannot delete expired credentials, that would require ApplyView&
// However we can check if credentials are expired. Expected transaction
@@ -228,7 +229,7 @@ validDomain(ReadView const& view, uint256 domainID, AccountID const& subject)
}
}
return foundExpired ? tecEXPIRED : tecNO_PERMISSION;
return foundExpired ? tecEXPIRED : tecNO_AUTH;
}
TER
@@ -338,8 +339,9 @@ verifyValidDomain(
return tefINTERNAL;
auto const issuer = h.getAccountID(sfIssuer);
auto const type = makeSlice(h.getFieldVL(sfCredentialType));
auto const keyletCredential = keylet::credential(account, issuer, type);
auto const type = h.getFieldVL(sfCredentialType);
auto const keyletCredential =
keylet::credential(account, issuer, makeSlice(type));
if (view.exists(keyletCredential))
credentials.push_back(keyletCredential.key);
}

5
src/xrpld/app/tx/detail/VaultDeposit.cpp
View File

@@ -119,7 +119,8 @@ VaultDeposit::doApply()
auto const& vaultAccount = vault->at(sfAccount);
MPTIssue const mptIssue(mptIssuanceID);
if (vault->getFlags() == tfVaultPrivate)
// Note, vault owner is always authorized
if (account_ != vault->at(sfOwner) && (vault->getFlags() & tfVaultPrivate))
{
if (auto const err = enforceMPTokenAuthorization(
ctx_.view(), mptIssue, account_, mPriorBalance, j_);
@@ -130,7 +131,7 @@ VaultDeposit::doApply()
{
// No authorization needed, but must ensure there is MPToken
auto sleMpt = view().read(keylet::mptoken(mptIssuanceID, account_));
if (!sleMpt && account_ != vaultAccount)
if (!sleMpt)
{
if (auto const err = MPTokenAuthorize::authorize(
view(),

11
src/xrpld/app/tx/detail/VaultSet.cpp
View File

@@ -95,6 +95,11 @@ VaultSet::doApply()
if (!vault)
return tecOBJECT_NOT_FOUND;
auto const mptIssuanceID = (*vault)[sfMPTokenIssuanceID];
auto const sleIssuance = view().peek(keylet::mptIssuance(mptIssuanceID));
if (!sleIssuance)
return tefINTERNAL;
// Update mutable flags and fields if given.
if (tx.isFieldPresent(sfData))
vault->at(sfData) = tx[sfData];
@@ -108,8 +113,10 @@ VaultSet::doApply()
{
// In VaultSet::preclaim we enforce that tfVaultPrivate must have been
// set in the vault. We currently do not support making such a vault
// public (i.e. removal of tfVaultPrivate flag)
vault->setFieldH256(sfDomainID, tx.getFieldH256(sfDomainID));
// public (i.e. removal of tfVaultPrivate flag). The sfDomainID flag
// must be set in the MPTokenIssuance object and can be freely updated.
sleIssuance->setFieldH256(sfDomainID, tx.getFieldH256(sfDomainID));
view().update(sleIssuance);
}
view().update(vault);

2
src/xrpld/app/tx/detail/VaultWithdraw.cpp
View File

@@ -116,7 +116,7 @@ VaultWithdraw::doApply()
account_,
share,
FreezeHandling::fhZERO_IF_FROZEN,
AuthHandling::ahZERO_IF_UNAUTHORIZED,
AuthHandling::ahIGNORE_AUTH,
j_) < shares)
{
return tecINSUFFICIENT_FUNDS;

65
src/xrpld/ledger/detail/View.cpp
View File

@@ -2216,7 +2216,7 @@ requireAuth(
return tesSUCCESS;
}
// err = tefINTERNAL | tecINVALID_DOMAIN | tecNO_PERMISSION | tecEXPIRED
// err = tefINTERNAL | tecINVALID_DOMAIN | tecNO_AUTH | tecEXPIRED
if (auto const err =
credentials::validDomain(view, *maybeDomainID, account);
!isTesSuccess(err))
@@ -2239,6 +2239,10 @@ enforceMPTokenAuthorization(
if (!sleIssuance)
return tefINTERNAL; // Should have called requireAuth earlier
XRPL_ASSERT(
sleIssuance->getFieldU32(sfFlags) & lsfMPTRequireAuth,
"ripple::verifyAuth : MPTokenIssuance requires authorization");
if (account == sleIssuance->at(sfIssuer))
return tesSUCCESS; // Won't create MPToken for the token issuer
@@ -2250,8 +2254,15 @@ enforceMPTokenAuthorization(
if (domainOwned || sleToken == nullptr)
{
// We check DomainID if:
//
// 1. Token not found or
// 2. Token found and has lsfMPTDomainCheck flag
//
// In case 1. we check authorization in order to create the token
// (below), but only if the account is authorized by the domain. In
// case 2. we re-check authorization in case the credentials are
// expired, in which case the token needs to be unauthorized (below)
auto const maybeDomainID = sleIssuance->at(~sfDomainID);
authorizedByDomain = maybeDomainID.has_value() &&
verifyValidDomain(view, account, *maybeDomainID, j) == tesSUCCESS;
@@ -2260,33 +2271,47 @@ enforceMPTokenAuthorization(
if (!authorizedByDomain && sleToken == nullptr)
{
// Intentionally empty. This could be either of:
// 1. DomainID not set in MPTokenIssuance or
// 2. account has no matching and accepted credentials or
// 3. only expired credentials (removed in verifyValidDomain)
// Either way, move to check at the end of this function
//
// 1. Field sfDomainID not set in MPTokenIssuance or
// 2. Account has no matching and accepted credentials or
// 3. Account has all expired credentials (removed in verifyValidDomain)
//
// Either way, will return tecNO_AUTH at the end of this function
}
else if (!authorizedByDomain && domainOwned)
{
auto sleMpt = view.peek(keylet::mptoken(mptIssuanceID, account));
XRPL_ASSERT(sleMpt, "ripple::verifyAuth : non-null old MPToken");
std::uint32_t const flags = sleMpt->getFieldU32(sfFlags);
// Remove lsfMPTAuthorized flag
sleMpt->setFieldU32(sfFlags, flags & ~lsfMPTAuthorized);
view.update(sleMpt);
// We found an MPToken with lsfMPTDomainCheck flag, but the account is
// no longer authorized.
if (sleToken->getFieldU32(sfFlags) & lsfMPTAuthorized)
{
// Must reset lsfMPTAuthorized, no current credentials
auto sleMpt = view.peek(keylet::mptoken(mptIssuanceID, account));
XRPL_ASSERT(sleMpt, "ripple::verifyAuth : non-null bad MPToken");
std::uint32_t const flags = sleMpt->getFieldU32(sfFlags);
sleMpt->setFieldU32(sfFlags, flags & ~lsfMPTAuthorized);
view.update(sleMpt);
sleToken = nullptr;
sleToken = nullptr; // return tecNO_AUTH at the end of function
}
}
else if (authorizedByDomain && sleToken)
else if (!authorizedByDomain)
{
XRPL_ASSERT(
sleToken->getFlags() & lsfMPTDomainCheck,
"ripple::verifyAuth : MPToken owned by domain");
sleToken != nullptr && !domainOwned,
"ripple::verifyAuth : MPToken not owned by domain");
// MPToken was created by other means, we will check its authorization
// at the end of this function. No need to do anything here.
}
else if (authorizedByDomain && sleToken != nullptr)
{
XRPL_ASSERT(
domainOwned, "ripple::verifyAuth : MPToken owned by domain");
if ((sleToken->getFlags() & lsfMPTAuthorized) == 0)
{
// Must set lsfMPTAuthorized, we found new credentials
auto sleMpt = view.peek(keylet::mptoken(mptIssuanceID, account));
XRPL_ASSERT(sleMpt, "ripple::verifyAuth : non-null new MPToken");
XRPL_ASSERT(sleMpt, "ripple::verifyAuth : non-null good MPToken");
std::uint32_t const flags = sleMpt->getFieldU32(sfFlags);
sleMpt->setFieldU32(sfFlags, flags | lsfMPTAuthorized);
view.update(sleMpt);
@@ -2310,7 +2335,7 @@ enforceMPTokenAuthorization(
return err;
auto sleMpt = view.peek(keylet::mptoken(mptIssuanceID, account));
XRPL_ASSERT(sleMpt, "ripple::verifyAuth : found new MPToken");
XRPL_ASSERT(sleMpt, "ripple::verifyAuth : non-null new MPToken");
std::uint32_t const flags = sleMpt->getFieldU32(sfFlags);
sleMpt->setFieldU32(
sfFlags, flags | lsfMPTDomainCheck | lsfMPTAuthorized);
@@ -2319,11 +2344,7 @@ enforceMPTokenAuthorization(
sleToken = sleMpt; // with lsfMPTAuthorized
}
if (!sleToken)
return tecNO_AUTH;
if (sleIssuance->getFieldU32(sfFlags) & lsfMPTRequireAuth &&
!(sleToken->getFlags() & lsfMPTAuthorized))
if (sleToken == nullptr || (sleToken->getFlags() & lsfMPTAuthorized) == 0)
return tecNO_AUTH;
return tesSUCCESS;