deps: Use ed25519-consensus instead of ed25519-dalek (informalsystems…

…#1067)

* Use ed25519-consensus instead of ed25519-dalek

Closes informalsystems#355 (see that issue for more context; `ed25519-consensus` is a fork of
`ed25519-zebra` that's Zcash-independent).

This change ensures that `tendermint-rs` has the same signature verification as
`tendermint`, which uses the validation criteria provided by the Go
`ed25519consensus` library.

* clippy fixes

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* Remove redundant dependency

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* cargo fmt

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Add changelog entries

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Co-authored-by: Henry de Valence <hdevalence@penumbra.zone>

.changelog/unreleased/breaking-changes/1046-p2p-ed25519-consensus.md

@@ -0,0 +1,3 @@
+- `[tendermint-p2p]` All public APIs using `ed25519-
+  dalek` types now use types from `ed25519-consensus`
+  ([#1046](https://github.com/informalsystems/tendermint-rs/pull/1046))

.changelog/unreleased/dependencies/1046-ed25519-consensus-dep.md

@@ -0,0 +1,3 @@
+- `[tendermint, tendermint-p2p]` Replaced the `ed25519-dalek` dependency with
+  `ed25519-consensus`
+  ([#1046](https://github.com/informalsystems/tendermint-rs/pull/1046))

config/src/node_key.rs

@@ -36,7 +36,7 @@ impl NodeKey {
     pub fn public_key(&self) -> PublicKey {
         #[allow(unreachable_patterns)]
         match &self.priv_key {
-            PrivateKey::Ed25519(keypair) => keypair.public.into(),
+            PrivateKey::Ed25519(signing_key) => PublicKey::Ed25519(signing_key.verification_key()),
             _ => unreachable!(),
         }
     }

p2p/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name        = "tendermint-p2p"
 version     = "0.23.9"
-edition     = "2018"
+edition     = "2021"
 license     = "Apache-2.0"
 repository  = "https://github.com/informalsystems/tendermint-rs"
 homepage    = "https://tendermint.com"
@@ -28,7 +28,7 @@ amino = ["prost-derive"]
 
 [dependencies]
 chacha20poly1305 = { version = "0.8", default-features = false, features = ["reduced-round"] }
-ed25519-dalek = { version = "1", default-features = false }
+ed25519-consensus = { version = "1.2", default-features = false }
 eyre = { version = "0.6", default-features = false }
 flume = { version = "0.10.7", default-features = false }
 hkdf = { version = "0.10.0", default-features = false }
@@ -37,7 +37,7 @@ prost = { version = "0.11", default-features = false }
 rand_core = { version = "0.5", default-features = false, features = ["std"] }
 sha2 = { version = "0.9", default-features = false }
 subtle = { version = "2", default-features = false }
-x25519-dalek = { version = "1.1", default-features = false }
+x25519-dalek = { version = "1.1", default-features = false, features = ["u64_backend"] }
 zeroize = { version = "1", default-features = false }
 signature = { version = "1", default-features = false }
 aead = { version = "0.4.1", default-features = false }

p2p/src/error.rs

@@ -6,7 +6,6 @@
 
 use flex_error::{define_error, DisplayOnly};
 use prost::DecodeError;
-use signature::Error as SignatureError;
 
 define_error! {
     Error {
@@ -40,7 +39,6 @@ define_error! {
             | _ | { "public key missing" },
 
         Signature
-            [ DisplayOnly<SignatureError> ]
             | _ | { "signature error" },
 
         UnsupportedKey

p2p/src/secret_connection.rs

@@ -16,7 +16,6 @@ use chacha20poly1305::{
     aead::{generic_array::GenericArray, AeadInPlace, NewAead},
     ChaCha20Poly1305,
 };
-use ed25519_dalek::{self as ed25519, Signer, Verifier};
 use merlin::Transcript;
 use rand_core::OsRng;
 use subtle::ConstantTimeEq;
@@ -61,7 +60,7 @@ pub struct Handshake<S> {
 
 /// `AwaitingEphKey` means we're waiting for the remote ephemeral pubkey.
 pub struct AwaitingEphKey {
-    local_privkey: ed25519::Keypair,
+    local_privkey: ed25519_consensus::SigningKey,
     local_eph_privkey: Option<EphemeralSecret>,
 }
 
@@ -71,15 +70,15 @@ pub struct AwaitingAuthSig {
     kdf: Kdf,
     recv_cipher: ChaCha20Poly1305,
     send_cipher: ChaCha20Poly1305,
-    local_signature: ed25519::Signature,
+    local_signature: ed25519_consensus::Signature,
 }
 
 #[allow(clippy::use_self)]
 impl Handshake<AwaitingEphKey> {
     /// Initiate a handshake.
     #[must_use]
     pub fn new(
-        local_privkey: ed25519::Keypair,
+        local_privkey: ed25519_consensus::SigningKey,
         protocol_version: Version,
     ) -> (Self, EphemeralPublic) {
         // Generate an ephemeral key for perfect forward secrecy.
@@ -151,9 +150,9 @@ impl Handshake<AwaitingEphKey> {
 
         // Sign the challenge bytes for authentication.
         let local_signature = if self.protocol_version.has_transcript() {
-            sign_challenge(&sc_mac, &self.state.local_privkey)?
+            self.state.local_privkey.sign(&sc_mac)
         } else {
-            sign_challenge(&kdf.challenge, &self.state.local_privkey)?
+            self.state.local_privkey.sign(&kdf.challenge)
         };
 
         Ok(Handshake {
@@ -186,22 +185,23 @@ impl Handshake<AwaitingAuthSig> {
 
         let remote_pubkey = match pk_sum {
             proto::crypto::public_key::Sum::Ed25519(ref bytes) => {
-                ed25519::PublicKey::from_bytes(bytes).map_err(Error::signature)
-            },
-            proto::crypto::public_key::Sum::Secp256k1(_) => Err(Error::unsupported_key()),
+                ed25519_consensus::VerificationKey::try_from(&bytes[..])
+                    .map_err(|_| Error::signature())
+            }
+            _ => Err(Error::unsupported_key()),
         }?;
 
-        let remote_sig =
-            ed25519::Signature::try_from(auth_sig_msg.sig.as_slice()).map_err(Error::signature)?;
+        let remote_sig = ed25519_consensus::Signature::try_from(auth_sig_msg.sig.as_slice())
+            .map_err(|_| Error::signature())?;
 
         if self.protocol_version.has_transcript() {
             remote_pubkey
-                .verify(&self.state.sc_mac, &remote_sig)
-                .map_err(Error::signature)?;
+                .verify(&remote_sig, &self.state.sc_mac)
+                .map_err(|_| Error::signature())?;
         } else {
             remote_pubkey
-                .verify(&self.state.kdf.challenge, &remote_sig)
-                .map_err(Error::signature)?;
+                .verify(&remote_sig, &self.state.kdf.challenge)
+                .map_err(|_| Error::signature())?;
         }
 
         // We've authorized.
@@ -279,7 +279,7 @@ impl<IoHandler: Read + Write + Send + Sync> SecretConnection<IoHandler> {
     /// * if receiving the signature fails
     pub fn new(
         mut io_handler: IoHandler,
-        local_privkey: ed25519::Keypair,
+        local_privkey: ed25519_consensus::SigningKey,
         protocol_version: Version,
     ) -> Result<Self, Error> {
         // Start a handshake process.
@@ -470,20 +470,12 @@ fn share_eph_pubkey<IoHandler: Read + Write + Send + Sync>(
     protocol_version.decode_initial_handshake(&buf)
 }
 
-/// Sign the challenge with the local private key
-fn sign_challenge(
-    challenge: &[u8; 32],
-    local_privkey: &dyn Signer<ed25519::Signature>,
-) -> Result<ed25519::Signature, Error> {
-    local_privkey.try_sign(challenge).map_err(Error::signature)
-}
-
 // TODO(ismail): change from DecodeError to something more generic
 // this can also fail while writing / sending
 fn share_auth_signature<IoHandler: Read + Write + Send + Sync>(
     sc: &mut SecretConnection<IoHandler>,
-    pubkey: &ed25519::PublicKey,
-    local_signature: &ed25519::Signature,
+    pubkey: &ed25519_consensus::VerificationKey,
+    local_signature: &ed25519_consensus::Signature,
 ) -> Result<proto::p2p::AuthSigMessage, Error> {
     let buf = sc
         .protocol_version

p2p/src/secret_connection/amino_types.rs

@@ -1,8 +1,6 @@
 //! Amino types used by Secret Connection
 
 use core::convert::TryFrom;
-
-use ed25519_dalek as ed25519;
 use prost_derive::Message;
 use tendermint_proto as proto;
 
@@ -24,13 +22,16 @@ pub struct AuthSigMessage {
 }
 
 impl AuthSigMessage {
-    pub fn new(pub_key: &ed25519::PublicKey, sig: &ed25519::Signature) -> Self {
+    pub fn new(
+        pub_key: &ed25519_consensus::VerificationKey,
+        sig: &ed25519_consensus::Signature,
+    ) -> Self {
         let mut pub_key_bytes = Vec::from(PUB_KEY_ED25519_AMINO_PREFIX);
-        pub_key_bytes.extend_from_slice(pub_key.as_ref());
+        pub_key_bytes.extend_from_slice(pub_key.as_bytes());
 
         Self {
             pub_key: pub_key_bytes,
-            sig: sig.as_ref().to_vec(),
+            sig: sig.to_bytes().to_vec(),
         }
     }
 }

p2p/src/secret_connection/protocol.rs

@@ -2,7 +2,6 @@
 
 use std::convert::TryInto;
 
-use ed25519_dalek as ed25519;
 use prost::Message as _;
 use tendermint_proto as proto;
 use x25519_dalek::PublicKey as EphemeralPublic;
@@ -110,8 +109,8 @@ impl Version {
     #[must_use]
     pub fn encode_auth_signature(
         self,
-        pub_key: &ed25519::PublicKey,
-        signature: &ed25519::Signature,
+        pub_key: &ed25519_consensus::VerificationKey,
+        signature: &ed25519_consensus::Signature,
     ) -> Vec<u8> {
         if self.is_protobuf() {
             // Protobuf `AuthSigMessage`
@@ -123,7 +122,7 @@ impl Version {
 
             let msg = proto::p2p::AuthSigMessage {
                 pub_key: Some(pub_key),
-                sig: signature.as_ref().to_vec(),
+                sig: signature.to_bytes().to_vec(),
             };
 
             let mut buf = Vec::new();
@@ -165,8 +164,8 @@ impl Version {
     #[cfg(feature = "amino")]
     fn encode_auth_signature_amino(
         self,
-        pub_key: &ed25519::PublicKey,
-        signature: &ed25519::Signature,
+        pub_key: &ed25519_consensus::VerificationKey,
+        signature: &ed25519_consensus::Signature,
     ) -> Vec<u8> {
         // Legacy Amino encoded `AuthSigMessage`
         let msg = amino_types::AuthSigMessage::new(pub_key, signature);
@@ -181,8 +180,8 @@ impl Version {
     #[cfg(not(feature = "amino"))]
     const fn encode_auth_signature_amino(
         self,
-        _: &ed25519::PublicKey,
-        _: &ed25519::Signature,
+        _: &ed25519_consensus::VerificationKey,
+        _: &ed25519_consensus::Signature,
     ) -> Vec<u8> {
         panic!("attempted to encode auth signature using amino, but 'amino' feature is not present")
     }

p2p/src/secret_connection/public_key.rs

@@ -2,15 +2,14 @@
 
 use std::fmt::{self, Display};
 
-use ed25519_dalek as ed25519;
 use sha2::{digest::Digest, Sha256};
 use tendermint::{error::Error, node};
 
 /// Secret Connection peer public keys (signing, presently Ed25519-only)
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum PublicKey {
     /// Ed25519 Secret Connection Keys
-    Ed25519(ed25519::PublicKey),
+    Ed25519(ed25519_consensus::VerificationKey),
 }
 
 impl PublicKey {
@@ -20,14 +19,14 @@ impl PublicKey {
     ///
     /// * if the bytes given are invalid
     pub fn from_raw_ed25519(bytes: &[u8]) -> Result<Self, Error> {
-        ed25519::PublicKey::from_bytes(bytes)
+        ed25519_consensus::VerificationKey::try_from(bytes)
             .map(Self::Ed25519)
-            .map_err(Error::signature)
+            .map_err(|_| Error::signature())
     }
 
     /// Get Ed25519 public key
     #[must_use]
-    pub const fn ed25519(self) -> Option<ed25519::PublicKey> {
+    pub const fn ed25519(self) -> Option<ed25519_consensus::VerificationKey> {
         match self {
             Self::Ed25519(pk) => Some(pk),
         }
@@ -54,14 +53,14 @@ impl Display for PublicKey {
     }
 }
 
-impl From<&ed25519::Keypair> for PublicKey {
-    fn from(sk: &ed25519::Keypair) -> Self {
-        Self::Ed25519(sk.public)
+impl From<&ed25519_consensus::SigningKey> for PublicKey {
+    fn from(sk: &ed25519_consensus::SigningKey) -> Self {
+        Self::Ed25519(sk.verification_key())
     }
 }
 
-impl From<ed25519::PublicKey> for PublicKey {
-    fn from(pk: ed25519::PublicKey) -> Self {
+impl From<ed25519_consensus::VerificationKey> for PublicKey {
+    fn from(pk: ed25519_consensus::VerificationKey) -> Self {
         Self::Ed25519(pk)
     }
 }

tendermint/Cargo.toml

@@ -33,7 +33,7 @@ rustdoc-args = ["--cfg", "docsrs"]
 async-trait = { version = "0.1", default-features = false }
 bytes = { version = "1.0", default-features = false, features = ["serde"] }
 ed25519 = { version = "1.3", default-features = false }
-ed25519-dalek = { version = "1", default-features = false, features = ["u64_backend"] }
+ed25519-consensus = { version = "1.2", default-features = false }
 futures = { version = "0.3", default-features = false }
 num-traits = { version = "0.2", default-features = false }
 once_cell = { version = "1.3", default-features = false }

tendermint/src/account.rs

@@ -161,7 +161,7 @@ mod tests {
         let id_bytes = Id::from_str(id_hex).expect("expected id_hex to decode properly");
 
         // get id for pubkey
-        let pubkey = Ed25519::from_bytes(pubkey_bytes).unwrap();
+        let pubkey = Ed25519::try_from(&pubkey_bytes[..]).unwrap();
         let id = Id::from(pubkey);
 
         assert_eq!(id_bytes.ct_eq(&id).unwrap_u8(), 1);

tendermint/src/error.rs

@@ -209,8 +209,7 @@ define_error! {
             |_| { format_args!("subtle encoding error") },
 
         Signature
-            [ DisplayOnly<signature::Error> ]
-            |_| { format_args!("signature error") },
+            |_| { "signature error" },
 
         TrustThresholdTooLarge
             |_| { "trust threshold is too large (must be <= 1)" },