From 6c9aee54ec0475299e4f306a9bafbc7f4c5d901d Mon Sep 17 00:00:00 2001
From: Matt Johnston <matt@ucc.asn.au>
Date: Sun, 31 Dec 2023 12:13:25 +0800
Subject: [PATCH] Only SSH client allows ignored banner lines
The server should expect a banner line straight away, avoids dangling
connections from port scanners etc
---
src/conn.rs | 2 +-
src/ident.rs | 23 ++++++++++++++++++++---
src/kex.rs | 4 ++--
3 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/src/conn.rs b/src/conn.rs
index 1c7ee3a..31fd569 100644
--- a/src/conn.rs
+++ b/src/conn.rs
@@ -100,7 +100,7 @@ impl Conn {
Ok(Conn {
sess_id: None,
kex: Kex::new(),
- remote_version: ident::RemoteVersion::new(),
+ remote_version: ident::RemoteVersion::new(cliserv.is_client()),
state: ConnState::SendIdent,
algo_conf,
cliserv,
diff --git a/src/ident.rs b/src/ident.rs
index 49f28d3..b38b214 100644
--- a/src/ident.rs
+++ b/src/ident.rs
@@ -33,6 +33,7 @@ pub struct RemoteVersion {
/// Parse state
st: VersPars,
num_lines: usize,
+ is_client: bool,
}
/// Version parsing state.
@@ -59,11 +60,12 @@ pub(crate) enum VersPars {
}
impl RemoteVersion {
- pub fn new() -> Self {
+ pub fn new(is_client: bool) -> Self {
RemoteVersion {
storage: [0; MAX_REMOTE_VERSION_LEN],
st: VersPars::Start(0),
num_lines: 0,
+ is_client,
}
}
@@ -113,7 +115,8 @@ impl RemoteVersion {
if b == LF {
self.st = VersPars::Start(0);
self.num_lines += 1;
- if self.num_lines > MAX_LINES {
+ // only client allows extra unknown lines
+ if !self.is_client || self.num_lines > MAX_LINES {
return Err(Error::NotSSH);
}
}
@@ -160,8 +163,9 @@ mod tests {
use crate::error::{Error,TrapBug,Result};
use crate::sunsetlog::init_test_log;
+ // Tests as a client, allowing leading ignored lines
fn test_version(v: &str, split: usize, expect: &str) -> Result<usize> {
- let mut r = ident::RemoteVersion::new();
+ let mut r = ident::RemoteVersion::new(true);
let split = split.min(v.len());
let (a, b) = v.as_bytes().split_at(split);
@@ -212,6 +216,19 @@ mod tests {
Ok(())
}
+
+ #[test]
+ /// check server doesn't allow leading lines
+ fn version_server_lines() -> Result<()> {
+ // server instance, is_client false
+ let mut r = ident::RemoteVersion::new(false);
+ r.consume(b"SSH-2.0-aaa").unwrap();
+ let mut r = ident::RemoteVersion::new(false);
+ // disallow leading lines
+ r.consume(b"zzz\x0d\x0aSSH-2.0-aaa").unwrap_err();
+ Ok(())
+ }
+
// // TODO: maybe fuzzing would work better.
// // also hits an ICE, perhaps
// // https://github.com/rust-lang/rust/pull/94391
diff --git a/src/kex.rs b/src/kex.rs
index f9e81f0..4d80fca 100644
--- a/src/kex.rs
+++ b/src/kex.rs
@@ -896,7 +896,7 @@ mod tests {
impl TrafCatcher {
fn new() -> Self {
let traf_in = traffic::TrafIn::new(vec![0u8; 3000].leak());
- let mut rv = RemoteVersion::new();
+ let mut rv = RemoteVersion::new(false);
rv.consume(b"SSH-2.0-thing\r\n").unwrap();
rv.version().unwrap();
@@ -958,7 +958,7 @@ mod tests {
// needs to be hardcoded because that's what we send.
let mut s = Vec::from(crate::ident::OUR_VERSION);
s.extend_from_slice(b"\r\n");
- let mut version = RemoteVersion::new();
+ let mut version = RemoteVersion::new(true);
version.consume(s.as_slice()).unwrap();
let mut keys = vec![];
--
GitLab