diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 5bdf8ce010a1..e31c53237be2 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -133,7 +133,7 @@ pub mod bls_crypto { ::Output: Into<[u8; 32]>, { fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { - // `w3f-bls` library uses IETF hashing standard and as such does not exposes + // `w3f-bls` library uses IETF hashing standard and as such does not expose // a choice of hash to field function. // We are directly calling into the library to avoid introducing new host call. // and because BeefyAuthorityId::verify is being called in the runtime so we don't have @@ -157,7 +157,7 @@ pub mod bls_crypto { pub mod ecdsa_bls_crypto { use super::{BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; use sp_application_crypto::{app_crypto, ecdsa_bls377}; - use sp_core::{crypto::Wraps, ecdsa_bls377::Pair as EcdsaBlsPair, Pair as _}; + use sp_core::{crypto::Wraps, ecdsa_bls377::Pair as EcdsaBlsPair}; app_crypto!(ecdsa_bls377, KEY_TYPE); @@ -167,17 +167,24 @@ pub mod ecdsa_bls_crypto { /// Signature for a BEEFY authority using (ECDSA,BLS) as its crypto. pub type AuthoritySignature = Signature; - impl BeefyAuthorityId for AuthorityId + impl BeefyAuthorityId for AuthorityId where - ::Output: Into<[u8; 32]>, + H: Hash, + H::Output: Into<[u8; 32]>, { fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { - // `w3f-bls` library uses IETF hashing standard and as such does not exposes - // a choice of hash to field function. - // We are directly calling into the library to avoid introducing new host call. - // and because BeefyAuthorityId::verify is being called in the runtime so we don't have - - EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref()) + // We can not simply call + // `EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref())` + // because that invokes ECDSA default verification which perfoms Blake2b hash + // which we don't want. This is because ECDSA signatures are meant to be verified + // on Ethereum network where Keccak hasher is significantly cheaper than Blake2b. + // See Figure 3 of [OnSc21](https://www.scitepress.org/Papers/2021/106066/106066.pdf) + // for comparison. + EcdsaBlsPair::verify_with_hasher::( + signature.as_inner_ref(), + msg, + self.as_inner_ref(), + ) } } } @@ -257,6 +264,7 @@ pub enum ConsensusLog { /// /// A vote message is a direct vote created by a BEEFY node on every voting round /// and is gossiped to its peers. +// TODO: Remove `Signature` generic type, instead get it from `Id::Signature`. #[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] pub struct VoteMessage { /// Commit to information extracted from a finalized block @@ -507,11 +515,15 @@ mod tests { let msg = &b"test-message"[..]; let (pair, _) = ecdsa_bls_crypto::Pair::generate(); - let signature: ecdsa_bls_crypto::Signature = pair.as_inner_ref().sign(&msg).into(); + let signature: ecdsa_bls_crypto::Signature = + pair.as_inner_ref().sign_with_hasher::(&msg).into(); // Verification works if same hashing function is used when signing and verifying. assert!(BeefyAuthorityId::::verify(&pair.public(), &signature, msg)); + // Verification doesn't work if we verify function provided by pair_crypto implementation + assert!(!ecdsa_bls_crypto::Pair::verify(&signature, msg, &pair.public())); + // Other public key doesn't work let (other_pair, _) = ecdsa_bls_crypto::Pair::generate(); assert!(!BeefyAuthorityId::::verify(&other_pair.public(), &signature, msg,)); diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index a97b657e7578..edf6156f268e 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -39,7 +39,13 @@ use sp_std::convert::TryFrom; /// ECDSA and BLS12-377 paired crypto scheme #[cfg(feature = "bls-experimental")] pub mod ecdsa_bls377 { - use crate::{bls377, crypto::CryptoTypeId, ecdsa}; + #[cfg(feature = "full_crypto")] + use crate::Hasher; + use crate::{ + bls377, + crypto::{CryptoTypeId, Pair as PairT, UncheckedFrom}, + ecdsa, + }; /// An identifier used to match public keys against BLS12-377 keys pub const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ecb7"); @@ -71,6 +77,60 @@ pub mod ecdsa_bls377 { impl super::CryptoType for Pair { type Pair = Pair; } + + #[cfg(feature = "full_crypto")] + impl Pair { + /// Hashes the `message` with the specified [`Hasher`] before signing sith the ECDSA secret + /// component. + /// + /// The hasher does not affect the BLS12-377 component. This generates BLS12-377 Signature + /// according to IETF standard. + pub fn sign_with_hasher(&self, message: &[u8]) -> Signature + where + H: Hasher, + H::Out: Into<[u8; 32]>, + { + let msg_hash = H::hash(message).into(); + + let mut raw: [u8; SIGNATURE_LEN] = [0u8; SIGNATURE_LEN]; + raw[..ecdsa::SIGNATURE_SERIALIZED_SIZE] + .copy_from_slice(self.left.sign_prehashed(&msg_hash).as_ref()); + raw[ecdsa::SIGNATURE_SERIALIZED_SIZE..] + .copy_from_slice(self.right.sign(message).as_ref()); + ::Signature::unchecked_from(raw) + } + + /// Hashes the `message` with the specified [`Hasher`] before verifying with the ECDSA + /// public component. + /// + /// The hasher does not affect the the BLS12-377 component. This verifies whether the + /// BLS12-377 signature was hashed and signed according to IETF standard + pub fn verify_with_hasher(sig: &Signature, message: &[u8], public: &Public) -> bool + where + H: Hasher, + H::Out: Into<[u8; 32]>, + { + let msg_hash = H::hash(message).into(); + + let Ok(left_pub) = public.0[..ecdsa::PUBLIC_KEY_SERIALIZED_SIZE].try_into() else { + return false + }; + let Ok(left_sig) = sig.0[0..ecdsa::SIGNATURE_SERIALIZED_SIZE].try_into() else { + return false + }; + if !ecdsa::Pair::verify_prehashed(&left_sig, &msg_hash, &left_pub) { + return false + } + + let Ok(right_pub) = public.0[ecdsa::PUBLIC_KEY_SERIALIZED_SIZE..].try_into() else { + return false + }; + let Ok(right_sig) = sig.0[ecdsa::SIGNATURE_SERIALIZED_SIZE..].try_into() else { + return false + }; + bls377::Pair::verify(&right_sig, message.as_ref(), &right_pub) + } + } } /// Secure seed length. @@ -455,12 +515,12 @@ where #[cfg(all(test, feature = "bls-experimental"))] mod test { use super::*; - use crate::crypto::DEV_PHRASE; + use crate::{crypto::DEV_PHRASE, KeccakHasher}; use ecdsa_bls377::{Pair, Signature}; use crate::{bls377, ecdsa}; - #[test] + #[test] fn test_length_of_paired_ecdsa_and_bls377_public_key_and_signature_is_correct() { assert_eq!( ::Public::LEN, @@ -617,6 +677,16 @@ mod test { assert_eq!(cmp, public); } + #[test] + fn sign_and_verify_with_hasher_works() { + let pair = + Pair::from_seed(&(b"12345678901234567890123456789012".as_slice().try_into().unwrap())); + let message = b"Something important"; + let signature = pair.sign_with_hasher::(&message[..]); + + assert!(Pair::verify_with_hasher::(&signature, &message[..], &pair.public())); + } + #[test] fn signature_serialization_works() { let pair =