From 47d551dce52550cafbae26a10dcdfe640ffd9e68 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Sun, 6 Nov 2022 19:29:17 +0100 Subject: [PATCH] refactor: use k256 crate --- Cargo.lock | 59 +++++++++++++++++++++++---- Cargo.toml | 3 ++ crates/net/discv4/Cargo.toml | 9 ++--- crates/net/discv4/src/error.rs | 4 +- crates/net/discv4/src/lib.rs | 27 +++++++------ crates/net/discv4/src/mock.rs | 20 +++++----- crates/net/discv4/src/proto.rs | 73 +++++++++++++++++++--------------- 7 files changed, 125 insertions(+), 70 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ba818364fba04..abb5fd5d13abd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1010,8 +1010,19 @@ checksum = "413301934810f597c1d19ca71c8710e99a3f1ba28a0d2ebc01551a2daeea3c5c" dependencies = [ "der", "elliptic-curve", - "rfc6979", - "signature", + "rfc6979 0.3.0", + "signature 1.6.4", +] + +[[package]] +name = "ecdsa" +version = "0.15.0-pre" +source = "git+https://github.com/RustCrypto/signatures.git#ea3e3f3bae3d6b2dac9e9e99466dd29b4b0ffe9b" +dependencies = [ + "der", + "elliptic-curve", + "rfc6979 0.3.1", + "signature 2.0.0-pre.2", ] [[package]] @@ -1020,7 +1031,7 @@ version = "1.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1e9c280362032ea4203659fc489832d0204ef09f247a0506f170dafcac08c369" dependencies = [ - "signature", + "signature 1.6.4", ] [[package]] @@ -1102,7 +1113,7 @@ dependencies = [ "bytes", "ed25519-dalek", "hex", - "k256", + "k256 0.11.6", "log", "rand 0.8.5", "rlp", @@ -1120,7 +1131,7 @@ dependencies = [ "bs58", "bytes", "hex", - "k256", + "k256 0.11.6", "log", "rand 0.8.5", "rlp", @@ -1216,7 +1227,7 @@ dependencies = [ "ethabi", "generic-array", "hex", - "k256", + "k256 0.11.6", "open-fastrlp", "rand 0.8.5", "rlp", @@ -2098,12 +2109,23 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72c1e0b51e7ec0a97369623508396067a486bd0cbed95a2659a4b863d28cfc8b" dependencies = [ "cfg-if", - "ecdsa", + "ecdsa 0.14.8", "elliptic-curve", "sha2 0.10.6", "sha3", ] +[[package]] +name = "k256" +version = "0.12.0-pre" +source = "git+https://github.com/RustCrypto/elliptic-curves#c87d391a107b5bc22f03acf0de4ded988797c4ec" +dependencies = [ + "cfg-if", + "ecdsa 0.15.0-pre", + "elliptic-curve", + "sha2 0.10.6", +] + [[package]] name = "keccak" version = "0.1.2" @@ -3080,12 +3102,13 @@ dependencies = [ "discv5", "generic-array", "hex", + "k256 0.12.0-pre", "public-ip", "rand 0.8.5", "reth-primitives", "reth-rlp", "reth-rlp-derive", - "secp256k1", + "sha3", "thiserror", "tokio", "tokio-stream", @@ -3422,6 +3445,16 @@ dependencies = [ "zeroize", ] +[[package]] +name = "rfc6979" +version = "0.3.1" +source = "git+https://github.com/RustCrypto/signatures.git#ea3e3f3bae3d6b2dac9e9e99466dd29b4b0ffe9b" +dependencies = [ + "crypto-bigint", + "hmac", + "zeroize", +] + [[package]] name = "ring" version = "0.16.20" @@ -3898,6 +3931,16 @@ dependencies = [ "rand_core 0.6.4", ] +[[package]] +name = "signature" +version = "2.0.0-pre.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b7d7471fcfa786b75b657061bd5bf1d51179b3aac54db33bba7e44f5a4b5b75" +dependencies = [ + "digest 0.10.5", + "rand_core 0.6.4", +] + [[package]] name = "slab" version = "0.4.7" diff --git a/Cargo.toml b/Cargo.toml index 53a94b14488fb..79e1191f305ae 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,3 +24,6 @@ members = [ "crates/libmdbx-rs/mdbx-sys" ] default-members = ["bin/reth"] + +[patch.crates-io] +ecdsa = { git = "https://github.com/RustCrypto/signatures.git" } diff --git a/crates/net/discv4/Cargo.toml b/crates/net/discv4/Cargo.toml index 14eeba4fdd045..f6a6738dd5043 100644 --- a/crates/net/discv4/Cargo.toml +++ b/crates/net/discv4/Cargo.toml @@ -17,11 +17,8 @@ reth-rlp-derive = { path = "../../common/rlp-derive" } # ethereum discv5 = { git = "https://github.com/sigp/discv5" } -secp256k1 = { version = "0.24", features = [ - "global-context", - "rand-std", - "recovery", -] } +k256 = { git = "https://github.com/RustCrypto/elliptic-curves", default-features = false, features = ["ecdsa", "std"] } +sha3 = { version = "0.10", default-features = false } # async/futures tokio = { version = "1", features = ["io-util", "net", "time"] } @@ -43,4 +40,4 @@ tokio = { version = "1", features = ["full"] } tracing-test = "0.2" [features] -mock = ["rand"] \ No newline at end of file +mock = ["rand"] diff --git a/crates/net/discv4/src/error.rs b/crates/net/discv4/src/error.rs index 804e084ba5894..6906beaf271a8 100644 --- a/crates/net/discv4/src/error.rs +++ b/crates/net/discv4/src/error.rs @@ -15,7 +15,9 @@ pub enum DecodePacketError { #[error("Message id {0} is not supported.")] UnknownMessage(u8), #[error("Failed to recover public key: {0:?}")] - Secp256k1(#[from] secp256k1::Error), + RecoveryError(#[from] k256::ecdsa::Error), + #[error("Recovery id {0} too large")] + InvalidRecoveryId(u8), } /// High level errors that can occur when interacting with the discovery service diff --git a/crates/net/discv4/src/lib.rs b/crates/net/discv4/src/lib.rs index a0638bcf3a7ac..a02fbc9d04955 100644 --- a/crates/net/discv4/src/lib.rs +++ b/crates/net/discv4/src/lib.rs @@ -30,8 +30,8 @@ use discv5::{ }, ConnectionDirection, ConnectionState, }; +use k256::ecdsa::SigningKey; use reth_primitives::{H256, H512}; -use secp256k1::SecretKey; use std::{ cell::RefCell, collections::{btree_map, hash_map::Entry, BTreeMap, HashMap, VecDeque}, @@ -122,10 +122,10 @@ impl Discv4 { pub async fn spawn( local_address: SocketAddr, local_enr: NodeRecord, - secret_key: SecretKey, + signing_key: SigningKey, config: Discv4Config, ) -> io::Result { - let (discv4, service) = Self::bind(local_address, local_enr, secret_key, config).await?; + let (discv4, service) = Self::bind(local_address, local_enr, signing_key, config).await?; let _ = service.spawn(); @@ -138,14 +138,15 @@ impl Discv4 { /// # use std::io; /// use std::net::SocketAddr; /// use std::str::FromStr; + /// use k256::ecdsa::{SigningKey, VerifyingKey}; /// use rand::thread_rng; - /// use secp256k1::SECP256K1; /// use reth_discv4::{Discv4, Discv4Config, NodeId, NodeRecord}; /// # async fn t() -> io::Result<()> { /// // generate a (random) keypair /// let mut rng = thread_rng(); - /// let (secret_key, pk) = SECP256K1.generate_keypair(&mut rng); - /// let id = NodeId::from_slice(&pk.serialize_uncompressed()[1..]); + /// let signing_key = SigningKey::random(&mut rng); + /// let verifying_key = VerifyingKey::from(&signing_key); + /// let id = NodeId::from_slice(&verifying_key.to_encoded_point(false).as_bytes()[1..]); /// /// let socket = SocketAddr::from_str("0.0.0.0:0").unwrap(); /// let local_enr = NodeRecord { @@ -156,7 +157,7 @@ impl Discv4 { /// }; /// let config = Discv4Config::default(); /// - /// let(discv4, mut service) = Discv4::bind(socket, local_enr, secret_key, config).await.unwrap(); + /// let(discv4, mut service) = Discv4::bind(socket, local_enr, signing_key, config).await.unwrap(); /// /// // get an update strea /// let updates = service.update_stream(); @@ -172,7 +173,7 @@ impl Discv4 { pub async fn bind( local_address: SocketAddr, mut local_enr: NodeRecord, - secret_key: SecretKey, + signing_key: SigningKey, config: Discv4Config, ) -> io::Result<(Self, Discv4Service)> { let socket = UdpSocket::bind(local_address).await?; @@ -183,7 +184,7 @@ impl Discv4 { // We don't expect many commands, so the buffer can be quite small here. let (to_service, rx) = mpsc::channel(5); let service = - Discv4Service::new(socket, local_addr, local_enr, secret_key, config, Some(rx)); + Discv4Service::new(socket, local_addr, local_enr, signing_key, config, Some(rx)); let discv4 = Discv4 { local_addr, to_service }; Ok((discv4, service)) } @@ -240,7 +241,7 @@ pub struct Discv4Service { /// Local ENR of the server. local_enr: NodeRecord, /// The secret key used to sign payloads - secret_key: SecretKey, + signing_key: SigningKey, /// The UDP socket for sending and receiving messages. _socket: Arc, /// The spawned UDP tasks. @@ -288,7 +289,7 @@ impl Discv4Service { socket: UdpSocket, local_address: SocketAddr, local_enr: NodeRecord, - secret_key: SecretKey, + signing_key: SigningKey, config: Discv4Config, commands_rx: Option>, ) -> Self { @@ -331,7 +332,7 @@ impl Discv4Service { local_enr, _socket: socket, kbuckets, - secret_key, + signing_key, _tasks: tasks, ingress: ingress_rx, egress: egress_tx, @@ -540,7 +541,7 @@ impl Discv4Service { /// Encodes the packet, sends it and returns the hash. pub(crate) fn send_packet(&mut self, msg: Message, to: SocketAddr) -> H256 { - let (payload, hash) = msg.encode(&self.secret_key); + let (payload, hash) = msg.encode(&self.signing_key).unwrap(); trace!(r#type=?msg.msg_type(), ?to, ?hash, target = "net::disc", "sending packet"); let _ = self.egress.try_send((payload, to)); hash diff --git a/crates/net/discv4/src/mock.rs b/crates/net/discv4/src/mock.rs index c4565911d7fe1..056ebb0695525 100644 --- a/crates/net/discv4/src/mock.rs +++ b/crates/net/discv4/src/mock.rs @@ -8,9 +8,9 @@ use crate::{ receive_loop, send_loop, Discv4, Discv4Config, Discv4Service, EgressSender, IngressEvent, IngressReceiver, NodeId, SAFE_MAX_DATAGRAM_NEIGHBOUR_RECORDS, }; +use k256::ecdsa::{SigningKey, VerifyingKey}; use rand::{thread_rng, Rng, RngCore}; use reth_primitives::H256; -use secp256k1::{SecretKey, SECP256K1}; use std::{ collections::{HashMap, HashSet}, io, @@ -33,7 +33,7 @@ use tracing::error; pub struct MockDiscovery { local_addr: SocketAddr, local_enr: NodeRecord, - secret_key: SecretKey, + signing_key: SigningKey, udp: Arc, _tasks: JoinSet<()>, /// Receiver for incoming messages @@ -50,8 +50,9 @@ impl MockDiscovery { pub async fn new() -> io::Result<(Self, mpsc::Sender)> { let mut rng = thread_rng(); let socket = SocketAddr::from_str("0.0.0.0:0").unwrap(); - let (secret_key, pk) = SECP256K1.generate_keypair(&mut rng); - let id = NodeId::from_slice(&pk.serialize_uncompressed()[1..]); + let signing_key = SigningKey::random(&mut rng); + let verifying_key = VerifyingKey::from(&signing_key); + let id = NodeId::from_slice(&verifying_key.to_encoded_point(false).as_bytes()[1..]); let socket = Arc::new(UdpSocket::bind(socket).await?); let local_addr = socket.local_addr()?; let local_enr = NodeRecord { @@ -78,7 +79,7 @@ impl MockDiscovery { egress: egress_tx, local_addr, local_enr, - secret_key, + signing_key, udp: socket, pending_pongs: Default::default(), pending_neighbours: Default::default(), @@ -114,7 +115,7 @@ impl MockDiscovery { /// Encodes the packet, sends it and returns the hash. fn send_packet(&mut self, msg: Message, to: SocketAddr) -> H256 { - let (payload, hash) = msg.encode(&self.secret_key); + let (payload, hash) = msg.encode(&self.signing_key); let _ = self.egress.try_send((payload, to)); hash } @@ -208,12 +209,13 @@ pub async fn create_discv4() -> (Discv4, Discv4Service) { pub async fn create_discv4_with_config(config: Discv4Config) -> (Discv4, Discv4Service) { let mut rng = thread_rng(); let socket = SocketAddr::from_str("0.0.0.0:0").unwrap(); - let (secret_key, pk) = SECP256K1.generate_keypair(&mut rng); - let id = NodeId::from_slice(&pk.serialize_uncompressed()[1..]); + let signing_key = SigningKey::random(&mut rng); + let verifying_key = VerifyingKey::from(&signing_key); + let id = NodeId::from_slice(&verifying_key.to_encoded_point(false).as_bytes()[1..]); let external_addr = public_ip::addr().await.unwrap_or_else(|| socket.ip()); let local_enr = NodeRecord { address: external_addr, tcp_port: socket.port(), udp_port: socket.port(), id }; - Discv4::bind(socket, local_enr, secret_key, config).await.unwrap() + Discv4::bind(socket, local_enr, signing_key, config).await.unwrap() } pub fn rng_endpoint(rng: &mut impl Rng) -> NodeEndpoint { diff --git a/crates/net/discv4/src/proto.rs b/crates/net/discv4/src/proto.rs index 81036a6282141..5e9ac06437034 100644 --- a/crates/net/discv4/src/proto.rs +++ b/crates/net/discv4/src/proto.rs @@ -2,13 +2,14 @@ use crate::{error::DecodePacketError, node::NodeRecord, NodeId, MAX_PACKET_SIZE, MIN_PACKET_SIZE}; use bytes::{Buf, BufMut, Bytes, BytesMut}; +use k256::{ + ecdsa::{hazmat::SignPrimitive, RecoveryId, Signature, SigningKey, VerifyingKey}, + sha2::Sha256, +}; use reth_primitives::{keccak256, H256}; use reth_rlp::{Decodable, DecodeError, Encodable, Header}; use reth_rlp_derive::{RlpDecodable, RlpEncodable}; -use secp256k1::{ - ecdsa::{RecoverableSignature, RecoveryId}, - SecretKey, SECP256K1, -}; +use sha3::{Digest, Keccak256}; use std::net::{IpAddr, Ipv6Addr}; // Note: this is adapted from https://github.com/vorot93/discv4 @@ -63,14 +64,14 @@ impl Message { /// /// The datagram is `header || payload` /// where header is `hash || signature || packet-type` - pub fn encode(&self, secret_key: &SecretKey) -> (Bytes, H256) { + pub fn encode(&self, signing_key: &SigningKey) -> Result<(Bytes, H256), k256::ecdsa::Error> { // allocate max packet size let mut datagram = BytesMut::with_capacity(MAX_PACKET_SIZE); // since signature has fixed len, we can split and fill the datagram buffer at fixed // positions, this way we can encode the message directly in the datagram buffer let mut sig_bytes = datagram.split_off(H256::len_bytes()); - let mut payload = sig_bytes.split_off(secp256k1::constants::COMPACT_SIGNATURE_SIZE + 1); + let mut payload = sig_bytes.split_off(64 + 1); match self { Message::Ping(message) => { @@ -91,22 +92,22 @@ impl Message { } } - let signature: RecoverableSignature = SECP256K1.sign_ecdsa_recoverable( - &secp256k1::Message::from_slice(keccak256(&payload).as_ref()) - .expect("is correct MESSAGE_SIZE; qed"), - secret_key, - ); + let digest = Keccak256::digest(&payload); + let (signature, recid) = + signing_key.as_nonzero_scalar().try_sign_prehashed_rfc6979::(digest, b"")?; - let (rec, sig) = signature.serialize_compact(); - sig_bytes.extend_from_slice(&sig); - sig_bytes.put_u8(rec.to_i32() as u8); + let recid = recid.ok_or_else(|| k256::ecdsa::Error::new())?; + + // let (rec, sig) = signature.serialize_compact(); + sig_bytes.extend_from_slice(signature.to_bytes().as_slice()); + sig_bytes.put_u8(recid.to_byte()); sig_bytes.unsplit(payload); let hash = keccak256(&sig_bytes); datagram.extend_from_slice(hash.as_bytes()); datagram.unsplit(sig_bytes); - (datagram.freeze(), hash) + Ok((datagram.freeze(), hash)) } /// Decodes the [`Message`] from the given buffer. @@ -128,15 +129,18 @@ impl Message { return Err(DecodePacketError::HashMismatch) } - let signature = &packet[32..96]; - let recovery_id = RecoveryId::from_i32(packet[96] as i32)?; - let recoverable_sig = RecoverableSignature::from_compact(signature, recovery_id)?; - - // recover the public key - let msg = secp256k1::Message::from_slice(keccak256(&packet[97..]).as_bytes())?; + let signature = Signature::try_from(&packet[32..96]).unwrap(); + let recid = packet[96]; + let recid = + RecoveryId::try_from(recid).map_err(|_| DecodePacketError::InvalidRecoveryId(recid))?; - let pk = SECP256K1.recover_ecdsa(&msg, &recoverable_sig)?; - let node_id = NodeId::from_slice(&pk.serialize_uncompressed()[1..]); + // recover the node id + let recovered_key = VerifyingKey::recover_from_digest( + Keccak256::new_with_prefix(&packet[97..]), + &signature, + recid, + )?; + let node_id = NodeId::from_slice(&recovered_key.to_encoded_point(false).as_bytes()[1..]); let msg_type = packet[97]; let payload = &mut &packet[98..]; @@ -391,6 +395,7 @@ mod tests { SAFE_MAX_DATAGRAM_NEIGHBOUR_RECORDS, }; use bytes::BytesMut; + use k256::ecdsa::VerifyingKey; use rand::{thread_rng, Rng, RngCore}; #[test] @@ -453,8 +458,8 @@ mod tests { fn test_hash_mismatch() { let mut rng = thread_rng(); let msg = rng_message(&mut rng); - let (secret_key, _) = SECP256K1.generate_keypair(&mut rng); - let (buf, _) = msg.encode(&secret_key); + let signing_key = SigningKey::random(&mut rng); + let (buf, _) = msg.encode(&signing_key).unwrap(); let mut buf = BytesMut::from(buf.as_ref()); buf.put_u8(0); match Message::decode(buf.as_ref()).unwrap_err() { @@ -475,10 +480,10 @@ mod tests { .collect(), expire: rng.gen(), }); - let (secret_key, _) = SECP256K1.generate_keypair(&mut rng); + let signing_key = SigningKey::random(&mut rng); - let (encoded, _) = msg.encode(&secret_key); - assert!(encoded.len() <= MAX_PACKET_SIZE, "{} {:?}", encoded.len(), msg); + let (encoded, _) = msg.encode(&signing_key).unwrap(); + assert!(encoded.len() <= MAX_PACKET_SIZE, "{} {msg:?}", encoded.len()); let mut neighbours = Neighbours { nodes: std::iter::repeat_with(|| rng_ipv6_record(&mut rng)) @@ -488,8 +493,8 @@ mod tests { }; neighbours.nodes.push(rng_ipv4_record(&mut rng)); let msg = Message::Neighbours(neighbours); - let (encoded, _) = msg.encode(&secret_key); - assert!(encoded.len() <= MAX_PACKET_SIZE, "{} {:?}", encoded.len(), msg); + let (encoded, _) = msg.encode(&signing_key).unwrap(); + assert!(encoded.len() <= MAX_PACKET_SIZE, "{} {msg:?}", encoded.len()); } } @@ -498,10 +503,12 @@ mod tests { let mut rng = thread_rng(); for _ in 0..100 { let msg = rng_message(&mut rng); - let (secret_key, pk) = SECP256K1.generate_keypair(&mut rng); - let sender_id = NodeId::from_slice(&pk.serialize_uncompressed()[1..]); + let signing_key = SigningKey::random(&mut rng); + let verifying_key = VerifyingKey::from(&signing_key); + let sender_id = + NodeId::from_slice(&verifying_key.to_encoded_point(false).as_bytes()[1..]); - let (buf, _) = msg.encode(&secret_key); + let (buf, _) = msg.encode(&signing_key).unwrap(); let packet = Message::decode(buf.as_ref()).unwrap();