ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-04-29 15:37:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add support for extra transaction signature validation

Browse Source 
- Restructures `STTx` signature checking code to be able to handle
  a `sigObject`, which may be the full transaction, or may be an object
  field containing a separate signature. Either way, the `sigObject` can
  be a single- or multi-sign signature.
- This is distinct from 550f90a75e (#5594), which changed the check in
  Transactor, which validates whether a given account is allowed to sign
  for the given transaction. This cryptographically checks the signature
  validity.
This commit is contained in:
Ed Hennis
2025-10-03 14:56:39 -04:00
parent 5d79bfc531
commit 23045fcbef
2 changed files with 129 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
include/xrpl/protocol/STTx.h
View File

@@ -88,7 +88,13 @@ public:
// Outer transaction functions / signature functions.
Blob
getSignature() const;
getSignature(STObject const& sigObject) const;
Blob
getSignature() const
{
return getSignature(*this);
}
uint256
getSigningHash() const;
@@ -119,13 +125,34 @@ public:
getJson(JsonOptions options, bool binary) const;
void
sign(PublicKey const& publicKey, SecretKey const& secretKey);
sign(
PublicKey const& publicKey,
SecretKey const& secretKey,
std::optional<std::reference_wrapper<SField const>> signatureTarget =
{});
/** Check the signature.
@return `true` if valid signature. If invalid, the error message string.
*/
enum class RequireFullyCanonicalSig : bool { no, yes };
/** Check the signature.
@param requireCanonicalSig If `true`, check that the signature is fully
canonical. If `false`, only check that the signature is valid.
@param rules The current ledger rules.
@param pSig Pointer to object that contains the signature fields, if not
using "this". Will most often be null
@return `true` if valid signature. If invalid, the error message string.
*/
Expected<void, std::string>
checkSign(
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules,
STObject const* pSig) const;
/** Check the signature.
@param requireCanonicalSig If `true`, check that the signature is fully
canonical. If `false`, only check that the signature is valid.
@param rules The current ledger rules.
@return `true` if valid signature. If invalid, the error message string.
*/
Expected<void, std::string>
checkSign(RequireFullyCanonicalSig requireCanonicalSig, Rules const& rules)
const;
@@ -155,12 +182,15 @@ public:
private:
Expected<void, std::string>
checkSingleSign(RequireFullyCanonicalSig requireCanonicalSig) const;
checkSingleSign(
RequireFullyCanonicalSig requireCanonicalSig,
STObject const* pSig) const;
Expected<void, std::string>
checkMultiSign(
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules) const;
Rules const& rules,
STObject const* pSig) const;
Expected<void, std::string>
checkBatchSingleSign(

134
src/libxrpl/protocol/STTx.cpp
View File

@@ -200,11 +200,11 @@ STTx::getSigningHash() const
}
Blob
STTx::getSignature() const
STTx::getSignature(STObject const& sigObject) const
{
try
{
return getFieldVL(sfTxnSignature);
return sigObject.getFieldVL(sfTxnSignature);
}
catch (std::exception const&)
{
@@ -234,35 +234,69 @@ STTx::getSeqValue() const
}
void
STTx::sign(PublicKey const& publicKey, SecretKey const& secretKey)
STTx::sign(
PublicKey const& publicKey,
SecretKey const& secretKey,
std::optional<std::reference_wrapper<SField const>> signatureTarget)
{
auto const data = getSigningData(*this);
auto const sig = ripple::sign(publicKey, secretKey, makeSlice(data));
setFieldVL(sfTxnSignature, sig);
if (signatureTarget)
{
auto& target = peekFieldObject(*signatureTarget);
target.setFieldVL(sfTxnSignature, sig);
}
else
{
setFieldVL(sfTxnSignature, sig);
}
tid_ = getHash(HashPrefix::transactionID);
}
Expected<void, std::string>
STTx::checkSign(
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules,
STObject const* pSig) const
{
try
{
// Determine whether we're single- or multi-signing by looking
// at the SigningPubKey. If it's empty we must be
// multi-signing. Otherwise we're single-signing.
STObject const& sigObject{pSig ? *pSig : *this};
Blob const& signingPubKey = sigObject.getFieldVL(sfSigningPubKey);
return signingPubKey.empty()
? checkMultiSign(requireCanonicalSig, rules, pSig)
: checkSingleSign(requireCanonicalSig, pSig);
}
catch (std::exception const&)
{
}
return Unexpected("Internal signature check failure.");
}
Expected<void, std::string>
STTx::checkSign(
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules) const
{
try
if (auto const ret = checkSign(requireCanonicalSig, rules, nullptr); !ret)
return ret;
/* Placeholder for field that will be added by Lending Protocol
if (isFieldPresent(sfCounterpartySignature))
{
// Determine whether we're single- or multi-signing by looking
// at the SigningPubKey. If it's empty we must be
// multi-signing. Otherwise we're single-signing.
Blob const& signingPubKey = getFieldVL(sfSigningPubKey);
return signingPubKey.empty()
? checkMultiSign(requireCanonicalSig, rules)
: checkSingleSign(requireCanonicalSig);
auto const counterSig = getFieldObject(sfCounterpartySignature);
if (auto const ret = checkSign(requireCanonicalSig, rules, &counterSig);
!ret)
return Unexpected("Counterparty: " + ret.error());
}
catch (std::exception const&)
{
}
return Unexpected("Internal signature check failure.");
*/
return {};
}
Expected<void, std::string>
@@ -382,23 +416,23 @@ STTx::getMetaSQL(
static Expected<void, std::string>
singleSignHelper(
STObject const& signer,
STObject const& sigObject,
Slice const& data,
bool const fullyCanonical)
{
// We don't allow both a non-empty sfSigningPubKey and an sfSigners.
// That would allow the transaction to be signed two ways. So if both
// fields are present the signature is invalid.
if (signer.isFieldPresent(sfSigners))
if (sigObject.isFieldPresent(sfSigners))
return Unexpected("Cannot both single- and multi-sign.");
bool validSig = false;
try
{
auto const spk = signer.getFieldVL(sfSigningPubKey);
auto const spk = sigObject.getFieldVL(sfSigningPubKey);
if (publicKeyType(makeSlice(spk)))
{
Blob const signature = signer.getFieldVL(sfTxnSignature);
Blob const signature = sigObject.getFieldVL(sfTxnSignature);
validSig = verify(
PublicKey(makeSlice(spk)),
data,
@@ -418,12 +452,15 @@ singleSignHelper(
}
Expected<void, std::string>
STTx::checkSingleSign(RequireFullyCanonicalSig requireCanonicalSig) const
STTx::checkSingleSign(
RequireFullyCanonicalSig requireCanonicalSig,
STObject const* pSig) const
{
STObject const& sigObject{pSig ? *pSig : *this};
auto const data = getSigningData(*this);
bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) ||
(requireCanonicalSig == STTx::RequireFullyCanonicalSig::yes);
return singleSignHelper(*this, makeSlice(data), fullyCanonical);
return singleSignHelper(sigObject, makeSlice(data), fullyCanonical);
}
Expected<void, std::string>
@@ -440,31 +477,29 @@ STTx::checkBatchSingleSign(
Expected<void, std::string>
multiSignHelper(
STObject const& signerObj,
STObject const& sigObject,
std::optional<AccountID> txnAccountID,
bool const fullyCanonical,
std::function<Serializer(AccountID const&)> makeMsg,
Rules const& rules)
{
// Make sure the MultiSigners are present. Otherwise they are not
// attempting multi-signing and we just have a bad SigningPubKey.
if (!signerObj.isFieldPresent(sfSigners))
if (!sigObject.isFieldPresent(sfSigners))
return Unexpected("Empty SigningPubKey.");
// We don't allow both an sfSigners and an sfTxnSignature. Both fields
// being present would indicate that the transaction is signed both ways.
if (signerObj.isFieldPresent(sfTxnSignature))
if (sigObject.isFieldPresent(sfTxnSignature))
return Unexpected("Cannot both single- and multi-sign.");
STArray const& signers{signerObj.getFieldArray(sfSigners)};
STArray const& signers{sigObject.getFieldArray(sfSigners)};
// There are well known bounds that the number of signers must be within.
if (signers.size() < STTx::minMultiSigners ||
signers.size() > STTx::maxMultiSigners(&rules))
return Unexpected("Invalid Signers array size.");
// We also use the sfAccount field inside the loop. Get it once.
auto const txnAccountID = signerObj.getAccountID(sfAccount);
// Signers must be in sorted order by AccountID.
AccountID lastAccountID(beast::zero);
@@ -472,8 +507,10 @@ multiSignHelper(
{
auto const accountID = signer.getAccountID(sfAccount);
// The account owner may not multisign for themselves.
if (accountID == txnAccountID)
// The account owner may not usually multisign for themselves.
// If they can, txnAccountID will be unseated, which is not equal to any
// value.
if (txnAccountID == accountID)
return Unexpected("Invalid multisigner.");
// No duplicate signers allowed.
@@ -489,6 +526,7 @@ multiSignHelper(
// Verify the signature.
bool validSig = false;
std::optional<std::string> errorWhat;
try
{
auto spk = signer.getFieldVL(sfSigningPubKey);
@@ -502,15 +540,16 @@ multiSignHelper(
fullyCanonical);
}
}
catch (std::exception const&)
catch (std::exception const& e)
{
// We assume any problem lies with the signature.
validSig = false;
errorWhat = e.what();
}
if (!validSig)
return Unexpected(
std::string("Invalid signature on account ") +
toBase58(accountID) + ".");
toBase58(accountID) + errorWhat.value_or("") + ".");
}
// All signatures verified.
return {};
@@ -532,8 +571,9 @@ STTx::checkBatchMultiSign(
serializeBatch(dataStart, getFlags(), getBatchTransactionIDs());
return multiSignHelper(
batchSigner,
std::nullopt,
fullyCanonical,
[&dataStart](AccountID const& accountID) mutable -> Serializer {
[&dataStart](AccountID const& accountID) -> Serializer {
Serializer s = dataStart;
finishMultiSigningData(accountID, s);
return s;
@@ -544,19 +584,28 @@ STTx::checkBatchMultiSign(
Expected<void, std::string>
STTx::checkMultiSign(
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules) const
Rules const& rules,
STObject const* pSig) const
{
STObject const& sigObject{pSig ? *pSig : *this};
bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) ||
(requireCanonicalSig == RequireFullyCanonicalSig::yes);
// Used inside the loop in multiSignHelper to enforce that
// the account owner may not multisign for themselves.
auto const txnAccountID =
pSig ? std::nullopt : std::optional<AccountID>(getAccountID(sfAccount));
// We can ease the computational load inside the loop a bit by
// pre-constructing part of the data that we hash. Fill a Serializer
// with the stuff that stays constant from signature to signature.
Serializer dataStart = startMultiSigningData(*this);
return multiSignHelper(
*this,
sigObject,
txnAccountID,
fullyCanonical,
[&dataStart](AccountID const& accountID) mutable -> Serializer {
[&dataStart](AccountID const& accountID) -> Serializer {
Serializer s = dataStart;
finishMultiSigningData(accountID, s);
return s;
@@ -588,11 +637,12 @@ STTx::getBatchTransactionIDs() const
XRPL_ASSERT(
getFieldArray(sfRawTransactions).size() != 0,
"STTx::getBatchTransactionIDs : empty raw transactions");
if (batch_txn_ids_.size() != 0)
return batch_txn_ids_;
for (STObject const& rb : getFieldArray(sfRawTransactions))
batch_txn_ids_.push_back(rb.getHash(HashPrefix::transactionID));
// Don't early return so that the size assert is always hit.
if (batch_txn_ids_.size() == 0)
{
for (STObject const& rb : getFieldArray(sfRawTransactions))
batch_txn_ids_.push_back(rb.getHash(HashPrefix::transactionID));
}
XRPL_ASSERT(
batch_txn_ids_.size() == getFieldArray(sfRawTransactions).size(),