diff --git a/src/libxrpl/protocol/ConfidentialTransfer.cpp b/src/libxrpl/protocol/ConfidentialTransfer.cpp index 0699571a0b..d6e4529f0e 100644 --- a/src/libxrpl/protocol/ConfidentialTransfer.cpp +++ b/src/libxrpl/protocol/ConfidentialTransfer.cpp @@ -90,6 +90,9 @@ getConvertBackContextHash( bool makeEcPair(Slice const& buffer, secp256k1_pubkey& out1, secp256k1_pubkey& out2) { + if (buffer.length() != 2 * ecGamalEncryptedLength) + return false; // LCOV_EXCL_LINE + auto parsePubKey = [](Slice const& slice, secp256k1_pubkey& out) { return secp256k1_ec_pubkey_parse( secp256k1Context(), &out, reinterpret_cast(slice.data()), slice.length()); @@ -275,10 +278,8 @@ verifyElGamalEncryption( Slice const& pubKeySlice, Slice const& ciphertext) { - if (blindingFactor.size() != ecBlindingFactorLength) - return tecINTERNAL; // LCOV_EXCL_LINE - - if (pubKeySlice.size() != ecPubKeyLength) + if (ciphertext.size() != ecGamalEncryptedTotalLength || blindingFactor.size() != ecBlindingFactorLength || + pubKeySlice.size() != ecPubKeyLength) return tecINTERNAL; // LCOV_EXCL_LINE secp256k1_pubkey pubKey; @@ -402,11 +403,12 @@ verifyClawbackEqualityProof( Slice const& ciphertext, uint256 const& contextHash) { - secp256k1_pubkey c1, c2; - if (!makeEcPair(ciphertext, c1, c2)) + if (ciphertext.size() != ecGamalEncryptedTotalLength || pubKeySlice.size() != ecPubKeyLength || + proof.size() != ecEqualityProofLength) return tecINTERNAL; // LCOV_EXCL_LINE - if (pubKeySlice.size() != ecPubKeyLength) + secp256k1_pubkey c1, c2; + if (!makeEcPair(ciphertext, c1, c2)) return tecINTERNAL; // LCOV_EXCL_LINE secp256k1_pubkey pubKey; @@ -537,9 +539,14 @@ verifyAggregatedBulletproof( std::vector const& compressedCommitments, uint256 const& contextHash) { - // 1. Validate Aggregation Factor (m), m to be a power of 2 + // 1. Validate input lengths + // This function could support any power-of-2 m, but current usage only requires m=1 or m=2 std::size_t const m = compressedCommitments.size(); - if (m == 0 || (m & (m - 1)) != 0) + if (m != 1 && m != 2) + return tecINTERNAL; // LCOV_EXCL_LINE + + std::size_t const expectedProofLen = (m == 1) ? ecSingleBulletproofLength : ecDoubleBulletproofLength; + if (proof.size() != expectedProofLen) return tecINTERNAL; // LCOV_EXCL_LINE // 2. Prepare Pedersen Commitments, parse from compressed format diff --git a/src/test/app/ConfidentialTransfer_test.cpp b/src/test/app/ConfidentialTransfer_test.cpp index e1bf792cf5..40096d01b7 100644 --- a/src/test/app/ConfidentialTransfer_test.cpp +++ b/src/test/app/ConfidentialTransfer_test.cpp @@ -2565,98 +2565,294 @@ class ConfidentialTransfer_test : public beast::unit_test::suite { testcase("Send deposit preauth"); using namespace test::jtx; - Env env(*this, features); - using namespace std::chrono; + // When an account enables lsfDepositAuth (via asfDepositAuth flag), + // it requires explicit authorization before accepting incoming payments. + // + // There are two authorization mechanisms: + // + // 1. DIRECT ACCOUNT AUTHORIZATION (deposit::auth) + // - Bob directly authorizes Carol: deposit::auth(bob, carol) + // - Simple 1-to-1 trust relationship + // - Carol can send to Bob without credentials + // + // 2. CREDENTIAL-BASED AUTHORIZATION (deposit::authCredentials) + // - A trusted third party (dpIssuer) issues credentials + // - Bob authorizes a credential TYPE from an issuer + // - Anyone holding that credential can send to Bob + // - Requires sender to include credential ID in transaction Account const alice("alice"); Account const bob("bob"); Account const carol("carol"); Account const dpIssuer("dpIssuer"); + char const credType[] = "KYC_VERIFIED"; - env.fund(XRP(50000), dpIssuer); - env.close(); - char const credType[] = "abcde"; - MPTTester mptAlice(env, alice, {.holders = {bob, carol}}); + // Common setup: create MPT with privacy, convert both carol and bob + auto setupMPT = [&](Env& env, MPTTester& mpt) { + mpt.create({.ownerCount = 1, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); + mpt.authorize({.account = bob}); + mpt.authorize({.account = carol}); + mpt.pay(alice, bob, 100); + mpt.pay(alice, carol, 100); - mptAlice.create({.ownerCount = 1, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); + mpt.generateKeyPair(alice); + mpt.generateKeyPair(bob); + mpt.generateKeyPair(carol); + mpt.set({.account = alice, .issuerPubKey = mpt.getPubKey(alice)}); - mptAlice.authorize({.account = bob}); - mptAlice.authorize({.account = carol}); + mpt.convert({.account = carol, .amt = 50, .holderPubKey = mpt.getPubKey(carol)}); + mpt.convert({.account = bob, .amt = 50, .holderPubKey = mpt.getPubKey(bob)}); + mpt.mergeInbox({.account = carol}); + mpt.mergeInbox({.account = bob}); - mptAlice.pay(alice, bob, 100); - mptAlice.pay(alice, carol, 50); + env(fset(bob, asfDepositAuth)); + env.close(); + }; - mptAlice.generateKeyPair(alice); - mptAlice.generateKeyPair(bob); - mptAlice.generateKeyPair(carol); + // Create and accept credential for an account + auto createCredential = [&](Env& env, Account const& subject) -> std::string { + env(credentials::create(subject, dpIssuer, credType)); + env.close(); + env(credentials::accept(subject, dpIssuer, credType)); + env.close(); + auto const jv = credentials::ledgerEntry(env, subject, dpIssuer, credType); + return jv[jss::result][jss::index].asString(); + }; - mptAlice.set({.account = alice, .issuerPubKey = mptAlice.getPubKey(alice)}); + // TEST 1: Direct Account Authorization + { + Env env(*this, features); + MPTTester mpt(env, alice, {.holders = {bob, carol}}); + setupMPT(env, mpt); - // Bob require preauthorization - env(fset(bob, asfDepositAuth)); - env.close(); + // Carol cannot send to Bob without authorization + mpt.send({.account = carol, .dest = bob, .amt = 10, .err = tecNO_PERMISSION}); - mptAlice.convert({ - .account = carol, - .amt = 50, - .holderPubKey = mptAlice.getPubKey(carol), - }); - mptAlice.convert({ - .account = bob, - .amt = 50, - .holderPubKey = mptAlice.getPubKey(bob), - }); + // Bob directly authorizes Carol + env(deposit::auth(bob, carol)); + env.close(); - // carol merge inbox - mptAlice.mergeInbox({ - .account = carol, - }); + // Now Carol can send to Bob + mpt.send({.account = carol, .dest = bob, .amt = 10}); + mpt.mergeInbox({.account = bob}); - // bob merge inbox - mptAlice.mergeInbox({ - .account = bob, - }); + // Bob revokes Carol's authorization + env(deposit::unauth(bob, carol)); + env.close(); - // carol sends 10 to bob, but not authorized - mptAlice.send({.account = carol, .dest = bob, .amt = 10, .err = tecNO_PERMISSION}); + // Carol can no longer send to Bob + mpt.send({.account = carol, .dest = bob, .amt = 10, .err = tecNO_PERMISSION}); + } - // Bob authorize alice - env(deposit::auth(bob, carol)); - env.close(); + // TEST 2: Credential-Based Authorization + { + Env env(*this, features); + env.fund(XRP(50000), dpIssuer); + env.close(); - mptAlice.send({.account = carol, .dest = bob, .amt = 10}); + MPTTester mpt(env, alice, {.holders = {bob, carol}}); + setupMPT(env, mpt); - // Create credentials - env(credentials::create(bob, dpIssuer, credType)); - env.close(); - env(credentials::accept(bob, dpIssuer, credType)); - env.close(); - auto const jv = credentials::ledgerEntry(env, bob, dpIssuer, credType); - std::string const credIdx = jv[jss::result][jss::index].asString(); + auto const credIdx = createCredential(env, carol); - mptAlice.send({.account = carol, .dest = bob, .amt = 10, .credentials = {{credIdx}}}); + // Carol cannot send yet - Bob hasn't authorized this credential type + mpt.send({.account = carol, .dest = bob, .amt = 10, .credentials = {{credIdx}}, .err = tecNO_PERMISSION}); - // Bob revoke authorization - env(deposit::unauth(bob, carol)); - env.close(); + // Bob authorizes the credential type from dpIssuer + env(deposit::authCredentials(bob, {{dpIssuer, credType}})); + env.close(); - mptAlice.send({.account = carol, .dest = bob, .amt = 10, .err = tecNO_PERMISSION}); + // Carol still cannot send without including credential + mpt.send({.account = carol, .dest = bob, .amt = 10, .err = tecNO_PERMISSION}); - mptAlice.send({.account = carol, .dest = bob, .amt = 10, .credentials = {{credIdx}}, .err = tecNO_PERMISSION}); + // Carol CAN send when including her credential + mpt.send({.account = carol, .dest = bob, .amt = 10, .credentials = {{credIdx}}}); + mpt.mergeInbox({.account = bob}); + } - // Bob authorize credentials - env(deposit::authCredentials(bob, {{dpIssuer, credType}})); - env.close(); + // TEST 3: Direct Auth Takes Precedence Over Credentials + { + Env env(*this, features); + env.fund(XRP(50000), dpIssuer); + env.close(); - mptAlice.send({.account = carol, .dest = bob, .amt = 10, .err = tecNO_PERMISSION}); + MPTTester mpt(env, alice, {.holders = {bob, carol}}); + setupMPT(env, mpt); - mptAlice.send({ - .account = carol, - .dest = bob, - .amt = 10, - .credentials = {{credIdx}}, - }); + auto const credIdx = createCredential(env, carol); + + // Bob directly authorizes Carol (no credential needed) + env(deposit::auth(bob, carol)); + env.close(); + + // Carol can send without credentials (direct auth) + mpt.send({.account = carol, .dest = bob, .amt = 10}); + mpt.mergeInbox({.account = bob}); + + // Carol can also send WITH credentials (still works) + mpt.send({.account = carol, .dest = bob, .amt = 10, .credentials = {{credIdx}}}); + mpt.mergeInbox({.account = bob}); + + // Bob revokes direct authorization + env(deposit::unauth(bob, carol)); + env.close(); + + // Carol cannot send without credentials anymore + mpt.send({.account = carol, .dest = bob, .amt = 10, .err = tecNO_PERMISSION}); + + // But credential-based auth not set up, so this also fails + mpt.send({.account = carol, .dest = bob, .amt = 10, .credentials = {{credIdx}}, .err = tecNO_PERMISSION}); + + // Bob authorizes the credential type + env(deposit::authCredentials(bob, {{dpIssuer, credType}})); + env.close(); + + // Now Carol can send with credentials + mpt.send({.account = carol, .dest = bob, .amt = 10, .credentials = {{credIdx}}}); + } + } + + void + testSendCredentialValidation(FeatureBitset features) + { + testcase("Send credential validation"); + using namespace test::jtx; + + // Tests for credentials::checkFields (preflight) and + // credentials::valid (preclaim) validation. + // + // Preflight checks (temMALFORMED): + // - Empty credentials array + // - Array size exceeds maxCredentialsArraySize (8) + // - Duplicate credential IDs in array + // + // Preclaim checks (tecBAD_CREDENTIALS): + // - Credential doesn't exist + // - Credential doesn't belong to source account + // - Credential not accepted (lsfAccepted flag not set) + + Account const alice("alice"); + Account const bob("bob"); + Account const carol("carol"); + Account const dpIssuer("dpIssuer"); + char const credType[] = "KYC"; + + // Common setup: create MPT with privacy, convert carol and bob to confidential + auto setupBasic = [&](Env& env, MPTTester& mpt) { + mpt.create({.ownerCount = 1, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); + mpt.authorize({.account = bob}); + mpt.authorize({.account = carol}); + mpt.pay(alice, bob, 100); + mpt.pay(alice, carol, 100); + + mpt.generateKeyPair(alice); + mpt.generateKeyPair(bob); + mpt.generateKeyPair(carol); + mpt.set({.account = alice, .issuerPubKey = mpt.getPubKey(alice)}); + + mpt.convert({.account = carol, .amt = 50, .holderPubKey = mpt.getPubKey(carol)}); + mpt.convert({.account = bob, .amt = 50, .holderPubKey = mpt.getPubKey(bob)}); + mpt.mergeInbox({.account = carol}); + mpt.mergeInbox({.account = bob}); + }; + + // TEST 1: Preflight - Empty Credentials Array + { + Env env(*this, features); + MPTTester mpt(env, alice, {.holders = {bob, carol}}); + setupBasic(env, mpt); + + mpt.send( + {.account = carol, + .dest = bob, + .amt = 10, + .credentials = std::vector{}, + .err = temMALFORMED}); + } + + // TEST 2: Preflight - Credentials Array Too Large + { + Env env(*this, features); + MPTTester mpt(env, alice, {.holders = {bob, carol}}); + setupBasic(env, mpt); + + std::vector tooManyCredentials; + for (int i = 0; i < 9; ++i) + tooManyCredentials.push_back(to_string(uint256(i))); + + mpt.send( + {.account = carol, .dest = bob, .amt = 10, .credentials = tooManyCredentials, .err = temMALFORMED}); + } + + // TEST 3: Preflight - Duplicate Credentials + { + Env env(*this, features); + env.fund(XRP(50000), dpIssuer); + env.close(); + MPTTester mpt(env, alice, {.holders = {bob, carol}}); + setupBasic(env, mpt); + + env(credentials::create(carol, dpIssuer, credType)); + env.close(); + env(credentials::accept(carol, dpIssuer, credType)); + env.close(); + + auto const jv = credentials::ledgerEntry(env, carol, dpIssuer, credType); + std::string const credIdx = jv[jss::result][jss::index].asString(); + + mpt.send( + {.account = carol, .dest = bob, .amt = 10, .credentials = {{credIdx, credIdx}}, .err = temMALFORMED}); + } + + // TEST 4: Preclaim - Credential Doesn't Exist + { + Env env(*this, features); + MPTTester mpt(env, alice, {.holders = {bob, carol}}); + setupBasic(env, mpt); + + std::string const fakeCredIdx = to_string(uint256(999)); + mpt.send( + {.account = carol, .dest = bob, .amt = 10, .credentials = {{fakeCredIdx}}, .err = tecBAD_CREDENTIALS}); + } + + // TEST 5: Preclaim - Credential Doesn't Belong to Source Account + { + Env env(*this, features); + env.fund(XRP(50000), dpIssuer); + env.close(); + MPTTester mpt(env, alice, {.holders = {bob, carol}}); + setupBasic(env, mpt); + + // Create credential for BOB (not carol) + env(credentials::create(bob, dpIssuer, credType)); + env.close(); + env(credentials::accept(bob, dpIssuer, credType)); + env.close(); + + auto const jv = credentials::ledgerEntry(env, bob, dpIssuer, credType); + std::string const credIdx = jv[jss::result][jss::index].asString(); + + mpt.send({.account = carol, .dest = bob, .amt = 10, .credentials = {{credIdx}}, .err = tecBAD_CREDENTIALS}); + } + + // TEST 6: Preclaim - Credential Not Accepted + { + Env env(*this, features); + env.fund(XRP(50000), dpIssuer); + env.close(); + MPTTester mpt(env, alice, {.holders = {bob, carol}}); + setupBasic(env, mpt); + + // Create credential but DON'T accept it + env(credentials::create(carol, dpIssuer, credType)); + env.close(); + + auto const jv = credentials::ledgerEntry(env, carol, dpIssuer, credType); + std::string const credIdx = jv[jss::result][jss::index].asString(); + + mpt.send({.account = carol, .dest = bob, .amt = 10, .credentials = {{credIdx}}, .err = tecBAD_CREDENTIALS}); + } } void @@ -4092,6 +4288,7 @@ class ConfidentialTransfer_test : public beast::unit_test::suite testSendPreclaim(features); testSendRangeProof(features); testSendDepositPreauth(features); + testSendCredentialValidation(features); testSendWithAuditor(features); // ConfidentialMPTClawback diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index 29f1cb7179..f1d573471d 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -1197,6 +1197,10 @@ MPTTester::send(MPTConfidentialSend const& arg) arr.append(hash); } + // Version counters before send + auto const prevSenderVersion = getMPTokenVersion(*arg.account); + auto const prevDestVersion = getMPTokenVersion(*arg.dest); + // Sender's previous confidential state auto const prevSenderInbox = getDecryptedBalance(*arg.account, HOLDER_ENCRYPTED_INBOX); auto const prevSenderSpending = getDecryptedBalance(*arg.account, HOLDER_ENCRYPTED_SPENDING); @@ -1372,6 +1376,10 @@ MPTTester::send(MPTConfidentialSend const& arg) env_.require(requireAny([&]() -> bool { return *postSenderInbox + *postSenderSpending == *postSenderIssuer; })); env_.require(requireAny([&]() -> bool { return *postDestInbox + *postDestSpending == *postDestIssuer; })); + // Version: sender increments by 1; receiver version is unchanged by incoming sends + env_.require(requireAny([&]() -> bool { return getMPTokenVersion(*arg.account) == prevSenderVersion + 1; })); + env_.require(requireAny([&]() -> bool { return getMPTokenVersion(*arg.dest) == prevDestVersion; })); + if (arg.auditorEncryptedAmt || auditor_) { auto const postSenderAuditor = getDecryptedBalance(*arg.account, AUDITOR_ENCRYPTED_BALANCE); @@ -1439,11 +1447,13 @@ MPTTester::confidentialClaw(MPTConfidentialClawback const& arg) auto const holderPubAmt = getBalance(*arg.holder); auto const prevCOA = getIssuanceConfidentialBalance(); auto const prevOA = getIssuanceOutstandingBalance(); + auto const prevVersion = getMPTokenVersion(*arg.holder); if (submit(arg, jv) == tesSUCCESS) { auto const postCOA = getIssuanceConfidentialBalance(); auto const postOA = getIssuanceOutstandingBalance(); + auto const postVersion = getMPTokenVersion(*arg.holder); // Verify holder's public balance is unchanged env_.require(mptbalance(*this, *arg.holder, holderPubAmt)); @@ -1461,6 +1471,9 @@ MPTTester::confidentialClaw(MPTConfidentialClawback const& arg) requireAny([&]() -> bool { return getDecryptedBalance(*arg.holder, ISSUER_ENCRYPTED_BALANCE) == 0; })); env_.require( requireAny([&]() -> bool { return getDecryptedBalance(*arg.holder, AUDITOR_ENCRYPTED_BALANCE) == 0; })); + + // Verify version is incremented + env_.require(requireAny([&]() -> bool { return postVersion == prevVersion + 1; })); } } diff --git a/src/xrpld/app/tx/detail/ConfidentialMPTClawback.cpp b/src/xrpld/app/tx/detail/ConfidentialMPTClawback.cpp index 2dc737a066..f8dac3913e 100644 --- a/src/xrpld/app/tx/detail/ConfidentialMPTClawback.cpp +++ b/src/xrpld/app/tx/detail/ConfidentialMPTClawback.cpp @@ -125,7 +125,7 @@ ConfidentialMPTClawback::doApply() (*sleHolderMPToken)[sfConfidentialBalanceInbox] = *encZeroForHolder; (*sleHolderMPToken)[sfConfidentialBalanceSpending] = *encZeroForHolder; (*sleHolderMPToken)[sfIssuerEncryptedBalance] = *encZeroForIssuer; - (*sleHolderMPToken)[sfConfidentialBalanceVersion] = 0; + incrementConfidentialVersion(*sleHolderMPToken); if (sleHolderMPToken->isFieldPresent(sfAuditorEncryptedBalance)) { diff --git a/src/xrpld/app/tx/detail/ConfidentialMPTSend.cpp b/src/xrpld/app/tx/detail/ConfidentialMPTSend.cpp index ef09d2081e..ba817b0575 100644 --- a/src/xrpld/app/tx/detail/ConfidentialMPTSend.cpp +++ b/src/xrpld/app/tx/detail/ConfidentialMPTSend.cpp @@ -60,6 +60,9 @@ ConfidentialMPTSend::preflight(PreflightContext const& ctx) if (hasAuditor && !isValidCiphertext(ctx.tx[sfAuditorEncryptedAmount])) return temBAD_CIPHERTEXT; + if (auto const err = credentials::checkFields(ctx.tx, ctx.j); !isTesSuccess(err)) + return err; + return tesSUCCESS; } @@ -291,6 +294,9 @@ ConfidentialMPTSend::preclaim(PreclaimContext const& ctx) if (auto const ter = requireAuth(ctx.view, mptIssue, destination); !isTesSuccess(ter)) return ter; + if (auto const err = credentials::valid(ctx.tx, ctx.view, ctx.tx[sfAccount], ctx.j); !isTesSuccess(err)) + return err; + return verifySendProofs(ctx, sleSenderMPToken, sleDestinationMPToken, sleIssuance); } @@ -386,9 +392,8 @@ ConfidentialMPTSend::doApply() (*sleDestinationMPToken)[sfAuditorEncryptedBalance] = newAuditorEnc; } - // increment version + // increment sender version only; receiver version is not modified by incoming sends incrementConfidentialVersion(*sleSenderMPToken); - incrementConfidentialVersion(*sleDestinationMPToken); view().update(sleSenderMPToken); view().update(sleDestinationMPToken);