Skip to content

Commit

Permalink
fixed #172 - update ed25519-dalek
Browse files Browse the repository at this point in the history
  • Loading branch information
Eugeny committed Aug 17, 2023
1 parent d5cc506 commit 43edc32
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 78 deletions.
4 changes: 2 additions & 2 deletions russh-keys/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ block-padding = { version = "0.3", features = ["std"] }
byteorder = "1.4"
data-encoding = "2.3"
dirs = "5.0"
ed25519-dalek = "1.0"
ed25519-dalek = { version= "2.0", features = ["rand_core"] }
futures = "0.3"
hmac = "0.12"
inout = { version = "0.1", features = ["std"] }
Expand All @@ -49,7 +49,7 @@ num-integer = "0.1"
openssl = { version = "0.10", optional = true }
pbkdf2 = "0.11"
rand = "0.7"
rand_core = { version = "0.5", features = ["std"] }
rand_core = { version = "0.6.4", features = ["std"] }
russh-cryptovec = { version = "0.7.0", path = "../cryptovec" }
serde = { version = "1.0", features = ["derive"] }
sha2 = "0.10"
Expand Down
12 changes: 7 additions & 5 deletions russh-keys/src/agent/client.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use std::convert::TryFrom;

use byteorder::{BigEndian, ByteOrder};
use log::{debug, info};
use russh_cryptovec::CryptoVec;
use tokio;
use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
use log::{debug, info};

use super::{msg, Constraint};
use crate::encoding::{Encoding, Reader};
Expand Down Expand Up @@ -102,10 +104,10 @@ impl<S: AsyncRead + AsyncWrite + Unpin> AgentClient<S> {
match *key {
key::KeyPair::Ed25519(ref pair) => {
self.buf.extend_ssh_string(b"ssh-ed25519");
self.buf.extend_ssh_string(pair.public.as_bytes());
self.buf.extend_ssh_string(pair.verifying_key().as_bytes());
self.buf.push_u32_be(64);
self.buf.extend(pair.secret.as_bytes());
self.buf.extend(pair.public.as_bytes());
self.buf.extend(pair.to_bytes().as_slice());
self.buf.extend(pair.verifying_key().as_bytes());
self.buf.extend_ssh_string(b"");
}
#[cfg(feature = "openssl")]
Expand Down Expand Up @@ -263,7 +265,7 @@ impl<S: AsyncRead + AsyncWrite + Unpin> AgentClient<S> {
})
}
b"ssh-ed25519" => keys.push(PublicKey::Ed25519(
ed25519_dalek::PublicKey::from_bytes(r.read_string()?)?,
ed25519_dalek::VerifyingKey::try_from(r.read_string()?)?,
)),
t => {
info!("Unsupported key type: {:?}", std::str::from_utf8(t))
Expand Down
18 changes: 6 additions & 12 deletions russh-keys/src/agent/server.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::collections::HashMap;
use std::sync::{Arc, RwLock};
use std::convert::TryFrom;
use std::marker::Sync;
use std::sync::{Arc, RwLock};
use std::time::{Duration, SystemTime};

use async_trait::async_trait;
Expand Down Expand Up @@ -211,7 +212,7 @@ impl<S: AsyncRead + AsyncWrite + Send + Unpin + 'static, A: Agent + Send + Sync
}
}
let len = writebuf.len() - 4;
BigEndian::write_u32(&mut writebuf[..], len as u32);
BigEndian::write_u32(&mut writebuf[..], len as u32);
Ok(())
}

Expand Down Expand Up @@ -255,25 +256,18 @@ impl<S: AsyncRead + AsyncWrite + Send + Unpin + 'static, A: Agent + Send + Sync
let t = r.read_string()?;
let (blob, key) = match t {
b"ssh-ed25519" => {
let public_ = r.read_string()?;
let pos1 = r.position;
let concat = r.read_string()?;
let _comment = r.read_string()?;
#[allow(clippy::indexing_slicing)] // length checked before
let public = ed25519_dalek::PublicKey::from_bytes(
public_.get(..32).ok_or(Error::KeyIsCorrupt)?,
)?;
let secret = ed25519_dalek::SecretKey::from_bytes(
let secret = ed25519_dalek::SigningKey::try_from(
concat.get(..32).ok_or(Error::KeyIsCorrupt)?,
)?;
).map_err(|_| Error::KeyIsCorrupt)?;

writebuf.push(msg::SUCCESS);

#[allow(clippy::indexing_slicing)] // positions checked before
(
self.buf[pos0..pos1].to_vec(),
key::KeyPair::Ed25519(ed25519_dalek::Keypair { public, secret }),
)
(self.buf[pos0..pos1].to_vec(), key::KeyPair::Ed25519(secret))
}
#[cfg(feature = "openssl")]
b"ssh-rsa" => {
Expand Down
10 changes: 4 additions & 6 deletions russh-keys/src/format/openssh.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::convert::TryFrom;

use aes::cipher::block_padding::NoPadding;
use aes::cipher::{BlockDecryptMut, KeyIvInit, StreamCipher};
use bcrypt_pbkdf;
Expand Down Expand Up @@ -42,14 +44,10 @@ pub fn decode_openssh(secret: &[u8], password: Option<&str>) -> Result<key::KeyP
if Some(pubkey) != seckey.get(32..) {
return Err(Error::KeyIsCorrupt);
}
let secret = ed25519_dalek::SecretKey::from_bytes(
let secret = ed25519_dalek::SigningKey::try_from(
seckey.get(..32).ok_or(Error::KeyIsCorrupt)?,
)?;
let public = (&secret).into();
return Ok(key::KeyPair::Ed25519(ed25519_dalek::Keypair {
secret,
public,
}));
return Ok(key::KeyPair::Ed25519(secret));
} else if key_type == KEYTYPE_RSA && cfg!(feature = "openssl") {
#[cfg(feature = "openssl")]
{
Expand Down
30 changes: 12 additions & 18 deletions russh-keys/src/format/pkcs8.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use openssl::pkey::Private;
use openssl::rsa::Rsa;
#[cfg(test)]
use rand_core::OsRng;
use std::convert::TryFrom;
use yasna::BERReaderSeq;
use {std, yasna};

Expand Down Expand Up @@ -116,8 +117,8 @@ fn asn1_read_aes256cbc(
Ok(Ok(Encryption::Aes256Cbc(i)))
}

fn write_key_v1(writer: &mut yasna::DERWriterSeq, secret: &ed25519_dalek::SecretKey) {
let public = ed25519_dalek::PublicKey::from(secret);
fn write_key_v1(writer: &mut yasna::DERWriterSeq, secret: &ed25519_dalek::SigningKey) {
let public = ed25519_dalek::VerifyingKey::from(secret);
writer.next().write_u32(1);
// write OID
writer.next().write_sequence(|writer| {
Expand All @@ -127,7 +128,7 @@ fn write_key_v1(writer: &mut yasna::DERWriterSeq, secret: &ed25519_dalek::Secret
});
let seed = yasna::construct_der(|writer| {
writer.write_bytes(
[secret.as_bytes().as_slice(), public.as_bytes().as_slice()]
[secret.to_bytes().as_slice(), public.as_bytes().as_slice()]
.concat()
.as_slice(),
)
Expand All @@ -145,22 +146,15 @@ fn read_key_v1(reader: &mut BERReaderSeq) -> Result<key::KeyPair, Error> {
.next()
.read_sequence(|reader| reader.next().read_oid())?;
if oid.components().as_slice() == ED25519 {
use ed25519_dalek::{Keypair, PublicKey, SecretKey};
use ed25519_dalek::SigningKey;
let secret = {
let s = yasna::parse_der(&reader.next().read_bytes()?, |reader| reader.read_bytes())?;

s.get(..ed25519_dalek::SECRET_KEY_LENGTH)
.ok_or(Error::KeyIsCorrupt)
.and_then(|s| SecretKey::from_bytes(s).map_err(|_| Error::CouldNotReadKey))?
.and_then(|s| SigningKey::try_from(s).map_err(|_| Error::CouldNotReadKey))?
};
let public = {
let public = reader
.next()
.read_tagged(yasna::Tag::context(1), |reader| reader.read_bitvec())?
.to_bytes();
PublicKey::from_bytes(&public).map_err(|_| Error::CouldNotReadKey)?
};
Ok(key::KeyPair::Ed25519(Keypair { public, secret }))
Ok(key::KeyPair::Ed25519(secret))
} else {
Err(Error::CouldNotReadKey)
}
Expand Down Expand Up @@ -255,12 +249,12 @@ fn read_key_v0(_: &mut BERReaderSeq) -> Result<key::KeyPair, Error> {

#[test]
fn test_read_write_pkcs8() {
let ed25519_dalek::Keypair { public, secret } = ed25519_dalek::Keypair::generate(&mut OsRng {});
let secret = ed25519_dalek::SigningKey::generate(&mut OsRng {});
assert_eq!(
public.as_bytes(),
ed25519_dalek::PublicKey::from(&secret).as_bytes()
secret.verifying_key().as_bytes(),
ed25519_dalek::VerifyingKey::from(&secret).as_bytes()
);
let key = key::KeyPair::Ed25519(ed25519_dalek::Keypair { public, secret });
let key = key::KeyPair::Ed25519(secret);
let password = b"blabla";
let ciphertext = encode_pkcs8_encrypted(password, 100, &key).unwrap();
let key = decode_pkcs8(&ciphertext, Some(password)).unwrap();
Expand Down Expand Up @@ -317,7 +311,7 @@ pub fn encode_pkcs8_encrypted(
pub fn encode_pkcs8(key: &key::KeyPair) -> Vec<u8> {
yasna::construct_der(|writer| {
writer.write_sequence(|writer| match *key {
key::KeyPair::Ed25519(ref pair) => write_key_v1(writer, &pair.secret),
key::KeyPair::Ed25519(ref pair) => write_key_v1(writer, pair),
#[cfg(feature = "openssl")]
key::KeyPair::RSA { ref key, .. } => write_key_v0(writer, key),
})
Expand Down
66 changes: 35 additions & 31 deletions russh-keys/src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.
//
use serde::{Serialize, Deserialize};
use ed25519_dalek::ed25519::signature::Signature as EdSignature;
use ed25519_dalek::{Signer, Verifier};
#[cfg(feature = "openssl")]
use openssl::pkey::{Private, Public};
use rand::rngs::OsRng;
use rand_core::OsRng;
use russh_cryptovec::CryptoVec;
use serde::{Deserialize, Serialize};
use std::convert::TryFrom;

use crate::encoding::{Encoding, Reader};
pub use crate::signature::*;
Expand Down Expand Up @@ -109,7 +109,7 @@ impl SignatureHash {
#[derive(Eq, Debug, Clone)]
pub enum PublicKey {
#[doc(hidden)]
Ed25519(ed25519_dalek::PublicKey),
Ed25519(ed25519_dalek::VerifyingKey),
#[doc(hidden)]
#[cfg(feature = "openssl")]
RSA {
Expand Down Expand Up @@ -137,7 +137,7 @@ pub struct OpenSSLPKey(pub openssl::pkey::PKey<Public>);

#[cfg(feature = "openssl")]
use std::cmp::{Eq, PartialEq};
use std::convert::TryInto;

#[cfg(feature = "openssl")]
impl PartialEq for OpenSSLPKey {
fn eq(&self, b: &OpenSSLPKey) -> bool {
Expand All @@ -161,11 +161,13 @@ impl PublicKey {
let mut p = pubkey.reader(0);
let key_algo = p.read_string()?;
let key_bytes = p.read_string()?;
if key_algo != b"ssh-ed25519" || key_bytes.len() != ed25519_dalek::PUBLIC_KEY_LENGTH
{
if key_algo != b"ssh-ed25519" {
return Err(Error::CouldNotReadKey);
}
ed25519_dalek::PublicKey::from_bytes(key_bytes)
let Ok(key_bytes) = <&[u8; ed25519_dalek::PUBLIC_KEY_LENGTH]>::try_from(key_bytes) else {
return Err(Error::CouldNotReadKey);
};
ed25519_dalek::VerifyingKey::from_bytes(key_bytes)
.map(PublicKey::Ed25519)
.map_err(Error::from)
}
Expand Down Expand Up @@ -217,9 +219,13 @@ impl PublicKey {
/// Verify a signature.
pub fn verify_detached(&self, buffer: &[u8], sig: &[u8]) -> bool {
match self {
PublicKey::Ed25519(ref public) => ed25519_dalek::Signature::from_bytes(sig)
.and_then(|sig| public.verify(buffer, &sig))
.is_ok(),
PublicKey::Ed25519(ref public) => {
let Ok(sig) = ed25519_dalek::ed25519::SignatureBytes::try_from(sig) else {
return false;
};
let sig = ed25519_dalek::Signature::from_bytes(&sig);
public.verify(buffer, &sig).is_ok()
}

#[cfg(feature = "openssl")]
PublicKey::RSA { ref key, ref hash } => {
Expand Down Expand Up @@ -273,7 +279,7 @@ impl Verify for PublicKey {
/// Public key exchange algorithms.
#[allow(clippy::large_enum_variant)]
pub enum KeyPair {
Ed25519(ed25519_dalek::Keypair),
Ed25519(ed25519_dalek::SigningKey),
#[cfg(feature = "openssl")]
RSA {
key: openssl::rsa::Rsa<Private>,
Expand All @@ -285,10 +291,9 @@ impl Clone for KeyPair {
fn clone(&self) -> Self {
match self {
#[allow(clippy::expect_used)]
Self::Ed25519(kp) => Self::Ed25519(
ed25519_dalek::Keypair::from_bytes(&kp.to_bytes())
.expect("expected to clone keypair"),
),
Self::Ed25519(kp) => {
Self::Ed25519(ed25519_dalek::SigningKey::from_bytes(&kp.to_bytes()))
}
#[cfg(feature = "openssl")]
Self::RSA { key, hash } => Self::RSA {
key: key.clone(),
Expand All @@ -304,7 +309,7 @@ impl std::fmt::Debug for KeyPair {
KeyPair::Ed25519(ref key) => write!(
f,
"Ed25519 {{ public: {:?}, secret: (hidden) }}",
key.public.as_bytes()
key.verifying_key().as_bytes()
),
#[cfg(feature = "openssl")]
KeyPair::RSA { .. } => write!(f, "RSA {{ (hidden) }}"),
Expand All @@ -322,7 +327,7 @@ impl KeyPair {
/// Copy the public key of this algorithm.
pub fn clone_public_key(&self) -> Result<PublicKey, Error> {
Ok(match self {
KeyPair::Ed25519(ref key) => PublicKey::Ed25519(key.public),
KeyPair::Ed25519(ref key) => PublicKey::Ed25519(key.verifying_key()),
#[cfg(feature = "openssl")]
KeyPair::RSA { ref key, ref hash } => {
use openssl::pkey::PKey;
Expand All @@ -347,10 +352,10 @@ impl KeyPair {

/// Generate a key pair.
pub fn generate_ed25519() -> Option<Self> {
let keypair = ed25519_dalek::Keypair::generate(&mut OsRng {});
let keypair = ed25519_dalek::SigningKey::generate(&mut OsRng {});
assert_eq!(
keypair.public.as_bytes(),
ed25519_dalek::PublicKey::from(&keypair.secret).as_bytes()
keypair.verifying_key().as_bytes(),
ed25519_dalek::VerifyingKey::from(&keypair).as_bytes()
);
Some(KeyPair::Ed25519(keypair))
}
Expand All @@ -366,9 +371,7 @@ impl KeyPair {
match self {
#[allow(clippy::unwrap_used)]
KeyPair::Ed25519(ref secret) => Ok(Signature::Ed25519(SignatureBytes(
ed25519_dalek::ed25519::signature::Signature::as_bytes(&secret.sign(to_sign))
.try_into()
.unwrap(),
secret.sign(to_sign).to_bytes(),
))),
#[cfg(feature = "openssl")]
KeyPair::RSA { ref key, ref hash } => Ok(Signature::RSA {
Expand All @@ -391,11 +394,9 @@ impl KeyPair {
KeyPair::Ed25519(ref secret) => {
let signature = secret.sign(to_sign.as_ref());

buffer.push_u32_be(
(ED25519.0.len() + EdSignature::as_bytes(&signature).len() + 8) as u32,
);
buffer.push_u32_be((ED25519.0.len() + signature.to_bytes().len() + 8) as u32);
buffer.extend_ssh_string(ED25519.0.as_bytes());
buffer.extend_ssh_string(signature.as_bytes());
buffer.extend_ssh_string(signature.to_bytes().as_slice());
}
#[cfg(feature = "openssl")]
KeyPair::RSA { ref key, ref hash } => {
Expand All @@ -418,9 +419,9 @@ impl KeyPair {
match self {
KeyPair::Ed25519(ref secret) => {
let signature = secret.sign(buffer);
buffer.push_u32_be((ED25519.0.len() + signature.as_bytes().len() + 8) as u32);
buffer.push_u32_be((ED25519.0.len() + signature.to_bytes().len() + 8) as u32);
buffer.extend_ssh_string(ED25519.0.as_bytes());
buffer.extend_ssh_string(signature.as_bytes());
buffer.extend_ssh_string(signature.to_bytes().as_slice());
}
#[cfg(feature = "openssl")]
KeyPair::RSA { ref key, ref hash } => {
Expand Down Expand Up @@ -482,7 +483,10 @@ pub fn parse_public_key(
let t = pos.read_string()?;
if t == b"ssh-ed25519" {
if let Ok(pubkey) = pos.read_string() {
let p = ed25519_dalek::PublicKey::from_bytes(pubkey).map_err(Error::from)?;
let Ok(pubkey) = <&[u8; ed25519_dalek::PUBLIC_KEY_LENGTH]>::try_from(pubkey) else {
return Err(Error::CouldNotReadKey);
};
let p = ed25519_dalek::VerifyingKey::from_bytes(pubkey).map_err(Error::from)?;
return Ok(PublicKey::Ed25519(p));
}
}
Expand Down
4 changes: 2 additions & 2 deletions russh-keys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,10 @@ impl PublicKeyBase64 for key::KeyPair {
s.extend_from_slice(name);
match *self {
key::KeyPair::Ed25519(ref key) => {
let public = key.public.as_bytes();
let public = key.verifying_key().to_bytes();
#[allow(clippy::unwrap_used)] // Vec<>.write can't fail
s.write_u32::<BigEndian>(public.len() as u32).unwrap();
s.extend_from_slice(public);
s.extend_from_slice(public.as_slice());
}
#[cfg(feature = "openssl")]
key::KeyPair::RSA { ref key, .. } => {
Expand Down

0 comments on commit 43edc32

Please sign in to comment.