From 9f5d9fa96f88007292c2d59810b7c3c855da58a7 Mon Sep 17 00:00:00 2001
From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
Date: Sat, 9 Mar 2024 11:14:06 +0100
Subject: [PATCH] core: replace secp256k with k256 in crypto::ecdsa (#3525)
This PR replaces the usage of
[secp256k](https://crates.io/crates/secp256k1) crate with
[k256](https://crates.io/crates/k256) in `core::crypto::ecdsa` for
`non-std` environments as outcome of discussion in #3448.
`secp256k1` is used in `std`, meaning that we should not affect host
performance with this PR.
`k256` is enabled in runtimes (`no-std`), and is required to proceed
with #2044.
If desirable, in future we can switch to `k256` also for `std`. That
would require some performance evaluation (e.g. for EVM chains as per
https://github.com/paritytech/polkadot-sdk/issues/3448#issuecomment-1976780391).
Closes https://github.com/paritytech/polkadot-sdk/issues/3448
---------
Co-authored-by: command-bot <>
Co-authored-by: Davide Galassi <davxy@datawok.net>
---
Cargo.lock | 27 ++++-
substrate/primitives/core/Cargo.toml | 9 +-
substrate/primitives/core/src/ecdsa.rs | 145 +++++++++++++++++--------
3 files changed, 129 insertions(+), 52 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
index e38f09db4d3..0b7ea71a536 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -4815,6 +4815,7 @@ dependencies = [
"digest 0.10.7",
"elliptic-curve",
"rfc6979",
+ "serdect",
"signature",
"spki",
]
@@ -4881,9 +4882,9 @@ checksum = "a26ae43d7bcc3b814de94796a5e736d4029efb0ee900c12e2d54c993ad1a1e07"
[[package]]
name = "elliptic-curve"
-version = "0.13.5"
+version = "0.13.8"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "968405c8fdc9b3bf4df0a6638858cc0b52462836ab6b1c87377785dd09cf1c0b"
+checksum = "b5e6043086bf7973472e0c7dff2142ea0b680d30e18d9cc40f267efbf222bd47"
dependencies = [
"base16ct",
"crypto-bigint",
@@ -4894,6 +4895,7 @@ dependencies = [
"pkcs8",
"rand_core 0.6.4",
"sec1",
+ "serdect",
"subtle 2.5.0",
"zeroize",
]
@@ -6971,14 +6973,15 @@ dependencies = [
[[package]]
name = "k256"
-version = "0.13.1"
+version = "0.13.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "cadb76004ed8e97623117f3df85b17aaa6626ab0b0831e6573f104df16cd1bcc"
+checksum = "956ff9b67e26e1a6a866cb758f12c6f8746208489e3e4a4b5580802f2f0a587b"
dependencies = [
"cfg-if",
"ecdsa",
"elliptic-curve",
"once_cell",
+ "serdect",
"sha2 0.10.7",
]
@@ -17101,6 +17104,7 @@ dependencies = [
"der",
"generic-array 0.14.7",
"pkcs8",
+ "serdect",
"subtle 2.5.0",
"zeroize",
]
@@ -17359,6 +17363,16 @@ dependencies = [
"unsafe-libyaml",
]
+[[package]]
+name = "serdect"
+version = "0.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a84f14a19e9a014bb9f4512488d9829a68e04ecabffb0f9904cd1ace94598177"
+dependencies = [
+ "base16ct",
+ "serde",
+]
+
[[package]]
name = "serial_test"
version = "2.0.0"
@@ -18567,6 +18581,7 @@ dependencies = [
"hash256-std-hasher",
"impl-serde",
"itertools 0.10.5",
+ "k256",
"lazy_static",
"libsecp256k1",
"log",
@@ -22553,9 +22568,9 @@ dependencies = [
[[package]]
name = "zeroize"
-version = "1.6.0"
+version = "1.7.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2a0956f1ba7c7909bfb66c2e9e4124ab6f6482560f6628b5aaeba39207c9aad9"
+checksum = "525b4ec142c6b68a2d10f01f7bbf6755599ca3f81ea53b8431b7dd348f5fdb2d"
dependencies = [
"zeroize_derive",
]
diff --git a/substrate/primitives/core/Cargo.toml b/substrate/primitives/core/Cargo.toml
index 70ca7be6a2d..f7cc2b4fcf7 100644
--- a/substrate/primitives/core/Cargo.toml
+++ b/substrate/primitives/core/Cargo.toml
@@ -52,9 +52,12 @@ blake2 = { version = "0.10.4", default-features = false, optional = true }
libsecp256k1 = { version = "0.7", default-features = false, features = ["static-context"], optional = true }
schnorrkel = { version = "0.11.4", features = ["preaudit_deprecated"], default-features = false }
merlin = { version = "3.0", default-features = false }
-secp256k1 = { version = "0.28.0", default-features = false, features = ["alloc", "recovery"], optional = true }
sp-crypto-hashing = { path = "../crypto/hashing", default-features = false, optional = true }
sp-runtime-interface = { path = "../runtime-interface", default-features = false }
+# k256 crate, better portability, intended to be used in substrate-runtimes (no-std)
+k256 = { version = "0.13.3", features = ["alloc", "ecdsa"], default-features = false }
+# secp256k1 crate, better performance, intended to be used on host side (std)
+secp256k1 = { version = "0.28.0", default-features = false, features = ["alloc", "recovery"], optional = true }
# bls crypto
w3f-bls = { version = "0.1.3", default-features = false, optional = true }
@@ -76,6 +79,7 @@ bench = false
[features]
default = ["std"]
+
std = [
"array-bytes",
"bandersnatch_vrfs?/std",
@@ -94,6 +98,7 @@ std = [
"hash256-std-hasher/std",
"impl-serde/std",
"itertools",
+ "k256/std",
"libsecp256k1/std",
"log/std",
"merlin/std",
@@ -132,6 +137,7 @@ serde = [
"bs58/alloc",
"dep:serde",
"impl-serde",
+ "k256/serde",
"primitive-types/serde_no_std",
"scale-info/serde",
"secrecy/alloc",
@@ -147,7 +153,6 @@ full_crypto = [
"blake2",
"ed25519-zebra",
"libsecp256k1",
- "secp256k1",
"sp-crypto-hashing",
"sp-runtime-interface/disable_target_static_assertions",
]
diff --git a/substrate/primitives/core/src/ecdsa.rs b/substrate/primitives/core/src/ecdsa.rs
index f172b3a7d02..e6bd2c7eb57 100644
--- a/substrate/primitives/core/src/ecdsa.rs
+++ b/substrate/primitives/core/src/ecdsa.rs
@@ -28,14 +28,15 @@ use crate::crypto::{
};
#[cfg(feature = "full_crypto")]
use crate::crypto::{DeriveError, DeriveJunction, Pair as TraitPair, SecretStringError};
-#[cfg(all(feature = "full_crypto", not(feature = "std")))]
-use secp256k1::Secp256k1;
-#[cfg(feature = "std")]
-use secp256k1::SECP256K1;
-#[cfg(feature = "full_crypto")]
+
+#[cfg(all(not(feature = "std"), feature = "full_crypto"))]
+use k256::ecdsa::SigningKey as SecretKey;
+#[cfg(not(feature = "std"))]
+use k256::ecdsa::VerifyingKey;
+#[cfg(all(feature = "std", feature = "full_crypto"))]
use secp256k1::{
ecdsa::{RecoverableSignature, RecoveryId},
- Message, PublicKey, SecretKey,
+ Message, PublicKey, SecretKey, SECP256K1,
};
#[cfg(feature = "serde")]
use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
@@ -53,9 +54,9 @@ pub const PUBLIC_KEY_SERIALIZED_SIZE: usize = 33;
/// The byte length of signature
pub const SIGNATURE_SERIALIZED_SIZE: usize = 65;
-/// A secret seed (which is bytewise essentially equivalent to a SecretKey).
+/// The secret seed.
///
-/// We need it as a different type because `Seed` is expected to be AsRef<[u8]>.
+/// The raw secret seed, which can be used to create the `Pair`.
#[cfg(feature = "full_crypto")]
type Seed = [u8; 32];
@@ -96,18 +97,21 @@ impl Public {
/// Create a new instance from the given full public key.
///
/// This will convert the full public key into the compressed format.
- #[cfg(feature = "std")]
pub fn from_full(full: &[u8]) -> Result<Self, ()> {
- let pubkey = if full.len() == 64 {
+ let mut tagged_full = [0u8; 65];
+ let full = if full.len() == 64 {
// Tag it as uncompressed public key.
- let mut tagged_full = [0u8; 65];
tagged_full[0] = 0x04;
tagged_full[1..].copy_from_slice(full);
- secp256k1::PublicKey::from_slice(&tagged_full)
+ &tagged_full
} else {
- secp256k1::PublicKey::from_slice(full)
+ full
};
- pubkey.map(|k| Self(k.serialize())).map_err(|_| ())
+ #[cfg(feature = "std")]
+ let pubkey = PublicKey::from_slice(&full);
+ #[cfg(not(feature = "std"))]
+ let pubkey = VerifyingKey::from_sec1_bytes(&full);
+ pubkey.map(|k| k.into()).map_err(|_| ())
}
}
@@ -131,6 +135,24 @@ impl AsMut<[u8]> for Public {
}
}
+#[cfg(feature = "std")]
+impl From<PublicKey> for Public {
+ fn from(pubkey: PublicKey) -> Self {
+ Self(pubkey.serialize())
+ }
+}
+
+#[cfg(not(feature = "std"))]
+impl From<VerifyingKey> for Public {
+ fn from(pubkey: VerifyingKey) -> Self {
+ Self::unchecked_from(
+ pubkey.to_sec1_bytes()[..]
+ .try_into()
+ .expect("valid key is serializable to [u8,33]. qed."),
+ )
+ }
+}
+
impl TryFrom<&[u8]> for Public {
type Error = ();
@@ -331,23 +353,34 @@ impl Signature {
/// Recover the public key from this signature and a pre-hashed message.
#[cfg(feature = "full_crypto")]
pub fn recover_prehashed(&self, message: &[u8; 32]) -> Option<Public> {
- let rid = RecoveryId::from_i32(self.0[64] as i32).ok()?;
- let sig = RecoverableSignature::from_compact(&self.0[..64], rid).ok()?;
- let message = Message::from_digest_slice(message).expect("Message is 32 bytes; qed");
-
#[cfg(feature = "std")]
- let context = SECP256K1;
+ {
+ let rid = RecoveryId::from_i32(self.0[64] as i32).ok()?;
+ let sig = RecoverableSignature::from_compact(&self.0[..64], rid).ok()?;
+ let message = Message::from_digest_slice(message).expect("Message is 32 bytes; qed");
+ SECP256K1.recover_ecdsa(&message, &sig).ok().map(Public::from)
+ }
+
#[cfg(not(feature = "std"))]
- let context = Secp256k1::verification_only();
+ {
+ let rid = k256::ecdsa::RecoveryId::from_byte(self.0[64])?;
+ let sig = k256::ecdsa::Signature::from_bytes((&self.0[..64]).into()).ok()?;
+ VerifyingKey::recover_from_prehash(message, &sig, rid).map(Public::from).ok()
+ }
+ }
+}
- context
- .recover_ecdsa(&message, &sig)
- .ok()
- .map(|pubkey| Public(pubkey.serialize()))
+#[cfg(not(feature = "std"))]
+impl From<(k256::ecdsa::Signature, k256::ecdsa::RecoveryId)> for Signature {
+ fn from(recsig: (k256::ecdsa::Signature, k256::ecdsa::RecoveryId)) -> Signature {
+ let mut r = Self::default();
+ r.0[..64].copy_from_slice(&recsig.0.to_bytes());
+ r.0[64] = recsig.1.to_byte();
+ r
}
}
-#[cfg(feature = "full_crypto")]
+#[cfg(all(feature = "std", feature = "full_crypto"))]
impl From<RecoverableSignature> for Signature {
fn from(recsig: RecoverableSignature) -> Signature {
let mut r = Self::default();
@@ -384,17 +417,19 @@ impl TraitPair for Pair {
///
/// You should never need to use this; generate(), generate_with_phrase
fn from_seed_slice(seed_slice: &[u8]) -> Result<Pair, SecretStringError> {
- let secret =
- SecretKey::from_slice(seed_slice).map_err(|_| SecretStringError::InvalidSeedLength)?;
-
#[cfg(feature = "std")]
- let context = SECP256K1;
- #[cfg(not(feature = "std"))]
- let context = Secp256k1::signing_only();
+ {
+ let secret = SecretKey::from_slice(seed_slice)
+ .map_err(|_| SecretStringError::InvalidSeedLength)?;
+ Ok(Pair { public: PublicKey::from_secret_key(&SECP256K1, &secret).into(), secret })
+ }
- let public = PublicKey::from_secret_key(&context, &secret);
- let public = Public(public.serialize());
- Ok(Pair { public, secret })
+ #[cfg(not(feature = "std"))]
+ {
+ let secret = SecretKey::from_slice(seed_slice)
+ .map_err(|_| SecretStringError::InvalidSeedLength)?;
+ Ok(Pair { public: VerifyingKey::from(&secret).into(), secret })
+ }
}
/// Derive a child key from a series of given junctions.
@@ -438,7 +473,14 @@ impl TraitPair for Pair {
impl Pair {
/// Get the seed for this key.
pub fn seed(&self) -> Seed {
- self.secret.secret_bytes()
+ #[cfg(feature = "std")]
+ {
+ self.secret.secret_bytes()
+ }
+ #[cfg(not(feature = "std"))]
+ {
+ self.secret.to_bytes().into()
+ }
}
/// Exactly as `from_string` except that if no matches are found then, the the first 32
@@ -455,14 +497,19 @@ impl Pair {
/// Sign a pre-hashed message
pub fn sign_prehashed(&self, message: &[u8; 32]) -> Signature {
- let message = Message::from_digest_slice(message).expect("Message is 32 bytes; qed");
-
#[cfg(feature = "std")]
- let context = SECP256K1;
- #[cfg(not(feature = "std"))]
- let context = Secp256k1::signing_only();
+ {
+ let message = Message::from_digest_slice(message).expect("Message is 32 bytes; qed");
+ SECP256K1.sign_ecdsa_recoverable(&message, &self.secret).into()
+ }
- context.sign_ecdsa_recoverable(&message, &self.secret).into()
+ #[cfg(not(feature = "std"))]
+ {
+ self.secret
+ .sign_prehash_recoverable(message)
+ .expect("signing may not fail (???). qed.")
+ .into()
+ }
}
/// Verify a signature on a pre-hashed message. Return `true` if the signature is valid
@@ -503,7 +550,7 @@ impl Pair {
// NOTE: this solution is not effective when `Pair` is moved around memory.
// The very same problem affects other cryptographic backends that are just using
// `zeroize`for their secrets.
-#[cfg(feature = "full_crypto")]
+#[cfg(all(feature = "std", feature = "full_crypto"))]
impl Drop for Pair {
fn drop(&mut self) {
self.secret.non_secure_erase()
@@ -770,8 +817,18 @@ mod test {
let msg = [0u8; 32];
let sig1 = pair.sign_prehashed(&msg);
let sig2: Signature = {
- let message = Message::from_digest_slice(&msg).unwrap();
- SECP256K1.sign_ecdsa_recoverable(&message, &pair.secret).into()
+ #[cfg(feature = "std")]
+ {
+ let message = Message::from_digest_slice(&msg).unwrap();
+ SECP256K1.sign_ecdsa_recoverable(&message, &pair.secret).into()
+ }
+ #[cfg(not(feature = "std"))]
+ {
+ pair.secret
+ .sign_prehash_recoverable(&msg)
+ .expect("signing may not fail (???). qed.")
+ .into()
+ }
};
assert_eq!(sig1, sig2);
--
GitLab