From 7a0ab1d9a0f96c707a54dcb209b6a469ee74302f Mon Sep 17 00:00:00 2001 From: Eugene Date: Sun, 5 Jan 2025 22:53:29 +0100 Subject: [PATCH 1/6] client gex --- russh/examples/client_exec_simple.rs | 5 + russh/src/client/kex.rs | 95 ++++++++++++++--- russh/src/client/mod.rs | 36 +++++-- russh/src/kex/dh/gex.rs | 0 russh/src/kex/dh/groups.rs | 100 +++++++++++++----- russh/src/kex/dh/mod.rs | 147 ++++++++++++++++++++------- russh/src/kex/mod.rs | 63 ++++++++---- russh/src/lib.rs | 4 +- russh/src/msg.rs | 64 +++++++++++- russh/src/negotiation.rs | 1 + russh/src/server/mod.rs | 10 +- russh/src/session.rs | 3 + 12 files changed, 412 insertions(+), 116 deletions(-) create mode 100644 russh/src/kex/dh/gex.rs diff --git a/russh/examples/client_exec_simple.rs b/russh/examples/client_exec_simple.rs index e3cc0d8c..0b445da1 100644 --- a/russh/examples/client_exec_simple.rs +++ b/russh/examples/client_exec_simple.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; /// /// Run this example with: /// cargo run --example client_exec_simple -- -k @@ -84,6 +85,10 @@ impl Session { let key_pair = load_secret_key(key_path, None)?; let config = client::Config { inactivity_timeout: Some(Duration::from_secs(5)), + preferred: Preferred { + kex: Cow::Owned(vec![russh::kex::DH_GEX_SHA1]), + ..Default::default() + }, ..<_>::default() }; diff --git a/russh/src/client/kex.rs b/russh/src/client/kex.rs index 446f4452..89055609 100644 --- a/russh/src/client/kex.rs +++ b/russh/src/client/kex.rs @@ -9,10 +9,11 @@ use russh_cryptovec::CryptoVec; use russh_keys::key::parse_public_key; use signature::Verifier; use ssh_encoding::{Decode, Encode}; -use ssh_key::{PublicKey, Signature}; +use ssh_key::{Mpint, PublicKey, Signature}; use super::IncomingSshPacket; use crate::client::{Config, NewKeys}; +use crate::kex::dh::groups::DhGroup; use crate::kex::{Kex, KexAlgorithm, KexAlgorithmImplementor, KexCause, KexProgress, KEXES}; use crate::negotiation::{Names, Select}; use crate::session::Exchange; @@ -27,7 +28,11 @@ thread_local! { #[allow(clippy::large_enum_variant)] enum ClientKexState { Created, - WaitingForKexReply { + WaitingForGexResponse { + names: Names, + kex: KexAlgorithm, + }, + WaitingForServerDh { // both KexInit and DH init sent names: Names, kex: KexAlgorithm, @@ -53,8 +58,11 @@ impl Debug for ClientKex { ClientKexState::Created => { s.field("state", &"created"); } - ClientKexState::WaitingForKexReply { .. } => { - s.field("state", &"waiting for a reply"); + ClientKexState::WaitingForGexResponse { .. } => { + s.field("state", &"waiting for GEX response"); + } + ClientKexState::WaitingForServerDh { .. } => { + s.field("state", &"waiting for DH response"); } ClientKexState::WaitingForNewKeys { .. } => { s.field("state", &"waiting for NEWKEYS"); @@ -126,11 +134,6 @@ impl Kex for ClientKex { let mut kex = KEXES.get(&names.kex).ok_or(Error::UnknownAlgo)?.make(); - output.packet(|w| { - kex.client_dh(&mut self.exchange.client_ephemeral, w)?; - Ok(()) - })?; - if kex.skip_exchange() { // Non-standard no-kex exchange let newkeys = compute_keys( @@ -152,14 +155,75 @@ impl Kex for ClientKex { }); } - self.state = ClientKexState::WaitingForKexReply { names, kex }; + if kex.is_dh_gex() { + // TODO values + output.packet(|w| { + kex.client_dh_gex_init( + self.config.gex.min_group_size, + self.config.gex.preferred_group_size, + self.config.gex.max_group_size, + w, + )?; + Ok(()) + })?; + + self.state = ClientKexState::WaitingForGexResponse { names, kex }; + } else { + output.packet(|w| { + kex.client_dh(&mut self.exchange.client_ephemeral, w)?; + Ok(()) + })?; + + self.state = ClientKexState::WaitingForServerDh { names, kex }; + } + + Ok(KexProgress::NeedsReply { + kex: self, + reset_seqn: false, + }) + } + ClientKexState::WaitingForGexResponse { names, mut kex } => { + let Some(input) = input else { + return Err(Error::KexInit); + }; + + if input.buffer.first() != Some(&msg::KEX_DH_GEX_GROUP) { + error!( + "Unexpected kex message at this stage: {:?}", + input.buffer.first() + ); + return Err(Error::KexInit); + } + + #[allow(clippy::indexing_slicing)] // length checked + let mut r = &input.buffer[1..]; + + let prime = Mpint::decode(&mut r)?; + let gen = Mpint::decode(&mut r)?; + debug!("received gex group: prime={}, gen={}", prime, gen); + //todo validate group + + let group = DhGroup { + prime: prime.as_bytes().to_vec().into(), + generator: gen.as_bytes().to_vec().into(), + exp_size: 0, + }; + + let exchange = &mut self.exchange; + exchange.gex = Some((self.config.gex.clone(), group.clone())); + kex.client_dh_gex_group(group)?; + output.packet(|w| { + kex.client_dh(&mut exchange.client_ephemeral, w)?; + Ok(()) + })?; + self.state = ClientKexState::WaitingForServerDh { names, kex }; Ok(KexProgress::NeedsReply { kex: self, reset_seqn: false, }) } - ClientKexState::WaitingForKexReply { mut names, mut kex } => { + ClientKexState::WaitingForServerDh { mut names, mut kex } => { // At this point, we've sent ECDH_INTI and // are waiting for the ECDH_REPLY from the server. @@ -171,14 +235,19 @@ impl Kex for ClientKex { // Ignore the next packet if (1) it follows and (2) it's not the correct guess. debug!("ignoring guessed kex"); names.ignore_guessed = false; - self.state = ClientKexState::WaitingForKexReply { names, kex }; + self.state = ClientKexState::WaitingForServerDh { names, kex }; return Ok(KexProgress::NeedsReply { kex: self, reset_seqn: false, }); } - if input.buffer.first() != Some(&msg::KEX_ECDH_REPLY) { + if input.buffer.first() + != Some(match kex.is_dh_gex() { + true => &msg::KEX_DH_GEX_REPLY, + false => &msg::KEX_ECDH_REPLY, + }) + { error!( "Unexpected kex message at this stage: {:?}", input.buffer.first() diff --git a/russh/src/client/mod.rs b/russh/src/client/mod.rs index a7900ebc..fa45c473 100644 --- a/russh/src/client/mod.rs +++ b/russh/src/client/mod.rs @@ -60,13 +60,13 @@ use tokio::sync::oneshot; use crate::channels::{Channel, ChannelMsg, ChannelRef, WindowSizeRef}; use crate::cipher::{self, clear, OpeningKey}; use crate::kex::{Kex, KexCause, KexProgress, SessionKexState}; -use crate::msg::{is_kex_msg, STRICT_KEX_MSG_ORDER}; +use crate::msg::{is_kex_msg, validate_server_msg_strict_kex}; use crate::session::{CommonSession, EncryptedState, GlobalRequestResponse, NewKeys}; use crate::ssh_read::SshRead; use crate::sshbuffer::{IncomingSshPacket, PacketWriter, SSHBuffer, SshId}; use crate::{ - auth, msg, negotiation, strict_kex_violation, ChannelId, ChannelOpenFailure, CryptoVec, - Disconnect, Error, Limits, Sig, + auth, msg, negotiation, ChannelId, ChannelOpenFailure, CryptoVec, Disconnect, Error, Limits, + Sig, }; mod encrypted; @@ -1273,11 +1273,7 @@ async fn reply( ); if session.common.strict_kex && session.common.encrypted.is_none() { let seqno = pkt.seqn.0 - 1; // was incremented after read() - if let Some(expected) = STRICT_KEX_MSG_ORDER.get(seqno as usize) { - if message_type != expected { - return Err(strict_kex_violation(*message_type, seqno as usize).into()); - } - } + validate_server_msg_strict_kex(*message_type, seqno as usize)?; } if [msg::IGNORE, msg::UNIMPLEMENTED, msg::DEBUG].contains(message_type) { @@ -1375,6 +1371,27 @@ fn initial_encrypted_state(session: &Session) -> EncryptedState { } } +/// The configuration of clients. +#[derive(Debug, Clone)] +pub struct GexParams { + /// Minimum DH group size (in bits) for DH GEX exchanges. + pub min_group_size: u32, + /// Preferred DH group size (in bits) for DH GEX exchanges. + pub preferred_group_size: u32, + /// Maximum DH group size (in bits) for DH GEX exchanges. + pub max_group_size: u32, +} + +impl Default for GexParams { + fn default() -> GexParams { + GexParams { + min_group_size: 3072, + preferred_group_size: 3072, + max_group_size: 8192, + } + } +} + /// The configuration of clients. #[derive(Debug)] pub struct Config { @@ -1398,6 +1415,8 @@ pub struct Config { pub keepalive_max: usize, /// Whether to expect and wait for an authentication call. pub anonymous: bool, + /// DH dynamic group exchange parameters. + pub gex: GexParams, } impl Default for Config { @@ -1417,6 +1436,7 @@ impl Default for Config { keepalive_interval: None, keepalive_max: 3, anonymous: false, + gex: Default::default(), } } } diff --git a/russh/src/kex/dh/gex.rs b/russh/src/kex/dh/gex.rs new file mode 100644 index 00000000..e69de29b diff --git a/russh/src/kex/dh/groups.rs b/russh/src/kex/dh/groups.rs index bb1e992d..9826ba58 100644 --- a/russh/src/kex/dh/groups.rs +++ b/russh/src/kex/dh/groups.rs @@ -1,16 +1,63 @@ +use std::fmt::Debug; +use std::ops::Deref; + use hex_literal::hex; use num_bigint::{BigUint, RandBigInt}; use rand; +#[derive(Clone)] +pub enum DhGroupUInt { + Static(&'static [u8]), + Owned(Vec), +} + +impl From> for DhGroupUInt { + fn from(x: Vec) -> Self { + Self::Owned(x) + } +} + +impl DhGroupUInt { + pub const fn new(x: &'static [u8]) -> Self { + Self::Static(x) + } +} + +impl Deref for DhGroupUInt { + type Target = [u8]; + + fn deref(&self) -> &Self::Target { + match self { + Self::Static(x) => x, + Self::Owned(x) => x, + } + } +} + +#[derive(Clone)] pub struct DhGroup { - pub(crate) prime: &'static [u8], - pub(crate) generator: usize, + pub(crate) prime: DhGroupUInt, + pub(crate) generator: DhGroupUInt, pub(crate) exp_size: u64, } +impl Debug for DhGroup { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("DhGroup") + .field("prime", &format!("<{} bytes>", self.prime.deref().len())) + .field( + "generator", + &format!("<{} bytes>", self.generator.deref().len()), + ) + .field("exp_size", &self.exp_size) + .finish() + } +} + pub const DH_GROUP1: DhGroup = DhGroup { - prime: hex!( - " + prime: DhGroupUInt::new( + hex!( + " FFFFFFFF FFFFFFFF C90FDAA2 2168C234 C4C6628B 80DC1CD1 29024E08 8A67CC74 020BBEA6 3B139B22 514A0879 8E3404DD EF9519B3 CD3A431B 302B0A6D F25F1437 4FE1356D 6D51C245 @@ -18,15 +65,17 @@ pub const DH_GROUP1: DhGroup = DhGroup { EE386BFB 5A899FA5 AE9F2411 7C4B1FE6 49286651 ECE65381 FFFFFFFF FFFFFFFF " - ) - .as_slice(), - generator: 2, + ) + .as_slice(), + ), + generator: DhGroupUInt::new(&[2]), exp_size: 256, }; pub const DH_GROUP14: DhGroup = DhGroup { - prime: hex!( - " + prime: DhGroupUInt::new( + hex!( + " FFFFFFFF FFFFFFFF C90FDAA2 2168C234 C4C6628B 80DC1CD1 29024E08 8A67CC74 020BBEA6 3B139B22 514A0879 8E3404DD EF9519B3 CD3A431B 302B0A6D F25F1437 4FE1356D 6D51C245 @@ -39,15 +88,17 @@ pub const DH_GROUP14: DhGroup = DhGroup { DE2BCBF6 95581718 3995497C EA956AE5 15D22618 98FA0510 15728E5A 8AACAA68 FFFFFFFF FFFFFFFF " - ) - .as_slice(), - generator: 2, + ) + .as_slice(), + ), + generator: DhGroupUInt::new(&[2]), exp_size: 256, }; pub const DH_GROUP16: DhGroup = DhGroup { - prime: hex!( - " + prime: DhGroupUInt::new( + hex!( + " FFFFFFFF FFFFFFFF C90FDAA2 2168C234 C4C6628B 80DC1CD1 29024E08 8A67CC74 020BBEA6 3B139B22 514A0879 8E3404DD EF9519B3 CD3A431B 302B0A6D F25F1437 4FE1356D 6D51C245 @@ -71,17 +122,18 @@ pub const DH_GROUP16: DhGroup = DhGroup { 93B4EA98 8D8FDDC1 86FFB7DC 90A6C08F 4DF435C9 34063199 FFFFFFFF FFFFFFFF " - ) - .as_slice(), - generator: 2, + ) + .as_slice(), + ), + generator: DhGroupUInt::new(&[2]), exp_size: 512, }; #[derive(Debug, PartialEq, Eq, Clone)] -pub struct DH { +pub(crate) struct DH { prime_num: BigUint, - generator: usize, - exp_size: u64, + generator: BigUint, + // exp_size: u64, private_key: BigUint, public_key: BigUint, shared_secret: BigUint, @@ -90,9 +142,9 @@ pub struct DH { impl DH { pub fn new(group: &DhGroup) -> Self { Self { - prime_num: BigUint::from_bytes_be(group.prime), - generator: group.generator, - exp_size: group.exp_size, + prime_num: BigUint::from_bytes_be(&*group.prime), + generator: BigUint::from_bytes_be(&*group.generator), + // exp_size: group.exp_size, private_key: BigUint::default(), public_key: BigUint::default(), shared_secret: BigUint::default(), @@ -108,7 +160,7 @@ impl DH { } pub fn generate_public_key(&mut self) -> BigUint { - self.public_key = BigUint::from(self.generator).modpow(&self.private_key, &self.prime_num); + self.public_key = self.generator.modpow(&self.private_key, &self.prime_num); self.public_key.clone() } diff --git a/russh/src/kex/dh/mod.rs b/russh/src/kex/dh/mod.rs index 75e19b58..b820962e 100644 --- a/russh/src/kex/dh/mod.rs +++ b/russh/src/kex/dh/mod.rs @@ -1,11 +1,10 @@ -// mod gex; -pub(crate) mod groups; +pub mod groups; use std::marker::PhantomData; use byteorder::{BigEndian, ByteOrder}; use digest::Digest; use groups::DH; -use log::debug; +use log::{debug, error}; use num_bigint::BigUint; use sha1::Sha1; use sha2::{Sha256, Sha512}; @@ -14,13 +13,29 @@ use ssh_encoding::{Encode, Writer}; use self::groups::{DhGroup, DH_GROUP1, DH_GROUP14, DH_GROUP16}; use super::{compute_keys, KexAlgorithm, KexAlgorithmImplementor, KexType}; use crate::session::Exchange; -use crate::{cipher, mac, msg, CryptoVec}; +use crate::{cipher, mac, msg, CryptoVec, Error}; + +pub struct DhGexSha1KexType {} + +impl KexType for DhGexSha1KexType { + fn make(&self) -> KexAlgorithm { + DhGroupKex::::new(None).into() + } +} + +pub struct DhGexSha256KexType {} + +impl KexType for DhGexSha256KexType { + fn make(&self) -> KexAlgorithm { + DhGroupKex::::new(None).into() + } +} pub struct DhGroup1Sha1KexType {} impl KexType for DhGroup1Sha1KexType { fn make(&self) -> KexAlgorithm { - DhGroupKex::::new(&DH_GROUP1).into() + DhGroupKex::::new(Some(&DH_GROUP1)).into() } } @@ -28,7 +43,7 @@ pub struct DhGroup14Sha1KexType {} impl KexType for DhGroup14Sha1KexType { fn make(&self) -> KexAlgorithm { - DhGroupKex::::new(&DH_GROUP14).into() + DhGroupKex::::new(Some(&DH_GROUP14)).into() } } @@ -36,7 +51,7 @@ pub struct DhGroup14Sha256KexType {} impl KexType for DhGroup14Sha256KexType { fn make(&self) -> KexAlgorithm { - DhGroupKex::::new(&DH_GROUP14).into() + DhGroupKex::::new(Some(&DH_GROUP14)).into() } } @@ -44,23 +59,24 @@ pub struct DhGroup16Sha512KexType {} impl KexType for DhGroup16Sha512KexType { fn make(&self) -> KexAlgorithm { - DhGroupKex::::new(&DH_GROUP16).into() + DhGroupKex::::new(Some(&DH_GROUP16)).into() } } #[doc(hidden)] pub struct DhGroupKex { - dh: DH, + dh: Option, shared_secret: Option>, + is_dh_gex: bool, _digest: PhantomData, } impl DhGroupKex { - pub fn new(group: &DhGroup) -> DhGroupKex { - let dh = DH::new(group); + pub fn new(group: Option<&DhGroup>) -> DhGroupKex { DhGroupKex { - dh, + dh: group.map(DH::new), shared_secret: None, + is_dh_gex: group.is_none(), _digest: PhantomData, } } @@ -92,33 +108,62 @@ impl KexAlgorithmImplementor for DhGroupKex { false } + fn is_dh_gex(&self) -> bool { + self.is_dh_gex + } + + fn client_dh_gex_init( + &mut self, + gex_min: u32, + gex_n: u32, + gex_max: u32, + writer: &mut impl Writer, + ) -> Result<(), Error> { + msg::KEX_DH_GEX_REQUEST.encode(writer)?; + gex_min.encode(writer)?; + gex_n.encode(writer)?; + gex_max.encode(writer)?; + Ok(()) + } + + #[allow(dead_code)] + fn client_dh_gex_group(&mut self, group: DhGroup) -> Result<(), crate::Error> { + self.dh = Some(DH::new(&group)); + Ok(()) + } + #[doc(hidden)] - fn server_dh(&mut self, exchange: &mut Exchange, payload: &[u8]) -> Result<(), crate::Error> { + fn server_dh(&mut self, exchange: &mut Exchange, payload: &[u8]) -> Result<(), Error> { debug!("server_dh"); + let Some(dh) = self.dh.as_mut() else { + error!("DH kex sequence error, dh is None in server_dh"); + return Err(Error::Inconsistent); + }; + let client_pubkey = { if payload.first() != Some(&msg::KEX_ECDH_INIT) { - return Err(crate::Error::Inconsistent); + return Err(Error::Inconsistent); } #[allow(clippy::indexing_slicing)] // length checked let pubkey_len = BigEndian::read_u32(&payload[1..]) as usize; if payload.len() < 5 + pubkey_len { - return Err(crate::Error::Inconsistent); + return Err(Error::Inconsistent); } &payload .get(5..(5 + pubkey_len)) - .ok_or(crate::Error::Inconsistent)? + .ok_or(Error::Inconsistent)? }; debug!("client_pubkey: {:?}", client_pubkey); - self.dh.generate_private_key(true); - let server_pubkey = &self.dh.generate_public_key(); - if !self.dh.validate_public_key(server_pubkey) { - return Err(crate::Error::Inconsistent); + dh.generate_private_key(true); + let server_pubkey = &dh.generate_public_key(); + if !dh.validate_public_key(server_pubkey) { + return Err(Error::Inconsistent); } let encoded_server_pubkey = biguint_to_mpint(server_pubkey); @@ -128,13 +173,13 @@ impl KexAlgorithmImplementor for DhGroupKex { exchange.server_ephemeral.extend(&encoded_server_pubkey); let decoded_client_pubkey = DH::decode_public_key(client_pubkey); - if !self.dh.validate_public_key(&decoded_client_pubkey) { - return Err(crate::Error::Inconsistent); + if !dh.validate_public_key(&decoded_client_pubkey) { + return Err(Error::Inconsistent); } - let shared = self.dh.compute_shared_secret(decoded_client_pubkey); - if !self.dh.validate_shared_secret(&shared) { - return Err(crate::Error::Inconsistent); + let shared = dh.compute_shared_secret(decoded_client_pubkey); + if !dh.validate_shared_secret(&shared) { + return Err(Error::Inconsistent); } self.shared_secret = Some(biguint_to_mpint(&shared)); Ok(()) @@ -145,12 +190,17 @@ impl KexAlgorithmImplementor for DhGroupKex { &mut self, client_ephemeral: &mut CryptoVec, writer: &mut impl Writer, - ) -> Result<(), crate::Error> { - self.dh.generate_private_key(false); - let client_pubkey = &self.dh.generate_public_key(); + ) -> Result<(), Error> { + let Some(dh) = self.dh.as_mut() else { + error!("DH kex sequence error, dh is None in client_dh"); + return Err(Error::Inconsistent); + }; + + dh.generate_private_key(false); + let client_pubkey = &dh.generate_public_key(); - if !self.dh.validate_public_key(client_pubkey) { - return Err(crate::Error::Inconsistent); + if !dh.validate_public_key(client_pubkey) { + return Err(Error::Inconsistent); } // fill exchange. @@ -158,22 +208,32 @@ impl KexAlgorithmImplementor for DhGroupKex { client_ephemeral.clear(); client_ephemeral.extend(&encoded_pubkey); - msg::KEX_ECDH_INIT.encode(writer)?; + if self.is_dh_gex { + msg::KEX_DH_GEX_INIT.encode(writer)?; + } else { + msg::KEX_ECDH_INIT.encode(writer)?; + } + encoded_pubkey.encode(writer)?; Ok(()) } - fn compute_shared_secret(&mut self, remote_pubkey_: &[u8]) -> Result<(), crate::Error> { + fn compute_shared_secret(&mut self, remote_pubkey_: &[u8]) -> Result<(), Error> { + let Some(dh) = self.dh.as_mut() else { + error!("DH kex sequence error, dh is None in compute_shared_secret"); + return Err(Error::Inconsistent); + }; + let remote_pubkey = DH::decode_public_key(remote_pubkey_); - if !self.dh.validate_public_key(&remote_pubkey) { - return Err(crate::Error::Inconsistent); + if !dh.validate_public_key(&remote_pubkey) { + return Err(Error::Inconsistent); } - let shared = self.dh.compute_shared_secret(remote_pubkey); - if !self.dh.validate_shared_secret(&shared) { - return Err(crate::Error::Inconsistent); + let shared = dh.compute_shared_secret(remote_pubkey); + if !dh.validate_shared_secret(&shared) { + return Err(Error::Inconsistent); } self.shared_secret = Some(biguint_to_mpint(&shared)); Ok(()) @@ -184,7 +244,7 @@ impl KexAlgorithmImplementor for DhGroupKex { key: &CryptoVec, exchange: &Exchange, buffer: &mut CryptoVec, - ) -> Result { + ) -> Result { // Computing the exchange hash, see page 7 of RFC 5656. buffer.clear(); exchange.client_id.encode(buffer)?; @@ -193,6 +253,15 @@ impl KexAlgorithmImplementor for DhGroupKex { exchange.server_kex_init.encode(buffer)?; buffer.extend(key); + + if let Some((gex_params, dh_group)) = &exchange.gex { + gex_params.min_group_size.encode(buffer)?; + gex_params.preferred_group_size.encode(buffer)?; + gex_params.max_group_size.encode(buffer)?; + dh_group.prime.encode(buffer)?; + dh_group.generator.encode(buffer)?; + } + exchange.client_ephemeral.encode(buffer)?; exchange.server_ephemeral.encode(buffer)?; @@ -216,7 +285,7 @@ impl KexAlgorithmImplementor for DhGroupKex { remote_to_local_mac: mac::Name, local_to_remote_mac: mac::Name, is_server: bool, - ) -> Result { + ) -> Result { compute_keys::( self.shared_secret.as_deref(), session_id, diff --git a/russh/src/kex/mod.rs b/russh/src/kex/mod.rs index 82197993..0a1d4ee3 100644 --- a/russh/src/kex/mod.rs +++ b/russh/src/kex/mod.rs @@ -16,7 +16,7 @@ //! //! This module exports kex algorithm names for use with [Preferred]. mod curve25519; -mod dh; +pub(crate) mod dh; mod ecdh_nistp; mod none; use std::cell::RefCell; @@ -27,8 +27,9 @@ use std::ops::DerefMut; use curve25519::Curve25519KexType; use delegate::delegate; +use dh::groups::DhGroup; use dh::{ - DhGroup14Sha1KexType, DhGroup14Sha256KexType, DhGroup16Sha512KexType, DhGroup1Sha1KexType, + DhGexSha1KexType, DhGexSha256KexType, DhGroup14Sha1KexType, DhGroup14Sha256KexType, DhGroup16Sha512KexType, DhGroup1Sha1KexType }; use digest::Digest; use ecdh_nistp::{EcdhNistP256KexType, EcdhNistP384KexType, EcdhNistP521KexType}; @@ -160,25 +161,37 @@ impl Debug for KexAlgorithm { #[enum_dispatch] pub(crate) trait KexAlgorithmImplementor { fn skip_exchange(&self) -> bool; + fn is_dh_gex(&self) -> bool { + false + } + + fn server_dh_gex_init( + &mut self, + _exchange: &mut Exchange, + _payload: &[u8], + ) -> Result<(), crate::Error> { + Err(crate::Error::KexInit) + } + + #[allow(dead_code)] + #[allow(unused_variables)] + fn client_dh_gex_init( + &mut self, + gex_min: u32, + gex_n: u32, + gex_max: u32, + writer: &mut impl Writer, + ) -> Result<(), crate::Error> { + Err(crate::Error::KexInit) + } - // fn server_dh_gex_init( - // &mut self, - // _exchange: &mut Exchange, - // _payload: &[u8], - // ) -> Result<(), crate::Error> { - // Err(crate::Error::KexInit) - // } - - // #[allow(dead_code)] - // fn client_dh_gex_init( - // &mut self, - // _gex_min: u32, - // _gex_n: u32, - // _gex_max: u32, - // _buf: &mut CryptoVec, - // ) -> Result<(), crate::Error> { - // Err(crate::Error::KexInit) - // } + #[allow(dead_code)] + fn client_dh_gex_group( + &mut self, + _group: DhGroup, + ) -> Result<(), crate::Error> { + Err(crate::Error::KexInit) + } #[cfg_attr(target_arch = "wasm32", allow(dead_code))] fn server_dh(&mut self, exchange: &mut Exchange, payload: &[u8]) -> Result<(), crate::Error>; @@ -235,6 +248,10 @@ impl TryFrom<&str> for Name { pub const CURVE25519: Name = Name("curve25519-sha256"); /// `curve25519-sha256@libssh.org` pub const CURVE25519_PRE_RFC_8731: Name = Name("curve25519-sha256@libssh.org"); +/// `diffie-hellman-group-exchange-sha1` +pub const DH_GEX_SHA1: Name = Name("diffie-hellman-group-exchange-sha1"); +/// `diffie-hellman-group-exchange-sha256` +pub const DH_GEX_SHA256: Name = Name("diffie-hellman-group-exchange-sha256"); /// `diffie-hellman-group1-sha1` pub const DH_G1_SHA1: Name = Name("diffie-hellman-group1-sha1"); /// `diffie-hellman-group14-sha1` @@ -261,6 +278,8 @@ pub const EXTENSION_OPENSSH_STRICT_KEX_AS_CLIENT: Name = Name("kex-strict-c-v00@ pub const EXTENSION_OPENSSH_STRICT_KEX_AS_SERVER: Name = Name("kex-strict-s-v00@openssh.com"); const _CURVE25519: Curve25519KexType = Curve25519KexType {}; +const _DH_GEX_SHA1: DhGexSha1KexType = DhGexSha1KexType {}; +const _DH_GEX_SHA256: DhGexSha256KexType = DhGexSha256KexType {}; const _DH_G1_SHA1: DhGroup1Sha1KexType = DhGroup1Sha1KexType {}; const _DH_G14_SHA1: DhGroup14Sha1KexType = DhGroup14Sha1KexType {}; const _DH_G14_SHA256: DhGroup14Sha256KexType = DhGroup14Sha256KexType {}; @@ -273,6 +292,8 @@ const _NONE: none::NoneKexType = none::NoneKexType {}; pub const ALL_KEX_ALGORITHMS: &[&Name] = &[ &CURVE25519, &CURVE25519_PRE_RFC_8731, + &DH_GEX_SHA1, + &DH_GEX_SHA256, &DH_G1_SHA1, &DH_G14_SHA1, &DH_G14_SHA256, @@ -288,6 +309,8 @@ pub(crate) static KEXES: Lazy = HashMap::new(); h.insert(&CURVE25519, &_CURVE25519); h.insert(&CURVE25519_PRE_RFC_8731, &_CURVE25519); + h.insert(&DH_GEX_SHA1, &_DH_GEX_SHA1); + h.insert(&DH_GEX_SHA256, &_DH_GEX_SHA256); h.insert(&DH_G16_SHA512, &_DH_G16_SHA512); h.insert(&DH_G14_SHA256, &_DH_G14_SHA256); h.insert(&DH_G14_SHA1, &_DH_G14_SHA1); diff --git a/russh/src/lib.rs b/russh/src/lib.rs index 94fd6dce..c6812453 100644 --- a/russh/src/lib.rs +++ b/russh/src/lib.rs @@ -92,7 +92,7 @@ use std::convert::TryFrom; use std::fmt::{Debug, Display, Formatter}; -use log::debug; +use log::{debug, warn}; use parsing::ChannelOpenConfirmation; pub use russh_cryptovec::CryptoVec; use ssh_encoding::{Decode, Encode}; @@ -316,7 +316,7 @@ pub enum Error { } pub(crate) fn strict_kex_violation(message_type: u8, sequence_number: usize) -> crate::Error { - debug!( + warn!( "strict kex violated at sequence no. {:?}, message type: {:?}", sequence_number, message_type ); diff --git a/russh/src/msg.rs b/russh/src/msg.rs index 68a21b36..717661c1 100644 --- a/russh/src/msg.rs +++ b/russh/src/msg.rs @@ -17,6 +17,8 @@ #[cfg(not(target_arch = "wasm32"))] pub use server::*; +use crate::{strict_kex_violation, Error}; + pub const DISCONNECT: u8 = 1; #[allow(dead_code)] pub const IGNORE: u8 = 2; @@ -34,6 +36,10 @@ pub const NEWKEYS: u8 = 21; // http://tools.ietf.org/html/rfc5656#section-7.1 pub const KEX_ECDH_INIT: u8 = 30; pub const KEX_ECDH_REPLY: u8 = 31; +pub const KEX_DH_GEX_REQUEST: u8 = 34; +pub const KEX_DH_GEX_GROUP: u8 = 31; +pub const KEX_DH_GEX_INIT: u8 = 32; +pub const KEX_DH_GEX_REPLY: u8 = 33; // https://tools.ietf.org/html/rfc4250#section-4.1.2 pub const USERAUTH_REQUEST: u8 = 50; @@ -77,8 +83,62 @@ mod server { pub const SSH_OPEN_ADMINISTRATIVELY_PROHIBITED: u8 = 1; } -pub(crate) const STRICT_KEX_MSG_ORDER: &[u8] = &[KEXINIT, KEX_ECDH_REPLY, NEWKEYS]; -const ALL_KEX_MESSAGES: &[u8] = &[KEXINIT, KEX_ECDH_INIT, KEX_ECDH_REPLY, NEWKEYS]; +/// Validate a message+seqno against a strict kex order pattern +/// Returns: +/// - `Some(true)` if the message is valid at this position +/// - `Some(false)` if the message is invalid at this position +/// - `None` if the `seqno` is not covered by strict kex +fn validate_msg_strict_kex(msg_type: u8, seqno: usize, order: &[u8]) -> Option { + order.get(seqno).map(|expected| expected == &msg_type) +} + +/// Validate a message+seqno against multiple strict kex order patterns +fn validate_msg_strict_kex_alt_order(msg_type: u8, seqno: usize, orders: &[&[u8]]) -> Option { + let mut valid = None; // did not match yet + for order in orders { + let result = validate_msg_strict_kex(msg_type, seqno, order); + valid = match (valid, result) { + // If we matched a valid msg, it's now valid forever + (Some(true), _ ) | (_, Some(true)) => Some(true), + // If we matched an invalid msg and we didn't find a valid one yet, it's now invalid + (None | Some(false), Some(false)) => Some(false), + // If the message was beyond the current pattern, no change + (x, None) => x, + }; + } + valid +} + +pub(crate) fn validate_client_msg_strict_kex(msg_type: u8, seqno: usize) -> Result<(), Error> { + if Some(false) == validate_msg_strict_kex_alt_order(msg_type, seqno, &[ + &[KEXINIT, KEX_ECDH_INIT, NEWKEYS], + &[KEXINIT, KEX_DH_GEX_REQUEST, KEX_DH_GEX_INIT, NEWKEYS], + ]) { + return Err(strict_kex_violation(msg_type, seqno)) + } + Ok(()) +} + +pub(crate) fn validate_server_msg_strict_kex(msg_type: u8, seqno: usize) -> Result<(), Error> { + if Some(false) == validate_msg_strict_kex_alt_order(msg_type, seqno, &[ + &[KEXINIT, KEX_ECDH_REPLY, NEWKEYS], + &[KEXINIT, KEX_DH_GEX_GROUP, KEX_DH_GEX_REPLY, NEWKEYS] + ]) { + return Err(strict_kex_violation(msg_type, seqno)) + } + Ok(()) +} + +const ALL_KEX_MESSAGES: &[u8] = &[ + KEXINIT, + KEX_ECDH_INIT, + KEX_ECDH_REPLY, + KEX_DH_GEX_GROUP, + KEX_DH_GEX_INIT, + KEX_DH_GEX_REPLY, + KEX_DH_GEX_REQUEST, + NEWKEYS, +]; pub(crate) fn is_kex_msg(msg: u8) -> bool { ALL_KEX_MESSAGES.contains(&msg) diff --git a/russh/src/negotiation.rs b/russh/src/negotiation.rs index 5b479caf..e1e3790a 100644 --- a/russh/src/negotiation.rs +++ b/russh/src/negotiation.rs @@ -91,6 +91,7 @@ impl Preferred { const SAFE_KEX_ORDER: &[kex::Name] = &[ kex::CURVE25519, kex::CURVE25519_PRE_RFC_8731, + kex::DH_GEX_SHA256, kex::DH_G16_SHA512, kex::DH_G14_SHA256, kex::EXTENSION_SUPPORT_AS_CLIENT, diff --git a/russh/src/server/mod.rs b/russh/src/server/mod.rs index 850dcc33..2379f329 100644 --- a/russh/src/server/mod.rs +++ b/russh/src/server/mod.rs @@ -39,7 +39,7 @@ use async_trait::async_trait; use bytes::Bytes; use futures::future::Future; use log::{debug, error, info, warn}; -use msg::is_kex_msg; +use msg::{is_kex_msg, validate_client_msg_strict_kex}; use russh_keys::map_err; use russh_util::runtime::JoinHandle; use russh_util::time::Instant; @@ -934,8 +934,6 @@ async fn read_ssh_id( Ok(session) } -const STRICT_KEX_MSG_ORDER: &[u8] = &[msg::KEXINIT, msg::KEX_ECDH_INIT, msg::NEWKEYS]; - async fn reply( session: &mut Session, handler: &mut H, @@ -949,11 +947,7 @@ async fn reply( ); if session.common.strict_kex && session.common.encrypted.is_none() { let seqno = pkt.seqn.0 - 1; // was incremented after read() - if let Some(expected) = STRICT_KEX_MSG_ORDER.get(seqno as usize) { - if message_type != expected { - return Err(strict_kex_violation(*message_type, seqno as usize).into()); - } - } + validate_client_msg_strict_kex(*message_type, seqno as usize)?; } if [msg::IGNORE, msg::UNIMPLEMENTED, msg::DEBUG].contains(message_type) { diff --git a/russh/src/session.rs b/russh/src/session.rs index f202e3f7..d25fa4be 100644 --- a/russh/src/session.rs +++ b/russh/src/session.rs @@ -24,6 +24,8 @@ use ssh_encoding::Encode; use tokio::sync::oneshot; use crate::cipher::OpeningKey; +use crate::client::GexParams; +use crate::kex::dh::groups::DhGroup; use crate::kex::{KexAlgorithm, KexAlgorithmImplementor}; use crate::sshbuffer::PacketWriter; use crate::{ @@ -529,6 +531,7 @@ pub struct Exchange { pub server_kex_init: CryptoVec, pub client_ephemeral: CryptoVec, pub server_ephemeral: CryptoVec, + pub gex: Option<(GexParams, DhGroup)>, } impl Exchange { From 599629b6ea73788247959921daf5e1bce7a994d7 Mon Sep 17 00:00:00 2001 From: Eugene Date: Mon, 6 Jan 2025 00:40:08 +0100 Subject: [PATCH 2/6] client gex --- russh/examples/client_exec_simple.rs | 2 +- russh/src/client/kex.rs | 22 +++++++---- russh/src/client/mod.rs | 55 ++++++++++++++++++++++++---- russh/src/kex/dh/groups.rs | 15 +++----- russh/src/kex/dh/mod.rs | 28 +++++++++----- russh/src/kex/mod.rs | 19 +++++----- russh/src/lib.rs | 3 ++ 7 files changed, 100 insertions(+), 44 deletions(-) diff --git a/russh/examples/client_exec_simple.rs b/russh/examples/client_exec_simple.rs index 0b445da1..af987d82 100644 --- a/russh/examples/client_exec_simple.rs +++ b/russh/examples/client_exec_simple.rs @@ -86,7 +86,7 @@ impl Session { let config = client::Config { inactivity_timeout: Some(Duration::from_secs(5)), preferred: Preferred { - kex: Cow::Owned(vec![russh::kex::DH_GEX_SHA1]), + kex: Cow::Owned(vec![russh::kex::DH_GEX_SHA256]), ..Default::default() }, ..<_>::default() diff --git a/russh/src/client/kex.rs b/russh/src/client/kex.rs index 89055609..bb50d18f 100644 --- a/russh/src/client/kex.rs +++ b/russh/src/client/kex.rs @@ -4,7 +4,7 @@ use std::fmt::{Debug, Formatter}; use std::sync::Arc; use bytes::Bytes; -use log::{debug, error}; +use log::{debug, error, warn}; use russh_cryptovec::CryptoVec; use russh_keys::key::parse_public_key; use signature::Verifier; @@ -158,12 +158,7 @@ impl Kex for ClientKex { if kex.is_dh_gex() { // TODO values output.packet(|w| { - kex.client_dh_gex_init( - self.config.gex.min_group_size, - self.config.gex.preferred_group_size, - self.config.gex.max_group_size, - w, - )?; + kex.client_dh_gex_init(&self.config.gex, w)?; Ok(()) })?; @@ -203,10 +198,21 @@ impl Kex for ClientKex { debug!("received gex group: prime={}, gen={}", prime, gen); //todo validate group + let Some(prime_bit_length) = prime.as_positive_bytes().map(|x| x.len() * 8) else { + warn!("negative DH prime received"); + return Err(Error::KexInit); + }; + + if prime_bit_length < self.config.gex.min_group_size + || prime_bit_length > self.config.gex.max_group_size + { + warn!("DH prime size ({prime_bit_length} bits) not within acceptable range"); + return Err(Error::KexInit); + } + let group = DhGroup { prime: prime.as_bytes().to_vec().into(), generator: gen.as_bytes().to_vec().into(), - exp_size: 0, }; let exchange = &mut self.exchange; diff --git a/russh/src/client/mod.rs b/russh/src/client/mod.rs index fa45c473..19255b70 100644 --- a/russh/src/client/mod.rs +++ b/russh/src/client/mod.rs @@ -1371,15 +1371,56 @@ fn initial_encrypted_state(session: &Session) -> EncryptedState { } } -/// The configuration of clients. +/// Parameters for dynamic group Diffie-Hellman key exchanges. #[derive(Debug, Clone)] pub struct GexParams { - /// Minimum DH group size (in bits) for DH GEX exchanges. - pub min_group_size: u32, - /// Preferred DH group size (in bits) for DH GEX exchanges. - pub preferred_group_size: u32, - /// Maximum DH group size (in bits) for DH GEX exchanges. - pub max_group_size: u32, + /// Minimum DH group size (in bits) + min_group_size: usize, + /// Preferred DH group size (in bits) + preferred_group_size: usize, + /// Maximum DH group size (in bits) + max_group_size: usize, +} + +impl GexParams { + pub fn new( + min_group_size: usize, + preferred_group_size: usize, + max_group_size: usize, + ) -> Result { + if min_group_size < 2048 { + return Err(Error::InvalidConfig( + "min_group_size must be at least 2048 bits".into(), + )); + } + if preferred_group_size < min_group_size { + return Err(Error::InvalidConfig( + "preferred_group_size must be at least as large as min_group_size".into(), + )); + } + if max_group_size < preferred_group_size { + return Err(Error::InvalidConfig( + "max_group_size must be at least as large as preferred_group_size".into(), + )); + } + Ok(GexParams { + min_group_size, + preferred_group_size, + max_group_size, + }) + } + + pub fn min_group_size(&self) -> usize { + self.min_group_size + } + + pub fn preferred_group_size(&self) -> usize { + self.preferred_group_size + } + + pub fn max_group_size(&self) -> usize { + self.max_group_size + } } impl Default for GexParams { diff --git a/russh/src/kex/dh/groups.rs b/russh/src/kex/dh/groups.rs index 9826ba58..df50ba1d 100644 --- a/russh/src/kex/dh/groups.rs +++ b/russh/src/kex/dh/groups.rs @@ -38,7 +38,7 @@ impl Deref for DhGroupUInt { pub struct DhGroup { pub(crate) prime: DhGroupUInt, pub(crate) generator: DhGroupUInt, - pub(crate) exp_size: u64, + // pub(crate) exp_size: u64, } impl Debug for DhGroup { @@ -49,7 +49,6 @@ impl Debug for DhGroup { "generator", &format!("<{} bytes>", self.generator.deref().len()), ) - .field("exp_size", &self.exp_size) .finish() } } @@ -69,7 +68,7 @@ pub const DH_GROUP1: DhGroup = DhGroup { .as_slice(), ), generator: DhGroupUInt::new(&[2]), - exp_size: 256, + // exp_size: 256, }; pub const DH_GROUP14: DhGroup = DhGroup { @@ -92,7 +91,7 @@ pub const DH_GROUP14: DhGroup = DhGroup { .as_slice(), ), generator: DhGroupUInt::new(&[2]), - exp_size: 256, + // exp_size: 256, }; pub const DH_GROUP16: DhGroup = DhGroup { @@ -126,14 +125,13 @@ pub const DH_GROUP16: DhGroup = DhGroup { .as_slice(), ), generator: DhGroupUInt::new(&[2]), - exp_size: 512, + // exp_size: 512, }; #[derive(Debug, PartialEq, Eq, Clone)] pub(crate) struct DH { prime_num: BigUint, generator: BigUint, - // exp_size: u64, private_key: BigUint, public_key: BigUint, shared_secret: BigUint, @@ -142,9 +140,8 @@ pub(crate) struct DH { impl DH { pub fn new(group: &DhGroup) -> Self { Self { - prime_num: BigUint::from_bytes_be(&*group.prime), - generator: BigUint::from_bytes_be(&*group.generator), - // exp_size: group.exp_size, + prime_num: BigUint::from_bytes_be(&group.prime), + generator: BigUint::from_bytes_be(&group.generator), private_key: BigUint::default(), public_key: BigUint::default(), shared_secret: BigUint::default(), diff --git a/russh/src/kex/dh/mod.rs b/russh/src/kex/dh/mod.rs index b820962e..5c527bcf 100644 --- a/russh/src/kex/dh/mod.rs +++ b/russh/src/kex/dh/mod.rs @@ -12,6 +12,7 @@ use ssh_encoding::{Encode, Writer}; use self::groups::{DhGroup, DH_GROUP1, DH_GROUP14, DH_GROUP16}; use super::{compute_keys, KexAlgorithm, KexAlgorithmImplementor, KexType}; +use crate::client::GexParams; use crate::session::Exchange; use crate::{cipher, mac, msg, CryptoVec, Error}; @@ -114,15 +115,13 @@ impl KexAlgorithmImplementor for DhGroupKex { fn client_dh_gex_init( &mut self, - gex_min: u32, - gex_n: u32, - gex_max: u32, + gex: &GexParams, writer: &mut impl Writer, ) -> Result<(), Error> { msg::KEX_DH_GEX_REQUEST.encode(writer)?; - gex_min.encode(writer)?; - gex_n.encode(writer)?; - gex_max.encode(writer)?; + (gex.min_group_size() as u32).encode(writer)?; + (gex.preferred_group_size() as u32).encode(writer)?; + (gex.max_group_size() as u32).encode(writer)?; Ok(()) } @@ -255,9 +254,7 @@ impl KexAlgorithmImplementor for DhGroupKex { buffer.extend(key); if let Some((gex_params, dh_group)) = &exchange.gex { - gex_params.min_group_size.encode(buffer)?; - gex_params.preferred_group_size.encode(buffer)?; - gex_params.max_group_size.encode(buffer)?; + gex_params.encode(buffer)?; dh_group.prime.encode(buffer)?; dh_group.generator.encode(buffer)?; } @@ -297,3 +294,16 @@ impl KexAlgorithmImplementor for DhGroupKex { ) } } + +impl Encode for GexParams { + fn encoded_len(&self) -> Result { + Ok(0u32.encoded_len()? * 3) + } + + fn encode(&self, writer: &mut impl Writer) -> Result<(), ssh_encoding::Error> { + (self.min_group_size() as u32).encode(writer)?; + (self.preferred_group_size() as u32).encode(writer)?; + (self.max_group_size() as u32).encode(writer)?; + Ok(()) + } +} diff --git a/russh/src/kex/mod.rs b/russh/src/kex/mod.rs index 0a1d4ee3..6dd36a0c 100644 --- a/russh/src/kex/mod.rs +++ b/russh/src/kex/mod.rs @@ -44,6 +44,7 @@ use ssh_encoding::{Encode, Writer}; use ssh_key::PublicKey; use crate::cipher::CIPHERS; +use crate::client::GexParams; use crate::mac::{self, MACS}; use crate::negotiation::Names; use crate::session::{Exchange, NewKeys}; @@ -165,21 +166,19 @@ pub(crate) trait KexAlgorithmImplementor { false } - fn server_dh_gex_init( - &mut self, - _exchange: &mut Exchange, - _payload: &[u8], - ) -> Result<(), crate::Error> { - Err(crate::Error::KexInit) - } + // fn server_dh_gex_init( + // &mut self, + // _exchange: &mut Exchange, + // _payload: &[u8], + // ) -> Result<(), crate::Error> { + // Err(crate::Error::KexInit) + // } #[allow(dead_code)] #[allow(unused_variables)] fn client_dh_gex_init( &mut self, - gex_min: u32, - gex_n: u32, - gex_max: u32, + gex: &GexParams, writer: &mut impl Writer, ) -> Result<(), crate::Error> { Err(crate::Error::KexInit) diff --git a/russh/src/lib.rs b/russh/src/lib.rs index c6812453..cbb7df08 100644 --- a/russh/src/lib.rs +++ b/russh/src/lib.rs @@ -313,6 +313,9 @@ pub enum Error { #[error("SshEncoding: {0}")] SshEncoding(#[from] ssh_encoding::Error), + + #[error("Invalid config: {0}")] + InvalidConfig(String), } pub(crate) fn strict_kex_violation(message_type: u8, sequence_number: usize) -> crate::Error { From 9003b16f4f8a91068ec9a7f0160c2155519a254a Mon Sep 17 00:00:00 2001 From: Eugene Date: Mon, 6 Jan 2025 01:46:56 +0100 Subject: [PATCH 3/6] server gex --- russh/examples/echoserver.rs | 13 +++- russh/src/client/kex.rs | 22 +++---- russh/src/client/mod.rs | 24 ++++--- russh/src/kex/dh/gex.rs | 0 russh/src/kex/dh/mod.rs | 51 +++++++++------ russh/src/kex/mod.rs | 55 +++++++--------- russh/src/server/kex.rs | 119 +++++++++++++++++++++++++++-------- russh/src/server/mod.rs | 26 +++++++- russh/src/server/session.rs | 2 +- 9 files changed, 211 insertions(+), 101 deletions(-) delete mode 100644 russh/src/kex/dh/gex.rs diff --git a/russh/examples/echoserver.rs b/russh/examples/echoserver.rs index 79487891..1642a4ef 100644 --- a/russh/examples/echoserver.rs +++ b/russh/examples/echoserver.rs @@ -1,7 +1,10 @@ +use std::borrow::Cow; use std::collections::HashMap; use std::sync::Arc; use async_trait::async_trait; +use russh::client::GexParams; +use kex::dh::groups::DhGroup; use rand_core::OsRng; use russh::keys::*; use russh::server::{Msg, Server as _, Session}; @@ -23,7 +26,7 @@ async fn main() { russh_keys::PrivateKey::random(&mut OsRng, russh_keys::Algorithm::Ed25519).unwrap(), ], preferred: Preferred { - // key: Cow::Borrowed(&[CERT_ECDSA_SHA2_P256]), + kex: Cow::Owned(vec![russh::kex::DH_GEX_SHA256]), ..Preferred::default() }, ..Default::default() @@ -133,6 +136,14 @@ impl server::Handler for Server { }); Ok(true) } + + async fn lookup_dh_gex_group( + &mut self, + gex_params: &GexParams, + ) -> Result, Self::Error> { + Ok(Some(russh::kex::dh::groups::DH_GROUP16)) + } + } impl Drop for Server { diff --git a/russh/src/client/kex.rs b/russh/src/client/kex.rs index bb50d18f..ea3545b3 100644 --- a/russh/src/client/kex.rs +++ b/russh/src/client/kex.rs @@ -28,11 +28,11 @@ thread_local! { #[allow(clippy::large_enum_variant)] enum ClientKexState { Created, - WaitingForGexResponse { + WaitingForGexReply { names: Names, kex: KexAlgorithm, }, - WaitingForServerDh { + WaitingForDhReply { // both KexInit and DH init sent names: Names, kex: KexAlgorithm, @@ -58,10 +58,10 @@ impl Debug for ClientKex { ClientKexState::Created => { s.field("state", &"created"); } - ClientKexState::WaitingForGexResponse { .. } => { + ClientKexState::WaitingForGexReply { .. } => { s.field("state", &"waiting for GEX response"); } - ClientKexState::WaitingForServerDh { .. } => { + ClientKexState::WaitingForDhReply { .. } => { s.field("state", &"waiting for DH response"); } ClientKexState::WaitingForNewKeys { .. } => { @@ -162,14 +162,14 @@ impl Kex for ClientKex { Ok(()) })?; - self.state = ClientKexState::WaitingForGexResponse { names, kex }; + self.state = ClientKexState::WaitingForGexReply { names, kex }; } else { output.packet(|w| { kex.client_dh(&mut self.exchange.client_ephemeral, w)?; Ok(()) })?; - self.state = ClientKexState::WaitingForServerDh { names, kex }; + self.state = ClientKexState::WaitingForDhReply { names, kex }; } Ok(KexProgress::NeedsReply { @@ -177,7 +177,7 @@ impl Kex for ClientKex { reset_seqn: false, }) } - ClientKexState::WaitingForGexResponse { names, mut kex } => { + ClientKexState::WaitingForGexReply { names, mut kex } => { let Some(input) = input else { return Err(Error::KexInit); }; @@ -217,19 +217,19 @@ impl Kex for ClientKex { let exchange = &mut self.exchange; exchange.gex = Some((self.config.gex.clone(), group.clone())); - kex.client_dh_gex_group(group)?; + kex.dh_gex_set_group(group)?; output.packet(|w| { kex.client_dh(&mut exchange.client_ephemeral, w)?; Ok(()) })?; - self.state = ClientKexState::WaitingForServerDh { names, kex }; + self.state = ClientKexState::WaitingForDhReply { names, kex }; Ok(KexProgress::NeedsReply { kex: self, reset_seqn: false, }) } - ClientKexState::WaitingForServerDh { mut names, mut kex } => { + ClientKexState::WaitingForDhReply { mut names, mut kex } => { // At this point, we've sent ECDH_INTI and // are waiting for the ECDH_REPLY from the server. @@ -241,7 +241,7 @@ impl Kex for ClientKex { // Ignore the next packet if (1) it follows and (2) it's not the correct guess. debug!("ignoring guessed kex"); names.ignore_guessed = false; - self.state = ClientKexState::WaitingForServerDh { names, kex }; + self.state = ClientKexState::WaitingForDhReply { names, kex }; return Ok(KexProgress::NeedsReply { kex: self, reset_seqn: false, diff --git a/russh/src/client/mod.rs b/russh/src/client/mod.rs index 19255b70..4b9bcd62 100644 --- a/russh/src/client/mod.rs +++ b/russh/src/client/mod.rs @@ -1388,26 +1388,32 @@ impl GexParams { preferred_group_size: usize, max_group_size: usize, ) -> Result { - if min_group_size < 2048 { + let this = Self { + min_group_size, + preferred_group_size, + max_group_size, + }; + this.validate()?; + Ok(this) + } + + pub(crate) fn validate(&self) -> Result<(), Error> { + if self.min_group_size < 2048 { return Err(Error::InvalidConfig( "min_group_size must be at least 2048 bits".into(), )); } - if preferred_group_size < min_group_size { + if self.preferred_group_size < self.min_group_size { return Err(Error::InvalidConfig( "preferred_group_size must be at least as large as min_group_size".into(), )); } - if max_group_size < preferred_group_size { + if self.max_group_size < self.preferred_group_size { return Err(Error::InvalidConfig( "max_group_size must be at least as large as preferred_group_size".into(), )); } - Ok(GexParams { - min_group_size, - preferred_group_size, - max_group_size, - }) + Ok(()) } pub fn min_group_size(&self) -> usize { @@ -1427,7 +1433,7 @@ impl Default for GexParams { fn default() -> GexParams { GexParams { min_group_size: 3072, - preferred_group_size: 3072, + preferred_group_size: 8192, max_group_size: 8192, } } diff --git a/russh/src/kex/dh/gex.rs b/russh/src/kex/dh/gex.rs deleted file mode 100644 index e69de29b..00000000 diff --git a/russh/src/kex/dh/mod.rs b/russh/src/kex/dh/mod.rs index 5c527bcf..bbd9b975 100644 --- a/russh/src/kex/dh/mod.rs +++ b/russh/src/kex/dh/mod.rs @@ -4,11 +4,11 @@ use std::marker::PhantomData; use byteorder::{BigEndian, ByteOrder}; use digest::Digest; use groups::DH; -use log::{debug, error}; +use log::{error, trace}; use num_bigint::BigUint; use sha1::Sha1; use sha2::{Sha256, Sha512}; -use ssh_encoding::{Encode, Writer}; +use ssh_encoding::{Decode, Encode, Reader, Writer}; use self::groups::{DhGroup, DH_GROUP1, DH_GROUP14, DH_GROUP16}; use super::{compute_keys, KexAlgorithm, KexAlgorithmImplementor, KexType}; @@ -16,7 +16,7 @@ use crate::client::GexParams; use crate::session::Exchange; use crate::{cipher, mac, msg, CryptoVec, Error}; -pub struct DhGexSha1KexType {} +pub(crate) struct DhGexSha1KexType {} impl KexType for DhGexSha1KexType { fn make(&self) -> KexAlgorithm { @@ -24,7 +24,7 @@ impl KexType for DhGexSha1KexType { } } -pub struct DhGexSha256KexType {} +pub(crate) struct DhGexSha256KexType {} impl KexType for DhGexSha256KexType { fn make(&self) -> KexAlgorithm { @@ -32,7 +32,7 @@ impl KexType for DhGexSha256KexType { } } -pub struct DhGroup1Sha1KexType {} +pub(crate) struct DhGroup1Sha1KexType {} impl KexType for DhGroup1Sha1KexType { fn make(&self) -> KexAlgorithm { @@ -40,7 +40,7 @@ impl KexType for DhGroup1Sha1KexType { } } -pub struct DhGroup14Sha1KexType {} +pub(crate) struct DhGroup14Sha1KexType {} impl KexType for DhGroup14Sha1KexType { fn make(&self) -> KexAlgorithm { @@ -48,7 +48,7 @@ impl KexType for DhGroup14Sha1KexType { } } -pub struct DhGroup14Sha256KexType {} +pub(crate) struct DhGroup14Sha256KexType {} impl KexType for DhGroup14Sha256KexType { fn make(&self) -> KexAlgorithm { @@ -56,7 +56,7 @@ impl KexType for DhGroup14Sha256KexType { } } -pub struct DhGroup16Sha512KexType {} +pub(crate) struct DhGroup16Sha512KexType {} impl KexType for DhGroup16Sha512KexType { fn make(&self) -> KexAlgorithm { @@ -65,7 +65,7 @@ impl KexType for DhGroup16Sha512KexType { } #[doc(hidden)] -pub struct DhGroupKex { +pub(crate) struct DhGroupKex { dh: Option, shared_secret: Option>, is_dh_gex: bool, @@ -73,7 +73,7 @@ pub struct DhGroupKex { } impl DhGroupKex { - pub fn new(group: Option<&DhGroup>) -> DhGroupKex { + pub(crate) fn new(group: Option<&DhGroup>) -> DhGroupKex { DhGroupKex { dh: group.map(DH::new), shared_secret: None, @@ -92,7 +92,7 @@ impl std::fmt::Debug for DhGroupKex { } } -fn biguint_to_mpint(biguint: &BigUint) -> Vec { +pub(crate) fn biguint_to_mpint(biguint: &BigUint) -> Vec { let mut mpint = Vec::new(); let bytes = biguint.to_bytes_be(); if let Some(b) = bytes.first() { @@ -126,22 +126,22 @@ impl KexAlgorithmImplementor for DhGroupKex { } #[allow(dead_code)] - fn client_dh_gex_group(&mut self, group: DhGroup) -> Result<(), crate::Error> { + fn dh_gex_set_group(&mut self, group: DhGroup) -> Result<(), crate::Error> { self.dh = Some(DH::new(&group)); Ok(()) } #[doc(hidden)] fn server_dh(&mut self, exchange: &mut Exchange, payload: &[u8]) -> Result<(), Error> { - debug!("server_dh"); - let Some(dh) = self.dh.as_mut() else { error!("DH kex sequence error, dh is None in server_dh"); return Err(Error::Inconsistent); }; let client_pubkey = { - if payload.first() != Some(&msg::KEX_ECDH_INIT) { + if payload.first() != Some(&msg::KEX_ECDH_INIT) + && payload.first() != Some(&msg::KEX_DH_GEX_INIT) + { return Err(Error::Inconsistent); } @@ -157,7 +157,7 @@ impl KexAlgorithmImplementor for DhGroupKex { .ok_or(Error::Inconsistent)? }; - debug!("client_pubkey: {:?}", client_pubkey); + trace!("client_pubkey: {:?}", client_pubkey); dh.generate_private_key(true); let server_pubkey = &dh.generate_public_key(); @@ -255,8 +255,8 @@ impl KexAlgorithmImplementor for DhGroupKex { if let Some((gex_params, dh_group)) = &exchange.gex { gex_params.encode(buffer)?; - dh_group.prime.encode(buffer)?; - dh_group.generator.encode(buffer)?; + biguint_to_mpint(&BigUint::from_bytes_be(&*dh_group.prime)).encode(buffer)?; + biguint_to_mpint(&BigUint::from_bytes_be(&*dh_group.generator)).encode(buffer)?; } exchange.client_ephemeral.encode(buffer)?; @@ -307,3 +307,18 @@ impl Encode for GexParams { Ok(()) } } + +impl Decode for GexParams { + fn decode(reader: &mut impl Reader) -> Result { + let min_group_size = u32::decode(reader)? as usize; + let preferred_group_size = u32::decode(reader)? as usize; + let max_group_size = u32::decode(reader)? as usize; + Ok(GexParams::new( + min_group_size, + preferred_group_size, + max_group_size, + )?) + } + + type Error = Error; +} diff --git a/russh/src/kex/mod.rs b/russh/src/kex/mod.rs index 6dd36a0c..dfb5427c 100644 --- a/russh/src/kex/mod.rs +++ b/russh/src/kex/mod.rs @@ -16,7 +16,7 @@ //! //! This module exports kex algorithm names for use with [Preferred]. mod curve25519; -pub(crate) mod dh; +pub mod dh; mod ecdh_nistp; mod none; use std::cell::RefCell; @@ -29,7 +29,8 @@ use curve25519::Curve25519KexType; use delegate::delegate; use dh::groups::DhGroup; use dh::{ - DhGexSha1KexType, DhGexSha256KexType, DhGroup14Sha1KexType, DhGroup14Sha256KexType, DhGroup16Sha512KexType, DhGroup1Sha1KexType + DhGexSha1KexType, DhGexSha256KexType, DhGroup14Sha1KexType, DhGroup14Sha256KexType, + DhGroup16Sha512KexType, DhGroup1Sha1KexType, }; use digest::Digest; use ecdh_nistp::{EcdhNistP256KexType, EcdhNistP384KexType, EcdhNistP521KexType}; @@ -52,19 +53,19 @@ use crate::sshbuffer::{IncomingSshPacket, PacketWriter}; use crate::{cipher, CryptoVec, Error}; #[derive(Debug)] -pub(crate) enum SessionKexState { +pub(crate) enum SessionKexState { Idle, InProgress(K), Taken, // some async activity still going on such as host key checks } -impl PartialEq for SessionKexState { +impl PartialEq for SessionKexState { fn eq(&self, other: &Self) -> bool { core::mem::discriminant(self) == core::mem::discriminant(other) } } -impl SessionKexState { +impl SessionKexState { pub fn active(&self) -> bool { match self { SessionKexState::Idle => false, @@ -166,49 +167,37 @@ pub(crate) trait KexAlgorithmImplementor { false } - // fn server_dh_gex_init( - // &mut self, - // _exchange: &mut Exchange, - // _payload: &[u8], - // ) -> Result<(), crate::Error> { - // Err(crate::Error::KexInit) - // } - - #[allow(dead_code)] #[allow(unused_variables)] fn client_dh_gex_init( &mut self, gex: &GexParams, writer: &mut impl Writer, - ) -> Result<(), crate::Error> { - Err(crate::Error::KexInit) + ) -> Result<(), Error> { + Err(Error::KexInit) } - #[allow(dead_code)] - fn client_dh_gex_group( - &mut self, - _group: DhGroup, - ) -> Result<(), crate::Error> { - Err(crate::Error::KexInit) + #[allow(unused_variables)] + fn dh_gex_set_group(&mut self, group: DhGroup) -> Result<(), Error> { + Err(Error::KexInit) } #[cfg_attr(target_arch = "wasm32", allow(dead_code))] - fn server_dh(&mut self, exchange: &mut Exchange, payload: &[u8]) -> Result<(), crate::Error>; + fn server_dh(&mut self, exchange: &mut Exchange, payload: &[u8]) -> Result<(), Error>; fn client_dh( &mut self, client_ephemeral: &mut CryptoVec, writer: &mut impl Writer, - ) -> Result<(), crate::Error>; + ) -> Result<(), Error>; - fn compute_shared_secret(&mut self, remote_pubkey_: &[u8]) -> Result<(), crate::Error>; + fn compute_shared_secret(&mut self, remote_pubkey_: &[u8]) -> Result<(), Error>; fn compute_exchange_hash( &self, key: &CryptoVec, exchange: &Exchange, buffer: &mut CryptoVec, - ) -> Result; + ) -> Result; fn compute_keys( &self, @@ -218,7 +207,7 @@ pub(crate) trait KexAlgorithmImplementor { remote_to_local_mac: mac::Name, local_to_remote_mac: mac::Name, is_server: bool, - ) -> Result; + ) -> Result; } #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] @@ -337,21 +326,21 @@ pub(crate) fn compute_keys( remote_to_local_mac: mac::Name, local_to_remote_mac: mac::Name, is_server: bool, -) -> Result { - let cipher = CIPHERS.get(&cipher).ok_or(crate::Error::UnknownAlgo)?; +) -> Result { + let cipher = CIPHERS.get(&cipher).ok_or(Error::UnknownAlgo)?; let remote_to_local_mac = MACS .get(&remote_to_local_mac) - .ok_or(crate::Error::UnknownAlgo)?; + .ok_or(Error::UnknownAlgo)?; let local_to_remote_mac = MACS .get(&local_to_remote_mac) - .ok_or(crate::Error::UnknownAlgo)?; + .ok_or(Error::UnknownAlgo)?; // https://tools.ietf.org/html/rfc4253#section-7.2 BUFFER.with(|buffer| { KEY_BUF.with(|key| { NONCE_BUF.with(|nonce| { MAC_BUF.with(|mac| { - let compute_key = |c, key: &mut CryptoVec, len| -> Result<(), crate::Error> { + let compute_key = |c, key: &mut CryptoVec, len| -> Result<(), Error> { let mut buffer = buffer.borrow_mut(); buffer.clear(); key.clear(); @@ -446,7 +435,7 @@ pub(crate) fn compute_keys( // NOTE: using MpInt::from_bytes().encode() will randomly fail, // I'm assuming it's due to specific byte values / padding but no time to investigate #[allow(clippy::indexing_slicing)] // length is known -pub(crate) fn encode_mpint(s: &[u8], w: &mut W) -> Result<(), crate::Error> { +pub(crate) fn encode_mpint(s: &[u8], w: &mut W) -> Result<(), Error> { // Skip initial 0s. let mut i = 0; while i < s.len() && s[i] == 0 { diff --git a/russh/src/server/kex.rs b/russh/src/server/kex.rs index 52b3bda8..e7e0cd46 100644 --- a/russh/src/server/kex.rs +++ b/russh/src/server/kex.rs @@ -1,14 +1,17 @@ use core::fmt; use std::cell::RefCell; +use client::GexParams; use log::debug; +use num_bigint::BigUint; use russh_keys::helpers::sign_with_hash_alg; use russh_keys::key::PrivateKeyWithHashAlg; use ssh_encoding::Encode; use ssh_key::Algorithm; use super::*; -use crate::kex::{Kex, KexAlgorithm, KexAlgorithmImplementor, KexCause, KEXES}; +use crate::kex::dh::biguint_to_mpint; +use crate::kex::{KexAlgorithm, KexAlgorithmImplementor, KexCause, KEXES}; use crate::negotiation::{is_key_compatible_with_algo, Names, Select}; use crate::{msg, negotiation}; @@ -20,7 +23,11 @@ thread_local! { #[allow(clippy::large_enum_variant)] enum ServerKexState { Created, - WaitingForKexReply { + WaitingForGexRequest { + names: Names, + kex: KexAlgorithm, + }, + WaitingForDhInit { // both KexInit and DH init sent names: Names, kex: KexAlgorithm, @@ -45,8 +52,11 @@ impl Debug for ServerKex { ServerKexState::Created => { s.field("state", &"created"); } - ServerKexState::WaitingForKexReply { .. } => { - s.field("state", &"waiting for a reply"); + ServerKexState::WaitingForGexRequest { .. } => { + s.field("state", &"waiting for GEX request"); + } + ServerKexState::WaitingForDhInit { .. } => { + s.field("state", &"waiting for DH reply"); } ServerKexState::WaitingForNewKeys { .. } => { s.field("state", &"waiting for NEWKEYS"); @@ -71,32 +81,31 @@ impl ServerKex { state: ServerKexState::Created, } } -} -impl Kex for ServerKex { - fn kexinit(&mut self, output: &mut PacketWriter) -> Result<(), Error> { + pub fn kexinit(&mut self, output: &mut PacketWriter) -> Result<(), Error> { self.exchange.server_kex_init = negotiation::write_kex(&self.config.preferred, output, Some(self.config.as_ref()))?; Ok(()) } - fn step( + pub async fn step( mut self, input: Option<&mut IncomingSshPacket>, output: &mut PacketWriter, - ) -> Result, Error> { + handler: &mut H, + ) -> Result, H::Error> { match self.state { ServerKexState::Created => { let Some(input) = input else { - return Err(Error::KexInit); + return Err(Error::KexInit)?; }; if input.buffer.first() != Some(&msg::KEXINIT) { error!( "Unexpected kex message at this stage: {:?}", input.buffer.first() ); - return Err(Error::KexInit); + return Err(Error::KexInit)?; } let names = { @@ -114,7 +123,7 @@ impl Kex for ServerKex { return Err(strict_kex_violation( msg::KEXINIT, input.seqn.0 as usize - 1, - )); + ))?; } let kex = KEXES.get(&names.kex).ok_or(Error::UnknownAlgo)?.make(); @@ -139,36 +148,88 @@ impl Kex for ServerKex { }); } - self.state = ServerKexState::WaitingForKexReply { names, kex }; + if kex.is_dh_gex() { + self.state = ServerKexState::WaitingForGexRequest { names, kex }; + } else { + self.state = ServerKexState::WaitingForDhInit { names, kex }; + } + + Ok(KexProgress::NeedsReply { + kex: self, + reset_seqn: false, + }) + } + ServerKexState::WaitingForGexRequest { names, mut kex } => { + let Some(input) = input else { + return Err(Error::KexInit)?; + }; + if input.buffer.first() != Some(&msg::KEX_DH_GEX_REQUEST) { + error!( + "Unexpected kex message at this stage: {:?}", + input.buffer.first() + ); + return Err(Error::KexInit)?; + } + + #[allow(clippy::indexing_slicing)] // length checked + let gex_params = GexParams::decode(&mut &input.buffer[1..])?; + debug!("client requests a gex group: {:?}", gex_params); + + let Some(dh_group) = handler.lookup_dh_gex_group(&gex_params).await? else { + debug!("server::Handler impl did not find a matching DH group (is lookup_dh_gex_group implemented?)"); + return Err(Error::Kex)?; + }; + + let prime = biguint_to_mpint(&BigUint::from_bytes_be(&*dh_group.prime)); + let generator = biguint_to_mpint(&BigUint::from_bytes_be(&*dh_group.generator)); + + dbg!(&prime); + dbg!(&generator); + + self.exchange.gex = Some((gex_params, dh_group.clone())); + kex.dh_gex_set_group(dh_group)?; + + output.packet(|w| { + msg::KEX_DH_GEX_GROUP.encode(w)?; + prime.encode(w)?; + generator.encode(w)?; + Ok(()) + })?; + + self.state = ServerKexState::WaitingForDhInit { names, kex }; Ok(KexProgress::NeedsReply { kex: self, reset_seqn: false, }) } - ServerKexState::WaitingForKexReply { mut names, mut kex } => { + ServerKexState::WaitingForDhInit { mut names, mut kex } => { let Some(input) = input else { - return Err(Error::KexInit); + return Err(Error::KexInit)?; }; if names.ignore_guessed { // Ignore the next packet if (1) it follows and (2) it's not the correct guess. debug!("ignoring guessed kex"); names.ignore_guessed = false; - self.state = ServerKexState::WaitingForKexReply { names, kex }; + self.state = ServerKexState::WaitingForDhInit { names, kex }; return Ok(KexProgress::NeedsReply { kex: self, reset_seqn: false, }); } - // We've received ECDH_REPLY - if input.buffer.first() != Some(&msg::KEX_ECDH_INIT) { + if input.buffer.first() + != Some(match kex.is_dh_gex() { + true => &msg::KEX_DH_GEX_INIT, + false => &msg::KEX_ECDH_INIT, + }) + { error!( "Unexpected kex message at this stage: {:?}", input.buffer.first() ); - return Err(Error::KexInit); + return Err(Error::KexInit)?; } #[allow(clippy::indexing_slicing)] // length checked @@ -176,7 +237,7 @@ impl Kex for ServerKex { self.exchange .client_ephemeral - .extend(&Bytes::decode(&mut r)?); + .extend(&Bytes::decode(&mut r).map_err(Into::into)?); let exchange = &mut self.exchange; kex.server_dh(exchange, &input.buffer)?; @@ -188,7 +249,7 @@ impl Kex for ServerKex { .position(|key| is_key_compatible_with_algo(key, &names.key)) else { debug!("we don't have a host key of type {:?}", names.key); - return Err(Error::UnknownKey); + return Err(Error::UnknownKey.into()); }; // Look up the key we'll be using to sign the exchange hash @@ -214,12 +275,18 @@ impl Kex for ServerKex { // Hash signature debug!("signing with key {:?}", key); let signature = sign_with_hash_alg( - &PrivateKeyWithHashAlg::new(Arc::new(key.clone()), signature_hash_alg)?, + &PrivateKeyWithHashAlg::new(Arc::new(key.clone()), signature_hash_alg) + .map_err(Into::into)?, &hash, - )?; + ) + .map_err(Into::into)?; output.packet(|w| { - msg::KEX_ECDH_REPLY.encode(w)?; + match kex.is_dh_gex() { + true => &msg::KEX_DH_GEX_REPLY, + false => &msg::KEX_ECDH_REPLY, + } + .encode(w)?; key.public_key().to_bytes()?.encode(w)?; exchange.server_ephemeral.encode(w)?; signature.encode(w)?; @@ -250,7 +317,7 @@ impl Kex for ServerKex { } ServerKexState::WaitingForNewKeys { newkeys } => { let Some(input) = input else { - return Err(Error::KexInit); + return Err(Error::KexInit.into()); }; if input.buffer.first() != Some(&msg::NEWKEYS) { @@ -258,7 +325,7 @@ impl Kex for ServerKex { "Unexpected kex message at this stage: {:?}", input.buffer.first() ); - return Err(Error::Kex); + return Err(Error::Kex.into()); } debug!("new keys received"); diff --git a/russh/src/server/mod.rs b/russh/src/server/mod.rs index 2379f329..0ed8e9a0 100644 --- a/russh/src/server/mod.rs +++ b/russh/src/server/mod.rs @@ -37,6 +37,7 @@ use std::task::{Context, Poll}; use async_trait::async_trait; use bytes::Bytes; +use client::GexParams; use futures::future::Future; use log::{debug, error, info, warn}; use msg::{is_kex_msg, validate_client_msg_strict_kex}; @@ -49,7 +50,8 @@ use tokio::net::{TcpListener, ToSocketAddrs}; use tokio::pin; use crate::cipher::{clear, OpeningKey}; -use crate::kex::{Kex, KexProgress, SessionKexState}; +use crate::kex::dh::groups::DhGroup; +use crate::kex::{KexProgress, SessionKexState}; use crate::session::*; use crate::ssh_read::*; use crate::sshbuffer::*; @@ -727,6 +729,24 @@ pub trait Handler: Sized { ) -> Result { Ok(false) } + + /// Implement when enabling the `diffie-hellman-group-exchange-*` key exchange methods. + /// Should return a Diffie-Hellman group with a safe prime whose length is + /// between `gex_params.min_group_size` and `gex_params.max_group_size` and + /// (if possible) over and as close as possible to `gex_params.preferred_group_size`. + /// + /// OpenSSH uses a pre-generated database of safe primes stored in `/etc/ssh/moduli` + /// + /// The default implementation returns `None`, aborting the key exchange + /// + /// See https://datatracker.ietf.org/doc/html/rfc4419#section-3 + #[allow(unused_variables)] + async fn lookup_dh_gex_group( + &mut self, + gex_params: &GexParams, + ) -> Result, Self::Error> { + Ok(None) + } } #[async_trait] @@ -966,7 +986,9 @@ async fn reply( if is_kex_msg { if let SessionKexState::InProgress(kex) = session.kex.take() { - let progress = kex.step(Some(pkt), &mut session.common.packet_writer)?; + let progress = kex + .step(Some(pkt), &mut session.common.packet_writer, handler) + .await?; match progress { KexProgress::NeedsReply { kex, reset_seqn } => { diff --git a/russh/src/server/session.rs b/russh/src/server/session.rs index c9b43e5f..7c833a27 100644 --- a/russh/src/server/session.rs +++ b/russh/src/server/session.rs @@ -13,7 +13,7 @@ use tokio::sync::oneshot; use super::*; use crate::channels::{Channel, ChannelMsg, ChannelRef}; -use crate::kex::{Kex, KexCause, SessionKexState, EXTENSION_SUPPORT_AS_CLIENT}; +use crate::kex::{KexCause, SessionKexState, EXTENSION_SUPPORT_AS_CLIENT}; use crate::msg; /// A connected server session. This type is unique to a client. From 1fddb083296bc19c7ebb1985199c3f5da59eafc9 Mon Sep 17 00:00:00 2001 From: Eugene Date: Mon, 6 Jan 2025 01:54:29 +0100 Subject: [PATCH 4/6] cleanup --- russh/examples/echoserver.rs | 3 +-- russh/src/kex/dh/mod.rs | 10 +++------- russh/src/kex/mod.rs | 8 ++------ russh/src/msg.rs | 36 ++++++++++++++++++++++++------------ russh/src/server/kex.rs | 4 ++-- 5 files changed, 32 insertions(+), 29 deletions(-) diff --git a/russh/examples/echoserver.rs b/russh/examples/echoserver.rs index 1642a4ef..fd2b145a 100644 --- a/russh/examples/echoserver.rs +++ b/russh/examples/echoserver.rs @@ -3,9 +3,9 @@ use std::collections::HashMap; use std::sync::Arc; use async_trait::async_trait; -use russh::client::GexParams; use kex::dh::groups::DhGroup; use rand_core::OsRng; +use russh::client::GexParams; use russh::keys::*; use russh::server::{Msg, Server as _, Session}; use russh::*; @@ -143,7 +143,6 @@ impl server::Handler for Server { ) -> Result, Self::Error> { Ok(Some(russh::kex::dh::groups::DH_GROUP16)) } - } impl Drop for Server { diff --git a/russh/src/kex/dh/mod.rs b/russh/src/kex/dh/mod.rs index bbd9b975..3ea6a365 100644 --- a/russh/src/kex/dh/mod.rs +++ b/russh/src/kex/dh/mod.rs @@ -255,8 +255,8 @@ impl KexAlgorithmImplementor for DhGroupKex { if let Some((gex_params, dh_group)) = &exchange.gex { gex_params.encode(buffer)?; - biguint_to_mpint(&BigUint::from_bytes_be(&*dh_group.prime)).encode(buffer)?; - biguint_to_mpint(&BigUint::from_bytes_be(&*dh_group.generator)).encode(buffer)?; + biguint_to_mpint(&BigUint::from_bytes_be(&dh_group.prime)).encode(buffer)?; + biguint_to_mpint(&BigUint::from_bytes_be(&dh_group.generator)).encode(buffer)?; } exchange.client_ephemeral.encode(buffer)?; @@ -313,11 +313,7 @@ impl Decode for GexParams { let min_group_size = u32::decode(reader)? as usize; let preferred_group_size = u32::decode(reader)? as usize; let max_group_size = u32::decode(reader)? as usize; - Ok(GexParams::new( - min_group_size, - preferred_group_size, - max_group_size, - )?) + GexParams::new(min_group_size, preferred_group_size, max_group_size) } type Error = Error; diff --git a/russh/src/kex/mod.rs b/russh/src/kex/mod.rs index dfb5427c..7d2a66fb 100644 --- a/russh/src/kex/mod.rs +++ b/russh/src/kex/mod.rs @@ -328,12 +328,8 @@ pub(crate) fn compute_keys( is_server: bool, ) -> Result { let cipher = CIPHERS.get(&cipher).ok_or(Error::UnknownAlgo)?; - let remote_to_local_mac = MACS - .get(&remote_to_local_mac) - .ok_or(Error::UnknownAlgo)?; - let local_to_remote_mac = MACS - .get(&local_to_remote_mac) - .ok_or(Error::UnknownAlgo)?; + let remote_to_local_mac = MACS.get(&remote_to_local_mac).ok_or(Error::UnknownAlgo)?; + let local_to_remote_mac = MACS.get(&local_to_remote_mac).ok_or(Error::UnknownAlgo)?; // https://tools.ietf.org/html/rfc4253#section-7.2 BUFFER.with(|buffer| { diff --git a/russh/src/msg.rs b/russh/src/msg.rs index 717661c1..cfc5bc1b 100644 --- a/russh/src/msg.rs +++ b/russh/src/msg.rs @@ -97,9 +97,9 @@ fn validate_msg_strict_kex_alt_order(msg_type: u8, seqno: usize, orders: &[&[u8] let mut valid = None; // did not match yet for order in orders { let result = validate_msg_strict_kex(msg_type, seqno, order); - valid = match (valid, result) { + valid = match (valid, result) { // If we matched a valid msg, it's now valid forever - (Some(true), _ ) | (_, Some(true)) => Some(true), + (Some(true), _) | (_, Some(true)) => Some(true), // If we matched an invalid msg and we didn't find a valid one yet, it's now invalid (None | Some(false), Some(false)) => Some(false), // If the message was beyond the current pattern, no change @@ -110,21 +110,33 @@ fn validate_msg_strict_kex_alt_order(msg_type: u8, seqno: usize, orders: &[&[u8] } pub(crate) fn validate_client_msg_strict_kex(msg_type: u8, seqno: usize) -> Result<(), Error> { - if Some(false) == validate_msg_strict_kex_alt_order(msg_type, seqno, &[ - &[KEXINIT, KEX_ECDH_INIT, NEWKEYS], - &[KEXINIT, KEX_DH_GEX_REQUEST, KEX_DH_GEX_INIT, NEWKEYS], - ]) { - return Err(strict_kex_violation(msg_type, seqno)) + if Some(false) + == validate_msg_strict_kex_alt_order( + msg_type, + seqno, + &[ + &[KEXINIT, KEX_ECDH_INIT, NEWKEYS], + &[KEXINIT, KEX_DH_GEX_REQUEST, KEX_DH_GEX_INIT, NEWKEYS], + ], + ) + { + return Err(strict_kex_violation(msg_type, seqno)); } Ok(()) } pub(crate) fn validate_server_msg_strict_kex(msg_type: u8, seqno: usize) -> Result<(), Error> { - if Some(false) == validate_msg_strict_kex_alt_order(msg_type, seqno, &[ - &[KEXINIT, KEX_ECDH_REPLY, NEWKEYS], - &[KEXINIT, KEX_DH_GEX_GROUP, KEX_DH_GEX_REPLY, NEWKEYS] - ]) { - return Err(strict_kex_violation(msg_type, seqno)) + if Some(false) + == validate_msg_strict_kex_alt_order( + msg_type, + seqno, + &[ + &[KEXINIT, KEX_ECDH_REPLY, NEWKEYS], + &[KEXINIT, KEX_DH_GEX_GROUP, KEX_DH_GEX_REPLY, NEWKEYS], + ], + ) + { + return Err(strict_kex_violation(msg_type, seqno)); } Ok(()) } diff --git a/russh/src/server/kex.rs b/russh/src/server/kex.rs index e7e0cd46..efd82716 100644 --- a/russh/src/server/kex.rs +++ b/russh/src/server/kex.rs @@ -180,8 +180,8 @@ impl ServerKex { return Err(Error::Kex)?; }; - let prime = biguint_to_mpint(&BigUint::from_bytes_be(&*dh_group.prime)); - let generator = biguint_to_mpint(&BigUint::from_bytes_be(&*dh_group.generator)); + let prime = biguint_to_mpint(&BigUint::from_bytes_be(&dh_group.prime)); + let generator = biguint_to_mpint(&BigUint::from_bytes_be(&dh_group.generator)); dbg!(&prime); dbg!(&generator); From 4c753ce3583453fb7601856e452a089b00d81908 Mon Sep 17 00:00:00 2001 From: Eugene Date: Mon, 6 Jan 2025 02:17:36 +0100 Subject: [PATCH 5/6] fixes --- Cargo.toml | 2 +- russh/examples/echoserver.rs | 12 +----------- russh/src/client/kex.rs | 22 +++++++++------------- russh/src/kex/dh/groups.rs | 11 +++++++++++ russh/src/kex/mod.rs | 4 ++-- russh/src/server/kex.rs | 3 --- russh/src/server/mod.rs | 30 ++++++++++++++++++++++++++---- 7 files changed, 50 insertions(+), 34 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 29d95b8b..9e9e9ce3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,7 +29,7 @@ signature = "2.2" ssh-encoding = { version = "0.2", features = [ "bytes", ] } -ssh-key = { version = "=0.6.8+upstream-0.6.7", features = [ +ssh-key = { version = "=0.6.8", features = [ "ed25519", "rsa", "rsa-sha1", diff --git a/russh/examples/echoserver.rs b/russh/examples/echoserver.rs index fd2b145a..cbb6a962 100644 --- a/russh/examples/echoserver.rs +++ b/russh/examples/echoserver.rs @@ -1,11 +1,8 @@ -use std::borrow::Cow; use std::collections::HashMap; use std::sync::Arc; use async_trait::async_trait; -use kex::dh::groups::DhGroup; use rand_core::OsRng; -use russh::client::GexParams; use russh::keys::*; use russh::server::{Msg, Server as _, Session}; use russh::*; @@ -26,7 +23,7 @@ async fn main() { russh_keys::PrivateKey::random(&mut OsRng, russh_keys::Algorithm::Ed25519).unwrap(), ], preferred: Preferred { - kex: Cow::Owned(vec![russh::kex::DH_GEX_SHA256]), + // kex: std::borrow::Cow::Owned(vec![russh::kex::DH_GEX_SHA256]), ..Preferred::default() }, ..Default::default() @@ -136,13 +133,6 @@ impl server::Handler for Server { }); Ok(true) } - - async fn lookup_dh_gex_group( - &mut self, - gex_params: &GexParams, - ) -> Result, Self::Error> { - Ok(Some(russh::kex::dh::groups::DH_GROUP16)) - } } impl Drop for Server { diff --git a/russh/src/client/kex.rs b/russh/src/client/kex.rs index ea3545b3..3157cd61 100644 --- a/russh/src/client/kex.rs +++ b/russh/src/client/kex.rs @@ -156,7 +156,6 @@ impl Kex for ClientKex { } if kex.is_dh_gex() { - // TODO values output.packet(|w| { kex.client_dh_gex_init(&self.config.gex, w)?; Ok(()) @@ -196,25 +195,22 @@ impl Kex for ClientKex { let prime = Mpint::decode(&mut r)?; let gen = Mpint::decode(&mut r)?; debug!("received gex group: prime={}, gen={}", prime, gen); - //todo validate group - let Some(prime_bit_length) = prime.as_positive_bytes().map(|x| x.len() * 8) else { - warn!("negative DH prime received"); - return Err(Error::KexInit); + let group = DhGroup { + prime: prime.as_bytes().to_vec().into(), + generator: gen.as_bytes().to_vec().into(), }; - if prime_bit_length < self.config.gex.min_group_size - || prime_bit_length > self.config.gex.max_group_size + if group.bit_size() < self.config.gex.min_group_size + || group.bit_size() > self.config.gex.max_group_size { - warn!("DH prime size ({prime_bit_length} bits) not within acceptable range"); + warn!( + "DH prime size ({} bits) not within requested range", + group.bit_size() + ); return Err(Error::KexInit); } - let group = DhGroup { - prime: prime.as_bytes().to_vec().into(), - generator: gen.as_bytes().to_vec().into(), - }; - let exchange = &mut self.exchange; exchange.gex = Some((self.config.gex.clone(), group.clone())); kex.dh_gex_set_group(group)?; diff --git a/russh/src/kex/dh/groups.rs b/russh/src/kex/dh/groups.rs index df50ba1d..409af0e6 100644 --- a/russh/src/kex/dh/groups.rs +++ b/russh/src/kex/dh/groups.rs @@ -41,6 +41,15 @@ pub struct DhGroup { // pub(crate) exp_size: u64, } +impl DhGroup { + pub fn bit_size(&self) -> usize { + let Some(fsb_idx) = self.prime.deref().iter().position(|&x| x != 0) else { + return 0; + }; + (self.prime.deref().len() - fsb_idx) * 8 + } +} + impl Debug for DhGroup { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("DhGroup") @@ -184,3 +193,5 @@ impl DH { public_key > &one && public_key < &prime_minus_one } } + +pub(crate) const BUILTIN_SAFE_DH_GROUPS: &[&DhGroup] = &[&DH_GROUP14, &DH_GROUP16]; diff --git a/russh/src/kex/mod.rs b/russh/src/kex/mod.rs index 7d2a66fb..50a2f998 100644 --- a/russh/src/kex/mod.rs +++ b/russh/src/kex/mod.rs @@ -236,9 +236,9 @@ impl TryFrom<&str> for Name { pub const CURVE25519: Name = Name("curve25519-sha256"); /// `curve25519-sha256@libssh.org` pub const CURVE25519_PRE_RFC_8731: Name = Name("curve25519-sha256@libssh.org"); -/// `diffie-hellman-group-exchange-sha1` +/// `diffie-hellman-group-exchange-sha1`. pub const DH_GEX_SHA1: Name = Name("diffie-hellman-group-exchange-sha1"); -/// `diffie-hellman-group-exchange-sha256` +/// `diffie-hellman-group-exchange-sha256`. pub const DH_GEX_SHA256: Name = Name("diffie-hellman-group-exchange-sha256"); /// `diffie-hellman-group1-sha1` pub const DH_G1_SHA1: Name = Name("diffie-hellman-group1-sha1"); diff --git a/russh/src/server/kex.rs b/russh/src/server/kex.rs index efd82716..defe961c 100644 --- a/russh/src/server/kex.rs +++ b/russh/src/server/kex.rs @@ -183,9 +183,6 @@ impl ServerKex { let prime = biguint_to_mpint(&BigUint::from_bytes_be(&dh_group.prime)); let generator = biguint_to_mpint(&BigUint::from_bytes_be(&dh_group.generator)); - dbg!(&prime); - dbg!(&generator); - self.exchange.gex = Some((gex_params, dh_group.clone())); kex.dh_gex_set_group(dh_group)?; diff --git a/russh/src/server/mod.rs b/russh/src/server/mod.rs index 0ed8e9a0..0f7415ee 100644 --- a/russh/src/server/mod.rs +++ b/russh/src/server/mod.rs @@ -50,7 +50,7 @@ use tokio::net::{TcpListener, ToSocketAddrs}; use tokio::pin; use crate::cipher::{clear, OpeningKey}; -use crate::kex::dh::groups::DhGroup; +use crate::kex::dh::groups::{DhGroup, BUILTIN_SAFE_DH_GROUPS, DH_GROUP14}; use crate::kex::{KexProgress, SessionKexState}; use crate::session::*; use crate::ssh_read::*; @@ -730,14 +730,16 @@ pub trait Handler: Sized { Ok(false) } - /// Implement when enabling the `diffie-hellman-group-exchange-*` key exchange methods. + /// Override when enabling the `diffie-hellman-group-exchange-*` key exchange methods. /// Should return a Diffie-Hellman group with a safe prime whose length is /// between `gex_params.min_group_size` and `gex_params.max_group_size` and /// (if possible) over and as close as possible to `gex_params.preferred_group_size`. /// /// OpenSSH uses a pre-generated database of safe primes stored in `/etc/ssh/moduli` /// - /// The default implementation returns `None`, aborting the key exchange + /// The default implementation picks a group from a very short static list + /// of built-in standard groups and is not really taking advantage of the security + /// offered by these kex methods. /// /// See https://datatracker.ietf.org/doc/html/rfc4419#section-3 #[allow(unused_variables)] @@ -745,7 +747,27 @@ pub trait Handler: Sized { &mut self, gex_params: &GexParams, ) -> Result, Self::Error> { - Ok(None) + let mut best_group = &DH_GROUP14; + + // Find _some_ matching group + for group in BUILTIN_SAFE_DH_GROUPS.iter() { + if group.bit_size() >= gex_params.min_group_size() + && group.bit_size() <= gex_params.max_group_size() + { + best_group = *group; + break; + } + } + + // Find _closest_ matching group + for group in BUILTIN_SAFE_DH_GROUPS.iter() { + if group.bit_size() > gex_params.preferred_group_size() { + best_group = *group; + break; + } + } + + Ok(Some(best_group.clone())) } } From 81bdb8dbc476dd1b6bc99344a7271801383472f3 Mon Sep 17 00:00:00 2001 From: Eugene Date: Mon, 6 Jan 2025 02:21:13 +0100 Subject: [PATCH 6/6] removed kex trait --- russh/src/client/kex.rs | 8 +++----- russh/src/client/mod.rs | 2 +- russh/src/kex/mod.rs | 14 -------------- 3 files changed, 4 insertions(+), 20 deletions(-) diff --git a/russh/src/client/kex.rs b/russh/src/client/kex.rs index 3157cd61..24627491 100644 --- a/russh/src/client/kex.rs +++ b/russh/src/client/kex.rs @@ -14,7 +14,7 @@ use ssh_key::{Mpint, PublicKey, Signature}; use super::IncomingSshPacket; use crate::client::{Config, NewKeys}; use crate::kex::dh::groups::DhGroup; -use crate::kex::{Kex, KexAlgorithm, KexAlgorithmImplementor, KexCause, KexProgress, KEXES}; +use crate::kex::{KexAlgorithm, KexAlgorithmImplementor, KexCause, KexProgress, KEXES}; use crate::negotiation::{Names, Select}; use crate::session::Exchange; use crate::sshbuffer::PacketWriter; @@ -87,17 +87,15 @@ impl ClientKex { state: ClientKexState::Created, } } -} -impl Kex for ClientKex { - fn kexinit(&mut self, output: &mut PacketWriter) -> Result<(), Error> { + pub fn kexinit(&mut self, output: &mut PacketWriter) -> Result<(), Error> { self.exchange.client_kex_init = negotiation::write_kex(&self.config.preferred, output, None)?; Ok(()) } - fn step( + pub fn step( mut self, input: Option<&mut IncomingSshPacket>, output: &mut PacketWriter, diff --git a/russh/src/client/mod.rs b/russh/src/client/mod.rs index 4b9bcd62..63167613 100644 --- a/russh/src/client/mod.rs +++ b/russh/src/client/mod.rs @@ -59,7 +59,7 @@ use tokio::sync::oneshot; use crate::channels::{Channel, ChannelMsg, ChannelRef, WindowSizeRef}; use crate::cipher::{self, clear, OpeningKey}; -use crate::kex::{Kex, KexCause, KexProgress, SessionKexState}; +use crate::kex::{KexCause, KexProgress, SessionKexState}; use crate::msg::{is_kex_msg, validate_server_msg_strict_kex}; use crate::session::{CommonSession, EncryptedState, GlobalRequestResponse, NewKeys}; use crate::ssh_read::SshRead; diff --git a/russh/src/kex/mod.rs b/russh/src/kex/mod.rs index 50a2f998..96fd4aab 100644 --- a/russh/src/kex/mod.rs +++ b/russh/src/kex/mod.rs @@ -49,7 +49,6 @@ use crate::client::GexParams; use crate::mac::{self, MACS}; use crate::negotiation::Names; use crate::session::{Exchange, NewKeys}; -use crate::sshbuffer::{IncomingSshPacket, PacketWriter}; use crate::{cipher, CryptoVec, Error}; #[derive(Debug)] @@ -125,19 +124,6 @@ pub(crate) enum KexProgress { }, } -pub(crate) trait Kex -where - Self: Sized, -{ - fn kexinit(&mut self, output: &mut PacketWriter) -> Result<(), Error>; - - fn step( - self, - input: Option<&mut IncomingSshPacket>, - output: &mut PacketWriter, - ) -> Result, Error>; -} - #[enum_dispatch(KexAlgorithmImplementor)] pub(crate) enum KexAlgorithm { DhGroupKexSha1(dh::DhGroupKex),