Skip to content

Commit

Permalink
[FOLD] Add support for featureCofactoredED25519 amendment:
Browse files Browse the repository at this point in the history
Transaction-related Ed25519 signatures are verified using
cofactored verification once the amendment goes live.

Those Ed25519 signatures that are not transaction related
are immediately switched to using cofactored verification,
since that is the conservative thing to do.  Such items
include

- validator lists and
- validator keys.
  • Loading branch information
scottschurr committed Feb 3, 2024
1 parent e9af179 commit 3fe47bb
Show file tree
Hide file tree
Showing 26 changed files with 227 additions and 155 deletions.
66 changes: 0 additions & 66 deletions conan_debug

This file was deleted.

5 changes: 4 additions & 1 deletion src/ripple/app/consensus/RCLCxPeerPos.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ bool
RCLCxPeerPos::checkSign() const
{
return verifyDigest(
publicKey(), proposal_.signingHash(), signature(), false);
publicKey(),
proposal_.signingHash(),
signature(),
RequireFullyCanonicalSig::no);
}

Json::Value
Expand Down
9 changes: 7 additions & 2 deletions src/ripple/app/misc/impl/Manifest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,16 @@ Manifest::verify() const

// Signing key and signature are not required for
// master key revocations
if (!revoked() && !ripple::verify(st, HashPrefix::manifest, signingKey))
if (!revoked() &&
!ripple::verify(st, HashPrefix::manifest, signingKey, Cofactored::yes))
return false;

return ripple::verify(
st, HashPrefix::manifest, masterKey, sfMasterSignature);
st,
HashPrefix::manifest,
masterKey,
Cofactored::yes,
sfMasterSignature);
}

uint256
Expand Down
5 changes: 3 additions & 2 deletions src/ripple/app/misc/impl/ValidatorList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -924,7 +924,7 @@ ValidatorList::applyLists(
std::uint32_t version,
std::vector<ValidatorBlobInfo> const& blobs,
std::string siteUri,
std::optional<uint256> const& hash /* = {} */)
std::optional<uint256> const& hash)
{
if (std::count(
std::begin(supportedListVersions),
Expand Down Expand Up @@ -1290,7 +1290,8 @@ ValidatorList::verify(
!ripple::verify(
publisherManifests_.getSigningKey(pubKey),
makeSlice(data),
makeSlice(*sig)))
makeSlice(*sig),
Cofactored::yes))
return ListDisposition::invalid;

Json::Reader r;
Expand Down
7 changes: 6 additions & 1 deletion src/ripple/app/tx/impl/PayChan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,12 @@ PayChanClaim::preflight(PreflightContext const& ctx)
PublicKey const pk(ctx.tx[sfPublicKey]);
Serializer msg;
serializePayChanAuthorization(msg, k.key, authAmt);
if (!verify(pk, msg.slice(), *sig, /*canonical*/ true))
if (!verify(
pk,
msg.slice(),
*sig,
cofactoredRules(ctx.rules),
RequireFullyCanonicalSig::yes))
return temBAD_SIGNATURE;
}

Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/tx/impl/XChainBridge.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1219,7 +1219,7 @@ attestationPreflight(PreflightContext const& ctx)
return temMALFORMED;

STXChainBridge const bridgeSpec = ctx.tx[sfXChainBridge];
if (!att->verify(bridgeSpec))
if (!att->verify(bridgeSpec, ctx.rules))
return temXCHAIN_BAD_PROOF;
if (!att->validAmounts())
return temXCHAIN_BAD_PROOF;
Expand Down
4 changes: 2 additions & 2 deletions src/ripple/app/tx/impl/apply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ checkValidity(
// Don't know signature state. Check it.
auto const requireCanonicalSig =
rules.enabled(featureRequireFullyCanonicalSig)
? STTx::RequireFullyCanonicalSig::yes
: STTx::RequireFullyCanonicalSig::no;
? RequireFullyCanonicalSig::yes
: RequireFullyCanonicalSig::no;

auto const sigVerify = tx.checkSign(requireCanonicalSig, rules);
if (!sigVerify)
Expand Down
6 changes: 5 additions & 1 deletion src/ripple/overlay/impl/Handshake.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,11 @@ verifyHandshake(

auto sig = base64_decode(iter->value());

if (!verifyDigest(publicKey, sharedValue, makeSlice(sig), false))
if (!verifyDigest(
publicKey,
sharedValue,
makeSlice(sig),
RequireFullyCanonicalSig::no))
throw std::runtime_error("Failed to verify session");
}

Expand Down
7 changes: 6 additions & 1 deletion src/ripple/overlay/impl/PeerImp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1393,7 +1393,12 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMPeerShardInfoV2> const& m)
return badData("Invalid public key");

// Verify signature
if (!verify(publicKey, s.slice(), makeSlice(m->signature()), false))
if (!verify(
publicKey,
s.slice(),
makeSlice(m->signature()),
Cofactored::yes,
RequireFullyCanonicalSig::no))
return badData("Invalid signature");

// Forward the message if a peer chain exists
Expand Down
3 changes: 2 additions & 1 deletion src/ripple/protocol/Feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ namespace detail {
// Feature.cpp. Because it's only used to reserve storage, and determine how
// large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
// the actual number of amendments. A LogicError on startup will verify this.
static constexpr std::size_t numFeatures = 65;
static constexpr std::size_t numFeatures = 66;

/** Amendments that this server supports and the default voting behavior.
Whether they are enabled depends on the Rules defined in the validated
Expand Down Expand Up @@ -352,6 +352,7 @@ extern uint256 const featureXChainBridge;
extern uint256 const fixDisallowIncomingV1;
extern uint256 const featureDID;
extern uint256 const fixFillOrKill;
extern uint256 const featureCofactoredED25519;

} // namespace ripple

Expand Down
16 changes: 14 additions & 2 deletions src/ripple/protocol/PublicKey.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include <ripple/basics/Slice.h>
#include <ripple/protocol/KeyType.h>
#include <ripple/protocol/Rules.h>
#include <ripple/protocol/STExchange.h>
#include <ripple/protocol/UintTypes.h>
#include <ripple/protocol/json_get_or_throw.h>
Expand Down Expand Up @@ -241,12 +242,21 @@ publicKeyType(PublicKey const& publicKey)
/** @} */

/** Verify a secp256k1 signature on the digest of a message. */
enum class RequireFullyCanonicalSig : bool { no, yes };
[[nodiscard]] bool
verifyDigest(
PublicKey const& publicKey,
uint256 const& digest,
Slice const& sig,
bool mustBeFullyCanonical = true) noexcept;
RequireFullyCanonicalSig canonicalSig =
RequireFullyCanonicalSig::yes) noexcept;

/** Given the Rules, return whether signature verification should be Cofactored
*/
enum class Cofactored : bool { no, yes };

[[nodiscard]] Cofactored
cofactoredRules(Rules const& rules);

/** Verify a signature on a message.
With secp256k1 signatures, the data is first hashed with
Expand All @@ -257,7 +267,9 @@ verify(
PublicKey const& publicKey,
Slice const& m,
Slice const& sig,
bool mustBeFullyCanonical = true) noexcept;
Cofactored cofactored,
RequireFullyCanonicalSig canonicalSig =
RequireFullyCanonicalSig::yes) noexcept;

/** Calculate the 160-bit node ID from a node public key. */
NodeID
Expand Down
12 changes: 5 additions & 7 deletions src/ripple/protocol/STTx.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,8 @@ class STTx final : public STObject, public CountedObject<STTx>
/** Check the signature.
@return `true` if valid signature. If invalid, the error message string.
*/
enum class RequireFullyCanonicalSig : bool { no, yes };
Expected<void, std::string>
checkSign(RequireFullyCanonicalSig requireCanonicalSig, Rules const& rules)
const;
checkSign(RequireFullyCanonicalSig canonicalSig, Rules const& rules) const;

// SQL Functions with metadata.
static std::string const&
Expand All @@ -141,12 +139,12 @@ class STTx final : public STObject, public CountedObject<STTx>

private:
Expected<void, std::string>
checkSingleSign(RequireFullyCanonicalSig requireCanonicalSig) const;
checkSingleSign(RequireFullyCanonicalSig canonicalSig, Rules const& rules)
const;

Expected<void, std::string>
checkMultiSign(
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules) const;
checkMultiSign(RequireFullyCanonicalSig canonicalSig, Rules const& rules)
const;

STBase*
copy(std::size_t n, void* buf) const override;
Expand Down
2 changes: 2 additions & 0 deletions src/ripple/protocol/Sign.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include <ripple/protocol/HashPrefix.h>
#include <ripple/protocol/PublicKey.h>
#include <ripple/protocol/Rules.h>
#include <ripple/protocol/STObject.h>
#include <ripple/protocol/SecretKey.h>
#include <utility>
Expand Down Expand Up @@ -60,6 +61,7 @@ verify(
STObject const& st,
HashPrefix const& prefix,
PublicKey const& pk,
Cofactored cofactored,
SF_VL const& sigField = sfSignature);

/** Return a Serializer suitable for computing a multisigning TxnSignature. */
Expand Down
3 changes: 2 additions & 1 deletion src/ripple/protocol/XChainAttestations.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <ripple/protocol/AccountID.h>
#include <ripple/protocol/Issue.h>
#include <ripple/protocol/PublicKey.h>
#include <ripple/protocol/Rules.h>
#include <ripple/protocol/SField.h>
#include <ripple/protocol/STBase.h>
#include <ripple/protocol/STXChainBridge.h>
Expand Down Expand Up @@ -80,7 +81,7 @@ struct AttestationBase

// verify that the signature attests to the data.
bool
verify(STXChainBridge const& bridge) const;
verify(STXChainBridge const& bridge, Rules const& rules) const;

protected:
explicit AttestationBase(STObject const& o);
Expand Down
3 changes: 2 additions & 1 deletion src/ripple/protocol/impl/Feature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,8 @@ REGISTER_FEATURE(AMM, Supported::yes, VoteBehavior::De
REGISTER_FEATURE(XChainBridge, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FIX (fixDisallowIncomingV1, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FEATURE(DID, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FIX(fixFillOrKill, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FIX (fixFillOrKill, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FEATURE(CofactoredED25519, Supported::yes, VoteBehavior::DefaultNo);

// The following amendments are obsolete, but must remain supported
// because they could potentially get enabled.
Expand Down
37 changes: 29 additions & 8 deletions src/ripple/protocol/impl/PublicKey.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,16 @@
*/
//==============================================================================

#include <ripple/protocol/PublicKey.h>

#include <ripple/basics/contract.h>
#include <ripple/basics/strHex.h>
#include <ripple/protocol/PublicKey.h>
#include <ripple/protocol/Feature.h>
#include <ripple/protocol/digest.h>
#include <ripple/protocol/impl/secp256k1.h>

#include <boost/multiprecision/cpp_int.hpp>

#include <ed25519.h>
#include <type_traits>

Expand Down Expand Up @@ -223,14 +227,14 @@ verifyDigest(
PublicKey const& publicKey,
uint256 const& digest,
Slice const& sig,
bool mustBeFullyCanonical) noexcept
RequireFullyCanonicalSig canonicalSig) noexcept
{
if (publicKeyType(publicKey) != KeyType::secp256k1)
LogicError("sign: secp256k1 required for digest signing");
auto const canonicality = ecdsaCanonicality(sig);
if (!canonicality)
return false;
if (mustBeFullyCanonical &&
if (canonicalSig == RequireFullyCanonicalSig::yes &&
(*canonicality != ECDSACanonicality::fullyCanonical))
return false;

Expand Down Expand Up @@ -268,19 +272,26 @@ verifyDigest(
&pubkey_imp) == 1;
}

Cofactored
cofactoredRules(Rules const& rules)
{
return rules.enabled(featureCofactoredED25519) ? Cofactored::yes
: Cofactored::no;
}

bool
verify(
PublicKey const& publicKey,
Slice const& m,
Slice const& sig,
bool mustBeFullyCanonical) noexcept
Cofactored cofactored,
RequireFullyCanonicalSig canonicalSig) noexcept
{
if (auto const type = publicKeyType(publicKey))
{
if (*type == KeyType::secp256k1)
{
return verifyDigest(
publicKey, sha512Half(m), sig, mustBeFullyCanonical);
return verifyDigest(publicKey, sha512Half(m), sig, canonicalSig);
}
else if (*type == KeyType::ed25519)
{
Expand All @@ -291,8 +302,18 @@ verify(
// byte to distinguish them from secp256k1 keys
// so when verifying the signature, we need to
// first strip that prefix.
return ed25519_sign_open_cofactored(
m.data(), m.size(), publicKey.data() + 1, sig.data()) == 0;
if (cofactored == Cofactored::no)
return ed25519_sign_open(
m.data(),
m.size(),
publicKey.data() + 1,
sig.data()) == 0;
else
return ed25519_sign_open_cofactored(
m.data(),
m.size(),
publicKey.data() + 1,
sig.data()) == 0;
}
}
return false;
Expand Down
Loading

0 comments on commit 3fe47bb

Please sign in to comment.