From b062471ace9b6898d936de7c738c359fc2b70f23 Mon Sep 17 00:00:00 2001 From: akeamc Date: Sat, 9 Dec 2023 23:02:22 +0100 Subject: [PATCH 1/4] Support for NIST P-521 public keys --- russh-keys/Cargo.toml | 1 + russh-keys/src/agent/client.rs | 12 ++++++-- russh-keys/src/key.rs | 54 +++++++++++++++++++++++++++++++++- russh-keys/src/lib.rs | 10 +++++-- russh/src/negotiation.rs | 4 ++- 5 files changed, 75 insertions(+), 6 deletions(-) diff --git a/russh-keys/Cargo.toml b/russh-keys/Cargo.toml index 643c7e55..5c361fa7 100644 --- a/russh-keys/Cargo.toml +++ b/russh-keys/Cargo.toml @@ -64,6 +64,7 @@ tokio = { version = "1.17.0", features = [ ] } tokio-stream = { version = "0.1", features = ["net"] } yasna = { version = "0.5.0", features = ["bit-vec", "num-bigint"] } +p521 = "0.13.3" [features] vendored-openssl = ["openssl", "openssl/vendored"] diff --git a/russh-keys/src/agent/client.rs b/russh-keys/src/agent/client.rs index e25f17fa..36075c35 100644 --- a/russh-keys/src/agent/client.rs +++ b/russh-keys/src/agent/client.rs @@ -278,11 +278,19 @@ impl AgentClient { b"ecdsa-sha2-nistp256" => { let curve = r.read_string()?; if curve != b"nistp256" { - return Err(Error::P256KeyError(p256::elliptic_curve::Error)); + return Err(Error::EcdsaKeyError(p256::elliptic_curve::Error)); } let key = r.read_string()?; keys.push(PublicKey::P256(p256::PublicKey::from_sec1_bytes(key)?)); } + b"ecdsa-sha2-nistp512" => { + let curve = r.read_string()?; + if curve != b"nistp521" { + return Err(Error::EcdsaKeyError(p521::elliptic_curve::Error)); + } + let key = r.read_string()?; + keys.push(PublicKey::P521(p521::PublicKey::from_sec1_bytes(key)?)); + } t => { info!("Unsupported key type: {:?}", std::str::from_utf8(t)) } @@ -542,7 +550,7 @@ fn key_blob(public: &key::PublicKey, buf: &mut CryptoVec) -> Result<(), Error> { #[allow(clippy::indexing_slicing)] // length is known BigEndian::write_u32(&mut buf[5..], (len1 - len0) as u32); } - PublicKey::P256(_) => { + PublicKey::P256(_) | PublicKey::P521(_) => { buf.extend_ssh_string(&public.public_key_bytes()); } } diff --git a/russh-keys/src/key.rs b/russh-keys/src/key.rs index 97db830a..e97ef4bd 100644 --- a/russh-keys/src/key.rs +++ b/russh-keys/src/key.rs @@ -14,6 +14,7 @@ // use std::convert::TryFrom; +use block_padding::generic_array::GenericArray; use ed25519_dalek::{Signer, Verifier}; #[cfg(feature = "openssl")] use openssl::pkey::{Private, Public}; @@ -38,6 +39,8 @@ impl AsRef for Name { /// The name of the ecdsa-sha2-nistp256 algorithm for SSH. pub const ECDSA_SHA2_NISTP256: Name = Name("ecdsa-sha2-nistp256"); +/// The name of the ecdsa-sha2-nistp521 algorithm for SSH. +pub const ECDSA_SHA2_NISTP521: Name = Name("ecdsa-sha2-nistp521"); /// The name of the Ed25519 algorithm for SSH. pub const ED25519: Name = Name("ssh-ed25519"); /// The name of the ssh-sha2-512 algorithm for SSH. @@ -53,7 +56,7 @@ impl Name { /// Base name of the private key file for a key name. pub fn identity_file(&self) -> &'static str { match *self { - ECDSA_SHA2_NISTP256 => "id_ecdsa", + ECDSA_SHA2_NISTP256 | ECDSA_SHA2_NISTP521 => "id_ecdsa", ED25519 => "id_ed25519", RSA_SHA2_512 => "id_rsa", RSA_SHA2_256 => "id_rsa", @@ -123,6 +126,8 @@ pub enum PublicKey { }, #[doc(hidden)] P256(p256::PublicKey), + #[doc(hidden)] + P521(p521::PublicKey), } impl PartialEq for PublicKey { @@ -132,6 +137,7 @@ impl PartialEq for PublicKey { (Self::RSA { key: a, .. }, Self::RSA { key: b, .. }) => a == b, (Self::Ed25519(a), Self::Ed25519(b)) => a == b, (Self::P256(a), Self::P256(b)) => a == b, + (Self::P521(a), Self::P521(b)) => a == b, _ => false, } } @@ -223,6 +229,18 @@ impl PublicKey { .map_err(|_| Error::CouldNotReadKey)?; Ok(PublicKey::P256(key)) } + b"ecdsa-sha2-nistp521" => { + let mut p = pubkey.reader(0); + let key_algo = p.read_string()?; + let curve = p.read_string()?; + if key_algo != b"ecdsa-sha2-nistp521" || curve != b"nistp521" { + return Err(Error::CouldNotReadKey); + } + let sec1_bytes = p.read_string()?; + let key = p521::PublicKey::from_sec1_bytes(sec1_bytes) + .map_err(|_| Error::CouldNotReadKey)?; + Ok(PublicKey::P521(key)) + } _ => Err(Error::CouldNotReadKey), } } @@ -234,6 +252,7 @@ impl PublicKey { #[cfg(feature = "openssl")] PublicKey::RSA { ref hash, .. } => hash.name().0, PublicKey::P256(_) => ECDSA_SHA2_NISTP256.0, + PublicKey::P521(_) => ECDSA_SHA2_NISTP521.0, } } @@ -282,6 +301,31 @@ impl PublicKey { .verify(buffer, &signature) .is_ok() } + PublicKey::P521(ref public) => { + const FIELD_LEN: usize = + ::FieldBytesSize::USIZE; + let mut reader = sig.reader(0); + let mut read_field = || -> Option { + let f = reader.read_mpint().ok()?; + let f = f.strip_prefix(&[0]).unwrap_or(f); + let mut result = [0; FIELD_LEN]; + if f.len() > FIELD_LEN { + return None; + } + #[allow(clippy::indexing_slicing)] // length is known + result[FIELD_LEN - f.len()..].copy_from_slice(f); + // Some(result.into()) + Some(GenericArray::clone_from_slice(&result)) // ew + }; + let Some(r) = read_field() else { return false }; + let Some(s) = read_field() else { return false }; + let Ok(signature) = p521::ecdsa::Signature::from_scalars(r, s) else { + return false; + }; + p521::ecdsa::VerifyingKey::from_sec1_bytes(&public.to_sec1_bytes()).unwrap() // also ew + .verify(buffer, &signature) + .is_ok() + } } } @@ -560,5 +604,13 @@ pub fn parse_public_key( let key = p256::PublicKey::from_sec1_bytes(sec1_bytes)?; return Ok(PublicKey::P256(key)); } + if t == b"ecdsa-sha2-nistp521" { + if pos.read_string()? != b"nistp521" { + return Err(Error::CouldNotReadKey); + } + let sec1_bytes = pos.read_string()?; + let key = p521::PublicKey::from_sec1_bytes(sec1_bytes)?; + return Ok(PublicKey::P521(key)); + } Err(Error::CouldNotReadKey) } diff --git a/russh-keys/src/lib.rs b/russh-keys/src/lib.rs index f23263a1..ed362993 100644 --- a/russh-keys/src/lib.rs +++ b/russh-keys/src/lib.rs @@ -102,8 +102,8 @@ pub enum Error { #[error("Invalid Ed25519 key data")] Ed25519KeyError(#[from] ed25519_dalek::SignatureError), /// The type of the key is unsupported - #[error("Invalid NIST-P256 key data")] - P256KeyError(#[from] p256::elliptic_curve::Error), + #[error("Invalid ECDSA key data")] + EcdsaKeyError(#[from] p256::elliptic_curve::Error), /// The key is encrypted (should supply a password?) #[error("The key is encrypted")] KeyIsEncrypted, @@ -242,6 +242,12 @@ impl PublicKeyBase64 for key::PublicKey { s.extend_ssh_string(b"nistp256"); s.extend_ssh_string(&publickey.to_sec1_bytes()); } + key::PublicKey::P521(ref publickey) => { + use encoding::Encoding; + s.extend_ssh_string(b"ecdsa-sha2-nistp521"); + s.extend_ssh_string(b"nistp521"); + s.extend_ssh_string(&publickey.to_sec1_bytes()); + } } s } diff --git a/russh/src/negotiation.rs b/russh/src/negotiation.rs index 5b0cf0af..c578cf60 100644 --- a/russh/src/negotiation.rs +++ b/russh/src/negotiation.rs @@ -81,6 +81,7 @@ impl Preferred { key: &[ key::ED25519, key::ECDSA_SHA2_NISTP256, + key::ECDSA_SHA2_NISTP521, #[cfg(feature = "openssl")] key::RSA_SHA2_256, #[cfg(feature = "openssl")] @@ -120,13 +121,14 @@ impl Named for () { #[cfg(feature = "openssl")] use russh_keys::key::SSH_RSA; -use russh_keys::key::{ECDSA_SHA2_NISTP256, ED25519}; +use russh_keys::key::{ECDSA_SHA2_NISTP256, ECDSA_SHA2_NISTP521, ED25519}; impl Named for PublicKey { fn name(&self) -> &'static str { match self { PublicKey::Ed25519(_) => ED25519.0, PublicKey::P256(_) => ECDSA_SHA2_NISTP256.0, + PublicKey::P521(_) => ECDSA_SHA2_NISTP521.0, #[cfg(feature = "openssl")] PublicKey::RSA { .. } => SSH_RSA.0, } From c6244dcad406f6bc3764791eb75f794a368807e6 Mon Sep 17 00:00:00 2001 From: akeamc Date: Sat, 9 Dec 2023 23:05:30 +0100 Subject: [PATCH 2/4] oops --- russh/src/key.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/russh/src/key.rs b/russh/src/key.rs index 5930231b..804c619e 100644 --- a/russh/src/key.rs +++ b/russh/src/key.rs @@ -30,7 +30,7 @@ impl PubKey for PublicKey { buffer.extend_ssh_string(ED25519.0.as_bytes()); buffer.extend_ssh_string(public.as_bytes()); } - PublicKey::P256(_) => { + PublicKey::P256(_) | PublicKey::P521(_) => { buffer.extend_ssh_string(&self.public_key_bytes()); } #[cfg(feature = "openssl")] From b47c0f58c618fb29ac13dfabdc51b2435276b6b6 Mon Sep 17 00:00:00 2001 From: Eugene Date: Tue, 23 Jan 2024 09:47:33 +0100 Subject: [PATCH 3/4] misc fixes --- russh-keys/Cargo.toml | 2 +- russh-keys/src/key.rs | 11 +++++++---- russh/Cargo.toml | 4 ++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/russh-keys/Cargo.toml b/russh-keys/Cargo.toml index 37cf8cc6..a71fb1c3 100644 --- a/russh-keys/Cargo.toml +++ b/russh-keys/Cargo.toml @@ -25,7 +25,7 @@ keywords = ["ssh"] license = "Apache-2.0" name = "russh-keys" repository = "https://github.com/warp-tech/russh" -version = "0.40.1" +version = "0.41.0-beta.1" rust-version = "1.65" [dependencies] diff --git a/russh-keys/src/key.rs b/russh-keys/src/key.rs index e97ef4bd..4c336a58 100644 --- a/russh-keys/src/key.rs +++ b/russh-keys/src/key.rs @@ -14,11 +14,11 @@ // use std::convert::TryFrom; -use block_padding::generic_array::GenericArray; use ed25519_dalek::{Signer, Verifier}; #[cfg(feature = "openssl")] use openssl::pkey::{Private, Public}; use p256::elliptic_curve::generic_array::typenum::Unsigned; +use p521::elliptic_curve::generic_array::GenericArray; use rand_core::OsRng; use russh_cryptovec::CryptoVec; use serde::{Deserialize, Serialize}; @@ -314,15 +314,18 @@ impl PublicKey { } #[allow(clippy::indexing_slicing)] // length is known result[FIELD_LEN - f.len()..].copy_from_slice(f); - // Some(result.into()) - Some(GenericArray::clone_from_slice(&result)) // ew + Some(GenericArray::clone_from_slice(&result)) }; let Some(r) = read_field() else { return false }; let Some(s) = read_field() else { return false }; let Ok(signature) = p521::ecdsa::Signature::from_scalars(r, s) else { return false; }; - p521::ecdsa::VerifyingKey::from_sec1_bytes(&public.to_sec1_bytes()).unwrap() // also ew + + // Length known + #[allow(clippy::unwrap_used)] + p521::ecdsa::VerifyingKey::from_sec1_bytes(&public.to_sec1_bytes()) + .unwrap() .verify(buffer, &signature) .is_ok() } diff --git a/russh/Cargo.toml b/russh/Cargo.toml index 61f04e35..fc6a2fea 100644 --- a/russh/Cargo.toml +++ b/russh/Cargo.toml @@ -9,7 +9,7 @@ license = "Apache-2.0" name = "russh" readme = "../README.md" repository = "https://github.com/warp-tech/russh" -version = "0.41.0-beta.1" +version = "0.41.0-beta.2" rust-version = "1.65" [features] @@ -37,7 +37,7 @@ once_cell = "1.13" openssl = { version = "0.10", optional = true } rand = "0.8" russh-cryptovec = { version = "0.7.0", path = "../cryptovec" } -russh-keys = { version = "0.40", path = "../russh-keys" } +russh-keys = { version = "0.41.0-beta.1", path = "../russh-keys" } sha1 = "0.10" sha2 = "0.10" hex-literal = "0.4" From e249365e9bd06dcfb1d58a334ec154a549f10447 Mon Sep 17 00:00:00 2001 From: akeamc Date: Tue, 23 Jan 2024 12:05:12 +0100 Subject: [PATCH 4/4] Add p521 parse test, loosen p521 version --- russh-keys/Cargo.toml | 2 +- russh-keys/src/lib.rs | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/russh-keys/Cargo.toml b/russh-keys/Cargo.toml index a71fb1c3..d55fdabf 100644 --- a/russh-keys/Cargo.toml +++ b/russh-keys/Cargo.toml @@ -49,6 +49,7 @@ num-bigint = "0.4" num-integer = "0.1" openssl = { version = "0.10", optional = true } p256 = "0.13" +p521 = "0.13" pbkdf2 = "0.11" rand = "0.7" rand_core = { version = "0.6.4", features = ["std"] } @@ -65,7 +66,6 @@ tokio = { version = "1.17.0", features = [ ] } tokio-stream = { version = "0.1", features = ["net"] } yasna = { version = "0.5.0", features = ["bit-vec", "num-bigint"] } -p521 = "0.13.3" [features] vendored-openssl = ["openssl", "openssl/vendored"] diff --git a/russh-keys/src/lib.rs b/russh-keys/src/lib.rs index ed362993..9d211edb 100644 --- a/russh-keys/src/lib.rs +++ b/russh-keys/src/lib.rs @@ -613,6 +613,14 @@ QR+u0AypRPmzHnOPAAAAEXJvb3RAMTQwOTExNTQ5NDBkAQ== parse_public_key_base64(key).unwrap(); } + #[test] + fn test_parse_p521_public_key() { + env_logger::try_init().unwrap_or(()); + let key = "AAAAE2VjZHNhLXNoYTItbmlzdHA1MjEAAAAIbmlzdHA1MjEAAACFBAAQepXEpOrzlX22r4E5zEHjhHWeZUe//zaevTanOWRBnnaCGWJFGCdjeAbNOuAmLtXc+HZdJTCZGREeSLSrpJa71QDCgZl0N7DkDUanCpHZJe/DCK6qwtHYbEMn28iLMlGCOrCIa060EyJHbp1xcJx4I1SKj/f/fm3DhhID/do6zyf8Cg=="; + + parse_public_key_base64(key).unwrap(); + } + #[test] #[cfg(feature = "openssl")] fn test_srhb() {