Skip to content

Commit

Permalink
[ed25519] Use verify_strict for signature verification in ed25519 p…
Browse files Browse the repository at this point in the history 
…recompile (anza-xyz#1876)

* use `verify_strict` for signature verification in ed25519 precompile

* add test

* clippy

* increase ed25519 precompile cost constant by 5%

* put ed25519 strict verification cost change under feature gate

---------

Co-authored-by: Emanuele Cesena <ecesena@jumptrading.com>
  • Loading branch information
@samkim-crypto @0x0ece
samkim-crypto and 0x0ece authored Jul 10, 2024
1 parent d441c0f commit ddda6c3
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 9 deletions.
2 changes: 2 additions & 0 deletions cost-model/src/block_cost_limits.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ pub const SIGNATURE_COST: u64 = COMPUTE_UNIT_TO_US_RATIO * 24;
pub const SECP256K1_VERIFY_COST: u64 = COMPUTE_UNIT_TO_US_RATIO * 223;
/// Number of compute units for one ed25519 signature verification.
pub const ED25519_VERIFY_COST: u64 = COMPUTE_UNIT_TO_US_RATIO * 76;
/// Number of compute units for one ed25519 strict signature verification.
pub const ED25519_VERIFY_STRICT_COST: u64 = COMPUTE_UNIT_TO_US_RATIO * 80;
/// Number of compute units for one write lock
pub const WRITE_LOCK_UNITS: u64 = COMPUTE_UNIT_TO_US_RATIO * 10;
/// Number of data bytes per compute units
Expand Down
20 changes: 16 additions & 4 deletions cost-model/src/cost_model.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl CostModel {
} else {
let mut tx_cost = UsageCostDetails::new_with_default_capacity();

Self::get_signature_cost(&mut tx_cost, transaction);
Self::get_signature_cost(&mut tx_cost, transaction, feature_set);
Self::get_write_lock_cost(&mut tx_cost, transaction, feature_set);
Self::get_transaction_cost(&mut tx_cost, transaction, feature_set);
tx_cost.allocated_accounts_data_size =
Expand All @@ -66,7 +66,7 @@ impl CostModel {
} else {
let mut tx_cost = UsageCostDetails::new_with_default_capacity();

Self::get_signature_cost(&mut tx_cost, transaction);
Self::get_signature_cost(&mut tx_cost, transaction, feature_set);
Self::get_write_lock_cost(&mut tx_cost, transaction, feature_set);
Self::get_instructions_data_cost(&mut tx_cost, transaction);
tx_cost.allocated_accounts_data_size =
Expand All @@ -82,13 +82,25 @@ impl CostModel {
}
}

fn get_signature_cost(tx_cost: &mut UsageCostDetails, transaction: &SanitizedTransaction) {
fn get_signature_cost(
tx_cost: &mut UsageCostDetails,
transaction: &SanitizedTransaction,
feature_set: &FeatureSet,
) {
let signatures_count_detail = transaction.message().get_signature_details();
tx_cost.num_transaction_signatures = signatures_count_detail.num_transaction_signatures();
tx_cost.num_secp256k1_instruction_signatures =
signatures_count_detail.num_secp256k1_instruction_signatures();
tx_cost.num_ed25519_instruction_signatures =
signatures_count_detail.num_ed25519_instruction_signatures();

let ed25519_verify_cost =
if feature_set.is_active(&feature_set::ed25519_precompile_verify_strict::id()) {
ED25519_VERIFY_STRICT_COST
} else {
ED25519_VERIFY_COST
};

tx_cost.signature_cost = signatures_count_detail
.num_transaction_signatures()
.saturating_mul(SIGNATURE_COST)
Expand All @@ -100,7 +112,7 @@ impl CostModel {
.saturating_add(
signatures_count_detail
.num_ed25519_instruction_signatures()
.saturating_mul(ED25519_VERIFY_COST),
.saturating_mul(ed25519_verify_cost),
);
}

Expand Down
121 changes: 116 additions & 5 deletions sdk/src/ed25519_instruction.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
#![cfg(feature = "full")]

use {
crate::{feature_set::FeatureSet, instruction::Instruction, precompiles::PrecompileError},
crate::{
feature_set::{ed25519_precompile_verify_strict, FeatureSet},
instruction::Instruction,
precompiles::PrecompileError,
},
bytemuck::bytes_of,
bytemuck_derive::{Pod, Zeroable},
ed25519_dalek::{ed25519::signature::Signature, Signer, Verifier},
Expand Down Expand Up @@ -86,7 +90,7 @@ pub fn new_ed25519_instruction(keypair: &ed25519_dalek::Keypair, message: &[u8])
pub fn verify(
data: &[u8],
instruction_datas: &[&[u8]],
_feature_set: &FeatureSet,
feature_set: &FeatureSet,
) -> Result<(), PrecompileError> {
if data.len() < SIGNATURE_OFFSETS_START {
return Err(PrecompileError::InvalidInstructionDataSize);
Expand Down Expand Up @@ -145,9 +149,15 @@ pub fn verify(
offsets.message_data_size as usize,
)?;

publickey
.verify(message, &signature)
.map_err(|_| PrecompileError::InvalidSignature)?;
if feature_set.is_active(&ed25519_precompile_verify_strict::id()) {
publickey
.verify_strict(message, &signature)
.map_err(|_| PrecompileError::InvalidSignature)?;
} else {
publickey
.verify(message, &signature)
.map_err(|_| PrecompileError::InvalidSignature)?;
}
}
Ok(())
}
Expand Down Expand Up @@ -189,9 +199,64 @@ pub mod test {
signature::{Keypair, Signer},
transaction::Transaction,
},
hex,
rand0_7::{thread_rng, Rng},
};

pub fn new_ed25519_instruction_raw(
pubkey: &[u8],
signature: &[u8],
message: &[u8],
) -> Instruction {
assert_eq!(pubkey.len(), PUBKEY_SERIALIZED_SIZE);
assert_eq!(signature.len(), SIGNATURE_SERIALIZED_SIZE);

let mut instruction_data = Vec::with_capacity(
DATA_START
.saturating_add(SIGNATURE_SERIALIZED_SIZE)
.saturating_add(PUBKEY_SERIALIZED_SIZE)
.saturating_add(message.len()),
);

let num_signatures: u8 = 1;
let public_key_offset = DATA_START;
let signature_offset = public_key_offset.saturating_add(PUBKEY_SERIALIZED_SIZE);
let message_data_offset = signature_offset.saturating_add(SIGNATURE_SERIALIZED_SIZE);

// add padding byte so that offset structure is aligned
instruction_data.extend_from_slice(bytes_of(&[num_signatures, 0]));

let offsets = Ed25519SignatureOffsets {
signature_offset: signature_offset as u16,
signature_instruction_index: u16::MAX,
public_key_offset: public_key_offset as u16,
public_key_instruction_index: u16::MAX,
message_data_offset: message_data_offset as u16,
message_data_size: message.len() as u16,
message_instruction_index: u16::MAX,
};

instruction_data.extend_from_slice(bytes_of(&offsets));

debug_assert_eq!(instruction_data.len(), public_key_offset);

instruction_data.extend_from_slice(pubkey);

debug_assert_eq!(instruction_data.len(), signature_offset);

instruction_data.extend_from_slice(signature);

debug_assert_eq!(instruction_data.len(), message_data_offset);

instruction_data.extend_from_slice(message);

Instruction {
program_id: solana_sdk::ed25519_program::id(),
accounts: vec![],
data: instruction_data,
}
}

fn test_case(
num_signatures: u16,
offsets: &Ed25519SignatureOffsets,
Expand Down Expand Up @@ -380,4 +445,50 @@ pub mod test {
);
assert!(tx.verify_precompiles(&feature_set).is_err());
}

#[test]
fn test_ed25519_malleability() {
solana_logger::setup();
let mint_keypair = Keypair::new();

// sig created via ed25519_dalek: both pass
let privkey = ed25519_dalek::Keypair::generate(&mut thread_rng());
let message_arr = b"hello";
let instruction = new_ed25519_instruction(&privkey, message_arr);
let tx = Transaction::new_signed_with_payer(
&[instruction.clone()],
Some(&mint_keypair.pubkey()),
&[&mint_keypair],
Hash::default(),
);

let feature_set = FeatureSet::default();
assert!(tx.verify_precompiles(&feature_set).is_ok());

let feature_set = FeatureSet::all_enabled();
assert!(tx.verify_precompiles(&feature_set).is_ok());

// malleable sig: verify_strict does NOT pass
// for example, test number 5:
// https://github.com/C2SP/CCTV/tree/main/ed25519
// R has low order (in fact R == 0)
let pubkey =
&hex::decode("10eb7c3acfb2bed3e0d6ab89bf5a3d6afddd1176ce4812e38d9fd485058fdb1f")
.unwrap();
let signature = &hex::decode("00000000000000000000000000000000000000000000000000000000000000009472a69cd9a701a50d130ed52189e2455b23767db52cacb8716fb896ffeeac09").unwrap();
let message = b"ed25519vectors 3";
let instruction = new_ed25519_instruction_raw(pubkey, signature, message);
let tx = Transaction::new_signed_with_payer(
&[instruction.clone()],
Some(&mint_keypair.pubkey()),
&[&mint_keypair],
Hash::default(),
);

let feature_set = FeatureSet::default();
assert!(tx.verify_precompiles(&feature_set).is_ok());

let feature_set = FeatureSet::all_enabled();
assert!(tx.verify_precompiles(&feature_set).is_err()); // verify_strict does NOT pass
}
}
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,10 @@ pub mod move_stake_and_move_lamports_ixs {
solana_sdk::declare_id!("7bTK6Jis8Xpfrs8ZoUfiMDPazTcdPcTWheZFJTA5Z6X4");
}

pub mod ed25519_precompile_verify_strict {
solana_sdk::declare_id!("ed9tNscbWLYBooxWA7FE2B5KHWs8A6sxfY8EzezEcoo");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -1041,6 +1045,7 @@ lazy_static! {
(zk_elgamal_proof_program_enabled::id(), "Enable ZkElGamalProof program SIMD-0153"),
(verify_retransmitter_signature::id(), "Verify retransmitter signature #1840"),
(move_stake_and_move_lamports_ixs::id(), "Enable MoveStake and MoveLamports stake program instructions #1610"),
(ed25519_precompile_verify_strict::id(), "Use strict verification in ed25519 precompile SIMD-0152"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down

0 comments on commit ddda6c3

Please sign in to comment.