diff --git a/Cargo.lock b/Cargo.lock index cd82e46296f2acdf65bff6cc3c01c0ae05d419ae..3b8a0f9cb1312e056227b1e2bcc6935118a81005 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2860,7 +2860,6 @@ dependencies = [ name = "sunset-demo-embassy-common" version = "0.1.0" dependencies = [ - "anyhow", "bcrypt", "defmt", "ed25519-dalek 2.0.0-rc.3", @@ -2878,7 +2877,6 @@ dependencies = [ "sunset", "sunset-embassy", "sunset-sshwire-derive", - "tokio", ] [[package]] diff --git a/embassy/src/embassy_sunset.rs b/embassy/src/embassy_sunset.rs index f9806f1e73d81063ee46c8e9ee1d16272403bf8a..f06dce788001e845f8971691ad53ba82655d604b 100644 --- a/embassy/src/embassy_sunset.rs +++ b/embassy/src/embassy_sunset.rs @@ -173,6 +173,8 @@ impl<'a, C: CliBehaviour, S: ServBehaviour> EmbassySunset<'a, C, S> { } Ok::<_, sunset::Error>(()) }; + + // TODO: if RX fails (bad decrypt etc) it doesn't cancel prog, so gets stuck let rx = select(rx, rx_stop.wait()); let rx = async { let r = rx.await; diff --git a/src/conn.rs b/src/conn.rs index ab2c401c39d2e24949478ce5d8c7075070b12e4c..7f52e6e8178352aec99b946e695218ca9b2067c3 100644 --- a/src/conn.rs +++ b/src/conn.rs @@ -15,7 +15,6 @@ use heapless::Vec; use crate::*; use sshnames::*; use client::Client; -use encrypt::KeyState; use packets::{Packet,ParseContext}; use server::Server; use traffic::TrafSend; diff --git a/src/encrypt.rs b/src/encrypt.rs index 8d9c0949c00fbe23cf5cdeac2b28e9646277945c..fb1c6334ce03a87f14e0216d01df5f3be93a0c43 100644 --- a/src/encrypt.rs +++ b/src/encrypt.rs @@ -45,9 +45,12 @@ const MAX_KEY_LEN: usize = 64; #[derive(Debug)] pub(crate) struct KeyState { keys: Keys, - // Packet sequence numbers. These don't reset with rekeying. + // Packet sequence numbers. + // These reset on newkeys when strict kex is in effect. pub seq_encrypt: Wrapping<u32>, pub seq_decrypt: Wrapping<u32>, + strict_kex: bool, + done_first_kex: bool, } impl KeyState { @@ -57,6 +60,8 @@ impl KeyState { keys: Keys::new_cleartext(), seq_encrypt: Wrapping(0), seq_decrypt: Wrapping(0), + strict_kex: false, + done_first_kex: false } } @@ -65,10 +70,21 @@ impl KeyState { || matches!(self.keys.dec, DecKey::NoCipher) } - /// Updates with new keys, keeping the same sequence numbers + /// Updates with new keys pub fn rekey(&mut self, keys: Keys) { trace!("rekey"); - self.keys = keys + self.keys = keys; + self.done_first_kex = true; + if self.strict_kex { + self.seq_decrypt = Wrapping(0); + self.seq_encrypt = Wrapping(0); + } + } + + pub fn enable_strict_kex(&mut self) { + if !self.done_first_kex { + self.strict_kex = true + } } pub fn recv_seq(&self) -> u32 { diff --git a/src/kex.rs b/src/kex.rs index 9ae87e4f3113f9ab85fda0989bb39e2a84cc800a..877c99e5870e1cb2242c18444d1c68a89d8659f1 100644 --- a/src/kex.rs +++ b/src/kex.rs @@ -37,7 +37,8 @@ const fixed_options_kex: &[&str] = /// Options that can't be negotiated const marker_only_kexs: &[&str] = - &[SSH_NAME_EXT_INFO_C, SSH_NAME_EXT_INFO_S, SSH_NAME_KEXGUESS2]; + &[SSH_NAME_EXT_INFO_C, SSH_NAME_EXT_INFO_S, SSH_NAME_KEXGUESS2, + SSH_NAME_STRICT_KEX_C, SSH_NAME_STRICT_KEX_S]; const fixed_options_hostsig: &[&str] = &[ SSH_NAME_ED25519, @@ -68,12 +69,14 @@ impl AlgoConfig { // Only clients are interested in ext-info // TODO perhaps it could go behind cfg rsa? if is_client { - // OK unwrap: static arrays are < MAX_LOCAL_NAMES+slack + // OK unwrap: static arrays are <= MAX_LOCAL_NAMES kexs.0.push(SSH_NAME_EXT_INFO_C).unwrap(); - + kexs.0.push(SSH_NAME_STRICT_KEX_C).unwrap(); + } else { + kexs.0.push(SSH_NAME_STRICT_KEX_S).unwrap(); } - // OK unwrap: static arrays are < MAX_LOCAL_NAMES+slack + // OK unwrap: static arrays are <= MAX_LOCAL_NAMES kexs.0.push(SSH_NAME_KEXGUESS2).unwrap(); AlgoConfig { @@ -222,6 +225,10 @@ pub(crate) struct Algos { // whether the remote side supports ext-info pub send_ext_info: bool, + + // whether the remote side supports strict kex. will be ignored + // for non-first KEX + pub strict_kex: bool, } impl fmt::Display for Algos { @@ -387,6 +394,9 @@ impl Kex { // The first KEX's H becomes the persistent sess_id let sess_id = sess_id.get_or_insert(output.h.clone()); let keys = Keys::derive(output, sess_id, &algos)?; + if algos.strict_kex { + s.enable_strict_kex() + } s.rekey(keys); *self = Kex::Idle; Ok(()) @@ -431,6 +441,14 @@ impl Kex { p.kex.has_algo(SSH_NAME_EXT_INFO_C).unwrap() }; + // we always send strict-kex, so just check if the other had it + let other_strict = if is_client { + SSH_NAME_STRICT_KEX_S + } else { + SSH_NAME_STRICT_KEX_C + }; + let strict_kex = p.kex.has_algo(other_strict).unwrap(); + debug!("hostsig {:?} vs {:?}", p.hostsig, conf.hostsig); let hostsig_method = p .hostsig @@ -500,6 +518,7 @@ impl Kex { discard_next, is_client, send_ext_info, + strict_kex, }) } } diff --git a/src/namelist.rs b/src/namelist.rs index 9babd64fb966c90d838732fa7c58f34f8403f8e6..6e4dfcd9cc7cdcd4e744715b8cbff2ff66afcca8 100644 --- a/src/namelist.rs +++ b/src/namelist.rs @@ -20,8 +20,8 @@ use heapless::Vec; // - auth types /// Max count of LocalNames entries /// -/// Current max is for kex, [curve25519, curve25519@libssh, ext-info, kexguess2] -pub const MAX_LOCAL_NAMES: usize = 4; +/// Current max is for kex, [curve25519, curve25519@libssh, ext-info, strictkex, kexguess2] +pub const MAX_LOCAL_NAMES: usize = 5; static EMPTY_LOCALNAMES: LocalNames = LocalNames::new(); /// A comma separated string, can be decoded or encoded. diff --git a/src/sshnames.rs b/src/sshnames.rs index d631a912f93682cabb8b78d6e6f9eeda9abcab68..d86fc5e1762e3717835c0671c6b88386f9b4aa5f 100644 --- a/src/sshnames.rs +++ b/src/sshnames.rs @@ -18,6 +18,12 @@ pub const SSH_NAME_EXT_INFO_C: &str = "ext-info-c"; /// Implemented by Dropbear to improve first_kex_packet_follows, described /// [https://mailarchive.ietf.org/arch/msg/secsh/3n6lNzDHmsGsIQSqhmHHwigIbuo/](https://mailarchive.ietf.org/arch/msg/secsh/3n6lNzDHmsGsIQSqhmHHwigIbuo/) pub const SSH_NAME_KEXGUESS2: &str = "kexguess2@matt.ucc.asn.au"; +/// Strict Kex +/// TODO +pub const SSH_NAME_STRICT_KEX_S: &str = "kex-strict-s-v00@openssh.com"; +/// Strict Kex +/// TODO +pub const SSH_NAME_STRICT_KEX_C: &str = "kex-strict-c-v00@openssh.com"; /// [RFC8709](https://tools.ietf.org/html/rfc8709) pub const SSH_NAME_ED25519: &str = "ssh-ed25519"; diff --git a/src/traffic.rs b/src/traffic.rs index 5261d41e82c821d20424772a3fbe062d2ad6e016..d691ac5615b25b585b3ee47138fa5f1d960dabf6 100644 --- a/src/traffic.rs +++ b/src/traffic.rs @@ -437,11 +437,14 @@ impl<'s, 'a> TrafSend<'s, 'a> { self.out.send_packet(p.into(), self.keys) } - pub fn rekey(&mut self, keys: encrypt::Keys) { self.keys.rekey(keys) } + pub fn enable_strict_kex(&mut self) { + self.keys.enable_strict_kex() + } + pub fn send_version(&mut self) -> Result<(), Error> { self.out.send_version() }