From 32b9034f997945117ed8782cf489935210be8000 Mon Sep 17 00:00:00 2001 From: hanabi1224 Date: Mon, 29 Apr 2024 17:05:53 +0800 Subject: [PATCH 1/4] feat: syscall for aggregated bls verification (FIP-0079) --- fvm/src/gas/price_list.rs | 30 +++++ fvm/src/kernel/default.rs | 50 +++++++ fvm/src/kernel/mod.rs | 16 +++ fvm/src/syscalls/context.rs | 23 ++++ fvm/src/syscalls/crypto.rs | 55 +++++++- fvm/src/syscalls/mod.rs | 5 + sdk/src/crypto.rs | 47 ++++++- sdk/src/sys/crypto.rs | 41 +++++- shared/src/crypto/signature.rs | 87 +++++++------ testing/calibration/shared/src/lib.rs | 10 ++ .../integration/tests/gas_calibration_test.rs | 53 ++++++++ .../fil-gas-calibration-actor/src/actor.rs | 11 ++ .../actors/fil-syscall-actor/src/actor.rs | 123 +++++++++++++++++- 13 files changed, 501 insertions(+), 50 deletions(-) diff --git a/fvm/src/gas/price_list.rs b/fvm/src/gas/price_list.rs index 7db8b7441..7f79a80e5 100644 --- a/fvm/src/gas/price_list.rs +++ b/fvm/src/gas/price_list.rs @@ -115,6 +115,11 @@ lazy_static! { } }, secp256k1_recover_cost: Gas::new(1637292), + bls_pairing_cost: Gas::new(8299302), + bls_hashing_cost: ScalingCost { + flat: Gas::zero(), + scale: Gas::new(7), + }, hashing_cost: total_enum_map! { SupportedHashes { Sha2_256 => ScalingCost { @@ -404,6 +409,9 @@ pub struct PriceList { /// Gas cost for recovering secp256k1 signer public key pub(crate) secp256k1_recover_cost: Gas, + pub(crate) bls_pairing_cost: Gas, + pub(crate) bls_hashing_cost: ScalingCost, + pub(crate) hashing_cost: HashMap, /// Gas cost for walking up the chain. @@ -598,6 +606,28 @@ impl PriceList { GasCharge::new("OnVerifySignature", gas, Zero::zero()) } + /// Returns gas required for BLS aggregate signature verification. + #[inline] + pub fn on_verify_aggregate_signature(&self, num_sigs: usize, data_len: usize) -> GasCharge { + // When `num_sigs` BLS signatures are aggregated into a single signature, the aggregate + // signature verifier must perform `num_sigs + 1` expensive pairing operations (one + // pairing on the aggregate signature, and one pairing for each signed plaintext's digest). + // + // Note that `bls_signatures` rearranges the textbook verifier equation (containing + // `num_sigs + 1` full pairings) into a more efficient equation containing `num_sigs + 1` + // Miller loops and one final exponentiation. + let num_pairings = num_sigs as u64 + 1; + + let gas_pairings = self.bls_pairing_cost * num_pairings; + let gas_hashing = self.bls_hashing_cost.apply(data_len); + + GasCharge::new( + "OnVerifyBlsAggregateSignature", + gas_pairings + gas_hashing, + Zero::zero(), + ) + } + /// Returns gas required for recovering signer pubkey from signature #[inline] pub fn on_recover_secp_public_key(&self) -> GasCharge { diff --git a/fvm/src/kernel/default.rs b/fvm/src/kernel/default.rs index 31840dc67..079023998 100644 --- a/fvm/src/kernel/default.rs +++ b/fvm/src/kernel/default.rs @@ -605,6 +605,56 @@ where })) } + fn verify_bls_aggregate( + &self, + aggregate_sig: &[u8; signature::BLS_SIG_LEN], + pub_keys: &[[u8; signature::BLS_PUB_LEN]], + plaintexts_concat: &[u8], + plaintext_lens: &[u32], + ) -> Result { + let num_signers = pub_keys.len(); + + if num_signers != plaintext_lens.len() { + return Err(syscall_error!( + IllegalArgument; + "unequal numbers of bls public keys and plaintexts" + ) + .into()); + } + + let t = self.call_manager.charge_gas( + self.call_manager + .price_list() + .on_verify_aggregate_signature(num_signers, plaintexts_concat.len()), + )?; + + let mut offset: usize = 0; + let plaintexts = plaintext_lens + .iter() + .map(|&len| { + let start = offset; + offset = start + .checked_add(len as usize) + .context("invalid bls plaintext length") + .or_illegal_argument()?; + plaintexts_concat + .get(start..offset) + .context("bls signature plaintext out of bounds") + .or_illegal_argument() + }) + .collect::>>()?; + if offset != plaintexts_concat.len() { + return Err( + syscall_error!(IllegalArgument; "plaintexts buffer length doesn't match").into(), + ); + } + + t.record( + signature::ops::verify_bls_aggregate(aggregate_sig, pub_keys, &plaintexts) + .or(Ok(false)), + ) + } + fn recover_secp_public_key( &self, hash: &[u8; SECP_SIG_MESSAGE_HASH_SIZE], diff --git a/fvm/src/kernel/mod.rs b/fvm/src/kernel/mod.rs index 559aa68b7..5597e093d 100644 --- a/fvm/src/kernel/mod.rs +++ b/fvm/src/kernel/mod.rs @@ -232,6 +232,22 @@ pub trait CryptoOps { plaintext: &[u8], ) -> Result; + /// Verifies a BLS aggregate signature. In the case where there is one signer/signed plaintext, + /// this is equivalent to verifying a non-aggregated BLS signature. + /// + /// Returns: + /// - `Ok(true)` on a valid signature. + /// - `Ok(false)` on an invalid signature or if the signature or public keys' bytes represent an + /// invalid curve point. + /// - `Err(IllegalArgument)` if `pub_keys.len() != plaintexts.len()`. + fn verify_bls_aggregate( + &self, + aggregate_sig: &[u8; fvm_shared::crypto::signature::BLS_SIG_LEN], + pub_keys: &[[u8; fvm_shared::crypto::signature::BLS_PUB_LEN]], + plaintexts_concat: &[u8], + plaintext_lens: &[u32], + ) -> Result; + /// Given a message hash and its signature, recovers the public key of the signer. fn recover_secp_public_key( &self, diff --git a/fvm/src/syscalls/context.rs b/fvm/src/syscalls/context.rs index dcac63a95..7b6def4e5 100644 --- a/fvm/src/syscalls/context.rs +++ b/fvm/src/syscalls/context.rs @@ -87,6 +87,29 @@ impl Memory { .or_error(ErrorNumber::IllegalArgument) } + /// Return a slice of byte arrays into the actor's memory. + /// + /// This slice of byte arrays is valid for the lifetime of the syscall, borrowing the actors memory without + /// copying. + pub fn try_chunks(&self, offset: u32, len: u32) -> Result<&[[u8; S]]> { + let num_chunks = { + let len = len as usize; + if len % S != 0 { + return Err(syscall_error!( + IllegalArgument; + "buffer length {len} is not divisible by chunk len {S}" + ) + .into()); + } + len / S + }; + + self.try_slice(offset, len).map(|bytes| { + let arr_ptr = bytes.as_ptr() as *const [u8; S]; + unsafe { std::slice::from_raw_parts(arr_ptr, num_chunks) } + }) + } + /// Read a CID from actor memory starting at the given offset. /// /// On failure, this method returns an [`ErrorNumber::IllegalArgument`] error. diff --git a/fvm/src/syscalls/crypto.rs b/fvm/src/syscalls/crypto.rs index 2681571f2..eb08aaf86 100644 --- a/fvm/src/syscalls/crypto.rs +++ b/fvm/src/syscalls/crypto.rs @@ -4,12 +4,15 @@ use std::cmp; use anyhow::Context as _; use fvm_shared::crypto::signature::{ - SignatureType, SECP_PUB_LEN, SECP_SIG_LEN, SECP_SIG_MESSAGE_HASH_SIZE, + SignatureType, BLS_PUB_LEN, BLS_SIG_LEN, SECP_PUB_LEN, SECP_SIG_LEN, SECP_SIG_MESSAGE_HASH_SIZE, }; use num_traits::FromPrimitive; use super::Context; -use crate::kernel::{ClassifyResult, CryptoOps, Result}; +use crate::{ + kernel::{ClassifyResult, CryptoOps, Result}, + syscall_error, +}; /// Verifies that a signature is valid for an address and plaintext. /// @@ -40,6 +43,54 @@ pub fn verify_signature( .map(|v| if v { 0 } else { -1 }) } +/// Verifies that a bls aggregate signature is valid for a list of public keys and plaintexts. +/// +/// The return i32 indicates the status code of the verification: +/// - 0: verification ok. +/// - -1: verification failed. +#[allow(clippy::too_many_arguments)] +pub fn verify_bls_aggregate( + context: Context<'_, impl CryptoOps>, + num_signers: u32, + sig_off: u32, + pub_keys_off: u32, + plaintexts_off: u32, + plaintext_lens_off: u32, +) -> Result { + // Check that the provided number of signatures aggregated does not cause `u32` overflow. + let pub_keys_len = num_signers + .checked_mul(BLS_PUB_LEN as u32) + .ok_or(syscall_error!( + IllegalArgument; + "number of signatures aggregated ({num_signers}) exceeds limit" + ))?; + + let sig: &[u8; BLS_SIG_LEN] = context + .memory + .try_slice(sig_off, BLS_SIG_LEN as u32)? + .try_into() + .expect("bls signature bytes slice-to-array conversion should not fail"); + + let pub_keys: &[[u8; BLS_PUB_LEN]] = context.memory.try_chunks(pub_keys_off, pub_keys_len)?; + + let plaintext_lens: &[u32] = context + .memory + .try_slice(plaintext_lens_off, num_signers * 4) + .map(|bytes| { + let ptr = bytes.as_ptr() as *const u32; + unsafe { std::slice::from_raw_parts(ptr, num_signers as usize) } + })?; + + let plaintexts_concat = context + .memory + .try_slice(plaintexts_off, plaintext_lens.iter().sum())?; + + context + .kernel + .verify_bls_aggregate(sig, pub_keys, plaintexts_concat, plaintext_lens) + .map(|v| if v { 0 } else { -1 }) +} + pub fn recover_secp_public_key( context: Context<'_, impl CryptoOps>, hash_off: u32, diff --git a/fvm/src/syscalls/mod.rs b/fvm/src/syscalls/mod.rs index 572d878f6..d3848a689 100644 --- a/fvm/src/syscalls/mod.rs +++ b/fvm/src/syscalls/mod.rs @@ -317,6 +317,11 @@ where } linker.link_syscall("crypto", "verify_signature", crypto::verify_signature)?; + linker.link_syscall( + "crypto", + "verify_bls_aggregate", + crypto::verify_bls_aggregate, + )?; linker.link_syscall( "crypto", "recover_secp_public_key", diff --git a/sdk/src/crypto.rs b/sdk/src/crypto.rs index f4deeacd0..a2969589a 100644 --- a/sdk/src/crypto.rs +++ b/sdk/src/crypto.rs @@ -4,10 +4,13 @@ use cid::Cid; use fvm_ipld_encoding::to_vec; use fvm_shared::address::Address; use fvm_shared::consensus::ConsensusFault; -use fvm_shared::crypto::hash::SupportedHashes; -use fvm_shared::crypto::signature::{ - Signature, SECP_PUB_LEN, SECP_SIG_LEN, SECP_SIG_MESSAGE_HASH_SIZE, +use fvm_shared::crypto::{ + hash::SupportedHashes, + signature::{ + Signature, BLS_PUB_LEN, BLS_SIG_LEN, SECP_PUB_LEN, SECP_SIG_LEN, SECP_SIG_MESSAGE_HASH_SIZE, + }, }; +use fvm_shared::error::ErrorNumber; use fvm_shared::piece::PieceInfo; use fvm_shared::sector::{ AggregateSealVerifyProofAndInfos, RegisteredSealProof, ReplicaUpdateInfo, SealVerifyInfo, @@ -43,6 +46,44 @@ pub fn verify_signature( } } +pub fn verify_bls_aggregate( + sig: &[u8; BLS_SIG_LEN], + pub_keys: &[[u8; BLS_PUB_LEN]], + plaintexts: &[&[u8]], +) -> SyscallResult { + let num_signers = { + let (num_signers, num_plaintexts) = (pub_keys.len(), plaintexts.len()); + if num_signers != num_plaintexts { + return Err(ErrorNumber::IllegalArgument); + }; + num_signers + .try_into() + .map_err(|_| ErrorNumber::IllegalArgument)? + }; + + let plaintext_lens = plaintexts + .iter() + .map(|msg| { + msg.len() + .try_into() + .map_err(|_| ErrorNumber::IllegalArgument) + }) + .collect::>>()?; + + let plaintexts_concat: Vec = plaintexts.concat(); + + unsafe { + sys::crypto::verify_bls_aggregate( + num_signers, + sig.as_ptr(), + pub_keys.as_ptr(), + plaintexts_concat.as_ptr(), + plaintext_lens.as_ptr(), + ) + .map(status_code_to_bool) + } +} + /// Recovers the signer public key from the message hash and signature. pub fn recover_secp_public_key( hash: &[u8; SECP_SIG_MESSAGE_HASH_SIZE], diff --git a/sdk/src/sys/crypto.rs b/sdk/src/sys/crypto.rs index 1b35626ac..bf1a52896 100644 --- a/sdk/src/sys/crypto.rs +++ b/sdk/src/sys/crypto.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0, MIT //! Syscalls for cryptographic operations. -use fvm_shared::crypto::signature::SECP_PUB_LEN; +use fvm_shared::crypto::signature::{BLS_PUB_LEN, SECP_PUB_LEN}; #[doc(inline)] pub use fvm_shared::sys::out::crypto::*; @@ -38,6 +38,45 @@ super::fvm_syscalls! { plaintext_len: u32, ) -> Result; + /// Verifies that a BLS aggregate signature is valid for a list of signers' BLS public keys and + /// and the digest of each signer's plaintext. + /// + /// # Arguments + /// + /// - `num_signers` - the number of signatures aggregated; for non-aggregate signatures `num_signers = 1`. + /// - `sig_off` - a pointer to the first byte of the signature being verified. + /// - `pub_keys_off` - a pointer to the first public key in an array containing each signer's public + /// key. + /// - `plaintexts_off` - a pointer to the first byte in an array containing each plaintext's bytes, + /// i.e. plaintexts must be allocated contiguously in memory (equivalent to the memory layout of an + /// array containing the concatenated plaintexts' bytes). + /// - `plaintext_lens_off` - a pointer to the first element of an array containing each plaintext's + /// byte length (each plaintexrt's byte length is represented as a `u32`). + /// + /// # Returns + /// + /// - `Err(IllegalArgument)`: + /// - any pointer and byte length pair are an invalid location in their Wasm module's memory. + /// + /// - `Ok(-1)`: + /// - the signature is an invalid G2 compressed curve point. + /// - any public key is an invalid G1 compressed curve point. + /// - there are duplicate plaintexts. + /// - any public key in is the G1 identity/zero point. + /// - the signature is invalid for the provided public keys and plaintexts. + /// + /// - `Ok(0)`: + /// - the provided signature is valid for the provided signers' public keys and plaintexts. + /// - the number of signers is zero + /// + pub fn verify_bls_aggregate( + num_signers: u32, + sig_off: *const u8, + pub_keys_off: *const [u8; BLS_PUB_LEN], + plaintexts_off: *const u8, + plaintext_lens_off: *const u32, + ) -> Result; + /// Recovers the signer public key from a signed message hash and its signature. /// /// Returns the public key in uncompressed 65 bytes form. diff --git a/shared/src/crypto/signature.rs b/shared/src/crypto/signature.rs index 36caf5afd..9f7fe6374 100644 --- a/shared/src/crypto/signature.rs +++ b/shared/src/crypto/signature.rs @@ -158,7 +158,6 @@ pub mod ops { use super::{Error, SECP_SIG_LEN, SECP_SIG_MESSAGE_HASH_SIZE}; use crate::address::{Address, Protocol}; - use crate::crypto::signature::Signature; /// Returns `String` error if a bls signature is invalid. pub fn verify_bls_sig(signature: &[u8], data: &[u8], addr: &Address) -> Result<(), String> { @@ -188,6 +187,38 @@ pub mod ops { } } + /// Verifies an aggregated BLS signature. Returns `Ok(false)` if signature verification fails + /// and `String` error if arguments are invalid. + pub fn verify_bls_aggregate( + aggregate_sig: &[u8; super::BLS_SIG_LEN], + pub_keys: &[[u8; super::BLS_PUB_LEN]], + plaintexts: &[&[u8]], + ) -> Result { + // If the number of public keys and data does not match, return false; + let (num_pub_keys, num_plaintexts) = (pub_keys.len(), plaintexts.len()); + if num_pub_keys != num_plaintexts { + return Err(format!( + "unequal numbers of public keys ({num_pub_keys}) and plaintexts ({num_plaintexts})", + )); + } + if num_pub_keys == 0 { + return Ok(true); + } + + // Deserialize signature bytes into a curve point. + let sig = BlsSignature::from_bytes(aggregate_sig) + .map_err(|_| "bls aggregate signature bytes are invalid G2 curve point".to_string())?; + + // Deserialize each public key's bytes into a curve point. + let pub_keys = pub_keys + .iter() + .map(|pub_key| BlsPubKey::from_bytes(pub_key.as_slice())) + .collect::, _>>() + .map_err(|_| "bls public key bytes are invalid G2 curve point".to_string())?; + + Ok(bls_signatures::verify_messages(&sig, plaintexts, &pub_keys)) + } + /// Returns `String` error if a secp256k1 signature is invalid. pub fn verify_secp256k1_sig( signature: &[u8], @@ -229,37 +260,6 @@ pub mod ops { } } - /// Aggregates and verifies bls signatures collectively. - pub fn verify_bls_aggregate( - data: &[&[u8]], - pub_keys: &[&[u8]], - aggregate_sig: &Signature, - ) -> bool { - // If the number of public keys and data does not match, then return false - if data.len() != pub_keys.len() { - return false; - } - if data.is_empty() { - return true; - } - - let sig = match BlsSignature::from_bytes(aggregate_sig.bytes()) { - Ok(v) => v, - Err(_) => return false, - }; - - let pk_map_results: Result, _> = - pub_keys.iter().map(|x| BlsPubKey::from_bytes(x)).collect(); - - let pks = match pk_map_results { - Ok(v) => v, - Err(_) => return false, - }; - - // Does the aggregate verification - verify_messages(&sig, data, &pks[..]) - } - /// Return the public key used for signing a message given it's signing bytes hash and signature. pub fn recover_secp_public_key( hash: &[u8; SECP_SIG_MESSAGE_HASH_SIZE], @@ -322,24 +322,27 @@ mod tests { let private_keys: Vec = (0..num_sigs).map(|_| PrivateKey::generate(rng)).collect(); - let public_keys: Vec<_> = private_keys + let public_keys: Vec<[u8; BLS_PUB_LEN]> = private_keys .iter() - .map(|x| x.public_key().as_bytes()) + .map(|x| { + x.public_key() + .as_bytes() + .try_into() + .expect("public key bytes to array conversion should not fail") + }) .collect(); let signatures: Vec = (0..num_sigs) .map(|x| private_keys[x].sign(data[x])) .collect(); - let public_keys_slice: Vec<&[u8]> = public_keys.iter().map(|x| &**x).collect(); + let agg_sig: [u8; BLS_SIG_LEN] = bls_signatures::aggregate(&signatures) + .expect("bls signature aggregation should not fail") + .as_bytes() + .try_into() + .expect("bls aggregate signature to bytes array should not fail"); - let calculated_bls_agg = - Signature::new_bls(bls_signatures::aggregate(&signatures).unwrap().as_bytes()); - assert!(verify_bls_aggregate( - &data, - &public_keys_slice, - &calculated_bls_agg - ),); + assert!(verify_bls_aggregate(&agg_sig, &public_keys, &data).unwrap()); } #[test] diff --git a/testing/calibration/shared/src/lib.rs b/testing/calibration/shared/src/lib.rs index b5cd46c02..5ef1eafa5 100644 --- a/testing/calibration/shared/src/lib.rs +++ b/testing/calibration/shared/src/lib.rs @@ -15,6 +15,8 @@ pub enum Method { OnBlock, /// Try (and fail) to verify random data with a public key and signature. OnVerifySignature, + /// Try to validate BLS aggregates (correctly). + OnVerifyBlsAggregate, /// Try (and fail) to recovery a public key from a signature, using random data. OnRecoverSecpPublicKey, /// Measure sends @@ -53,6 +55,14 @@ pub struct OnVerifySignatureParams { pub seed: u64, } +#[derive(Serialize, Deserialize)] +pub struct OnVerifyBlsAggregateParams { + pub iterations: usize, + pub signature: Vec, + pub keys: Vec>, + pub messages: Vec>, +} + #[derive(Serialize, Deserialize)] pub struct OnRecoverSecpPublicKeyParams { pub iterations: usize, diff --git a/testing/integration/tests/gas_calibration_test.rs b/testing/integration/tests/gas_calibration_test.rs index 3594a2ee8..2348877ec 100644 --- a/testing/integration/tests/gas_calibration_test.rs +++ b/testing/integration/tests/gas_calibration_test.rs @@ -442,6 +442,59 @@ fn on_verify_signature() { export(CHARGE_NAME, &obs, ®ression).unwrap(); } +#[test] +#[cfg(feature = "calibration")] +fn on_verify_bls_aggregate() { + use bls_signatures::Serialize; + use rand::{thread_rng, RngCore}; + + const CHARGE_NAME: &str = "OnVerifyBlsAggregateSignature"; + const METHOD: Method = Method::OnVerifyBlsAggregate; + + let iterations = 100; + + let mut te = instantiate_tester(); + let mut obs = Vec::new(); + let mut rng = thread_rng(); + + for &n in &[1, 4, 8, 20, 50, 200, 1000] { + let mut keys = Vec::new(); + let mut sks = Vec::new(); + let mut sigs = Vec::new(); + let mut messages = Vec::new(); + for _ in 0..n { + let mut data = vec![0u8; 100]; + rng.fill_bytes(&mut data); + let sk = bls_signatures::PrivateKey::generate(&mut rng); + let pk = sk.public_key(); + let sig = sk.sign(&data); + + keys.push(pk.as_bytes()); + sks.push(sk); + messages.push(data); + sigs.push(sig); + } + let signature = bls_signatures::aggregate(&sigs).unwrap().as_bytes(); + let params = OnVerifyBlsAggregateParams { + iterations, + signature, + keys, + messages, + }; + + let ret = te.execute_or_die(METHOD as u64, ¶ms); + + let iter_obs = collect_obs(&ret, CHARGE_NAME, "signers", n); + let iter_obs = eliminate_outliers(iter_obs, 0.02, Eliminate::Top); + + obs.extend(iter_obs); + } + + let regression = run_linear_regression(&obs); + + export(CHARGE_NAME, &obs, ®ression).unwrap(); +} + // Scan CBOR Fields with no links. #[test] #[cfg(feature = "calibration")] diff --git a/testing/test_actors/actors/fil-gas-calibration-actor/src/actor.rs b/testing/test_actors/actors/fil-gas-calibration-actor/src/actor.rs index 3d4e92f5a..b0a08a652 100644 --- a/testing/test_actors/actors/fil-gas-calibration-actor/src/actor.rs +++ b/testing/test_actors/actors/fil-gas-calibration-actor/src/actor.rs @@ -49,6 +49,7 @@ fn dispatch(method: Method, params_ptr: u32) -> Result<()> { Method::OnHashing => dispatch_to(on_hashing, params_ptr), Method::OnBlock => dispatch_to(on_block, params_ptr), Method::OnVerifySignature => dispatch_to(on_verify_signature, params_ptr), + Method::OnVerifyBlsAggregate => dispatch_to(on_verify_bls_aggregate, params_ptr), Method::OnRecoverSecpPublicKey => dispatch_to(on_recover_secp_public_key, params_ptr), Method::OnSend => dispatch_to(on_send, params_ptr), Method::OnEvent => dispatch_to(on_event, params_ptr), @@ -121,6 +122,16 @@ fn on_verify_signature(p: OnVerifySignatureParams) -> Result<()> { Ok(()) } +fn on_verify_bls_aggregate(p: OnVerifyBlsAggregateParams) -> Result<()> { + let sig = p.signature.try_into().unwrap(); + let keys: Vec<_> = p.keys.iter().map(|k| (&**k).try_into().unwrap()).collect(); + let messages: Vec<_> = p.messages.iter().map(|m| &**m).collect(); + for _ in 0..p.iterations { + fvm_sdk::crypto::verify_bls_aggregate(&sig, &keys, &messages)?; + } + Ok(()) +} + fn on_recover_secp_public_key(p: OnRecoverSecpPublicKeyParams) -> Result<()> { let mut data = random_bytes(p.size, p.seed); let sig: [u8; SECP_SIG_LEN] = p diff --git a/testing/test_actors/actors/fil-syscall-actor/src/actor.rs b/testing/test_actors/actors/fil-syscall-actor/src/actor.rs index 927aa4ac8..ba1d2a027 100644 --- a/testing/test_actors/actors/fil-syscall-actor/src/actor.rs +++ b/testing/test_actors/actors/fil-syscall-actor/src/actor.rs @@ -32,7 +32,9 @@ pub enum SupportedHashes { pub fn invoke(_: u32) -> u32 { sdk::initialize(); - test_signature(); + test_secp_signature(); + test_bls_signature(); + test_bls_aggregate(); test_expected_hash(); test_hash_syscall(); test_compute_unsealed_sector_cid(); @@ -46,7 +48,7 @@ pub fn invoke(_: u32) -> u32 { 0 } -fn test_signature() { +fn test_secp_signature() { // the following vectors represent a valid secp256k1 signatures for an address and plaintext message // let signature_bytes: Vec = vec![ @@ -141,6 +143,123 @@ fn test_signature() { } } +fn test_bls_signature() { + let msg = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; + + let pub_key = [ + 173, 154, 145, 188, 114, 85, 101, 250, 129, 225, 3, 205, 128, 61, 161, 185, 210, 18, 147, + 84, 160, 15, 233, 114, 178, 113, 115, 142, 4, 221, 81, 215, 188, 151, 11, 87, 4, 110, 23, + 219, 125, 143, 122, 176, 207, 123, 66, 146, + ]; + + let addr = Address::new_bls(&pub_key).unwrap(); + + let sig = Signature::new_bls(vec![ + 177, 209, 192, 174, 213, 199, 231, 9, 247, 201, 250, 193, 14, 250, 138, 252, 155, 27, 66, + 78, 14, 204, 165, 99, 192, 154, 96, 138, 179, 60, 59, 191, 58, 178, 229, 224, 43, 253, 43, + 254, 200, 37, 117, 247, 203, 45, 111, 195, 5, 188, 14, 121, 40, 59, 41, 48, 157, 88, 89, + 198, 177, 83, 24, 210, 254, 185, 78, 159, 230, 105, 29, 37, 169, 109, 247, 67, 111, 193, + 17, 31, 51, 17, 241, 96, 224, 254, 111, 101, 129, 18, 16, 242, 177, 61, 143, 64, + ]); + + // Test successful signature validation. + let res = sdk::crypto::verify_signature(&sig, &addr, &msg); + assert_eq!(res, Ok(true)); + + // Test invalid signature. The following signature bytes represent a valid G2 point. + let invalid_sig = Signature::new_bls(vec![ + 146, 72, 239, 152, 88, 59, 69, 25, 119, 24, 54, 37, 105, 220, 134, 131, 46, 186, 98, 35, + 46, 160, 88, 225, 195, 50, 135, 39, 24, 178, 11, 241, 46, 166, 214, 198, 67, 200, 61, 183, + 51, 108, 69, 115, 184, 150, 124, 32, 21, 192, 204, 174, 253, 151, 49, 111, 246, 60, 52, + 147, 90, 133, 90, 53, 9, 9, 78, 187, 127, 26, 207, 47, 240, 248, 109, 45, 104, 83, 99, 45, + 35, 78, 18, 219, 13, 50, 145, 26, 23, 6, 103, 32, 248, 188, 235, 111, + ]); + let res = sdk::crypto::verify_signature(&invalid_sig, &addr, &msg); + assert_eq!(res, Ok(false)); + + // Test invalid public key. The following public key bytes represent a valid G1 point. + let invalid_pub_key = [ + 146, 70, 145, 58, 25, 235, 94, 212, 41, 157, 27, 198, 144, 178, 157, 191, 218, 85, 23, 81, + 198, 2, 84, 171, 8, 212, 251, 62, 143, 46, 241, 61, 248, 22, 169, 138, 16, 19, 39, 179, + 114, 132, 67, 130, 45, 96, 1, 132, + ]; + let invalid_addr = Address::new_bls(&invalid_pub_key).unwrap(); + let res = sdk::crypto::verify_signature(&sig, &invalid_addr, &msg); + assert_eq!(res, Ok(false)); + + // Test invalid message. + let mut invalid_msg = msg; + invalid_msg[0] += 1; + let res = sdk::crypto::verify_signature(&sig, &addr, &invalid_msg); + assert_eq!(res, Ok(false)); +} + +fn test_bls_aggregate() { + let mut msgs_bytes = 0..; + let msg_1: Vec = (&mut msgs_bytes).take(10).collect(); + let msg_2: Vec = (&mut msgs_bytes).take(10).collect(); + let msg_3: Vec = (&mut msgs_bytes).take(10).collect(); + let msgs = [msg_1.as_slice(), msg_2.as_slice(), msg_3.as_slice()]; + + let pub_keys = [ + [ + 173, 154, 145, 188, 114, 85, 101, 250, 129, 225, 3, 205, 128, 61, 161, 185, 210, 18, + 147, 84, 160, 15, 233, 114, 178, 113, 115, 142, 4, 221, 81, 215, 188, 151, 11, 87, 4, + 110, 23, 219, 125, 143, 122, 176, 207, 123, 66, 146, + ], + [ + 166, 188, 253, 186, 140, 16, 193, 46, 218, 161, 3, 28, 70, 112, 192, 253, 195, 179, + 167, 181, 197, 130, 19, 216, 51, 188, 86, 179, 88, 40, 161, 215, 116, 189, 157, 29, 27, + 61, 144, 111, 195, 221, 100, 87, 107, 239, 25, 189, + ], + [ + 167, 241, 45, 72, 153, 172, 192, 10, 118, 144, 223, 120, 38, 106, 140, 48, 14, 57, 104, + 0, 67, 174, 148, 177, 204, 138, 35, 201, 92, 108, 208, 60, 109, 226, 9, 169, 2, 168, + 27, 73, 138, 221, 77, 74, 103, 186, 117, 225, + ], + ]; + + let sig = [ + 164, 39, 224, 212, 184, 193, 176, 129, 10, 127, 96, 36, 101, 63, 133, 5, 223, 148, 253, 34, + 139, 109, 244, 229, 242, 247, 83, 84, 6, 96, 9, 163, 87, 252, 234, 52, 105, 48, 87, 38, + 154, 48, 150, 34, 165, 53, 42, 108, 7, 106, 225, 93, 147, 11, 156, 109, 108, 226, 27, 126, + 213, 199, 148, 3, 77, 102, 248, 239, 41, 108, 177, 159, 14, 50, 153, 49, 47, 22, 250, 113, + 252, 170, 223, 150, 51, 97, 180, 19, 226, 171, 246, 197, 50, 92, 47, 182, + ]; + + // Assert that bls validation syscall succeeds. + let res = sdk::crypto::verify_bls_aggregate(&sig, &pub_keys, &msgs); + assert_eq!(res, Ok(true)); + + // Assert that bls validation syscall fails for an invalid aggregate signature. The following + // signature bytes represent as valid G2 point. + let invalid_sig = [ + 146, 72, 239, 152, 88, 59, 69, 25, 119, 24, 54, 37, 105, 220, 134, 131, 46, 186, 98, 35, + 46, 160, 88, 225, 195, 50, 135, 39, 24, 178, 11, 241, 46, 166, 214, 198, 67, 200, 61, 183, + 51, 108, 69, 115, 184, 150, 124, 32, 21, 192, 204, 174, 253, 151, 49, 111, 246, 60, 52, + 147, 90, 133, 90, 53, 9, 9, 78, 187, 127, 26, 207, 47, 240, 248, 109, 45, 104, 83, 99, 45, + 35, 78, 18, 219, 13, 50, 145, 26, 23, 6, 103, 32, 248, 188, 235, 111, + ]; + let res = sdk::crypto::verify_bls_aggregate(&invalid_sig, &pub_keys, &msgs); + assert_eq!(res, Ok(false)); + + // Assert that bls validation syscall fails for an invalid public key. The following public key + // bytes represent as valid G1 point. + let invalid_pub_key = [ + 146, 70, 145, 58, 25, 235, 94, 212, 41, 157, 27, 198, 144, 178, 157, 191, 218, 85, 23, 81, + 198, 2, 84, 171, 8, 212, 251, 62, 143, 46, 241, 61, 248, 22, 169, 138, 16, 19, 39, 179, + 114, 132, 67, 130, 45, 96, 1, 132, + ]; + let invalid_pub_keys = [invalid_pub_key, pub_keys[1], pub_keys[2]]; + let res = sdk::crypto::verify_bls_aggregate(&sig, &invalid_pub_keys, &msgs); + assert_eq!(res, Ok(false)); + + // Assert that bls validation syscall fails for invalid messages. + let invalid_msgs = [&[11, 22, 33, 44], msgs[1], msgs[2]]; + let res = sdk::crypto::verify_bls_aggregate(&sig, &pub_keys, &invalid_msgs); + assert_eq!(res, Ok(false)); +} + // use SDK methods to hash and compares against locally (inside the actor) hashed digest fn test_expected_hash() { use multihash::MultihashDigest; From 82418e1dca6ca17111c3db665ba8d70eb58a3c85 Mon Sep 17 00:00:00 2001 From: hanabi1224 Date: Thu, 2 May 2024 18:43:40 +0800 Subject: [PATCH 2/4] tests --- .github/workflows/ci.yml | 5 + fvm/Cargo.toml | 4 + fvm/src/kernel/default.rs | 32 ++++--- fvm/src/kernel/mod.rs | 35 +++++++ fvm/src/syscalls/crypto.rs | 9 +- fvm/src/syscalls/mod.rs | 2 +- sdk/Cargo.toml | 4 + sdk/src/crypto.rs | 53 ++++++++++ sdk/src/sys/crypto.rs | 1 + testing/integration/tests/main.rs | 20 +++- .../actors/fil-syscall-actor/Cargo.toml | 1 + .../actors/fil-syscall-actor/src/actor.rs | 1 + testing/test_actors/build.rs | 96 ++++++++++++++----- 13 files changed, 220 insertions(+), 43 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 77a6b2941..8c888efd4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,6 +47,11 @@ jobs: command: clippy # we disable default features because rust will otherwise unify them and turn on opencl in CI. args: --all --all-targets --no-default-features + - name: check-clippy(no-verify-signature) + key: v3 + command: clippy + # we disable default features because rust will otherwise unify them and turn on opencl in CI. + args: --all --all-targets --no-default-features --features no-verify-signature - name: test-fvm key: v3-cov push: true diff --git a/fvm/Cargo.toml b/fvm/Cargo.toml index 7f8fc850b..fbc812aa7 100644 --- a/fvm/Cargo.toml +++ b/fvm/Cargo.toml @@ -58,3 +58,7 @@ m2-native = [] upgrade-actor = [] gas_calibration = [] nv23-dev = [] +# Use this feature to remove `verify_signature` syscall that is supposed to be removed by FIP-0079, +# The current implementation keeps it by default for backward compatibility reason. +# See +no-verify-signature = [] diff --git a/fvm/src/kernel/default.rs b/fvm/src/kernel/default.rs index 079023998..01793d45d 100644 --- a/fvm/src/kernel/default.rs +++ b/fvm/src/kernel/default.rs @@ -1,14 +1,12 @@ // Copyright 2021-2023 Protocol Labs // SPDX-License-Identifier: Apache-2.0, MIT use std::convert::{TryFrom, TryInto}; -use std::panic::{self, UnwindSafe}; use std::path::PathBuf; use anyhow::{anyhow, Context as _}; use cid::Cid; use fvm_ipld_blockstore::Blockstore; use fvm_ipld_encoding::{CBOR, IPLD_RAW}; -use fvm_shared::address::Payload; use fvm_shared::crypto::signature; use fvm_shared::error::ErrorNumber; use fvm_shared::event::{ActorEvent, Entry, Flags}; @@ -575,6 +573,7 @@ impl CryptoOps for DefaultKernel where C: CallManager, { + #[cfg(not(feature = "no-verify-signature"))] fn verify_signature( &self, sig_type: SignatureType, @@ -582,6 +581,25 @@ where signer: &Address, plaintext: &[u8], ) -> Result { + use fvm_shared::address::Payload; + use std::panic::{self, UnwindSafe}; + + fn catch_and_log_panic Result + UnwindSafe, R>( + context: &str, + f: F, + ) -> Result { + match panic::catch_unwind(f) { + Ok(v) => v, + Err(e) => { + log::error!("caught panic when {}: {:?}", context, e); + Err( + syscall_error!(IllegalArgument; "caught panic when {}: {:?}", context, e) + .into(), + ) + } + } + } + let t = self.call_manager.charge_gas( self.call_manager .price_list() @@ -1130,13 +1148,3 @@ where Ok(()) } } - -fn catch_and_log_panic Result + UnwindSafe, R>(context: &str, f: F) -> Result { - match panic::catch_unwind(f) { - Ok(v) => v, - Err(e) => { - log::error!("caught panic when {}: {:?}", context, e); - Err(syscall_error!(IllegalArgument; "caught panic when {}: {:?}", context, e).into()) - } - } -} diff --git a/fvm/src/kernel/mod.rs b/fvm/src/kernel/mod.rs index 5597e093d..ef48b176b 100644 --- a/fvm/src/kernel/mod.rs +++ b/fvm/src/kernel/mod.rs @@ -220,6 +220,7 @@ pub trait ActorOps { fn balance_of(&self, actor_id: ActorID) -> Result; } +#[cfg(not(feature = "no-verify-signature"))] /// Cryptographic primitives provided by the kernel. #[delegatable_trait] pub trait CryptoOps { @@ -262,6 +263,40 @@ pub trait CryptoOps { fn hash(&self, code: u64, data: &[u8]) -> Result; } +#[cfg(feature = "no-verify-signature")] +/// Cryptographic primitives provided by the kernel. +#[delegatable_trait] +pub trait CryptoOps { + /// Verifies a BLS aggregate signature. In the case where there is one signer/signed plaintext, + /// this is equivalent to verifying a non-aggregated BLS signature. + /// + /// Returns: + /// - `Ok(true)` on a valid signature. + /// - `Ok(false)` on an invalid signature or if the signature or public keys' bytes represent an + /// invalid curve point. + /// - `Err(IllegalArgument)` if `pub_keys.len() != plaintexts.len()`. + fn verify_bls_aggregate( + &self, + aggregate_sig: &[u8; fvm_shared::crypto::signature::BLS_SIG_LEN], + pub_keys: &[[u8; fvm_shared::crypto::signature::BLS_PUB_LEN]], + plaintexts_concat: &[u8], + plaintext_lens: &[u32], + ) -> Result; + + /// Given a message hash and its signature, recovers the public key of the signer. + fn recover_secp_public_key( + &self, + hash: &[u8; SECP_SIG_MESSAGE_HASH_SIZE], + signature: &[u8; SECP_SIG_LEN], + ) -> Result<[u8; SECP_PUB_LEN]>; + + /// Hashes input `data_in` using with the specified hash function, writing the output to + /// `digest_out`, returning the size of the digest written to `digest_out`. If `digest_out` is + /// to small to fit the entire digest, it will be truncated. If too large, the leftover space + /// will not be overwritten. + fn hash(&self, code: u64, data: &[u8]) -> Result; +} + /// Randomness queries. #[delegatable_trait] pub trait RandomnessOps { diff --git a/fvm/src/syscalls/crypto.rs b/fvm/src/syscalls/crypto.rs index eb08aaf86..21fa7e472 100644 --- a/fvm/src/syscalls/crypto.rs +++ b/fvm/src/syscalls/crypto.rs @@ -2,11 +2,9 @@ // SPDX-License-Identifier: Apache-2.0, MIT use std::cmp; -use anyhow::Context as _; use fvm_shared::crypto::signature::{ - SignatureType, BLS_PUB_LEN, BLS_SIG_LEN, SECP_PUB_LEN, SECP_SIG_LEN, SECP_SIG_MESSAGE_HASH_SIZE, + BLS_PUB_LEN, BLS_SIG_LEN, SECP_PUB_LEN, SECP_SIG_LEN, SECP_SIG_MESSAGE_HASH_SIZE, }; -use num_traits::FromPrimitive; use super::Context; use crate::{ @@ -14,6 +12,7 @@ use crate::{ syscall_error, }; +#[cfg(not(feature = "no-verify-signature"))] /// Verifies that a signature is valid for an address and plaintext. /// /// The return i32 indicates the status code of the verification: @@ -30,6 +29,10 @@ pub fn verify_signature( plaintext_off: u32, plaintext_len: u32, ) -> Result { + use anyhow::Context as _; + use fvm_shared::crypto::signature::SignatureType; + use num_traits::FromPrimitive; + let sig_type = SignatureType::from_u32(sig_type) .with_context(|| format!("unknown signature type {}", sig_type)) .or_illegal_argument()?; diff --git a/fvm/src/syscalls/mod.rs b/fvm/src/syscalls/mod.rs index d3848a689..40b1dc9d2 100644 --- a/fvm/src/syscalls/mod.rs +++ b/fvm/src/syscalls/mod.rs @@ -315,7 +315,7 @@ where if cfg!(feature = "m2-native") { linker.link_syscall("actor", "install_actor", actor::install_actor)?; } - + #[cfg(not(feature = "no-verify-signature"))] linker.link_syscall("crypto", "verify_signature", crypto::verify_signature)?; linker.link_syscall( "crypto", diff --git a/sdk/Cargo.toml b/sdk/Cargo.toml index abb42a4ac..bf3f98c1e 100644 --- a/sdk/Cargo.toml +++ b/sdk/Cargo.toml @@ -24,3 +24,7 @@ fvm_ipld_encoding = { workspace = true } default = [] m2-native = [] upgrade-actor = [] +# Use this feature to remove `verify_signature` syscall that is supposed to be removed by FIP-0079, +# The current implementation keeps it by default for backward compatibility reason. +# See +no-verify-signature = [] diff --git a/sdk/src/crypto.rs b/sdk/src/crypto.rs index a2969589a..a342acf64 100644 --- a/sdk/src/crypto.rs +++ b/sdk/src/crypto.rs @@ -21,6 +21,7 @@ use num_traits::FromPrimitive; use crate::{status_code_to_bool, sys, SyscallResult}; +#[cfg(not(feature = "no-verify-signature"))] /// Verifies that a signature is valid for an address and plaintext. /// /// NOTE: This only supports f1 and f3 addresses. @@ -46,6 +47,58 @@ pub fn verify_signature( } } +#[cfg(feature = "no-verify-signature")] +/// Verifies that a signature is valid for an address and plaintext. +/// +/// NOTE: This only supports f1 and f3 addresses. +pub fn verify_signature( + signature: &Signature, + addr: &Address, + plaintext: &[u8], +) -> SyscallResult { + use fvm_shared::{ + address::{Payload, Protocol}, + crypto::signature::SignatureType, + }; + + let sig_type = signature.signature_type(); + let sig_bytes = signature.bytes(); + match sig_type { + SignatureType::BLS => { + let sig: &[u8; BLS_SIG_LEN] = sig_bytes + .try_into() + .map_err(|_| ErrorNumber::IllegalArgument)?; + + let pub_key = match addr.payload() { + Payload::BLS(pub_key) => *pub_key, + _ => return Err(ErrorNumber::IllegalArgument), + }; + + verify_bls_aggregate(sig, &[pub_key], &[plaintext]) + } + SignatureType::Secp256k1 => { + if addr.protocol() != Protocol::Secp256k1 { + return Err(ErrorNumber::IllegalArgument); + } + + let sig: &[u8; SECP_SIG_LEN] = sig_bytes + .try_into() + .map_err(|_| ErrorNumber::IllegalArgument)?; + + let digest = hash_blake2b(plaintext); + + let addr_recovered = { + let pub_key = recover_secp_public_key(&digest, sig)?; + Address::new_secp256k1(&pub_key).expect( + "recovered secp256k1 public key should always be a valid secp256k1 address", + ) + }; + + Ok(addr == &addr_recovered) + } + } +} + pub fn verify_bls_aggregate( sig: &[u8; BLS_SIG_LEN], pub_keys: &[[u8; BLS_PUB_LEN]], diff --git a/sdk/src/sys/crypto.rs b/sdk/src/sys/crypto.rs index bf1a52896..ec26569fd 100644 --- a/sdk/src/sys/crypto.rs +++ b/sdk/src/sys/crypto.rs @@ -28,6 +28,7 @@ super::fvm_syscalls! { /// | Error | Reason | /// |---------------------|------------------------------------------------------| /// | [`IllegalArgument`] | signature, address, or plaintext buffers are invalid | + #[cfg(not(feature = "no-verify-signature"))] pub fn verify_signature( sig_type: u32, sig_off: *const u8, diff --git a/testing/integration/tests/main.rs b/testing/integration/tests/main.rs index 0b83f0a5d..89515c43f 100644 --- a/testing/integration/tests/main.rs +++ b/testing/integration/tests/main.rs @@ -22,8 +22,8 @@ use fvm_shared::version::NetworkVersion; use fvm_test_actors::wasm_bin::{ ADDRESS_ACTOR_BINARY, CREATE_ACTOR_BINARY, CUSTOM_SYSCALL_ACTOR_BINARY, EXIT_DATA_ACTOR_BINARY, HELLO_WORLD_ACTOR_BINARY, IPLD_ACTOR_BINARY, OOM_ACTOR_BINARY, READONLY_ACTOR_BINARY, - SSELF_ACTOR_BINARY, STACK_OVERFLOW_ACTOR_BINARY, SYSCALL_ACTOR_BINARY, UPGRADE_ACTOR_BINARY, - UPGRADE_RECEIVE_ACTOR_BINARY, + SSELF_ACTOR_BINARY, STACK_OVERFLOW_ACTOR_BINARY, SYSCALL_ACTOR_BINARY, + SYSCALL_ACTOR_BINARY_FIP0079, UPGRADE_ACTOR_BINARY, UPGRADE_RECEIVE_ACTOR_BINARY, }; use num_traits::Zero; @@ -140,6 +140,20 @@ fn ipld() { #[test] fn syscalls() { + syscalls_inner(SYSCALL_ACTOR_BINARY) +} + +#[test] +fn syscalls_fip_0079() { + syscalls_inner(SYSCALL_ACTOR_BINARY_FIP0079) +} + +#[test] +fn syscalls_wasm_properly_imported() { + assert_ne!(SYSCALL_ACTOR_BINARY, SYSCALL_ACTOR_BINARY_FIP0079) +} + +fn syscalls_inner(wasm_bin: &[u8]) { // Instantiate tester let mut tester = new_tester( NV_FOR_TEST, @@ -151,8 +165,6 @@ fn syscalls() { let sender: [Account; 1] = tester.create_accounts().unwrap(); tester.set_account_sequence(sender[0].0, 100).unwrap(); - let wasm_bin = SYSCALL_ACTOR_BINARY; - // Set actor state let actor_state = State::default(); let state_cid = tester.set_state(&actor_state).unwrap(); diff --git a/testing/test_actors/actors/fil-syscall-actor/Cargo.toml b/testing/test_actors/actors/fil-syscall-actor/Cargo.toml index 1b9a740c5..a47287ea5 100644 --- a/testing/test_actors/actors/fil-syscall-actor/Cargo.toml +++ b/testing/test_actors/actors/fil-syscall-actor/Cargo.toml @@ -17,3 +17,4 @@ crate-type = ["cdylib"] ## cdylib is necessary for Wasm build [features] coverage = ["minicov"] +no-verify-signature = ["fvm_sdk/no-verify-signature"] diff --git a/testing/test_actors/actors/fil-syscall-actor/src/actor.rs b/testing/test_actors/actors/fil-syscall-actor/src/actor.rs index ba1d2a027..15f191ee0 100644 --- a/testing/test_actors/actors/fil-syscall-actor/src/actor.rs +++ b/testing/test_actors/actors/fil-syscall-actor/src/actor.rs @@ -98,6 +98,7 @@ fn test_secp_signature() { // test that calling sdk::sys::crypto::verify_signature with invalid parameters result // in correct error value // + #[cfg(not(feature = "no-verify-signature"))] unsafe { let sig_type = signature.signature_type(); let sig_bytes = signature.bytes(); diff --git a/testing/test_actors/build.rs b/testing/test_actors/build.rs index d5fdfa71f..c9d99595e 100644 --- a/testing/test_actors/build.rs +++ b/testing/test_actors/build.rs @@ -4,7 +4,7 @@ use std::error::Error; use std::fs::File; use std::io::{BufRead, BufReader, BufWriter, Write}; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; +use std::process::{Child, Command, ExitStatus, Stdio}; use std::thread; const ACTORS: &[(&str, &str)] = &[ @@ -55,7 +55,7 @@ fn main() -> Result<(), Box> { } // Cargo build command for all actors at once. - let mut cmd = Command::new(cargo); + let mut cmd = Command::new(cargo.clone()); cmd.arg("build") .args(ACTORS.iter().map(|(_, pkg)| "-p=".to_owned() + pkg)) .arg(format!("--target={WASM_TARGET}")) @@ -76,8 +76,77 @@ fn main() -> Result<(), Box> { println!("cargo:warning=cmd={:?}", &cmd); // Launch the command. - let mut child = cmd.spawn().expect("failed to launch cargo build"); + let child = cmd.spawn().expect("failed to launch cargo build"); + let result = wait_cmd_and_print_output(child)?; + if !result.success() { + return Err("actor build failed".into()); + } + let wasm_bin_file = + File::create(out_dir.join("wasm_bin.rs")).expect("failed to create manifest"); + let mut wasm_bin_file = BufWriter::new(wasm_bin_file); + for (var, pkg) in ACTORS { + let bin = bundle_dir + .join(WASM_TARGET) + .join("wasm") + .join(format!("{pkg}.wasm")); + let moved_bin = bundle_dir.join(format!("{var}.wasm")); + std::fs::rename(bin, &moved_bin).unwrap(); + writeln!( + &mut wasm_bin_file, + "pub const {var}: &[u8] = include_bytes!({moved_bin:?});" + ) + .expect("failed to write to manifest"); + } + + // Generate syscall actor with no-verify-signature feature. + { + let (var, pkg) = ("SYSCALL_ACTOR_BINARY_FIP0079", "fil_syscall_actor"); + let mut cmd = Command::new(cargo); + cmd.arg("build") + .arg(format!("-p={pkg}")) + // .arg(format!("--features=no-verify-signature")) + .arg(format!("--target={WASM_TARGET}")) + .arg("--profile=wasm") + .arg("--locked") + .arg("--manifest-path=".to_owned() + manifest_path.to_str().unwrap()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + // We are supposed to only generate artifacts under OUT_DIR, + // so set OUT_DIR as the target directory for this build. + .env("CARGO_TARGET_DIR", &bundle_dir) + // As we are being called inside a build-script, this env variable is set. However, we set + // our own `RUSTFLAGS` and thus, we need to remove this. Otherwise cargo favors this + // env variable. + .env_remove("CARGO_ENCODED_RUSTFLAGS"); + + // Print out the command line we're about to run. + println!("cargo:warning=cmd={:?}", &cmd); + + // Launch the command. + let child = cmd.spawn().expect("failed to launch cargo build"); + let result = wait_cmd_and_print_output(child)?; + if !result.success() { + return Err("actor build failed".into()); + } + let bin = bundle_dir + .join(WASM_TARGET) + .join("wasm") + .join(format!("{pkg}.wasm")); + let moved_bin = bundle_dir.join(format!("{var}.wasm")); + std::fs::rename(bin, &moved_bin).unwrap(); + writeln!( + &mut wasm_bin_file, + "pub const {var}: &[u8] = include_bytes!({moved_bin:?});" + ) + .expect("failed to write to manifest"); + } + + wasm_bin_file.flush().expect("failed to flush manifest"); + Ok(()) +} + +fn wait_cmd_and_print_output(mut child: Child) -> Result> { // Pipe the output as cargo warnings. Unfortunately this is the only way to // get cargo build to print the output. let stdout = child.stdout.take().expect("no stdout"); @@ -97,24 +166,5 @@ fn main() -> Result<(), Box> { j2.join().unwrap(); let result = child.wait().expect("failed to wait for build to finish"); - if !result.success() { - return Err("actor build failed".into()); - } - - let wasm_bin_file = - File::create(out_dir.join("wasm_bin.rs")).expect("failed to create manifest"); - let mut wasm_bin_file = BufWriter::new(wasm_bin_file); - for (var, pkg) in ACTORS { - let bin = bundle_dir - .join(WASM_TARGET) - .join("wasm") - .join(format!("{pkg}.wasm")); - writeln!( - &mut wasm_bin_file, - "pub const {var}: &[u8] = include_bytes!({bin:?});" - ) - .expect("failed to write to manifest"); - } - wasm_bin_file.flush().expect("failed to flush manifest"); - Ok(()) + Ok(result) } From 3d8fab236b5f33154fed32c6d5761f6c03c77efd Mon Sep 17 00:00:00 2001 From: hanabi1224 Date: Fri, 3 May 2024 06:20:46 +0800 Subject: [PATCH 3/4] use feature `verify-signature` instead of `no-verify-signature` --- .github/workflows/ci.yml | 6 +++--- fvm/Cargo.toml | 6 +++--- fvm/src/kernel/default.rs | 2 +- fvm/src/kernel/mod.rs | 4 ++-- fvm/src/syscalls/crypto.rs | 2 +- fvm/src/syscalls/mod.rs | 2 +- sdk/Cargo.toml | 6 +++--- sdk/src/crypto.rs | 4 ++-- sdk/src/sys/crypto.rs | 2 +- testing/integration/Cargo.toml | 2 +- testing/test_actors/actors/fil-syscall-actor/Cargo.toml | 2 +- testing/test_actors/actors/fil-syscall-actor/src/actor.rs | 2 +- testing/test_actors/build.rs | 4 ++-- 13 files changed, 22 insertions(+), 22 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8c888efd4..47aa31338 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,7 +28,7 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, macos-latest] - name: [build, check-m2-native, check-clippy, test-fvm, test, integration, conformance, calibration] + name: [build, check-m2-native, check-clippy, check-clippy-verify-signature, test-fvm, test, integration, conformance, calibration] include: - name: build key: v3 @@ -47,11 +47,11 @@ jobs: command: clippy # we disable default features because rust will otherwise unify them and turn on opencl in CI. args: --all --all-targets --no-default-features - - name: check-clippy(no-verify-signature) + - name: check-clippy-verify-signature key: v3 command: clippy # we disable default features because rust will otherwise unify them and turn on opencl in CI. - args: --all --all-targets --no-default-features --features no-verify-signature + args: --all --all-targets --no-default-features --features verify-signature - name: test-fvm key: v3-cov push: true diff --git a/fvm/Cargo.toml b/fvm/Cargo.toml index fbc812aa7..dde45376c 100644 --- a/fvm/Cargo.toml +++ b/fvm/Cargo.toml @@ -48,7 +48,7 @@ fvm = { path = ".", features = ["testing"], default-features = false } coverage-helper = { workspace = true } [features] -default = ["opencl"] +default = ["opencl", "verify-signature"] opencl = ["filecoin-proofs-api/opencl"] cuda = ["filecoin-proofs-api/cuda"] cuda-supraseal = ["filecoin-proofs-api/cuda-supraseal"] @@ -58,7 +58,7 @@ m2-native = [] upgrade-actor = [] gas_calibration = [] nv23-dev = [] -# Use this feature to remove `verify_signature` syscall that is supposed to be removed by FIP-0079, +# Use this feature to keep `verify_signature` syscall that is supposed to be removed by FIP-0079, # The current implementation keeps it by default for backward compatibility reason. # See -no-verify-signature = [] +verify-signature = [] diff --git a/fvm/src/kernel/default.rs b/fvm/src/kernel/default.rs index 01793d45d..b6814c7bd 100644 --- a/fvm/src/kernel/default.rs +++ b/fvm/src/kernel/default.rs @@ -573,7 +573,7 @@ impl CryptoOps for DefaultKernel where C: CallManager, { - #[cfg(not(feature = "no-verify-signature"))] + #[cfg(feature = "verify-signature")] fn verify_signature( &self, sig_type: SignatureType, diff --git a/fvm/src/kernel/mod.rs b/fvm/src/kernel/mod.rs index ef48b176b..0c40eeeda 100644 --- a/fvm/src/kernel/mod.rs +++ b/fvm/src/kernel/mod.rs @@ -220,7 +220,7 @@ pub trait ActorOps { fn balance_of(&self, actor_id: ActorID) -> Result; } -#[cfg(not(feature = "no-verify-signature"))] +#[cfg(feature = "verify-signature")] /// Cryptographic primitives provided by the kernel. #[delegatable_trait] pub trait CryptoOps { @@ -263,7 +263,7 @@ pub trait CryptoOps { fn hash(&self, code: u64, data: &[u8]) -> Result; } -#[cfg(feature = "no-verify-signature")] +#[cfg(not(feature = "verify-signature"))] /// Cryptographic primitives provided by the kernel. #[delegatable_trait] pub trait CryptoOps { diff --git a/fvm/src/syscalls/crypto.rs b/fvm/src/syscalls/crypto.rs index 21fa7e472..025242b4d 100644 --- a/fvm/src/syscalls/crypto.rs +++ b/fvm/src/syscalls/crypto.rs @@ -12,7 +12,7 @@ use crate::{ syscall_error, }; -#[cfg(not(feature = "no-verify-signature"))] +#[cfg(feature = "verify-signature")] /// Verifies that a signature is valid for an address and plaintext. /// /// The return i32 indicates the status code of the verification: diff --git a/fvm/src/syscalls/mod.rs b/fvm/src/syscalls/mod.rs index 40b1dc9d2..cd6f08f48 100644 --- a/fvm/src/syscalls/mod.rs +++ b/fvm/src/syscalls/mod.rs @@ -315,7 +315,7 @@ where if cfg!(feature = "m2-native") { linker.link_syscall("actor", "install_actor", actor::install_actor)?; } - #[cfg(not(feature = "no-verify-signature"))] + #[cfg(feature = "verify-signature")] linker.link_syscall("crypto", "verify_signature", crypto::verify_signature)?; linker.link_syscall( "crypto", diff --git a/sdk/Cargo.toml b/sdk/Cargo.toml index bf3f98c1e..6d918fc75 100644 --- a/sdk/Cargo.toml +++ b/sdk/Cargo.toml @@ -21,10 +21,10 @@ fvm_shared = { workspace = true } fvm_ipld_encoding = { workspace = true } [features] -default = [] +default = ["verify-signature"] m2-native = [] upgrade-actor = [] -# Use this feature to remove `verify_signature` syscall that is supposed to be removed by FIP-0079, +# Use this feature to keep `verify_signature` syscall that is supposed to be removed by FIP-0079, # The current implementation keeps it by default for backward compatibility reason. # See -no-verify-signature = [] +verify-signature = [] diff --git a/sdk/src/crypto.rs b/sdk/src/crypto.rs index a342acf64..1c5ca918b 100644 --- a/sdk/src/crypto.rs +++ b/sdk/src/crypto.rs @@ -21,7 +21,7 @@ use num_traits::FromPrimitive; use crate::{status_code_to_bool, sys, SyscallResult}; -#[cfg(not(feature = "no-verify-signature"))] +#[cfg(feature = "verify-signature")] /// Verifies that a signature is valid for an address and plaintext. /// /// NOTE: This only supports f1 and f3 addresses. @@ -47,7 +47,7 @@ pub fn verify_signature( } } -#[cfg(feature = "no-verify-signature")] +#[cfg(not(feature = "verify-signature"))] /// Verifies that a signature is valid for an address and plaintext. /// /// NOTE: This only supports f1 and f3 addresses. diff --git a/sdk/src/sys/crypto.rs b/sdk/src/sys/crypto.rs index ec26569fd..695bf2d0f 100644 --- a/sdk/src/sys/crypto.rs +++ b/sdk/src/sys/crypto.rs @@ -28,7 +28,7 @@ super::fvm_syscalls! { /// | Error | Reason | /// |---------------------|------------------------------------------------------| /// | [`IllegalArgument`] | signature, address, or plaintext buffers are invalid | - #[cfg(not(feature = "no-verify-signature"))] + #[cfg(feature = "verify-signature")] pub fn verify_signature( sig_type: u32, sig_off: *const u8, diff --git a/testing/integration/Cargo.toml b/testing/integration/Cargo.toml index e54b968e7..234139ee2 100644 --- a/testing/integration/Cargo.toml +++ b/testing/integration/Cargo.toml @@ -8,7 +8,7 @@ repository.workspace = true authors = ["Protocol Labs", "Filecoin Core Devs", "Polyphene"] [dependencies] -fvm = { workspace = true, default-features = false, features = ["testing", "upgrade-actor"] } +fvm = { workspace = true, default-features = false, features = ["testing", "upgrade-actor", "verify-signature"] } fvm_shared = { workspace = true, features = ["testing"] } fvm_ipld_car = { workspace = true } fvm_ipld_blockstore = { workspace = true } diff --git a/testing/test_actors/actors/fil-syscall-actor/Cargo.toml b/testing/test_actors/actors/fil-syscall-actor/Cargo.toml index a47287ea5..112a1af46 100644 --- a/testing/test_actors/actors/fil-syscall-actor/Cargo.toml +++ b/testing/test_actors/actors/fil-syscall-actor/Cargo.toml @@ -17,4 +17,4 @@ crate-type = ["cdylib"] ## cdylib is necessary for Wasm build [features] coverage = ["minicov"] -no-verify-signature = ["fvm_sdk/no-verify-signature"] +verify-signature = ["fvm_sdk/verify-signature"] diff --git a/testing/test_actors/actors/fil-syscall-actor/src/actor.rs b/testing/test_actors/actors/fil-syscall-actor/src/actor.rs index 15f191ee0..30ce98db3 100644 --- a/testing/test_actors/actors/fil-syscall-actor/src/actor.rs +++ b/testing/test_actors/actors/fil-syscall-actor/src/actor.rs @@ -98,7 +98,7 @@ fn test_secp_signature() { // test that calling sdk::sys::crypto::verify_signature with invalid parameters result // in correct error value // - #[cfg(not(feature = "no-verify-signature"))] + #[cfg(feature = "verify-signature")] unsafe { let sig_type = signature.signature_type(); let sig_bytes = signature.bytes(); diff --git a/testing/test_actors/build.rs b/testing/test_actors/build.rs index c9d99595e..eed96a14b 100644 --- a/testing/test_actors/build.rs +++ b/testing/test_actors/build.rs @@ -99,13 +99,13 @@ fn main() -> Result<(), Box> { .expect("failed to write to manifest"); } - // Generate syscall actor with no-verify-signature feature. + // Generate syscall actor with verify-signature feature. { let (var, pkg) = ("SYSCALL_ACTOR_BINARY_FIP0079", "fil_syscall_actor"); let mut cmd = Command::new(cargo); cmd.arg("build") .arg(format!("-p={pkg}")) - // .arg(format!("--features=no-verify-signature")) + .arg("--features=verify-signature") .arg(format!("--target={WASM_TARGET}")) .arg("--profile=wasm") .arg("--locked") From 169d61552ab897fd4f31217ec70a3c6e2b98c174 Mon Sep 17 00:00:00 2001 From: hanabi1224 Date: Tue, 7 May 2024 09:15:38 +0800 Subject: [PATCH 4/4] feature flag sig_cost and on_verify_signature --- fvm/src/gas/price_list.rs | 4 ++++ testing/test_actors/actors/fil-syscall-actor/Cargo.toml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/fvm/src/gas/price_list.rs b/fvm/src/gas/price_list.rs index 7f79a80e5..37c56bd37 100644 --- a/fvm/src/gas/price_list.rs +++ b/fvm/src/gas/price_list.rs @@ -7,6 +7,7 @@ use std::ops::Mul; use anyhow::Context; use fvm_shared::clock::ChainEpoch; +#[cfg(feature = "verify-signature")] use fvm_shared::crypto::signature::SignatureType; use fvm_shared::piece::PieceInfo; use fvm_shared::sector::{ @@ -102,6 +103,7 @@ lazy_static! { address_lookup: Gas::new(1_050_000), address_assignment: Gas::new(1_000_000), + #[cfg(feature = "verify-signature")] sig_cost: total_enum_map!{ SignatureType { Secp256k1 => ScalingCost { @@ -404,6 +406,7 @@ pub struct PriceList { pub(crate) actor_create_storage: Gas, /// Gas cost for verifying a cryptographic signature. + #[cfg(feature = "verify-signature")] pub(crate) sig_cost: HashMap, /// Gas cost for recovering secp256k1 signer public key @@ -599,6 +602,7 @@ impl PriceList { } /// Returns gas required for signature verification. + #[cfg(feature = "verify-signature")] #[inline] pub fn on_verify_signature(&self, sig_type: SignatureType, data_len: usize) -> GasCharge { let cost = self.sig_cost[&sig_type]; diff --git a/testing/test_actors/actors/fil-syscall-actor/Cargo.toml b/testing/test_actors/actors/fil-syscall-actor/Cargo.toml index 112a1af46..29fc9c27c 100644 --- a/testing/test_actors/actors/fil-syscall-actor/Cargo.toml +++ b/testing/test_actors/actors/fil-syscall-actor/Cargo.toml @@ -6,7 +6,7 @@ publish = false [target.'cfg(target_arch = "wasm32")'.dependencies] fvm_ipld_encoding = { workspace = true } -fvm_sdk = { workspace = true } +fvm_sdk = { workspace = true, default-features = false } fvm_shared = { workspace = true } multihash = { workspace = true, features = ["sha3", "sha2", "ripemd"] } minicov = {version = "0.3", optional = true}