Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deps: Use ed25519-consensus instead of ed25519-dalek (#1067) #1245

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- `[tendermint, tendermint-p2p]` Replaced the `ed25519-dalek` dependency with
`ed25519-consensus`
([#1046](https://github.com/informalsystems/tendermint-rs/pull/1046))
2 changes: 1 addition & 1 deletion config/src/node_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(),
}
}
Expand Down
6 changes: 3 additions & 3 deletions p2p/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "tendermint-p2p"
version = "0.28.0"
edition = "2018"
edition = "2021"
license = "Apache-2.0"
repository = "https://github.com/informalsystems/tendermint-rs"
homepage = "https://tendermint.com"
Expand All @@ -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 = "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 }
Expand All @@ -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 }
Expand Down
2 changes: 0 additions & 2 deletions p2p/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

use flex_error::{define_error, DisplayOnly};
use prost::DecodeError;
use signature::Error as SignatureError;

define_error! {
Error {
Expand Down Expand Up @@ -40,7 +39,6 @@ define_error! {
| _ | { "public key missing" },

Signature
[ DisplayOnly<SignatureError> ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error is deliberately opaque, but we can't make the Display impl transparent in flex-error, so we have our own message. OK.

| _ | { "signature error" },

UnsupportedKey
Expand Down
40 changes: 16 additions & 24 deletions p2p/src/secret_connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>,
}

Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
ed25519_consensus::VerificationKey::try_from(&bytes[..])
.map_err(|_| Error::signature())
},
proto::crypto::public_key::Sum::Secp256k1(_) => Err(Error::unsupported_key()),
mzabaluev marked this conversation as resolved.
Show resolved Hide resolved
}?;

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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions p2p/src/secret_connection/amino_types.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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(),
}
}
}
Expand Down
15 changes: 7 additions & 8 deletions p2p/src/secret_connection/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
Expand All @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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")
}
Expand Down
19 changes: 9 additions & 10 deletions p2p/src/secret_connection/public_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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),
}
Expand All @@ -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)
}
}
2 changes: 1 addition & 1 deletion tendermint/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ rustdoc-args = ["--cfg", "docsrs"]
[dependencies]
bytes = { version = "1.2", 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 = "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 }
Expand Down
2 changes: 1 addition & 1 deletion tendermint/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions tendermint/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)" },
Expand Down
36 changes: 28 additions & 8 deletions tendermint/src/private_key.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
//! Cryptographic private keys

pub use ed25519_dalek::{Keypair as Ed25519, EXPANDED_SECRET_KEY_LENGTH as ED25519_KEYPAIR_SIZE};
pub use ed25519_consensus::SigningKey as Ed25519;

use crate::prelude::*;
use crate::public_key::PublicKey;
use ed25519_consensus::VerificationKey;
use serde::{de, ser, Deserialize, Serialize};
use subtle_encoding::{Base64, Encoding};
use zeroize::Zeroizing;

use crate::{prelude::*, public_key::PublicKey};
pub const ED25519_KEYPAIR_SIZE: usize = 64;

/// Private keys as parsed from configuration files
#[derive(Serialize, Deserialize)]
Expand All @@ -25,24 +29,28 @@ impl PrivateKey {
/// Get the public key associated with this private key
pub fn public_key(&self) -> PublicKey {
match self {
PrivateKey::Ed25519(private_key) => private_key.public.into(),
PrivateKey::Ed25519(signing_key) => PublicKey::Ed25519(signing_key.verification_key()),
}
}

/// If applicable, borrow the Ed25519 keypair
pub fn ed25519_keypair(&self) -> Option<&Ed25519> {
pub fn ed25519_signing_key(&self) -> Option<&Ed25519> {
match self {
PrivateKey::Ed25519(keypair) => Some(keypair),
PrivateKey::Ed25519(signing_key) => Some(signing_key),
}
}
}

/// Serialize an Ed25519 keypair as Base64
fn serialize_ed25519_keypair<S>(keypair: &Ed25519, serializer: S) -> Result<S::Ok, S::Error>
fn serialize_ed25519_keypair<S>(signing_key: &Ed25519, serializer: S) -> Result<S::Ok, S::Error>
where
S: ser::Serializer,
{
let keypair_bytes = Zeroizing::new(keypair.to_bytes());
// Tendermint uses a serialization format inherited from Go that includes
// a cached copy of the public key as the second half.
let mut keypair_bytes = Zeroizing::new([0u8; ED25519_KEYPAIR_SIZE]);
keypair_bytes[0..32].copy_from_slice(signing_key.as_bytes());
keypair_bytes[32..64].copy_from_slice(signing_key.verification_key().as_bytes());
Zeroizing::new(String::from_utf8(Base64::default().encode(&keypair_bytes[..])).unwrap())
.serialize(serializer)
}
Expand All @@ -63,5 +71,17 @@ where
return Err(D::Error::custom("invalid Ed25519 keypair size"));
}

Ed25519::from_bytes(&*keypair_bytes).map_err(D::Error::custom)
// Tendermint uses a serialization format inherited from Go that includes a
// cached copy of the public key as the second half. This is somewhat
// dangerous, since there's no validation that the two parts are consistent
// with each other, so we ignore the second half and just check consistency
// with the re-derived data.
let signing_key = Ed25519::try_from(&keypair_bytes[0..32])
.map_err(|_| D::Error::custom("invalid signing key"))?;
let verification_key = VerificationKey::from(&signing_key);
if &keypair_bytes[32..64] != verification_key.as_bytes() {
return Err(D::Error::custom("keypair mismatch"));
}
Comment on lines +74 to +84
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!


Ok(signing_key)
}
Loading