Skip to content

Commit

Permalink
p2p: replace x25519-dalek with curve25519-dalek-ng
Browse files Browse the repository at this point in the history
The `tendermint-p2p` crate previously pulled in both an obsolete version
of `x25519-dalek`/`curve25519-dalek`, but also `curve25519-dalek-ng` by
way of `ed25519-consensus`.

This replaces `x25519-dalek` with `curve25519-dalek-ng` which is already
a dependency of `ed25519-consensus`. This reduces the overall number of
dependencies and ensures `curve25519-dalek-ng` is the main Curve25519
implementation used.
  • Loading branch information
tony-iqlusion committed Oct 11, 2023
1 parent bcc0b37 commit 7da8183
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 14 deletions.
4 changes: 2 additions & 2 deletions p2p/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@ amino = ["prost-derive"]

[dependencies]
chacha20poly1305 = { version = "0.8", default-features = false, features = ["reduced-round"] }
curve25519-dalek-ng = { version = "4", 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.12.3", default-features = false }
merlin = { version = "2", default-features = false }
prost = { version = "0.12", default-features = false }
rand_core = { version = "0.5", default-features = false, features = ["std"] }
rand_core = { version = "0.6", default-features = false, features = ["std"] }
sha2 = { version = "0.10", default-features = false }
subtle = { version = "2", default-features = false }
x25519-dalek = { version = "1.1", default-features = false, features = ["u64_backend"] }
zeroize = { version = "1", default-features = false }
signature = { version = "2", default-features = false }
aead = { version = "0.4.1", default-features = false }
Expand Down
13 changes: 8 additions & 5 deletions p2p/src/secret_connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@ use chacha20poly1305::{
aead::{generic_array::GenericArray, AeadInPlace, NewAead},
ChaCha20Poly1305,
};
use curve25519_dalek_ng::{
constants::X25519_BASEPOINT, montgomery::MontgomeryPoint as EphemeralPublic,
scalar::Scalar as EphemeralSecret,
};
use merlin::Transcript;
use rand_core::OsRng;
use subtle::ConstantTimeEq;
use tendermint_proto::v0_38 as proto;
use tendermint_std_ext::TryClone;
use x25519_dalek::{EphemeralSecret, PublicKey as EphemeralPublic};

pub use self::{
kdf::Kdf,
Expand Down Expand Up @@ -82,8 +85,8 @@ impl Handshake<AwaitingEphKey> {
protocol_version: Version,
) -> (Self, EphemeralPublic) {
// Generate an ephemeral key for perfect forward secrecy.
let local_eph_privkey = EphemeralSecret::new(OsRng);
let local_eph_pubkey = EphemeralPublic::from(&local_eph_privkey);
let local_eph_privkey = EphemeralSecret::random(&mut OsRng);
let local_eph_pubkey = X25519_BASEPOINT * &local_eph_privkey;

(
Self {
Expand Down Expand Up @@ -111,10 +114,10 @@ impl Handshake<AwaitingEphKey> {
let Some(local_eph_privkey) = self.state.local_eph_privkey.take() else {
return Err(Error::missing_secret());
};
let local_eph_pubkey = EphemeralPublic::from(&local_eph_privkey);
let local_eph_pubkey = X25519_BASEPOINT * &local_eph_privkey;

// Compute common shared secret.
let shared_secret = EphemeralSecret::diffie_hellman(local_eph_privkey, &remote_eph_pubkey);
let shared_secret = &local_eph_privkey * &remote_eph_pubkey;

let mut transcript = Transcript::new(b"TENDERMINT_SECRET_CONNECTION_TRANSCRIPT_HASH");

Expand Down
6 changes: 3 additions & 3 deletions p2p/src/secret_connection/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
use std::convert::TryInto;

use curve25519_dalek_ng::montgomery::MontgomeryPoint as EphemeralPublic;
use prost::Message as _;
use tendermint_proto::v0_38 as proto;
use x25519_dalek::PublicKey as EphemeralPublic;

#[cfg(feature = "amino")]
use super::amino_types;
Expand Down Expand Up @@ -83,7 +83,7 @@ impl Version {
}

let eph_pubkey_bytes: [u8; 32] = bytes[2..].try_into().expect("framing failed");
EphemeralPublic::from(eph_pubkey_bytes)
EphemeralPublic(eph_pubkey_bytes)
} else {
// Equivalent Go implementation:
// https://github.com/tendermint/tendermint/blob/013b9ce/p2p/conn/secret_connection.go#L220-L225
Expand All @@ -94,7 +94,7 @@ impl Version {
}

let eph_pubkey_bytes: [u8; 32] = bytes[1..].try_into().expect("framing failed");
EphemeralPublic::from(eph_pubkey_bytes)
EphemeralPublic(eph_pubkey_bytes)
};

// Reject the key if it is of low order
Expand Down
2 changes: 1 addition & 1 deletion test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ authors = ["Alexander Simmerl <[email protected]>"]
test = true

[dev-dependencies]
curve25519-dalek-ng = { version = "4", default-features = false }
ed25519-consensus = { version = "2", default-features = false }
flex-error = { version = "0.4.4", default-features = false }
flume = { version = "0.10", default-features = false }
rand_core = { version = "0.6", default-features = false, features = ["std"] }
readwrite = { version = "^0.1.1", default-features = false }
subtle-encoding = { version = "0.5", default-features = false }
x25519-dalek = { version = "1.1", default-features = false }

tendermint = { path = "../tendermint", default-features = false }
tendermint-p2p = { path = "../p2p", default-features = false }
Expand Down
8 changes: 5 additions & 3 deletions test/src/test/unit/p2p/secret_connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ use std::{
thread,
};

use curve25519_dalek_ng::{
constants::X25519_BASEPOINT, montgomery::MontgomeryPoint as EphemeralPublic,
};
use rand_core::OsRng;
use tendermint_p2p::secret_connection::{sort32, Handshake, SecretConnection, Version};
use tendermint_proto::v0_38 as proto;
use x25519_dalek::PublicKey as EphemeralPublic;

use crate::pipe;

Expand Down Expand Up @@ -66,7 +68,7 @@ fn test_evil_peer_shares_invalid_eph_key() {
let local_privkey = ed25519_consensus::SigningKey::new(csprng);
let (mut h, _) = Handshake::new(local_privkey, Version::V0_34);
let bytes: [u8; 32] = [0; 32];
let res = h.got_key(EphemeralPublic::from(bytes));
let res = h.got_key(EphemeralPublic(bytes));
assert!(res.is_err());
}

Expand All @@ -75,7 +77,7 @@ fn test_evil_peer_shares_invalid_auth_sig() {
let csprng = OsRng {};
let local_privkey = ed25519_consensus::SigningKey::new(csprng);
let (mut h, _) = Handshake::new(local_privkey, Version::V0_34);
let res = h.got_key(EphemeralPublic::from(x25519_dalek::X25519_BASEPOINT_BYTES));
let res = h.got_key(X25519_BASEPOINT);
assert!(res.is_ok());

let mut h = res.unwrap();
Expand Down

0 comments on commit 7da8183

Please sign in to comment.