From 2c343f475cc9af5af5547b0f97aa3922650bda01 Mon Sep 17 00:00:00 2001
From: Matt Johnston <matt@ucc.asn.au>
Date: Wed, 31 Aug 2022 01:08:25 +0800
Subject: [PATCH] bits of sig
---
async/examples/serv1.rs | 6 +++---
sshproto/src/channel.rs | 7 ++++++-
sshproto/src/conn.rs | 10 +++++++---
sshproto/src/error.rs | 3 +++
sshproto/src/servauth.rs | 2 +-
sshproto/src/sshwire.rs | 17 ++++++++++++++---
6 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/async/examples/serv1.rs b/async/examples/serv1.rs
index e2df57e..03ca144 100644
--- a/async/examples/serv1.rs
+++ b/async/examples/serv1.rs
@@ -69,9 +69,9 @@ fn setup_log(args: &Args) -> Result<()> {
.add_filter_allow_str("door")
.add_filter_allow_str("serv1")
// not debugging these bits of the stack at present
- // .add_filter_ignore_str("door_sshproto::traffic")
- // .add_filter_ignore_str("door_sshproto::runner")
- // .add_filter_ignore_str("door_async::async_door")
+ .add_filter_ignore_str("door_sshproto::traffic")
+ .add_filter_ignore_str("door_sshproto::runner")
+ .add_filter_ignore_str("door_async::async_door")
.set_time_offset_to_local().expect("Couldn't get local timezone")
.build();
diff --git a/sshproto/src/channel.rs b/sshproto/src/channel.rs
index 1414b43..5fd560f 100644
--- a/sshproto/src/channel.rs
+++ b/sshproto/src/channel.rs
@@ -233,6 +233,7 @@ impl Channels {
) -> Result<(), DispatchOpenError> {
if b.is_client() && matches!(p.ty, ChannelOpenType::Session) {
// only server should receive session opens
+ trace!("dispatch not server");
return Err(Error::SSHProtoError.into());
}
@@ -342,7 +343,10 @@ impl Channels {
ch.state = ChanState::Normal;
}
}
- _ => return Err(Error::SSHProtoError),
+ _ => {
+ trace!("Bad channel state");
+ return Err(Error::SSHProtoError)
+ }
}
}
@@ -350,6 +354,7 @@ impl Channels {
let ch = self.get(p.num)?;
if ch.send.is_some() {
// TODO: or just warn?
+ trace!("open failure late?");
return Err(Error::SSHProtoError);
} else {
self.remove(p.num)?;
diff --git a/sshproto/src/conn.rs b/sshproto/src/conn.rs
index 16ec256..0f28f70 100644
--- a/sshproto/src/conn.rs
+++ b/sshproto/src/conn.rs
@@ -1,6 +1,6 @@
#[allow(unused_imports)]
use {
- crate::error::{Error, Result, TrapBug},
+ crate::error::{*, Error, Result, TrapBug},
log::{debug, error, info, log, trace, warn},
};
@@ -163,7 +163,6 @@ impl<'a> Conn<'a> {
}
pub(crate) fn initial_sent(&self) -> bool {
- trace!("initial_sent state {:?}", self.state);
match self.state {
| ConnState::SendIdent
| ConnState::SendFirstKexInit
@@ -188,7 +187,10 @@ impl<'a> Conn<'a> {
s.send(packets::Unimplemented { seq })?;
Ok(Dispatched(None))
}
- Err(e) => return Err(e),
+ Err(e) => {
+ trace!("Error decoding packet: {e} {:#?}", payload.hex_dump());
+ return Err(e)
+ }
}
}
@@ -258,6 +260,7 @@ impl<'a> Conn<'a> {
ConnState::InKex { done_auth: _, ref mut output } => {
if self.cliserv.is_client() {
// TODO: client/server validity checks should move somewhere more general
+ trace!("kexdhinit not server");
return Err(Error::SSHProtoError);
}
if self.kex.maybe_discard_packet() {
@@ -284,6 +287,7 @@ impl<'a> Conn<'a> {
}
} else {
// TODO: client/server validity checks should move somewhere more general
+ trace!("Not kexdhreply not client");
return Err(Error::SSHProtoError);
}
}
diff --git a/sshproto/src/error.rs b/sshproto/src/error.rs
index e519c3e..fdf3e1e 100644
--- a/sshproto/src/error.rs
+++ b/sshproto/src/error.rs
@@ -38,6 +38,9 @@ pub enum Error {
/// Signature is incorrect
BadSig,
+ /// Integer overflow in packet
+ BadNumber,
+
/// Error in received SSH protocol. Will disconnect.
SSHProtoError,
diff --git a/sshproto/src/servauth.rs b/sshproto/src/servauth.rs
index 5764cc4..b2c59e6 100644
--- a/sshproto/src/servauth.rs
+++ b/sshproto/src/servauth.rs
@@ -134,7 +134,7 @@ impl ServAuth {
let msg = auth::AuthSigMsg::new(&p, sess_id);
match sig_type.verify(&m.pubkey.0, &msg, sig) {
Ok(()) => true,
- Err(_) => false,
+ Err(e) => { trace!("sig failde {e}"); false},
}
}
diff --git a/sshproto/src/sshwire.rs b/sshproto/src/sshwire.rs
index aef4f1c..c1c7a10 100644
--- a/sshproto/src/sshwire.rs
+++ b/sshproto/src/sshwire.rs
@@ -383,15 +383,26 @@ impl<B: SSHEncode> SSHEncode for Blob<B> {
impl<'de, B: SSHDecode<'de>> SSHDecode<'de> for Blob<B> {
fn dec<S>(s: &mut S) -> WireResult<Self>
where S: sshwire::SSHSource<'de> {
- let len = u32::dec(s)?;
+ let len = u32::dec(s)? as usize;
let pos1 = s.pos();
let inner = SSHDecode::dec(s)?;
let pos2 = s.pos();
+
// Sanity check the length matched
- if (pos2 - pos1) == len as usize {
+ let used_len = pos2 - pos1;
+ if used_len == len {
Ok(Blob(inner))
} else {
- Err(WireError::SSHProtoError)
+ let extra = len.checked_sub(used_len).ok_or_else(|| {
+ trace!("inner consumed past length of SSH Blob. \
+ Expected {} bytes, got {} bytes {}..{}",
+ len, pos2-pos1, pos1, pos2);
+ WireError::SSHProtoError
+ })?;
+ // Skip over unconsumed bytes in the blob.
+ // This can occur with Unknown variants
+ s.take(extra)?;
+ Ok(Blob(inner))
}
}
}
--
GitLab