From b2fddd99f90e767688dedaef638aa3d3c02dad8b Mon Sep 17 00:00:00 2001
From: Matt Johnston <matt@ucc.asn.au>
Date: Fri, 8 Dec 2023 23:13:14 +0800
Subject: [PATCH] Reset sequence numbers when in strict mode
---
Cargo.lock | 2 --
embassy/src/embassy_sunset.rs | 2 ++
src/conn.rs | 1 -
src/encrypt.rs | 22 +++++++++++++++++++---
src/kex.rs | 27 +++++++++++++++++++++++----
src/namelist.rs | 4 ++--
src/sshnames.rs | 6 ++++++
src/traffic.rs | 5 ++++-
8 files changed, 56 insertions(+), 13 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
index cd82e46..3b8a0f9 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 f9806f1..f06dce7 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 ab2c401..7f52e6e 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 8d9c094..fb1c633 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 9ae87e4..877c99e 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 9babd64..6e4dfcd 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 d631a91..d86fc5e 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 5261d41..d691ac5 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()
}
--
GitLab