From e9a903bd0f132497dce5da8a9b0d6aac6a37fe57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 10 Feb 2021 19:33:56 +0100 Subject: [PATCH] Replace `libsecp256k1` with `secp265k1` Fixes: https://github.com/paritytech/substrate/issues/8089 --- Cargo.lock | 23 ++++++++-- Cargo.toml | 2 +- client/executor/Cargo.toml | 1 - primitives/core/Cargo.toml | 6 +-- primitives/core/src/ecdsa.rs | 88 ++++++++++++++++++++++-------------- primitives/io/Cargo.toml | 4 +- primitives/io/src/lib.rs | 32 +++++++++---- 7 files changed, 101 insertions(+), 55 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 587d9af1a2131..9b2a104f612d3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7004,7 +7004,6 @@ dependencies = [ "derive_more", "hex-literal", "lazy_static", - "libsecp256k1", "log", "parity-scale-codec", "parity-wasm 0.41.0", @@ -7816,6 +7815,24 @@ dependencies = [ "untrusted", ] +[[package]] +name = "secp256k1" +version = "0.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "733b114f058f260c0af7591434eef4272ae1a8ec2751766d3cb89c6df8d5e450" +dependencies = [ + "secp256k1-sys", +] + +[[package]] +name = "secp256k1-sys" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "67e4b6455ee49f5901c8985b88f98fb0a0e1d90a6661f5a03f4888bd987dad29" +dependencies = [ + "cc", +] + [[package]] name = "secrecy" version = "0.7.0" @@ -8395,7 +8412,6 @@ dependencies = [ "hex-literal", "impl-serde", "lazy_static", - "libsecp256k1", "log", "merlin", "num-traits", @@ -8408,6 +8424,7 @@ dependencies = [ "rand_chacha 0.2.2", "regex", "schnorrkel", + "secp256k1", "secrecy", "serde", "serde_json", @@ -8498,10 +8515,10 @@ version = "2.0.1" dependencies = [ "futures 0.3.12", "hash-db", - "libsecp256k1", "log", "parity-scale-codec", "parking_lot 0.11.1", + "secp256k1", "sp-core", "sp-externalities", "sp-keystore", diff --git a/Cargo.toml b/Cargo.toml index 38b3a2bdcf296..749dd94b20345 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -233,7 +233,7 @@ integer-sqrt = { opt-level = 3 } keccak = { opt-level = 3 } libm = { opt-level = 3 } librocksdb-sys = { opt-level = 3 } -libsecp256k1 = { opt-level = 3 } +secp256k1 = { opt-level = 3 } libz-sys = { opt-level = 3 } mio = { opt-level = 3 } nalgebra = { opt-level = 3 } diff --git a/client/executor/Cargo.toml b/client/executor/Cargo.toml index 12a45d09c0b00..734b1e64c56a2 100644 --- a/client/executor/Cargo.toml +++ b/client/executor/Cargo.toml @@ -35,7 +35,6 @@ sc-executor-wasmi = { version = "0.8.0", path = "wasmi" } sc-executor-wasmtime = { version = "0.8.0", path = "wasmtime", optional = true } parking_lot = "0.11.1" log = "0.4.8" -libsecp256k1 = "0.3.4" [dev-dependencies] assert_matches = "1.3.0" diff --git a/primitives/core/Cargo.toml b/primitives/core/Cargo.toml index 36c980676827e..232dc56adc8bc 100644 --- a/primitives/core/Cargo.toml +++ b/primitives/core/Cargo.toml @@ -49,7 +49,7 @@ schnorrkel = { version = "0.9.1", features = ["preaudit_deprecated", "u64_backen sha2 = { version = "0.9.2", default-features = false, optional = true } hex = { version = "0.4", default-features = false, optional = true } twox-hash = { version = "1.5.0", default-features = false, optional = true } -libsecp256k1 = { version = "0.3.2", default-features = false, features = ["hmac"], optional = true } +secp256k1 = { version = "0.20.1", optional = true, default-features = false, features = [ "recovery" ] } merlin = { version = "2.0", default-features = false, optional = true } sp-runtime-interface = { version = "2.0.0", default-features = false, path = "../runtime-interface" } @@ -112,7 +112,7 @@ std = [ "secrecy/alloc", "futures", "futures/thread-pool", - "libsecp256k1/std", + "secp256k1/std", "dyn-clonable", ] @@ -127,7 +127,7 @@ full_crypto = [ "hex", "sha2", "twox-hash", - "libsecp256k1", + "secp256k1", "sp-runtime-interface/disable_target_static_assertions", "merlin", ] diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs index fc9b16beedd13..389d7553a6a46 100644 --- a/primitives/core/src/ecdsa.rs +++ b/primitives/core/src/ecdsa.rs @@ -26,7 +26,7 @@ use sp_std::cmp::Ordering; use codec::{Encode, Decode}; #[cfg(feature = "full_crypto")] -use core::convert::{TryFrom, TryInto}; +use core::convert::TryFrom; #[cfg(feature = "std")] use substrate_bip39::seed_from_entropy; #[cfg(feature = "std")] @@ -103,8 +103,18 @@ impl Public { /// This will convert the full public key into the compressed format. #[cfg(feature = "std")] pub fn from_full(full: &[u8]) -> Result { - secp256k1::PublicKey::parse_slice(full, None) - .map(|k| k.serialize_compressed()) + let pubkey = if full.len() == 64 { + let mut full_with_tag = [0u8; 65]; + full_with_tag[1..].copy_from_slice(full); + // Tag it as uncompressed public key + full_with_tag[0] = 0x04; + secp256k1::PublicKey::from_slice(&full_with_tag) + } else { + secp256k1::PublicKey::from_slice(&full) + }; + + pubkey + .map(|k| k.serialize()) .map(Self) .map_err(|_| ()) } @@ -165,7 +175,6 @@ impl sp_std::convert::TryFrom<&[u8]> for Public { if data.len() == 33 { Ok(Self::from_slice(data)) } else { - Err(()) } } @@ -348,31 +357,34 @@ impl Signature { /// Recover the public key from this signature and a message. #[cfg(feature = "full_crypto")] pub fn recover>(&self, message: M) -> Option { - let message = secp256k1::Message::parse(&blake2_256(message.as_ref())); - let sig: (_, _) = self.try_into().ok()?; - secp256k1::recover(&message, &sig.0, &sig.1) + let message = secp256k1::Message::from_slice(&blake2_256(message.as_ref())).ok()?; + + let id = secp256k1::recovery::RecoveryId::from_i32(self.0[64] as i32).ok()?; + let sig = secp256k1::recovery::RecoverableSignature::from_compact(&self.0[..64], id).ok()?; + + secp256k1::Secp256k1::verification_only().recover(&message, &sig) .ok() - .map(|recovered| Public(recovered.serialize_compressed())) + .map(|recovered| Public(recovered.serialize())) } } #[cfg(feature = "full_crypto")] -impl From<(secp256k1::Signature, secp256k1::RecoveryId)> for Signature { - fn from(x: (secp256k1::Signature, secp256k1::RecoveryId)) -> Signature { +impl From<(secp256k1::Signature, secp256k1::recovery::RecoveryId)> for Signature { + fn from(x: (secp256k1::Signature, secp256k1::recovery::RecoveryId)) -> Signature { let mut r = Self::default(); - r.0[0..64].copy_from_slice(&x.0.serialize()[..]); - r.0[64] = x.1.serialize(); + r.0[0..64].copy_from_slice(&x.0.serialize_compact()[..]); + r.0[64] = x.1.to_i32() as u8; r } } #[cfg(feature = "full_crypto")] -impl<'a> TryFrom<&'a Signature> for (secp256k1::Signature, secp256k1::RecoveryId) { +impl<'a> TryFrom<&'a Signature> for (secp256k1::Signature, secp256k1::recovery::RecoveryId) { type Error = (); - fn try_from(x: &'a Signature) -> Result<(secp256k1::Signature, secp256k1::RecoveryId), Self::Error> { + fn try_from(x: &'a Signature) -> Result<(secp256k1::Signature, secp256k1::recovery::RecoveryId), Self::Error> { Ok(( - secp256k1::Signature::parse_slice(&x.0[0..64]).expect("hardcoded to 64 bytes; qed"), - secp256k1::RecoveryId::parse(x.0[64]).map_err(|_| ())?, + secp256k1::Signature::from_compact(&x.0[0..64]).map_err(|_| ())?, + secp256k1::recovery::RecoveryId::from_i32(x.0[64] as i32).map_err(|_| ())?, )) } } @@ -450,9 +462,9 @@ impl TraitPair for Pair { /// /// You should never need to use this; generate(), generate_with_phrase fn from_seed_slice(seed_slice: &[u8]) -> Result { - let secret = SecretKey::parse_slice(seed_slice) + let secret = SecretKey::from_slice(seed_slice) .map_err(|_| SecretStringError::InvalidSeedLength)?; - let public = PublicKey::from_secret_key(&secret); + let public = PublicKey::from_secret_key(&secp256k1::Secp256k1::signing_only(), &secret); Ok(Pair{ secret, public }) } @@ -461,7 +473,7 @@ impl TraitPair for Pair { path: Iter, _seed: Option ) -> Result<(Pair, Option), DeriveError> { - let mut acc = self.secret.serialize(); + let mut acc = self.seed(); for j in path { match j { DeriveJunction::Soft(_cc) => return Err(DeriveError::SoftKeyInPath), @@ -473,22 +485,24 @@ impl TraitPair for Pair { /// Get the public key. fn public(&self) -> Public { - Public(self.public.serialize_compressed()) + Public(self.public.serialize()) } /// Sign a message. fn sign(&self, message: &[u8]) -> Signature { - let message = secp256k1::Message::parse(&blake2_256(message)); - secp256k1::sign(&message, &self.secret).into() + let message = secp256k1::Message::from_slice(&blake2_256(message)) + .expect("Message is 32 byte; qed"); + let sig = secp256k1::Secp256k1::signing_only().sign_recoverable(&message, &self.secret); + + let id = sig.serialize_compact().0; + (sig.to_standard(), id).into() } /// Verify a signature on a message. Returns true if the signature is good. fn verify>(sig: &Self::Signature, message: M, pubkey: &Self::Public) -> bool { - let message = secp256k1::Message::parse(&blake2_256(message.as_ref())); - let sig: (_, _) = match sig.try_into() { Ok(x) => x, _ => return false }; - match secp256k1::recover(&message, &sig.0, &sig.1) { - Ok(actual) => &pubkey.0[..] == &actual.serialize_compressed()[..], - _ => false, + match sig.recover(message) { + Some(actual) => actual == *pubkey, + None => false, } } @@ -497,13 +511,17 @@ impl TraitPair for Pair { /// This doesn't use the type system to ensure that `sig` and `pubkey` are the correct /// size. Use it only if you're coming from byte buffers and need the speed. fn verify_weak, M: AsRef<[u8]>>(sig: &[u8], message: M, pubkey: P) -> bool { - let message = secp256k1::Message::parse(&blake2_256(message.as_ref())); - if sig.len() != 65 { return false } - let ri = match secp256k1::RecoveryId::parse(sig[64]) { Ok(x) => x, _ => return false }; - let sig = match secp256k1::Signature::parse_slice(&sig[0..64]) { Ok(x) => x, _ => return false }; - match secp256k1::recover(&message, &sig, &ri) { - Ok(actual) => pubkey.as_ref() == &actual.serialize()[1..], - _ => false, + if sig.len() != 65 { + return false + } + + let mut signature = [0u8; 65]; + signature.copy_from_slice(sig); + let sig = Signature(signature); + + match sig.recover(message) { + None => false, + Some(actual) => pubkey.as_ref() == actual.as_ref(), } } @@ -517,7 +535,7 @@ impl TraitPair for Pair { impl Pair { /// Get the seed for this key. pub fn seed(&self) -> Seed { - self.secret.serialize() + *self.secret.as_ref() } /// Exactly as `from_string` except that if no matches are found then, the the first 32 diff --git a/primitives/io/Cargo.toml b/primitives/io/Cargo.toml index 1f509f7f9f214..343f1cab794e8 100644 --- a/primitives/io/Cargo.toml +++ b/primitives/io/Cargo.toml @@ -20,7 +20,7 @@ hash-db = { version = "0.15.2", default-features = false } sp-core = { version = "2.0.0", default-features = false, path = "../core" } sp-keystore = { version = "0.8.0", default-features = false, optional = true, path = "../keystore" } sp-std = { version = "2.0.0", default-features = false, path = "../std" } -libsecp256k1 = { version = "0.3.4", optional = true } +secp256k1 = { version = "0.20.1", optional = true, features = [ "recovery" ] } sp-state-machine = { version = "0.8.0", optional = true, path = "../state-machine" } sp-wasm-interface = { version = "2.0.0", path = "../wasm-interface", default-features = false } sp-runtime-interface = { version = "2.0.0", default-features = false, path = "../runtime-interface" } @@ -43,7 +43,7 @@ std = [ "hash-db/std", "sp-trie", "sp-state-machine", - "libsecp256k1", + "secp256k1", "sp-runtime-interface/std", "sp-externalities", "sp-wasm-interface/std", diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 397dd3c21712a..eb6bf8d532943 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -691,14 +691,20 @@ pub trait Crypto { sig: &[u8; 65], msg: &[u8; 32], ) -> Result<[u8; 64], EcdsaVerifyError> { - let rs = secp256k1::Signature::parse_slice(&sig[0..64]) + let v = secp256k1::recovery::RecoveryId::from_i32( + if sig[64] > 26 { sig[64] - 27 } else { sig[64] } as i32 + ).map_err(|_| EcdsaVerifyError::BadV)?; + + let rs = secp256k1::recovery::RecoverableSignature::from_compact(&sig[0..64], v) .map_err(|_| EcdsaVerifyError::BadRS)?; - let v = secp256k1::RecoveryId::parse(if sig[64] > 26 { sig[64] - 27 } else { sig[64] } as u8) - .map_err(|_| EcdsaVerifyError::BadV)?; - let pubkey = secp256k1::recover(&secp256k1::Message::parse(msg), &rs, &v) + + let msg = secp256k1::Message::from_slice(msg).map_err(|_| EcdsaVerifyError::BadSignature)?; + + let pubkey = secp256k1::Secp256k1::verification_only().recover(&msg, &rs) .map_err(|_| EcdsaVerifyError::BadSignature)?; + let mut res = [0u8; 64]; - res.copy_from_slice(&pubkey.serialize()[1..65]); + res.copy_from_slice(&pubkey.serialize_uncompressed()[1..65]); Ok(res) } @@ -712,13 +718,19 @@ pub trait Crypto { sig: &[u8; 65], msg: &[u8; 32], ) -> Result<[u8; 33], EcdsaVerifyError> { - let rs = secp256k1::Signature::parse_slice(&sig[0..64]) + let v = secp256k1::recovery::RecoveryId::from_i32( + if sig[64] > 26 { sig[64] - 27 } else { sig[64] } as i32 + ).map_err(|_| EcdsaVerifyError::BadV)?; + + let rs = secp256k1::recovery::RecoverableSignature::from_compact(&sig[0..64], v) .map_err(|_| EcdsaVerifyError::BadRS)?; - let v = secp256k1::RecoveryId::parse(if sig[64] > 26 { sig[64] - 27 } else { sig[64] } as u8) - .map_err(|_| EcdsaVerifyError::BadV)?; - let pubkey = secp256k1::recover(&secp256k1::Message::parse(msg), &rs, &v) + + let msg = secp256k1::Message::from_slice(msg).map_err(|_| EcdsaVerifyError::BadSignature)?; + + let pubkey = secp256k1::Secp256k1::verification_only().recover(&msg, &rs) .map_err(|_| EcdsaVerifyError::BadSignature)?; - Ok(pubkey.serialize_compressed()) + + Ok(pubkey.serialize()) } }