From 2e4647480dfe98e68ee51403d92587f55bdfc976 Mon Sep 17 00:00:00 2001
From: Matt Johnston <matt@ucc.asn.au>
Date: Thu, 22 Sep 2022 23:24:06 +0800
Subject: [PATCH] Convert LocalNames to an owned Vec
No changes yet, but is necessary to configure algorithm lists
---
sshproto/src/conn.rs | 19 +++++++-----
sshproto/src/kex.rs | 64 ++++++++++++++++++++--------------------
sshproto/src/namelist.rs | 62 ++++++++++++++++++++++++++++----------
sshproto/src/runner.rs | 2 +-
sshproto/src/servauth.rs | 16 ++++------
5 files changed, 96 insertions(+), 67 deletions(-)
diff --git a/sshproto/src/conn.rs b/sshproto/src/conn.rs
index 6999aea..fc1e941 100644
--- a/sshproto/src/conn.rs
+++ b/sshproto/src/conn.rs
@@ -20,10 +20,10 @@ use server::Server;
use traffic::TrafSend;
use channel::{Channels, ChanEvent, ChanEventMaker};
use config::MAX_CHANNELS;
-use kex::SessId;
+use kex::{SessId, AlgoConfig};
/// The core state of a SSH instance.
-pub struct Conn<'a> {
+pub struct Conn {
state: ConnState,
/// Next kex to run
@@ -33,7 +33,7 @@ pub struct Conn<'a> {
cliserv: ClientServer,
- algo_conf: kex::AlgoConfig<'a>,
+ algo_conf: AlgoConfig,
parse_ctx: ParseContext,
@@ -98,24 +98,27 @@ pub(crate) enum EventMaker {
pub(crate) struct Dispatched(pub Option<channel::DataIn>);
-impl<'a> Conn<'a> {
+impl Conn {
pub fn new_client() -> Result<Self> {
- Self::new(ClientServer::Client(client::Client::new()))
+ let algo_conf = AlgoConfig::new(true);
+ Self::new(ClientServer::Client(client::Client::new()), algo_conf)
}
pub fn new_server(
b: &mut dyn ServBehaviour,
) -> Result<Self> {
- Self::new(ClientServer::Server(server::Server::new(b)))
+ // XXX
+ let algo_conf = AlgoConfig::new(false);
+ Self::new(ClientServer::Server(server::Server::new(b)), algo_conf)
}
- fn new(cliserv: ClientServer) -> Result<Self, Error> {
+ fn new(cliserv: ClientServer, algo_conf: AlgoConfig) -> Result<Self, Error> {
Ok(Conn {
kex: kex::Kex::new()?,
sess_id: None,
remote_version: ident::RemoteVersion::new(),
state: ConnState::SendIdent,
- algo_conf: kex::AlgoConfig::new(cliserv.is_client()),
+ algo_conf,
cliserv,
channels: Channels::new(),
parse_ctx: ParseContext::new(),
diff --git a/sshproto/src/kex.rs b/sshproto/src/kex.rs
index 27453c5..de746a2 100644
--- a/sshproto/src/kex.rs
+++ b/sshproto/src/kex.rs
@@ -15,7 +15,7 @@ use crate::*;
use encrypt::{Cipher, Integ, Keys};
use ident::RemoteVersion;
use traffic::TrafSend;
-use namelist::LocalNames;
+use namelist::{NameList,LocalNames};
use packets::{Packet, PubKey, Signature};
use sign::SigType;
use sshnames::*;
@@ -29,40 +29,40 @@ pub type SessId = heapless::Vec<u8, MAX_SESSID>;
use pretty_hex::PrettyHex;
-const EMPTY_LOCALNAMES: LocalNames = LocalNames(&[]);
-
// TODO this will be configurable.
-const fixed_options_kex: LocalNames =
- LocalNames(&[SSH_NAME_CURVE25519, SSH_NAME_CURVE25519_LIBSSH]);
-const fixed_options_hostsig: LocalNames = LocalNames(&[
+const fixed_options_kex: &[&'static str] =
+ &[SSH_NAME_CURVE25519, SSH_NAME_CURVE25519_LIBSSH];
+
+const fixed_options_hostsig: &[&'static str] = &[
SSH_NAME_ED25519,
- #[cfg(alloc)]
+ #[cfg(rsa)]
SSH_NAME_RSA_SHA256,
-]);
-
-const fixed_options_cipher: LocalNames =
- LocalNames(&[SSH_NAME_CHAPOLY, SSH_NAME_AES256_CTR]);
-const fixed_options_mac: LocalNames = LocalNames(&[SSH_NAME_HMAC_SHA256]);
-const fixed_options_comp: LocalNames = LocalNames(&[SSH_NAME_NONE]);
-
-pub(crate) struct AlgoConfig<'a> {
- kexs: LocalNames<'a>,
- hostsig: LocalNames<'a>,
- ciphers: LocalNames<'a>,
- macs: LocalNames<'a>,
- comps: LocalNames<'a>,
+];
+
+const fixed_options_cipher: &[&'static str] =
+ &[SSH_NAME_CHAPOLY, SSH_NAME_AES256_CTR];
+const fixed_options_mac: &[&'static str] = &[SSH_NAME_HMAC_SHA256];
+const fixed_options_comp: &[&'static str] = &[SSH_NAME_NONE];
+
+pub(crate) struct AlgoConfig {
+ kexs: LocalNames,
+ hostsig: LocalNames,
+ ciphers: LocalNames,
+ macs: LocalNames,
+ comps: LocalNames,
}
-impl<'a> AlgoConfig<'a> {
+impl AlgoConfig {
/// Creates the standard algorithm configuration
/// TODO: ext-info-s and ext-info-c
pub fn new(_is_client: bool) -> Self {
AlgoConfig {
- kexs: fixed_options_kex,
- hostsig: fixed_options_hostsig,
- ciphers: fixed_options_cipher,
- macs: fixed_options_mac,
- comps: fixed_options_comp,
+ // OK unwrap: static arrays are < MAX_LOCAL_NAMES
+ kexs: fixed_options_kex.try_into().unwrap(),
+ hostsig: fixed_options_hostsig.try_into().unwrap(),
+ ciphers: fixed_options_cipher.try_into().unwrap(),
+ macs: fixed_options_mac.try_into().unwrap(),
+ comps: fixed_options_comp.try_into().unwrap(),
}
}
}
@@ -240,8 +240,8 @@ impl Kex {
mac_s2c: (&conf.macs).into(),
comp_c2s: (&conf.comps).into(),
comp_s2c: (&conf.comps).into(),
- lang_c2s: (&EMPTY_LOCALNAMES).into(),
- lang_s2c: (&EMPTY_LOCALNAMES).into(),
+ lang_c2s: NameList::empty(),
+ lang_s2c: NameList::empty(),
first_follows: false,
reserved: 0,
}.into()
@@ -578,16 +578,16 @@ mod tests {
#[test]
fn test_name_match() {
// check that the from_name() functions are complete
- for k in kex::fixed_options_kex.0.iter() {
+ for k in kex::fixed_options_kex.iter() {
kex::SharedSecret::from_name(k).unwrap();
}
- for k in kex::fixed_options_hostsig.0.iter() {
+ for k in kex::fixed_options_hostsig.iter() {
sign::SigType::from_name(k).unwrap();
}
- for k in kex::fixed_options_cipher.0.iter() {
+ for k in kex::fixed_options_cipher.iter() {
encrypt::Cipher::from_name(k).unwrap();
}
- for k in kex::fixed_options_mac.0.iter() {
+ for k in kex::fixed_options_mac.iter() {
encrypt::Integ::from_name(k).unwrap();
}
}
diff --git a/sshproto/src/namelist.rs b/sshproto/src/namelist.rs
index f459fb8..58ea3a6 100644
--- a/sshproto/src/namelist.rs
+++ b/sshproto/src/namelist.rs
@@ -11,6 +11,15 @@ use sshwire_derive::{SSHEncode, SSHDecode};
use crate::*;
use sshwire::{SSHEncode, SSHDecode, SSHSource, SSHSink, BinString, WireResult};
+use heapless::Vec;
+
+// Used for lists of:
+// - algorithm names
+// - key types
+// - signature types
+// - auth types
+pub const MAX_LOCAL_NAMES: usize = 5;
+static EMPTY_LOCALNAMES: LocalNames = LocalNames::new();
/// A comma separated string, can be decoded or encoded.
/// Used for remote name lists.
@@ -22,14 +31,14 @@ pub struct StringNames<'a>(pub &'a AsciiStr);
/// Deliberately `'static` since it should only come from hardcoded local strings
/// `SSH_NAME_*` in [`crate::sshnames`]. We don't validate string contents.
#[derive(Debug)]
-pub struct LocalNames<'a>(pub &'a [&'static str]);
+pub struct LocalNames(pub Vec<&'static str, MAX_LOCAL_NAMES>);
/// The general form that can store either representation
#[derive(SSHEncode, Debug)]
#[sshwire(no_variant_names)]
pub enum NameList<'a> {
String(StringNames<'a>),
- Local(LocalNames<'a>),
+ Local(&'a LocalNames),
}
impl<'de: 'a, 'a> SSHDecode<'de> for NameList<'a> {
@@ -42,10 +51,10 @@ impl<'de: 'a, 'a> SSHDecode<'de> for NameList<'a> {
}
/// Serialize the list of names with comma separators
-impl SSHEncode for LocalNames<'_> {
+impl SSHEncode for &LocalNames {
fn enc<S>(&self, e: &mut S) -> WireResult<()>
where S: sshwire::SSHSink {
- let names = &self.0;
+ let names = self.0.as_slice();
// space for names and commas
let strlen = names.iter().map(|n| n.len()).sum::<usize>()
+ names.len().saturating_sub(1);
@@ -74,17 +83,19 @@ impl<'a> TryFrom<&'a str> for NameList<'a> {
}
}
-impl<'a> From<&'a [&'static str]> for LocalNames<'a> {
- fn from(s: &'a [&'static str]) -> Self {
- Self(s)
+impl TryFrom<&[&'static str]> for LocalNames {
+ type Error = ();
+ fn try_from(s: &[&'static str]) -> Result<Self, ()> {
+ Ok(Self(Vec::from_slice(s)?))
}
}
-impl<'a> From<&LocalNames<'a>> for NameList<'a> {
- fn from(s: &LocalNames<'a>) -> Self {
- NameList::Local(LocalNames(s.0))
+impl<'a> From<&'a LocalNames> for NameList<'a> {
+ fn from(s: &'a LocalNames) -> Self {
+ NameList::Local(s)
}
}
+
impl<'a> NameList<'a> {
/// Returns the first name in this namelist that matches, based on SSH priority.
/// The SSH client's list (which could be either remote or ours) is used
@@ -123,6 +134,11 @@ impl<'a> NameList<'a> {
NameList::Local(s) => s.first(),
}
}
+
+ /// Returns an empty `Local` variant
+ pub fn empty() -> Self {
+ Self::Local(&EMPTY_LOCALNAMES)
+ }
}
impl<'a> StringNames<'a> {
@@ -160,7 +176,11 @@ impl<'a> StringNames<'a> {
}
}
-impl<'a> LocalNames<'a> {
+impl LocalNames {
+ pub const fn new() -> Self {
+ Self(Vec::new())
+ }
+
pub fn first(&self) -> &str {
if self.0.len() == 0 {
""
@@ -182,9 +202,9 @@ mod tests {
fn test_match() {
let r1 = NameList::String("rho,cog".try_into().unwrap());
let r2 = NameList::String("woe".try_into().unwrap());
- let l1 = LocalNames(&["rho", "cog"]);
- let l2 = LocalNames(&["cog", "rho"]);
- let l3 = LocalNames(&["now", "woe"]);
+ let l1 = LocalNames::try_from(["rho", "cog"].as_slice()).unwrap();
+ let l2 = LocalNames::try_from(["cog", "rho"].as_slice()).unwrap();
+ let l3 = LocalNames::try_from(["now", "woe"].as_slice()).unwrap();
assert_eq!(r1.first_match(true, &l1).unwrap(), Some("rho"));
assert_eq!(r1.first_match(false, &l1).unwrap(), Some("rho"));
assert_eq!(r1.first_match(true, &l2).unwrap(), Some("cog"));
@@ -207,7 +227,8 @@ mod tests {
&[",", ","], // not really valid
];
for t in tests.iter() {
- let n = NameList::Local(LocalNames(t));
+ let n = LocalNames::try_from(*t).unwrap();
+ let n = NameList::Local(&n);
let mut buf = vec![99; 30];
let l = sshwire::write_ssh(&mut buf, &n).unwrap();
buf.truncate(l);
@@ -227,7 +248,8 @@ mod tests {
];
for t in tests.iter() {
- let l = NameList::Local(LocalNames(t));
+ let l = LocalNames::try_from(*t).unwrap();
+ let l = NameList::Local(&l);
let x = t.join(",");
let s: NameList = x.as_str().try_into().unwrap();
assert_eq!(l.first(), s.first());
@@ -256,4 +278,12 @@ mod tests {
assert_eq!(n("zzz,boo", "boo"), true);
assert_eq!(n("zzz,boo", "urp"), false);
}
+
+ #[test]
+ fn localnames_max_size() {
+ let s = vec!["one"; MAX_LOCAL_NAMES + 1];
+ LocalNames::try_from(s.as_slice()).unwrap_err();
+ let s = vec!["one"; MAX_LOCAL_NAMES];
+ LocalNames::try_from(s.as_slice()).unwrap();
+ }
}
diff --git a/sshproto/src/runner.rs b/sshproto/src/runner.rs
index 814d8a3..12a6df0 100644
--- a/sshproto/src/runner.rs
+++ b/sshproto/src/runner.rs
@@ -16,7 +16,7 @@ use conn::{Conn, Dispatched};
use channel::ChanEventMaker;
pub struct Runner<'a> {
- conn: Conn<'a>,
+ conn: Conn,
/// Binary packet handling from the network buffer
traf_in: TrafIn<'a>,
diff --git a/sshproto/src/servauth.rs b/sshproto/src/servauth.rs
index b2c59e6..b7b2f70 100644
--- a/sshproto/src/servauth.rs
+++ b/sshproto/src/servauth.rs
@@ -4,8 +4,6 @@ use {
log::{debug, error, info, log, trace, warn},
};
-use heapless::Vec;
-
use crate::sshnames::*;
use crate::*;
use packets::{AuthMethod, Userauth60, UserauthPkOk};
@@ -88,8 +86,7 @@ impl ServAuth {
Ok(true)
}
AuthResp::Failure => {
- let mut n: Vec<&'static str, NUM_AUTHMETHOD> = Vec::new();
- let methods = self.avail_methods(username, &mut n, b);
+ let methods = self.avail_methods(username, b);
let methods = (&methods).into();
s.send(packets::UserauthFailure { methods, partial: false })?;
@@ -141,18 +138,17 @@ impl ServAuth {
fn avail_methods<'f>(
&self,
user: TextString,
- buf: &'f mut Vec<&'static str, NUM_AUTHMETHOD>,
b: &mut dyn ServBehaviour,
- ) -> namelist::LocalNames<'f> {
- buf.clear();
+ ) -> namelist::LocalNames {
+ let mut l = namelist::LocalNames::new();
// OK unwrap: buf is large enough
if b.have_auth_password(user) {
- buf.push(SSH_AUTHMETHOD_PASSWORD).unwrap()
+ l.0.push(SSH_AUTHMETHOD_PASSWORD).unwrap()
}
if b.have_auth_pubkey(user) {
- buf.push(SSH_AUTHMETHOD_PUBLICKEY).unwrap()
+ l.0.push(SSH_AUTHMETHOD_PUBLICKEY).unwrap()
}
- buf.as_slice().into()
+ l
}
}
--
GitLab