Skip to content

Commit 872429e

Browse files
committed
Migrate (back) to ed25519-dalek
We've had many requests to leverage `ed25519-dalek`'s `ExpandedSecretKey` functionality in order to support keys exported from YubiHSMs. See #978, #891, and #742. I have been a bit wary of the implementation though, because it has always involved having both `ed25519-consensus` and `ed25519-dalek`, often with enums over multiple key types, which seems incredibly overcomplicated just to implement this feature. We originally switched to `ed25519-consensus` because `tendermint-rs` did, and it makes sense there where ZIP-215 provides consensus-critical signature verification rules that don't diverge between implementations. However that's a bit irrelevant for our purposes as this is a signing service, and ZIP-215 allows us to use whatever signing implementation we want. Now that we've vendored `tendermint-p2p` as `tmkms-p2p` we can unilaterally switch the Ed25519 implementation (back) to `ed25519-dalek`, which should make adding `ExpandedSecretKey` support much easier and avoid having to worry about two implementations. This additionally switches (back) to upstream `curve25519-dalek` so as to support X25519 as part of the SecretConnection protocol.
1 parent f5f8d06 commit 872429e

File tree

9 files changed

+47
-50
lines changed

9 files changed

+47
-50
lines changed

Cargo.lock

Lines changed: 4 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ chrono = "0.4"
2222
clap = "4"
2323
cosmrs = "0.22"
2424
ed25519 = "2"
25-
ed25519-consensus = "2"
25+
ed25519-dalek = "2"
2626
elliptic-curve = { version = "0.13", features = ["pkcs8"], optional = true }
2727
eyre = "0.6"
2828
getrandom = "0.2"

src/keyring/ed25519/signing_key.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ type SigningKeyBytes = [u8; SigningKey::BYTE_SIZE];
77

88
/// Ed25519 signing key.
99
#[derive(Clone, Debug)]
10-
pub struct SigningKey(ed25519_consensus::SigningKey);
10+
pub struct SigningKey(ed25519_dalek::SigningKey);
1111

1212
impl SigningKey {
1313
/// Size of an encoded Ed25519 signing key in bytes.
@@ -20,7 +20,7 @@ impl SigningKey {
2020

2121
/// Get the verifying key for this signing key.
2222
pub fn verifying_key(&self) -> VerifyingKey {
23-
VerifyingKey(self.0.verification_key())
23+
VerifyingKey(self.0.verifying_key())
2424
}
2525
}
2626

@@ -30,8 +30,8 @@ impl From<SigningKeyBytes> for SigningKey {
3030
}
3131
}
3232

33-
impl From<SigningKey> for ed25519_consensus::SigningKey {
34-
fn from(signing_key: SigningKey) -> ed25519_consensus::SigningKey {
33+
impl From<SigningKey> for ed25519_dalek::SigningKey {
34+
fn from(signing_key: SigningKey) -> ed25519_dalek::SigningKey {
3535
signing_key.0
3636
}
3737
}
@@ -42,8 +42,8 @@ impl From<&SigningKey> for tmkms_p2p::PublicKey {
4242
}
4343
}
4444

45-
impl From<ed25519_consensus::SigningKey> for SigningKey {
46-
fn from(signing_key: ed25519_consensus::SigningKey) -> SigningKey {
45+
impl From<ed25519_dalek::SigningKey> for SigningKey {
46+
fn from(signing_key: ed25519_dalek::SigningKey) -> SigningKey {
4747
SigningKey(signing_key)
4848
}
4949
}

src/keyring/ed25519/verifying_key.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use signature::Verifier;
55

66
/// Ed25519 verification key.
77
#[derive(Clone, Debug)]
8-
pub struct VerifyingKey(pub(super) ed25519_consensus::VerificationKey);
8+
pub struct VerifyingKey(pub(super) ed25519_dalek::VerifyingKey);
99

1010
impl VerifyingKey {
1111
/// Size of an encoded Ed25519 verifying key in bytes.
@@ -45,10 +45,7 @@ impl From<&VerifyingKey> for tmkms_p2p::PublicKey {
4545

4646
impl Verifier<Signature> for VerifyingKey {
4747
fn verify(&self, msg: &[u8], sig: &Signature) -> signature::Result<()> {
48-
let sig = ed25519_consensus::Signature::from(sig.to_bytes());
49-
self.0
50-
.verify(&sig, msg)
51-
.map_err(|_| signature::Error::new())
48+
self.0.verify(msg, &sig)
5249
}
5350
}
5451

tmkms-p2p/Cargo.toml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,18 @@ description = """
2121
"""
2222

2323
[dependencies]
24-
chacha20poly1305 = { version = "0.10", default-features = false, features = ["reduced-round"] }
25-
curve25519-dalek-ng = { version = "4", default-features = false }
26-
ed25519-consensus = { version = "2", default-features = false }
24+
aead = { version = "0.5", default-features = false }
25+
chacha20poly1305 = { version = "0.10", default-features = false }
26+
curve25519-dalek = { version = "4", default-features = false, features = ["rand_core"] }
27+
ed25519-dalek = { version = "2", default-features = false }
2728
hkdf = { version = "0.12.3", default-features = false }
2829
merlin = { version = "3", default-features = false }
2930
prost = { version = "0.13", default-features = false, features = ["std"] }
3031
rand_core = { version = "0.6", default-features = false, features = ["std"] }
3132
sha2 = { version = "0.10", default-features = false }
32-
subtle = { version = "2", default-features = false }
33-
zeroize = { version = "1", default-features = false }
3433
signature = { version = "2", default-features = false }
35-
aead = { version = "0.5", default-features = false }
34+
subtle = { version = "2", default-features = false }
3635
tendermint = { version = "0.40.4", default-features = false }
3736
tendermint-proto = { version = "0.40.4", default-features = false }
3837
tendermint-std-ext = { version = "0.40.4", default-features = false }
38+
zeroize = { version = "1", default-features = false }

tmkms-p2p/src/handshake.rs

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ use crate::{
77
state::{ReceiveState, SendState},
88
};
99
use chacha20poly1305::{ChaCha20Poly1305, KeyInit};
10-
use curve25519_dalek_ng::{
11-
constants::X25519_BASEPOINT, montgomery::MontgomeryPoint as EphemeralPublic,
12-
scalar::Scalar as EphemeralSecret,
10+
use curve25519_dalek::{
11+
montgomery::MontgomeryPoint as EphemeralPublic, scalar::Scalar as EphemeralSecret,
1312
};
13+
use ed25519_dalek::{Signer, Verifier};
1414
use merlin::Transcript;
1515
use rand_core::OsRng;
1616
use subtle::ConstantTimeEq;
@@ -27,18 +27,18 @@ pub(crate) struct Handshake<S> {
2727

2828
/// `AwaitingEphKey` means we're waiting for the remote ephemeral pubkey.
2929
pub(crate) struct AwaitingEphKey {
30-
local_privkey: ed25519_consensus::SigningKey,
30+
local_privkey: ed25519_dalek::SigningKey,
3131
local_eph_privkey: Option<EphemeralSecret>,
3232
}
3333

3434
#[allow(clippy::use_self)]
3535
impl Handshake<AwaitingEphKey> {
3636
/// Initiate a handshake.
3737
#[must_use]
38-
pub fn new(local_privkey: ed25519_consensus::SigningKey) -> (Self, EphemeralPublic) {
38+
pub fn new(local_privkey: ed25519_dalek::SigningKey) -> (Self, EphemeralPublic) {
3939
// Generate an ephemeral key for perfect forward secrecy.
4040
let local_eph_privkey = EphemeralSecret::random(&mut OsRng);
41-
let local_eph_pubkey = X25519_BASEPOINT * local_eph_privkey;
41+
let local_eph_pubkey = EphemeralPublic::mul_base(&local_eph_privkey);
4242

4343
(
4444
Self {
@@ -67,9 +67,10 @@ impl Handshake<AwaitingEphKey> {
6767
let Some(local_eph_privkey) = self.state.local_eph_privkey.take() else {
6868
return Err(Error::MissingSecret);
6969
};
70-
let local_eph_pubkey = X25519_BASEPOINT * local_eph_privkey;
70+
let local_eph_pubkey = EphemeralPublic::mul_base(&local_eph_privkey);
7171

7272
// Compute common shared secret.
73+
// TODO(tarcieri): use `mul_clamped` instead?
7374
let shared_secret = local_eph_privkey * remote_eph_pubkey;
7475

7576
let mut transcript = Transcript::new(b"TENDERMINT_SECRET_CONNECTION_TRANSCRIPT_HASH");
@@ -122,7 +123,7 @@ pub(crate) struct AwaitingAuthSig {
122123
sc_mac: [u8; 32],
123124
recv_cipher: ChaCha20Poly1305,
124125
send_cipher: ChaCha20Poly1305,
125-
local_signature: ed25519_consensus::Signature,
126+
local_signature: ed25519_dalek::Signature,
126127
}
127128

128129
impl Handshake<AwaitingAuthSig> {
@@ -139,17 +140,17 @@ impl Handshake<AwaitingAuthSig> {
139140

140141
let remote_pubkey = match pk_sum {
141142
proto::crypto::public_key::Sum::Ed25519(ref bytes) => {
142-
ed25519_consensus::VerificationKey::try_from(&bytes[..])
143+
ed25519_dalek::VerifyingKey::try_from(&bytes[..])
143144
.map_err(|_| Error::SignatureInvalid)
144145
}
145146
proto::crypto::public_key::Sum::Secp256k1(_) => Err(Error::UnsupportedKey),
146147
}?;
147148

148-
let remote_sig = ed25519_consensus::Signature::try_from(auth_sig_msg.sig.as_slice())
149+
let remote_sig = ed25519_dalek::Signature::try_from(auth_sig_msg.sig.as_slice())
149150
.map_err(|_| Error::SignatureInvalid)?;
150151

151152
remote_pubkey
152-
.verify(&remote_sig, &self.state.sc_mac)
153+
.verify(&self.state.sc_mac, &remote_sig)
153154
.map_err(|_| Error::SignatureInvalid)?;
154155

155156
// We've authorized.
@@ -169,7 +170,7 @@ impl Handshake<AwaitingAuthSig> {
169170
}
170171

171172
/// Borrow the local signature.
172-
pub fn local_signature(&self) -> &ed25519_consensus::Signature {
173+
pub fn local_signature(&self) -> &ed25519_dalek::Signature {
173174
&self.state.local_signature
174175
}
175176
}

tmkms-p2p/src/protocol.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Secret Connection Protocol: message framing and versioning
22
33
use crate::{Error, Result, public_key};
4-
use curve25519_dalek_ng::montgomery::MontgomeryPoint as EphemeralPublic;
4+
use curve25519_dalek::montgomery::MontgomeryPoint as EphemeralPublic;
55
use prost::Message as _;
66
use tendermint_proto::v0_38 as proto;
77

@@ -47,8 +47,8 @@ pub(crate) fn decode_initial_handshake(bytes: &[u8]) -> Result<EphemeralPublic>
4747
/// Panics if the Protobuf encoding of `AuthSigMessage` fails
4848
#[must_use]
4949
pub(crate) fn encode_auth_signature(
50-
pub_key: &ed25519_consensus::VerificationKey,
51-
signature: &ed25519_consensus::Signature,
50+
pub_key: &ed25519_dalek::VerifyingKey,
51+
signature: &ed25519_dalek::Signature,
5252
) -> Vec<u8> {
5353
// Protobuf `AuthSigMessage`
5454
let pub_key = proto::crypto::PublicKey {

tmkms-p2p/src/public_key.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use tendermint::node;
1010
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
1111
pub enum PublicKey {
1212
/// Ed25519 Secret Connection Keys
13-
Ed25519(ed25519_consensus::VerificationKey),
13+
Ed25519(ed25519_dalek::VerifyingKey),
1414
}
1515

1616
impl PublicKey {
@@ -20,14 +20,14 @@ impl PublicKey {
2020
///
2121
/// * if the bytes given are invalid
2222
pub fn from_raw_ed25519(bytes: &[u8]) -> Result<Self> {
23-
ed25519_consensus::VerificationKey::try_from(bytes)
23+
ed25519_dalek::VerifyingKey::try_from(bytes)
2424
.map(Self::Ed25519)
2525
.map_err(|_| Error::SignatureInvalid)
2626
}
2727

2828
/// Get Ed25519 public key
2929
#[must_use]
30-
pub const fn ed25519(self) -> Option<ed25519_consensus::VerificationKey> {
30+
pub const fn ed25519(self) -> Option<ed25519_dalek::VerifyingKey> {
3131
match self {
3232
Self::Ed25519(pk) => Some(pk),
3333
}
@@ -54,14 +54,14 @@ impl Display for PublicKey {
5454
}
5555
}
5656

57-
impl From<&ed25519_consensus::SigningKey> for PublicKey {
58-
fn from(sk: &ed25519_consensus::SigningKey) -> Self {
59-
Self::Ed25519(sk.verification_key())
57+
impl From<&ed25519_dalek::SigningKey> for PublicKey {
58+
fn from(sk: &ed25519_dalek::SigningKey) -> Self {
59+
Self::Ed25519(sk.verifying_key())
6060
}
6161
}
6262

63-
impl From<ed25519_consensus::VerificationKey> for PublicKey {
64-
fn from(pk: ed25519_consensus::VerificationKey) -> Self {
63+
impl From<ed25519_dalek::VerifyingKey> for PublicKey {
64+
fn from(pk: ed25519_dalek::VerifyingKey) -> Self {
6565
Self::Ed25519(pk)
6666
}
6767
}

tmkms-p2p/src/secret_connection.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::{
66
proto, protocol,
77
state::{ReceiveState, SendState},
88
};
9-
use curve25519_dalek_ng::montgomery::MontgomeryPoint as EphemeralPublic;
9+
use curve25519_dalek::montgomery::MontgomeryPoint as EphemeralPublic;
1010
use std::{
1111
io::{self, Read, Write},
1212
slice,
@@ -92,7 +92,7 @@ impl<IoHandler: Read + Write + Send + Sync> SecretConnection<IoHandler> {
9292
/// * if receiving the signature fails
9393
pub fn new(
9494
mut io_handler: IoHandler,
95-
local_privkey: ed25519_consensus::SigningKey,
95+
local_privkey: ed25519_dalek::SigningKey,
9696
) -> Result<Self> {
9797
// Start a handshake process.
9898
let local_pubkey = PublicKey::from(&local_privkey);
@@ -129,8 +129,8 @@ impl<IoHandler: Read + Write + Send + Sync> SecretConnection<IoHandler> {
129129
/// Encode our auth signature and decode theirs.
130130
fn share_auth_signature(
131131
&mut self,
132-
pubkey: &ed25519_consensus::VerificationKey,
133-
local_signature: &ed25519_consensus::Signature,
132+
pubkey: &ed25519_dalek::VerifyingKey,
133+
local_signature: &ed25519_dalek::Signature,
134134
) -> Result<proto::p2p::AuthSigMessage> {
135135
/// Length of the auth message response
136136
// 32 + 64 + (proto overhead = 1 prefix + 2 fields + 2 lengths + total length)

0 commit comments

Comments
 (0)