diff --git a/async/examples/serv1.rs b/async/examples/serv1.rs index e2df57e34989e80dc70b1c117d2108b9c78348d8..03ca1446c9f86f2dc6ca55a2bd5aec28308c00ca 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 1414b43b91e816ebd80edf79a259fb83a467ed9d..5fd560f1b5163dfcab781f091631f282268e736d 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 16ec25627ffce6b64a1835742308839e71e9c229..0f28f708dcffda790b710f55287e7cf0688501ea 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 e519c3e6a419140586af16d5f2d5011af5bf43e1..fdf3e1eba6f1b2319357a499e9eb1ac7f48738ab 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 5764cc4bca69a3f9bd6946118bfc3919649aa118..b2c59e643ffa3a389af7c93cddb03f378d3cd25f 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 aef4f1cef5d071a7ba4c3d6ee400c556d887af83..c1c7a100c83dc4814e2011d92d4551b34a13a893 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)) } } }