From 04b196fc10cf29f686c505e63f216d9be7f87423 Mon Sep 17 00:00:00 2001
From: Matt Johnston <matt@ucc.asn.au>
Date: Fri, 7 Apr 2023 21:40:43 +0800
Subject: [PATCH] Add MSG_EXT_INFO handling for rsa2
Could eventually perhaps go behind a rsa feature
---
async/examples/sunsetc.rs | 6 ++---
async/src/cmdline_client.rs | 1 +
src/cliauth.rs | 18 ++++++++++---
src/client.rs | 2 --
src/conn.rs | 12 ++++++++-
src/kex.rs | 39 +++++++++++++++++++++++----
src/namelist.rs | 4 ++-
src/packets.rs | 53 +++++++++++++++++++++++++++++++++++--
src/sshnames.rs | 3 +++
9 files changed, 121 insertions(+), 17 deletions(-)
diff --git a/async/examples/sunsetc.rs b/async/examples/sunsetc.rs
index fb0171c..b4a36cc 100644
--- a/async/examples/sunsetc.rs
+++ b/async/examples/sunsetc.rs
@@ -179,9 +179,9 @@ fn setup_log(args: &Args) -> Result<()> {
.add_filter_allow_str("sunset")
.add_filter_allow_str("sshclient")
// not debugging these bits of the stack at present
- // .add_filter_ignore_str("sunset::traffic")
- // .add_filter_ignore_str("sunset::runner")
- // .add_filter_ignore_str("sunset_embassy")
+ .add_filter_ignore_str("sunset::traffic")
+ .add_filter_ignore_str("sunset::runner")
+ .add_filter_ignore_str("sunset_embassy")
.set_time_offset_to_local().expect("Couldn't get local timezone")
.build();
diff --git a/async/src/cmdline_client.rs b/async/src/cmdline_client.rs
index ecc54d9..a5c08af 100644
--- a/async/src/cmdline_client.rs
+++ b/async/src/cmdline_client.rs
@@ -271,6 +271,7 @@ impl<'a> CmdlineRunner<'a> {
/// Runs the `CmdlineClient` session. Requests a shell or command, performs
/// channel IO.
pub async fn run(&mut self, cli: &'a SSHClient<'a, CmdlineHooks<'a>>) -> Result<()> {
+ // chanio is only set once a channel is opened below
let chanio = Fuse::terminated();
pin_mut!(chanio);
diff --git a/src/cliauth.rs b/src/cliauth.rs
index 9d453a4..9ac2435 100644
--- a/src/cliauth.rs
+++ b/src/cliauth.rs
@@ -74,12 +74,15 @@ pub(crate) struct CliAuth {
username: ResponseString,
- // Starts as true, set to false if hook.auth_password() returns None.
- // Not set false if the server rejects auth.
+ /// Starts as true, set to false if hook.auth_password() returns None.
+ /// Not set false if the server rejects auth.
try_password: bool,
- // Set to false if hook.next_authkey() returns None.
+ /// Set to false if hook.next_authkey() returns None.
try_pubkey: bool,
+
+ /// Set once we are OKed from MSG_EXT_INFO
+ allow_rsa_sha2: bool,
}
impl CliAuth {
@@ -90,6 +93,7 @@ impl CliAuth {
username: ResponseString::new(),
try_password: true,
try_pubkey: true,
+ allow_rsa_sha2: false,
}
}
@@ -308,4 +312,12 @@ impl CliAuth {
// TODO errors
Ok(())
}
+
+ pub fn handle_ext_info(&mut self, p: &packets::ExtInfo<'_>) {
+ if let Some(ref algs) = p.server_sig_algs {
+ // OK unwrap: is a remote namelist
+ self.allow_rsa_sha2 = algs.has_algo(SSH_NAME_RSA_SHA256).unwrap();
+ trace!("setting allow_rsa_sha2 = {}", self.allow_rsa_sha2);
+ }
+ }
}
diff --git a/src/client.rs b/src/client.rs
index 4aaf7ac..d9f02f8 100644
--- a/src/client.rs
+++ b/src/client.rs
@@ -24,8 +24,6 @@ impl Client {
Client { auth: CliAuth::new() }
}
- // pub fn check_hostkey(hostkey: )
-
pub(crate) fn auth_success(
&mut self,
parse_ctx: &mut ParseContext,
diff --git a/src/conn.rs b/src/conn.rs
index a2502a3..e42c82f 100644
--- a/src/conn.rs
+++ b/src/conn.rs
@@ -263,6 +263,12 @@ impl<C: CliBehaviour, S: ServBehaviour> Conn<C, S> {
Packet::NewKeys(_) => {
self.kex.handle_newkeys(&mut self.sess_id, s)?;
}
+ Packet::ExtInfo(p) => {
+ if let ClientServer::Client(cli) = &mut self.cliserv {
+ cli.auth.handle_ext_info(&p);
+ }
+ // could potentially pass it to other handlers too
+ }
Packet::ServiceRequest(p) => {
if let ClientServer::Server(serv) = &mut self.cliserv {
serv.service_request(&p, s)?;
@@ -282,7 +288,11 @@ impl<C: CliBehaviour, S: ServBehaviour> Conn<C, S> {
warn!("Received SSH unimplemented message");
}
Packet::DebugPacket(p) => {
- warn!("SSH debug message from remote host: '{:?}'", p.message);
+ let level = match p.always_display {
+ true => log::Level::Info,
+ false => log::Level::Debug,
+ };
+ log!(level, "SSH debug message from remote host: {}", p.message);
}
Packet::Disconnect(p) => {
// We ignore p.reason.
diff --git a/src/kex.rs b/src/kex.rs
index b79b6ce..7ee639b 100644
--- a/src/kex.rs
+++ b/src/kex.rs
@@ -34,6 +34,10 @@ use pretty_hex::PrettyHex;
const fixed_options_kex: &[&'static str] =
&[SSH_NAME_CURVE25519, SSH_NAME_CURVE25519_LIBSSH];
+/// Options that can't be negotiated
+const marker_only_kexs: &[&'static str] =
+ &[SSH_NAME_EXT_INFO_C, SSH_NAME_EXT_INFO_S, SSH_NAME_KEXGUESS2];
+
const fixed_options_hostsig: &[&'static str] = &[
SSH_NAME_ED25519,
#[cfg(rsa)]
@@ -56,10 +60,20 @@ pub(crate) struct AlgoConfig {
impl AlgoConfig {
/// Creates the standard algorithm configuration
/// TODO: ext-info-s and ext-info-c
- pub fn new(_is_client: bool) -> Self {
- AlgoConfig {
+ pub fn new(is_client: bool) -> Self {
+ // OK unwrap: static arrays are < MAX_LOCAL_NAMES
+ let mut kexs: LocalNames = fixed_options_kex.try_into().unwrap();
+
+ // 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
- kexs: fixed_options_kex.try_into().unwrap(),
+ kexs.0.push(SSH_NAME_EXT_INFO_C).unwrap();
+
+ }
+
+ AlgoConfig {
+ kexs,
hostsig: fixed_options_hostsig.try_into().unwrap(),
ciphers: fixed_options_cipher.try_into().unwrap(),
macs: fixed_options_mac.try_into().unwrap(),
@@ -201,6 +215,9 @@ pub(crate) struct Algos {
// avoid having to keep passing it separately, though this
// is global state.
pub is_client: bool,
+
+ // whether the remote side supports ext-info
+ pub ext_info: bool,
}
impl fmt::Display for Algos {
@@ -367,10 +384,13 @@ impl Kex {
.kex
.first_match(is_client, &conf.kexs)?
.ok_or(Error::AlgoNoMatch { algo: "kex" })?;
- if kex_method == SSH_NAME_KEXGUESS2 {
- trace!("kexguess2 was negotiated, returning AlgoNoMatch");
+
+ // Certain kex method names aren't actual algorithms, just markers.
+ // If they are negotiated it means no valid method matched
+ if marker_only_kexs.contains(&kex_method) {
return Err(Error::AlgoNoMatch { algo: "kex" });
}
+
let kex = SharedSecret::from_name(kex_method)?;
let goodguess_kex = if kexguess2 {
p.kex.first() == kex_method
@@ -378,6 +398,14 @@ impl Kex {
p.kex.first() == conf.kexs.first()
};
+ // we only send MSG_EXT_INFO to a client, don't look
+ // for SSH_NAME_EXT_INFO_S
+ let ext_info = match is_client {
+ true => false,
+ // OK unwrap: p.kex is a remote list
+ false => p.kex.has_algo(SSH_NAME_EXT_INFO_C).unwrap(),
+ };
+
let hostsig_method = p
.hostsig
.first_match(is_client, &conf.hostsig)?
@@ -445,6 +473,7 @@ impl Kex {
integ_dec,
discard_next,
is_client,
+ ext_info,
})
}
}
diff --git a/src/namelist.rs b/src/namelist.rs
index 49460c4..318eeda 100644
--- a/src/namelist.rs
+++ b/src/namelist.rs
@@ -72,7 +72,6 @@ impl SSHEncode for &LocalNames {
}
}
-// for tests
impl<'a> TryFrom<&'a str> for StringNames<'a> {
type Error = ();
fn try_from(s: &'a str) -> Result<Self, Self::Error> {
@@ -86,6 +85,7 @@ impl<'a> TryFrom<&'a str> for NameList<'a> {
}
}
+// for tests
impl TryFrom<&[&'static str]> for LocalNames {
type Error = ();
fn try_from(s: &[&'static str]) -> Result<Self, ()> {
@@ -123,6 +123,8 @@ impl NameList<'_> {
}
/// Returns whether the `algo` is contained in this list
+ ///
+ /// Fails iff given a Local variant
pub fn has_algo(&self, algo: &str) -> Result<bool> {
match self {
NameList::String(s) => Ok(s.has_algo(algo)),
diff --git a/src/packets.rs b/src/packets.rs
index c4f58b2..b027891 100644
--- a/src/packets.rs
+++ b/src/packets.rs
@@ -97,6 +97,55 @@ pub struct ServiceAccept<'a> {
pub name: &'a str,
}
+/// MSG_EXT_INFO
+///
+/// `ExtInfo` differs from most packet structs, it only tracks known extension types
+/// rather than an unknown-sized list.
+#[derive(Debug)]
+pub struct ExtInfo<'a> {
+ // Wire format is
+ // byte SSH_MSG_EXT_INFO (value 7)
+ // uint32 nr-extensions
+ // repeat the following 2 fields "nr-extensions" times:
+ // string extension-name
+ // string extension-value (binary)
+
+ pub server_sig_algs: Option<NameList<'a>>,
+}
+
+impl<'de: 'a, 'a> SSHDecode<'de> for ExtInfo<'a> {
+ fn dec<S>(s: &mut S) -> WireResult<Self> where S: SSHSource<'de> {
+ let mut server_sig_algs = None;
+ let num = u32::dec(s)?;
+ for _ in 0..num {
+ let ext: &str = SSHDecode::dec(s)?;
+ match ext {
+ SSH_EXT_SERVER_SIG_ALGS => {
+ server_sig_algs = Some(SSHDecode::dec(s)?);
+ },
+ _ => {
+ // skip over
+ let _: BinString = SSHDecode::dec(s)?;
+ },
+ }
+ }
+ Ok(Self {
+ server_sig_algs
+ })
+ }
+}
+
+impl SSHEncode for ExtInfo<'_> {
+ fn enc<S>(&self, s: &mut S) -> WireResult<()> where S: SSHSink {
+ if let Some(ref algs) = self.server_sig_algs {
+ 1u32.enc(s)?;
+ SSH_EXT_SERVER_SIG_ALGS.enc(s)?;
+ algs.enc(s)?;
+ }
+ Ok(())
+ }
+}
+
#[derive(Debug, SSHEncode, SSHDecode)]
pub struct UserauthRequest<'a> {
pub username: TextString<'a>,
@@ -436,7 +485,7 @@ pub enum RequestSuccess {
}
impl<'de> SSHDecode<'de> for RequestSuccess {
- fn dec<S>(s: &mut S) -> WireResult<Self> where S: SSHSource<'de> {
+ fn dec<S>(_s: &mut S) -> WireResult<Self> where S: SSHSource<'de> {
// if s.ctx().last_req_port {
// Ok(Self::TcpPort(TcpPort::dec(s)?))
// } else {
@@ -852,7 +901,7 @@ messagetypes
pub const SSH_EXTENDED_DATA_STDERR: u32 = 1;
+/// [RFC8308](https://tools.ietf.org/html/rfc8308) Extension Negotiation
+pub const SSH_EXT_SERVER_SIG_ALGS: &str = "server-sig-algs";
+
/// [RFC4254](https://tools.ietf.org/html/rfc4254)
#[allow(non_camel_case_types)]
#[derive(Debug)]
--
GitLab