diff --git a/.changelog/unreleased/miscellaneous/1958-k256.md b/.changelog/unreleased/miscellaneous/1958-k256.md new file mode 100644 index 0000000000..82a38bb863 --- /dev/null +++ b/.changelog/unreleased/miscellaneous/1958-k256.md @@ -0,0 +1,2 @@ +- Switched from using `libsecp256k1` to `k256` crate. + ([\#1958](https://github.com/anoma/namada/pull/1958)) \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index 63ae872e56..41e6231cbd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1832,6 +1832,7 @@ dependencies = [ "digest 0.10.6", "elliptic-curve", "rfc6979", + "serdect", "signature 2.1.0", "spki", ] @@ -1898,6 +1899,7 @@ dependencies = [ "pkcs8", "rand_core 0.6.4", "sec1", + "serdect", "subtle 2.4.1", "zeroize", ] @@ -3032,17 +3034,6 @@ dependencies = [ "hmac 0.7.1", ] -[[package]] -name = "hmac-drbg" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "17ea0a1394df5b6574da6e0c1ade9e78868c9fb0a4e5ef4428e32da4676b85b1" -dependencies = [ - "digest 0.9.0", - "generic-array 0.14.7", - "hmac 0.8.1", -] - [[package]] name = "hmac-sha512" version = "0.1.9" @@ -3482,6 +3473,7 @@ dependencies = [ "ecdsa", "elliptic-curve", "once_cell", + "serdect", "sha2 0.10.6", "signature 2.1.0", ] @@ -3587,57 +3579,13 @@ dependencies = [ "arrayref", "crunchy", "digest 0.8.1", - "hmac-drbg 0.2.0", + "hmac-drbg", "rand 0.7.3", "sha2 0.8.2", "subtle 2.4.1", "typenum", ] -[[package]] -name = "libsecp256k1" -version = "0.7.0" -source = "git+https://github.com/heliaxdev/libsecp256k1?rev=bbb3bd44a49db361f21d9db80f9a087c194c0ae9#bbb3bd44a49db361f21d9db80f9a087c194c0ae9" -dependencies = [ - "arrayref", - "base64 0.13.1", - "digest 0.9.0", - "hmac-drbg 0.3.0", - "libsecp256k1-core", - "libsecp256k1-gen-ecmult", - "libsecp256k1-gen-genmult", - "rand 0.8.5", - "serde 1.0.163", - "sha2 0.9.9", - "typenum", -] - -[[package]] -name = "libsecp256k1-core" -version = "0.3.0" -source = "git+https://github.com/heliaxdev/libsecp256k1?rev=bbb3bd44a49db361f21d9db80f9a087c194c0ae9#bbb3bd44a49db361f21d9db80f9a087c194c0ae9" -dependencies = [ - "crunchy", - "digest 0.9.0", - "subtle 2.4.1", -] - -[[package]] -name = "libsecp256k1-gen-ecmult" -version = "0.3.0" -source = "git+https://github.com/heliaxdev/libsecp256k1?rev=bbb3bd44a49db361f21d9db80f9a087c194c0ae9#bbb3bd44a49db361f21d9db80f9a087c194c0ae9" -dependencies = [ - "libsecp256k1-core", -] - -[[package]] -name = "libsecp256k1-gen-genmult" -version = "0.3.0" -source = "git+https://github.com/heliaxdev/libsecp256k1?rev=bbb3bd44a49db361f21d9db80f9a087c194c0ae9#bbb3bd44a49db361f21d9db80f9a087c194c0ae9" -dependencies = [ - "libsecp256k1-core", -] - [[package]] name = "libssh2-sys" version = "0.2.23" @@ -4029,7 +3977,7 @@ dependencies = [ "fd-lock", "futures", "itertools", - "libsecp256k1 0.7.0", + "k256", "loupe", "masp_primitives", "masp_proofs", @@ -4215,7 +4163,7 @@ dependencies = [ "impl-num-traits", "index-set", "itertools", - "libsecp256k1 0.7.0", + "k256", "masp_primitives", "namada_macros", "num-integer", @@ -6085,6 +6033,7 @@ dependencies = [ "der", "generic-array 0.14.7", "pkcs8", + "serdect", "subtle 2.4.1", "zeroize", ] @@ -6263,6 +6212,16 @@ dependencies = [ "yaml-rust", ] +[[package]] +name = "serdect" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a84f14a19e9a014bb9f4512488d9829a68e04ecabffb0f9904cd1ace94598177" +dependencies = [ + "base16ct", + "serde 1.0.163", +] + [[package]] name = "sha1" version = "0.10.5" @@ -6856,7 +6815,7 @@ checksum = "01b874a4992538d4b2f4fbbac11b9419d685f4b39bdc3fed95b04e07bfd76040" dependencies = [ "base58 0.1.0", "hmac 0.7.1", - "libsecp256k1 0.3.5", + "libsecp256k1", "memzero", "sha2 0.8.2", ] diff --git a/Cargo.toml b/Cargo.toml index 813cf99d9a..3af3afa7cb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -86,10 +86,10 @@ git2 = "0.13.25" ics23 = "0.9.0" index-set = {git = "https://github.com/heliaxdev/index-set", tag = "v0.7.1", features = ["serialize-borsh", "serialize-serde"]} itertools = "0.10.0" +k256 = { version = "0.13.0", default-features = false, features = ["ecdsa", "pkcs8", "precomputed-tables", "serde", "std"]} lazy_static = "1.4.0" libc = "0.2.97" libloading = "0.7.2" -libsecp256k1 = {git = "https://github.com/heliaxdev/libsecp256k1", rev = "bbb3bd44a49db361f21d9db80f9a087c194c0ae9", default-features = false, features = ["std", "static-context"]} # branch = "murisi/namada-integration" masp_primitives = { git = "https://github.com/anoma/masp", rev = "50acc5028fbcd52a05970fe7991c7850ab04358e" } masp_proofs = { git = "https://github.com/anoma/masp", rev = "50acc5028fbcd52a05970fe7991c7850ab04358e", default-features = false, features = ["local-prover"] } diff --git a/apps/src/lib/node/ledger/shell/mod.rs b/apps/src/lib/node/ledger/shell/mod.rs index f871decbb4..8977a319a0 100644 --- a/apps/src/lib/node/ledger/shell/mod.rs +++ b/apps/src/lib/node/ledger/shell/mod.rs @@ -1617,11 +1617,11 @@ mod test_utils { ref sig, ref recovery_id, )) => { - let mut sig_bytes = sig.serialize(); - let recovery_id_bytes = recovery_id.serialize(); + let mut sig_bytes = sig.to_vec(); + let recovery_id_bytes = recovery_id.to_byte(); sig_bytes[0] = sig_bytes[0].wrapping_add(1); let bytes: [u8; 65] = - [sig_bytes.as_slice(), [recovery_id_bytes].as_slice()] + [sig_bytes.as_slice(), &[recovery_id_bytes]] .concat() .try_into() .unwrap(); diff --git a/core/Cargo.toml b/core/Cargo.toml index ebbb35f383..0dbd4b1822 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -25,11 +25,6 @@ ferveo-tpke = [ wasm-runtime = [ "rayon", ] -# secp256k1 key signing, disabled in WASM build by default as it bloats the -# build a lot -secp256k1-sign = [ - "libsecp256k1/hmac", -] abciplus = [ "ibc", @@ -78,7 +73,7 @@ ics23.workspace = true impl-num-traits = "0.1.2" index-set.workspace = true itertools.workspace = true -libsecp256k1.workspace = true +k256.workspace = true masp_primitives.workspace = true num256.workspace = true num-integer = "0.1.45" @@ -104,7 +99,6 @@ zeroize.workspace = true [dev-dependencies] assert_matches.workspace = true -libsecp256k1 = {workspace = true, features = ["hmac"]} pretty_assertions.workspace = true proptest.workspace = true rand.workspace = true diff --git a/core/src/types/key/mod.rs b/core/src/types/key/mod.rs index 1287956b13..e6e52f108b 100644 --- a/core/src/types/key/mod.rs +++ b/core/src/types/key/mod.rs @@ -692,17 +692,15 @@ mod more_tests { fn zeroize_keypair_secp256k1() { use rand::thread_rng; - let mut sk = secp256k1::SigScheme::generate(&mut thread_rng()); - let sk_scalar = sk.0.to_scalar_ref(); - let len = sk_scalar.0.len(); - let ptr = sk_scalar.0.as_ref().as_ptr(); - - let original_data = sk_scalar.0; - + let sk = secp256k1::SigScheme::generate(&mut thread_rng()); + let (ptr, original_data) = { + let sk_scalar = sk.0.as_scalar_primitive().as_ref(); + (sk_scalar.as_ptr(), sk_scalar.to_owned()) + }; drop(sk); - assert_ne!(&original_data, unsafe { - core::slice::from_raw_parts(ptr, len) + assert_ne!(original_data.as_slice(), unsafe { + core::slice::from_raw_parts(ptr, secp256k1::SECRET_KEY_SIZE) }); } } diff --git a/core/src/types/key/secp256k1.rs b/core/src/types/key/secp256k1.rs index 6fde8af5cd..e65ae7fcbd 100644 --- a/core/src/types/key/secp256k1.rs +++ b/core/src/types/key/secp256k1.rs @@ -9,9 +9,9 @@ use std::str::FromStr; use borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; use data_encoding::HEXLOWER; -use ethabi::ethereum_types::U256; use ethabi::Token; -use libsecp256k1::RecoveryId; +use k256::ecdsa::RecoveryId; +use k256::elliptic_curve::sec1::ToEncodedPoint; #[cfg(feature = "rand")] use rand::{CryptoRng, RngCore}; use serde::de::{Error, SeqAccess, Visitor}; @@ -22,7 +22,6 @@ use super::{ ParsePublicKeyError, ParseSecretKeyError, ParseSignatureError, RefTo, SchemeType, SigScheme as SigSchemeTrait, SignableBytes, VerifySigError, }; -use crate::hints; use crate::types::eth_abi::Encode; use crate::types::ethereum_events::EthAddress; use crate::types::key::StorageHasher; @@ -30,11 +29,16 @@ use crate::types::key::StorageHasher; /// The provided constant is for a traditional /// signature on this curve. For Ethereum, an extra byte is included /// that prevents malleability attacks. -pub const SIGNATURE_LENGTH: usize = libsecp256k1::util::SIGNATURE_SIZE + 1; +pub const SIGNATURE_SIZE: usize = 64 + 1; /// secp256k1 public key #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] -pub struct PublicKey(pub libsecp256k1::PublicKey); +pub struct PublicKey(pub k256::PublicKey); + +/// Size of a compressed public key bytes +const COMPRESSED_PUBLIC_KEY_SIZE: usize = 33; +/// Size of a secret key bytes +pub(crate) const SECRET_KEY_SIZE: usize = 32; impl super::PublicKey for PublicKey { const TYPE: SchemeType = SigScheme::TYPE; @@ -59,26 +63,23 @@ impl super::PublicKey for PublicKey { impl BorshDeserialize for PublicKey { fn deserialize(buf: &mut &[u8]) -> std::io::Result { // deserialize the bytes first - let pk = libsecp256k1::PublicKey::parse_compressed( - buf.get(0..libsecp256k1::util::COMPRESSED_PUBLIC_KEY_SIZE) - .ok_or_else(|| std::io::Error::from(ErrorKind::UnexpectedEof))? - .try_into() - .unwrap(), - ) - .map_err(|e| { + let bytes = buf + .get(0..COMPRESSED_PUBLIC_KEY_SIZE) + .ok_or_else(|| std::io::Error::from(ErrorKind::UnexpectedEof))?; + let pk = k256::PublicKey::from_sec1_bytes(bytes).map_err(|e| { std::io::Error::new( ErrorKind::InvalidInput, format!("Error decoding secp256k1 public key: {}", e), ) })?; - *buf = &buf[libsecp256k1::util::COMPRESSED_PUBLIC_KEY_SIZE..]; + *buf = &buf[COMPRESSED_PUBLIC_KEY_SIZE..]; Ok(PublicKey(pk)) } } impl BorshSerialize for PublicKey { fn serialize(&self, writer: &mut W) -> std::io::Result<()> { - writer.write_all(&self.0.serialize_compressed())?; + writer.write_all(&self.0.to_sec1_bytes())?; Ok(()) } } @@ -92,7 +93,7 @@ impl BorshSchema for PublicKey { ) { // Encoded as `[u8; COMPRESSED_PUBLIC_KEY_SIZE]` let elements = "u8".into(); - let length = libsecp256k1::util::COMPRESSED_PUBLIC_KEY_SIZE as u32; + let length = COMPRESSED_PUBLIC_KEY_SIZE as u32; let definition = borsh::schema::Definition::Array { elements, length }; definitions.insert(Self::declaration(), definition); } @@ -105,29 +106,25 @@ impl BorshSchema for PublicKey { #[allow(clippy::derived_hash_with_manual_eq)] impl Hash for PublicKey { fn hash(&self, state: &mut H) { - self.0.serialize_compressed().hash(state); + self.0.to_sec1_bytes().hash(state); } } impl PartialOrd for PublicKey { fn partial_cmp(&self, other: &Self) -> Option { - self.0 - .serialize_compressed() - .partial_cmp(&other.0.serialize_compressed()) + self.0.to_sec1_bytes().partial_cmp(&other.0.to_sec1_bytes()) } } impl Ord for PublicKey { fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.0 - .serialize_compressed() - .cmp(&other.0.serialize_compressed()) + self.0.to_sec1_bytes().cmp(&other.0.to_sec1_bytes()) } } impl Display for PublicKey { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", HEXLOWER.encode(&self.0.serialize_compressed())) + write!(f, "{}", HEXLOWER.encode(&self.0.to_sec1_bytes())) } } @@ -143,8 +140,8 @@ impl FromStr for PublicKey { } } -impl From for PublicKey { - fn from(pk: libsecp256k1::PublicKey) -> Self { +impl From for PublicKey { + fn from(pk: k256::PublicKey) -> Self { Self(pk) } } @@ -154,9 +151,7 @@ impl From<&PublicKey> for EthAddress { use tiny_keccak::Hasher; let mut hasher = tiny_keccak::Keccak::v256(); - // We're removing the first byte with - // `libsecp256k1::util::TAG_PUBKEY_FULL` - let pk_bytes = &pk.0.serialize()[1..]; + let pk_bytes = &pk.0.to_encoded_point(false).to_bytes()[1..]; hasher.update(pk_bytes); let mut output = [0_u8; 32]; hasher.finalize(&mut output); @@ -168,7 +163,7 @@ impl From<&PublicKey> for EthAddress { /// Secp256k1 secret key #[derive(Debug, Clone)] -pub struct SecretKey(pub Box); +pub struct SecretKey(pub Box); impl super::SecretKey for SecretKey { type PublicKey = PublicKey; @@ -197,7 +192,7 @@ impl Serialize for SecretKey { where S: Serializer, { - let arr = self.0.serialize(); + let arr: [u8; SECRET_KEY_SIZE] = self.0.to_bytes().into(); serde::Serialize::serialize(&arr, serializer) } } @@ -207,10 +202,10 @@ impl<'de> Deserialize<'de> for SecretKey { where D: serde::Deserializer<'de>, { - let arr_res: [u8; libsecp256k1::util::SECRET_KEY_SIZE] = + let arr_res: [u8; SECRET_KEY_SIZE] = serde::Deserialize::deserialize(deserializer)?; - let key = libsecp256k1::SecretKey::parse_slice(&arr_res) - .map_err(D::Error::custom); + let key = + k256::SecretKey::from_slice(&arr_res).map_err(D::Error::custom); Ok(SecretKey(Box::new(key.unwrap()))) } } @@ -218,23 +213,21 @@ impl<'de> Deserialize<'de> for SecretKey { impl BorshDeserialize for SecretKey { fn deserialize(buf: &mut &[u8]) -> std::io::Result { // deserialize the bytes first - Ok(SecretKey(Box::new( - libsecp256k1::SecretKey::parse( - &(BorshDeserialize::deserialize(buf)?), + let bytes: [u8; SECRET_KEY_SIZE] = BorshDeserialize::deserialize(buf)?; + let sk = k256::SecretKey::from_slice(&bytes).map_err(|e| { + std::io::Error::new( + ErrorKind::InvalidInput, + format!("Error decoding secp256k1 secret key: {}", e), ) - .map_err(|e| { - std::io::Error::new( - ErrorKind::InvalidInput, - format!("Error decoding secp256k1 secret key: {}", e), - ) - })?, - ))) + })?; + Ok(SecretKey(Box::new(sk))) } } impl BorshSerialize for SecretKey { fn serialize(&self, writer: &mut W) -> std::io::Result<()> { - BorshSerialize::serialize(&self.0.serialize(), writer) + let bytes: [u8; SECRET_KEY_SIZE] = self.0.to_bytes().into(); + BorshSerialize::serialize(&bytes, writer) } } @@ -247,7 +240,7 @@ impl BorshSchema for SecretKey { ) { // Encoded as `[u8; SECRET_KEY_SIZE]` let elements = "u8".into(); - let length = libsecp256k1::util::SECRET_KEY_SIZE as u32; + let length = SECRET_KEY_SIZE as u32; let definition = borsh::schema::Definition::Array { elements, length }; definitions.insert(Self::declaration(), definition); } @@ -259,7 +252,7 @@ impl BorshSchema for SecretKey { impl Display for SecretKey { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", HEXLOWER.encode(&self.0.serialize())) + write!(f, "{}", HEXLOWER.encode(&self.0.to_bytes())) } } @@ -277,13 +270,13 @@ impl FromStr for SecretKey { impl RefTo for SecretKey { fn ref_to(&self) -> PublicKey { - PublicKey(libsecp256k1::PublicKey::from_secret_key(&self.0)) + PublicKey(self.0.public_key()) } } /// Secp256k1 signature #[derive(Clone, Debug, Eq, PartialEq)] -pub struct Signature(pub libsecp256k1::Signature, pub RecoveryId); +pub struct Signature(pub k256::ecdsa::Signature, pub RecoveryId); impl super::Signature for Signature { const TYPE: SchemeType = SigScheme::TYPE; @@ -305,15 +298,14 @@ impl super::Signature for Signature { } } -// Would ideally like Serialize, Deserialize to be implemented in libsecp256k1, +// Would ideally like Serialize, Deserialize to be implemented in k256, // may try to do so and merge upstream in the future. - impl Serialize for Signature { fn serialize(&self, serializer: S) -> Result where S: Serializer, { - let arr = self.0.serialize(); + let arr = self.0.to_bytes(); // TODO: implement the line below, currently cannot support [u8; 64] // serde::Serialize::serialize(&arr, serializer) @@ -321,7 +313,7 @@ impl Serialize for Signature { for elem in &arr[..] { seq.serialize_element(elem)?; } - seq.serialize_element(&self.1.serialize())?; + seq.serialize_element(&self.1.to_byte())?; seq.end() } } @@ -334,22 +326,25 @@ impl<'de> Deserialize<'de> for Signature { struct ByteArrayVisitor; impl<'de> Visitor<'de> for ByteArrayVisitor { - type Value = [u8; SIGNATURE_LENGTH]; + type Value = [u8; SIGNATURE_SIZE]; fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { formatter.write_str(&format!( "an array of length {}", - SIGNATURE_LENGTH, + SIGNATURE_SIZE, )) } - fn visit_seq(self, mut seq: A) -> Result<[u8; 65], A::Error> + fn visit_seq( + self, + mut seq: A, + ) -> Result<[u8; SIGNATURE_SIZE], A::Error> where A: SeqAccess<'de>, { - let mut arr = [0u8; SIGNATURE_LENGTH]; + let mut arr = [0u8; SIGNATURE_SIZE]; #[allow(clippy::needless_range_loop)] - for i in 0..SIGNATURE_LENGTH { + for i in 0..SIGNATURE_SIZE { arr[i] = seq .next_element()? .ok_or_else(|| Error::invalid_length(i, &self))?; @@ -358,14 +353,15 @@ impl<'de> Deserialize<'de> for Signature { } } - let arr_res = deserializer - .deserialize_tuple(SIGNATURE_LENGTH, ByteArrayVisitor)?; + let arr_res = + deserializer.deserialize_tuple(SIGNATURE_SIZE, ByteArrayVisitor)?; let sig_array: [u8; 64] = arr_res[..64].try_into().unwrap(); - let sig = libsecp256k1::Signature::parse_standard(&sig_array) + let sig = k256::ecdsa::Signature::from_slice(&sig_array) .map_err(D::Error::custom); Ok(Signature( sig.unwrap(), - RecoveryId::parse(arr_res[64]).map_err(Error::custom)?, + RecoveryId::from_byte(arr_res[64]) + .ok_or_else(|| Error::custom("Invalid recovery byte"))?, )) } } @@ -373,33 +369,30 @@ impl<'de> Deserialize<'de> for Signature { impl BorshDeserialize for Signature { fn deserialize(buf: &mut &[u8]) -> std::io::Result { // deserialize the bytes first - let (sig_bytes, recovery_id) = BorshDeserialize::deserialize(buf)?; + let (sig_bytes, recovery_id): ([u8; 64], u8) = + BorshDeserialize::deserialize(buf)?; Ok(Signature( - libsecp256k1::Signature::parse_standard(&sig_bytes).map_err( - |e| { - std::io::Error::new( - ErrorKind::InvalidInput, - format!("Error decoding secp256k1 signature: {}", e), - ) - }, - )?, - RecoveryId::parse(recovery_id).map_err(|e| { + k256::ecdsa::Signature::from_slice(&sig_bytes).map_err(|e| { std::io::Error::new( ErrorKind::InvalidInput, format!("Error decoding secp256k1 signature: {}", e), ) })?, + RecoveryId::from_byte(recovery_id).ok_or_else(|| { + std::io::Error::new( + ErrorKind::InvalidInput, + "Error decoding secp256k1 signature recovery byte", + ) + })?, )) } } impl BorshSerialize for Signature { fn serialize(&self, writer: &mut W) -> std::io::Result<()> { - BorshSerialize::serialize( - &(self.0.serialize(), self.1.serialize()), - writer, - ) + let sig_bytes: [u8; 64] = self.0.to_bytes().into(); + BorshSerialize::serialize(&(sig_bytes, self.1.to_byte()), writer) } } @@ -411,11 +404,8 @@ impl BorshSchema for Signature { >, ) { // Encoded as `([u8; SIGNATURE_SIZE], u8)` - let signature = - <[u8; libsecp256k1::util::SIGNATURE_SIZE]>::declaration(); - <[u8; libsecp256k1::util::SIGNATURE_SIZE]>::add_definitions_recursively( - definitions, - ); + let signature = <[u8; SIGNATURE_SIZE]>::declaration(); + <[u8; SIGNATURE_SIZE]>::add_definitions_recursively(definitions); let recovery = "u8".into(); let definition = borsh::schema::Definition::Tuple { elements: vec![signature, recovery], @@ -429,52 +419,42 @@ impl BorshSchema for Signature { } impl Signature { - const S_MALLEABILITY_FIX: U256 = U256([ - 13822214165235122497, - 13451932020343611451, - 18446744073709551614, - 18446744073709551615, - ]); - // these constants are pulled from OpenZeppelin's ECDSA code - const S_MALLEABILITY_THRESHOLD: U256 = U256([ - 16134479119472337056, - 6725966010171805725, - 18446744073709551615, - 9223372036854775807, - ]); + /// OpenZeppelin consumes v values in the range [27, 28], + /// rather than [0, 1], the latter returned by `k256`. const V_FIX: u8 = 27; + /// Given a v signature parameter, flip its value + /// (i.e. negate the input). + /// + /// __INVARIANT__: The value of `v` must be in the range [0, 1]. + #[inline(always)] + fn flip_v(v: u8) -> u8 { + debug_assert!(v == 0 || v == 1); + v ^ 1 + } + /// Returns the `r`, `s` and `v` parameters of this [`Signature`], /// destroying the original value in the process. /// /// The returned signature is unique (i.e. non-malleable). This /// ensures OpenZeppelin considers the signature valid. pub fn into_eth_rsv(self) -> ([u8; 32], [u8; 32], u8) { - // assuming the value of v is either 0 or 1, - // the output is essentially the negated input - #[inline(always)] - fn flip_v(v: u8) -> u8 { - v ^ 1 - } - - let (v, s) = { - let s1: U256 = self.0.s.b32().into(); - let v = self.1.serialize(); - let (v, non_malleable_s) = - if hints::unlikely(s1 > Self::S_MALLEABILITY_THRESHOLD) { - // this code path seems quite rare. we often - // get non-malleable signatures, which is good - (flip_v(v) + Self::V_FIX, Self::S_MALLEABILITY_FIX - s1) - } else { - (v + Self::V_FIX, s1) - }; - let mut non_malleable_s: [u8; 32] = non_malleable_s.into(); - self.0.s.fill_b32(&mut non_malleable_s); - (v, self.0.s.b32()) + // A recovery id (dubbed v) is used by secp256k1 signatures + // to signal verifying code if a signature had been malleable + // or not (based on whether the s field of the signature was odd + // or not). In the `k256` dependency, the low-bit signifies the + // y-coordinate, associated with s, being odd. + let v = self.1.to_byte() & 1; + // Check if s needs to be normalized. In case it does, + // we must flip the value of v (e.g. 0 -> 1). + let (s, v) = if let Some(signature) = self.0.normalize_s() { + let normalized_s = signature.s(); + (normalized_s, Self::flip_v(v)) + } else { + (self.0.s(), v) }; - let r = self.0.r.b32(); - - (r, s, v) + let r = self.0.r(); + (r.to_bytes().into(), s.to_bytes().into(), v + Self::V_FIX) } } @@ -491,16 +471,14 @@ impl Encode<1> for Signature { #[allow(clippy::derived_hash_with_manual_eq)] impl Hash for Signature { fn hash(&self, state: &mut H) { - self.0.serialize().hash(state); + self.0.to_bytes().hash(state); } } impl PartialOrd for Signature { fn partial_cmp(&self, other: &Self) -> Option { - match self.0.serialize().partial_cmp(&other.0.serialize()) { - Some(Ordering::Equal) => { - self.1.serialize().partial_cmp(&other.1.serialize()) - } + match self.0.to_bytes().partial_cmp(&other.0.to_bytes()) { + Some(Ordering::Equal) => self.1.partial_cmp(&other.1), res => res, } } @@ -516,21 +494,20 @@ impl TryFrom<&[u8; 65]> for Signature { type Error = ParseSignatureError; fn try_from(sig: &[u8; 65]) -> Result { - let sig_bytes = sig[..64].try_into().unwrap(); - let recovery_id = RecoveryId::parse(sig[64]).map_err(|err| { + let recovery_id = RecoveryId::from_byte(sig[64]).ok_or_else(|| { ParseSignatureError::InvalidEncoding(std::io::Error::new( ErrorKind::Other, - err, + "Invalid recovery byte", )) })?; - libsecp256k1::Signature::parse_standard(&sig_bytes) - .map(|sig| Self(sig, recovery_id)) - .map_err(|err| { + let sig = + k256::ecdsa::Signature::from_slice(&sig[..64]).map_err(|err| { ParseSignatureError::InvalidEncoding(std::io::Error::new( ErrorKind::Other, err, )) - }) + })?; + Ok(Self(sig, recovery_id)) } } @@ -563,12 +540,12 @@ impl super::SigScheme for SigScheme { where R: CryptoRng + RngCore, { - SecretKey(Box::new(libsecp256k1::SecretKey::random(csprng))) + SecretKey(Box::new(k256::SecretKey::random(csprng))) } fn from_bytes(sk: [u8; 32]) -> SecretKey { SecretKey(Box::new( - libsecp256k1::SecretKey::parse_slice(&sk) + k256::SecretKey::from_slice(&sk) .expect("Secret key parsing should not fail."), )) } @@ -580,20 +557,12 @@ impl super::SigScheme for SigScheme { where H: 'static + StorageHasher, { - #[cfg(not(any(test, feature = "secp256k1-sign")))] - { - // to avoid `unused-variables` warn - let _ = (keypair, data); - panic!("\"secp256k1-sign\" feature must be enabled"); - } - - #[cfg(any(test, feature = "secp256k1-sign"))] - { - let message = - libsecp256k1::Message::parse(&data.signable_hash::()); - let (sig, recovery_id) = libsecp256k1::sign(&message, &keypair.0); - Signature(sig, recovery_id) - } + let sig_key = k256::ecdsa::SigningKey::from(keypair.0.as_ref()); + let msg = data.signable_hash::(); + let (sig, recovery_id) = sig_key + .sign_prehash_recoverable(&msg) + .expect("Must be able to sign"); + Signature(sig, recovery_id) } fn verify_signature_with_hasher( @@ -604,21 +573,23 @@ impl super::SigScheme for SigScheme { where H: 'static + StorageHasher, { - let message = libsecp256k1::Message::parse(&data.signable_hash::()); - let is_valid = libsecp256k1::verify(&message, &sig.0, &pk.0); - if is_valid { - Ok(()) - } else { - Err(VerifySigError::SigVerifyError(format!( + use k256::ecdsa::signature::hazmat::PrehashVerifier; + + let vrf_key = k256::ecdsa::VerifyingKey::from(&pk.0); + let msg = data.signable_hash::(); + vrf_key.verify_prehash(&msg, &sig.0).map_err(|e| { + VerifySigError::SigVerifyError(format!( "Error verifying secp256k1 signature: {}", - libsecp256k1::Error::InvalidSignature - ))) - } + e + )) + }) } } #[cfg(test)] mod test { + use k256::elliptic_curve::sec1::ToEncodedPoint; + use super::*; /// test vector from https://bitcoin.stackexchange.com/a/89848 @@ -635,9 +606,9 @@ mod test { let sk_bytes = HEXLOWER.decode(SECRET_KEY_HEX.as_bytes()).unwrap(); let sk = SecretKey::try_from_slice(&sk_bytes[..]).unwrap(); let pk: PublicKey = sk.ref_to(); - // We're removing the first byte with - // `libsecp256k1::util::TAG_PUBKEY_FULL` - let pk_hex = HEXLOWER.encode(&pk.0.serialize()[1..]); + // We're removing the first byte with tag + let pk_hex = + HEXLOWER.encode(&pk.0.to_encoded_point(false).to_bytes()[1..]); assert_eq!(expected_pk_hex, pk_hex); let eth_addr: EthAddress = (&pk).into(); @@ -653,7 +624,7 @@ mod test { let sk = SecretKey::try_from_slice(&sk_bytes[..]).unwrap(); let to_sign = "test".as_bytes(); let mut signature = SigScheme::sign(&sk, to_sign); - signature.1 = RecoveryId::parse(3).expect("Test failed"); + signature.1 = RecoveryId::from_byte(3).expect("Test failed"); let sig_json = serde_json::to_string(&signature).expect("Test failed"); let sig: Signature = serde_json::from_str(&sig_json).expect("Test failed"); @@ -668,28 +639,10 @@ mod test { let sk = SecretKey::try_from_slice(&sk_bytes[..]).unwrap(); let to_sign = "test".as_bytes(); let mut signature = SigScheme::sign(&sk, to_sign); - signature.1 = RecoveryId::parse(3).expect("Test failed"); + signature.1 = RecoveryId::from_byte(3).expect("Test failed"); let sig_bytes = signature.try_to_vec().expect("Test failed"); let sig = Signature::try_from_slice(sig_bytes.as_slice()) .expect("Test failed"); assert_eq!(sig, signature); } - - /// Ensures we are using the right malleability consts. - #[test] - fn test_signature_malleability_consts() { - let s_threshold = U256::from_str_radix( - "7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0", - 16, - ) - .unwrap(); - assert_eq!(Signature::S_MALLEABILITY_THRESHOLD, s_threshold); - - let malleable_const = U256::from_str_radix( - "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141", - 16, - ) - .unwrap(); - assert_eq!(Signature::S_MALLEABILITY_FIX, malleable_const); - } } diff --git a/ethereum_bridge/Cargo.toml b/ethereum_bridge/Cargo.toml index 1354c36a8c..85b0959106 100644 --- a/ethereum_bridge/Cargo.toml +++ b/ethereum_bridge/Cargo.toml @@ -29,7 +29,7 @@ testing = [ ] [dependencies] -namada_core = {path = "../core", default-features = false, features = ["secp256k1-sign", "ferveo-tpke", "ethers-derive"]} +namada_core = {path = "../core", default-features = false, features = ["ferveo-tpke", "ethers-derive"]} namada_macros = {path = "../macros"} namada_proof_of_stake = {path = "../proof_of_stake", default-features = false} borsh.workspace = true @@ -46,7 +46,7 @@ tracing = "0.1.30" [dev-dependencies] # Added "testing" feature. -namada_core = {path = "../core", default-features = false, features = ["secp256k1-sign", "ferveo-tpke", "ethers-derive", "testing"]} +namada_core = {path = "../core", default-features = false, features = ["ferveo-tpke", "ethers-derive", "testing"]} assert_matches.workspace = true data-encoding.workspace = true ethabi.workspace = true diff --git a/sdk/Cargo.toml b/sdk/Cargo.toml index f0eb9289ed..100db5949b 100644 --- a/sdk/Cargo.toml +++ b/sdk/Cargo.toml @@ -84,7 +84,7 @@ futures.workspace = true itertools.workspace = true masp_primitives.workspace = true masp_proofs = { workspace = true, features = ["download-params"] } -namada_core = {path = "../core", default-features = false, features = ["secp256k1-sign"]} +namada_core = {path = "../core", default-features = false} namada_ethereum_bridge = {path = "../ethereum_bridge", default-features = false} namada_proof_of_stake = {path = "../proof_of_stake", default-features = false} num256.workspace = true diff --git a/shared/Cargo.toml b/shared/Cargo.toml index 21eb023e18..0c6d903ddd 100644 --- a/shared/Cargo.toml +++ b/shared/Cargo.toml @@ -104,7 +104,7 @@ multicore = [ ] [dependencies] -namada_core = {path = "../core", default-features = false, features = ["secp256k1-sign"]} +namada_core = {path = "../core", default-features = false} namada_sdk = {path = "../sdk", default-features = false} namada_proof_of_stake = {path = "../proof_of_stake", default-features = false} namada_ethereum_bridge = {path = "../ethereum_bridge", default-features = false} @@ -166,14 +166,14 @@ tokio = {workspace = true, default-features = false, features = ["sync"]} wasmtimer = "0.2.0" [dev-dependencies] -namada_core = {path = "../core", default-features = false, features = ["secp256k1-sign", "testing", "ibc-mocks"]} +namada_core = {path = "../core", default-features = false, features = ["testing", "ibc-mocks"]} namada_ethereum_bridge = {path = "../ethereum_bridge", default-features = false, features = ["testing"]} namada_test_utils = {path = "../test_utils"} assert_matches.workspace = true async-trait.workspace = true base58.workspace = true byte-unit.workspace = true -libsecp256k1.workspace = true +k256.workspace = true pretty_assertions.workspace = true proptest.workspace = true tempfile.workspace = true diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index 8984c1708f..936d316b62 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -1447,6 +1447,7 @@ dependencies = [ "digest 0.10.6", "elliptic-curve", "rfc6979", + "serdect", "signature 2.1.0", "spki", ] @@ -1513,6 +1514,7 @@ dependencies = [ "pkcs8", "rand_core 0.6.4", "sec1", + "serdect", "subtle 2.4.1", "zeroize", ] @@ -2523,17 +2525,6 @@ dependencies = [ "hmac 0.7.1", ] -[[package]] -name = "hmac-drbg" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "17ea0a1394df5b6574da6e0c1ade9e78868c9fb0a4e5ef4428e32da4676b85b1" -dependencies = [ - "digest 0.9.0", - "generic-array 0.14.7", - "hmac 0.8.1", -] - [[package]] name = "hmac-sha512" version = "0.1.9" @@ -2939,6 +2930,7 @@ dependencies = [ "ecdsa", "elliptic-curve", "once_cell", + "serdect", "sha2 0.10.6", "signature 2.1.0", ] @@ -2995,57 +2987,13 @@ dependencies = [ "arrayref", "crunchy", "digest 0.8.1", - "hmac-drbg 0.2.0", + "hmac-drbg", "rand 0.7.3", "sha2 0.8.2", "subtle 2.4.1", "typenum", ] -[[package]] -name = "libsecp256k1" -version = "0.7.0" -source = "git+https://github.com/heliaxdev/libsecp256k1?rev=bbb3bd44a49db361f21d9db80f9a087c194c0ae9#bbb3bd44a49db361f21d9db80f9a087c194c0ae9" -dependencies = [ - "arrayref", - "base64 0.13.1", - "digest 0.9.0", - "hmac-drbg 0.3.0", - "libsecp256k1-core", - "libsecp256k1-gen-ecmult", - "libsecp256k1-gen-genmult", - "rand 0.8.5", - "serde", - "sha2 0.9.9", - "typenum", -] - -[[package]] -name = "libsecp256k1-core" -version = "0.3.0" -source = "git+https://github.com/heliaxdev/libsecp256k1?rev=bbb3bd44a49db361f21d9db80f9a087c194c0ae9#bbb3bd44a49db361f21d9db80f9a087c194c0ae9" -dependencies = [ - "crunchy", - "digest 0.9.0", - "subtle 2.4.1", -] - -[[package]] -name = "libsecp256k1-gen-ecmult" -version = "0.3.0" -source = "git+https://github.com/heliaxdev/libsecp256k1?rev=bbb3bd44a49db361f21d9db80f9a087c194c0ae9#bbb3bd44a49db361f21d9db80f9a087c194c0ae9" -dependencies = [ - "libsecp256k1-core", -] - -[[package]] -name = "libsecp256k1-gen-genmult" -version = "0.3.0" -source = "git+https://github.com/heliaxdev/libsecp256k1?rev=bbb3bd44a49db361f21d9db80f9a087c194c0ae9#bbb3bd44a49db361f21d9db80f9a087c194c0ae9" -dependencies = [ - "libsecp256k1-core", -] - [[package]] name = "linux-raw-sys" version = "0.3.7" @@ -3394,7 +3342,7 @@ dependencies = [ "impl-num-traits", "index-set", "itertools", - "libsecp256k1 0.7.0", + "k256", "masp_primitives", "namada_macros", "num-integer", @@ -4928,6 +4876,7 @@ dependencies = [ "der", "generic-array 0.14.7", "pkcs8", + "serdect", "subtle 2.4.1", "zeroize", ] @@ -5076,6 +5025,16 @@ dependencies = [ "serde", ] +[[package]] +name = "serdect" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a84f14a19e9a014bb9f4512488d9829a68e04ecabffb0f9904cd1ace94598177" +dependencies = [ + "base16ct", + "serde", +] + [[package]] name = "sha1" version = "0.10.5" @@ -5598,7 +5557,7 @@ checksum = "01b874a4992538d4b2f4fbbac11b9419d685f4b39bdc3fed95b04e07bfd76040" dependencies = [ "base58", "hmac 0.7.1", - "libsecp256k1 0.3.5", + "libsecp256k1", "memzero", "sha2 0.8.2", ] diff --git a/wasm_for_tests/wasm_source/Cargo.lock b/wasm_for_tests/wasm_source/Cargo.lock index 43c75ed658..8a4fdec901 100644 --- a/wasm_for_tests/wasm_source/Cargo.lock +++ b/wasm_for_tests/wasm_source/Cargo.lock @@ -1447,6 +1447,7 @@ dependencies = [ "digest 0.10.6", "elliptic-curve", "rfc6979", + "serdect", "signature 2.1.0", "spki", ] @@ -1513,6 +1514,7 @@ dependencies = [ "pkcs8", "rand_core 0.6.4", "sec1", + "serdect", "subtle 2.4.1", "zeroize", ] @@ -2523,17 +2525,6 @@ dependencies = [ "hmac 0.7.1", ] -[[package]] -name = "hmac-drbg" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "17ea0a1394df5b6574da6e0c1ade9e78868c9fb0a4e5ef4428e32da4676b85b1" -dependencies = [ - "digest 0.9.0", - "generic-array 0.14.7", - "hmac 0.8.1", -] - [[package]] name = "hmac-sha512" version = "0.1.9" @@ -2939,6 +2930,7 @@ dependencies = [ "ecdsa", "elliptic-curve", "once_cell", + "serdect", "sha2 0.10.6", "signature 2.1.0", ] @@ -2995,57 +2987,13 @@ dependencies = [ "arrayref", "crunchy", "digest 0.8.1", - "hmac-drbg 0.2.0", + "hmac-drbg", "rand 0.7.3", "sha2 0.8.2", "subtle 2.4.1", "typenum", ] -[[package]] -name = "libsecp256k1" -version = "0.7.0" -source = "git+https://github.com/heliaxdev/libsecp256k1?rev=bbb3bd44a49db361f21d9db80f9a087c194c0ae9#bbb3bd44a49db361f21d9db80f9a087c194c0ae9" -dependencies = [ - "arrayref", - "base64 0.13.1", - "digest 0.9.0", - "hmac-drbg 0.3.0", - "libsecp256k1-core", - "libsecp256k1-gen-ecmult", - "libsecp256k1-gen-genmult", - "rand 0.8.5", - "serde", - "sha2 0.9.9", - "typenum", -] - -[[package]] -name = "libsecp256k1-core" -version = "0.3.0" -source = "git+https://github.com/heliaxdev/libsecp256k1?rev=bbb3bd44a49db361f21d9db80f9a087c194c0ae9#bbb3bd44a49db361f21d9db80f9a087c194c0ae9" -dependencies = [ - "crunchy", - "digest 0.9.0", - "subtle 2.4.1", -] - -[[package]] -name = "libsecp256k1-gen-ecmult" -version = "0.3.0" -source = "git+https://github.com/heliaxdev/libsecp256k1?rev=bbb3bd44a49db361f21d9db80f9a087c194c0ae9#bbb3bd44a49db361f21d9db80f9a087c194c0ae9" -dependencies = [ - "libsecp256k1-core", -] - -[[package]] -name = "libsecp256k1-gen-genmult" -version = "0.3.0" -source = "git+https://github.com/heliaxdev/libsecp256k1?rev=bbb3bd44a49db361f21d9db80f9a087c194c0ae9#bbb3bd44a49db361f21d9db80f9a087c194c0ae9" -dependencies = [ - "libsecp256k1-core", -] - [[package]] name = "linux-raw-sys" version = "0.3.7" @@ -3394,7 +3342,7 @@ dependencies = [ "impl-num-traits", "index-set", "itertools", - "libsecp256k1 0.7.0", + "k256", "masp_primitives", "namada_macros", "num-integer", @@ -4921,6 +4869,7 @@ dependencies = [ "der", "generic-array 0.14.7", "pkcs8", + "serdect", "subtle 2.4.1", "zeroize", ] @@ -5069,6 +5018,16 @@ dependencies = [ "serde", ] +[[package]] +name = "serdect" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a84f14a19e9a014bb9f4512488d9829a68e04ecabffb0f9904cd1ace94598177" +dependencies = [ + "base16ct", + "serde", +] + [[package]] name = "sha1" version = "0.10.5" @@ -5591,7 +5550,7 @@ checksum = "01b874a4992538d4b2f4fbbac11b9419d685f4b39bdc3fed95b04e07bfd76040" dependencies = [ "base58", "hmac 0.7.1", - "libsecp256k1 0.3.5", + "libsecp256k1", "memzero", "sha2 0.8.2", ]