From df6a6586b73e3bc4dfea93d0b1d1700d48c07609 Mon Sep 17 00:00:00 2001
From: Matt Johnston <matt@ucc.asn.au>
Date: Sat, 27 Aug 2022 00:24:28 +0800
Subject: [PATCH] TrafSend seems OK now (untested)

Need to rework async progress() functions and cull the callbacks
---
 async/src/client.rs         |  3 ++-
 async/src/cmdline_client.rs | 14 +++++++-------
 async/src/pty.rs            |  2 +-
 sshproto/src/channel.rs     |  4 ++--
 sshproto/src/cliauth.rs     |  8 ++++----
 sshproto/src/conn.rs        | 27 ++++++++++++++++++++++++---
 sshproto/src/kex.rs         |  2 +-
 sshproto/src/runner.rs      | 19 ++++---------------
 sshproto/src/traffic.rs     | 13 +++++++------
 9 files changed, 52 insertions(+), 40 deletions(-)

diff --git a/async/src/client.rs b/async/src/client.rs
index 4a00dad..ad308f0 100644
--- a/async/src/client.rs
+++ b/async/src/client.rs
@@ -52,7 +52,8 @@ impl<'a> SSHClient<'a> {
         -> Result<Option<R>>
         where F: FnOnce(door::Event) -> Result<Option<R>> {
 
-        self.door.progress(b, f).await
+        let mut b = Behaviour::new_client(b);
+        self.door.progress(&mut b, f).await
     }
 
     // TODO: return a Channel object that gives events like WinChange or exit status
diff --git a/async/src/cmdline_client.rs b/async/src/cmdline_client.rs
index 26d395a..a1e40ce 100644
--- a/async/src/cmdline_client.rs
+++ b/async/src/cmdline_client.rs
@@ -6,7 +6,7 @@ use core::str::FromStr;
 use door::SignKey;
 use door_sshproto as door;
 use door_sshproto::{BhError, BhResult};
-use door_sshproto::{ChanMsg, ChanMsgDetails, Error, RespPackets, Result, Runner};
+use door_sshproto::{ChanMsg, ChanMsgDetails, Error, Result, Runner};
 
 use std::collections::VecDeque;
 
@@ -37,21 +37,21 @@ impl CmdlineClient {
 
 // #[async_trait(?Send)]
 #[async_trait]
-impl door::AsyncCliBehaviour for CmdlineClient {
-    async fn username(&mut self) -> BhResult<door::ResponseString> {
+impl door::CliBehaviour for CmdlineClient {
+    fn username(&mut self) -> BhResult<door::ResponseString> {
         door::ResponseString::from_str(&self.username).map_err(|_| BhError::Fail)
     }
 
-    async fn valid_hostkey(&mut self, key: &door::PubKey) -> BhResult<bool> {
+    fn valid_hostkey(&mut self, key: &door::PubKey) -> BhResult<bool> {
         trace!("valid_hostkey for {key:?}");
         Ok(true)
     }
 
-    async fn next_authkey(&mut self) -> BhResult<Option<door::SignKey>> {
+    fn next_authkey(&mut self) -> BhResult<Option<door::SignKey>> {
         Ok(self.authkeys.pop_front())
     }
 
-    async fn auth_password(
+    fn auth_password(
         &mut self,
         pwbuf: &mut door::ResponseString,
     ) -> BhResult<bool> {
@@ -68,7 +68,7 @@ impl door::AsyncCliBehaviour for CmdlineClient {
         }
     }
 
-    async fn authenticated(&mut self) {
+    fn authenticated(&mut self) {
         info!("Authentication succeeded");
         self.auth_done = true;
     }
diff --git a/async/src/pty.rs b/async/src/pty.rs
index 4876d35..10a3fa2 100644
--- a/async/src/pty.rs
+++ b/async/src/pty.rs
@@ -7,7 +7,7 @@ use std::io::Error as IoError;
 use libc::{ioctl, winsize, termios, tcgetattr, tcsetattr };
 
 use door_sshproto as door;
-use door::{Behaviour, AsyncCliBehaviour, Runner, Result, Pty};
+use door::{Behaviour, Runner, Result, Pty};
 use door::config::*;
 use door::packets::WinChange;
 
diff --git a/sshproto/src/channel.rs b/sshproto/src/channel.rs
index b2c8667..6203f2c 100644
--- a/sshproto/src/channel.rs
+++ b/sshproto/src/channel.rs
@@ -298,7 +298,7 @@ impl Channels {
     async fn dispatch_inner(
         &mut self,
         packet: Packet<'_>,
-        s: &mut TrafSend<'_>,
+        s: &mut TrafSend<'_, '_>,
         b: &mut Behaviour<'_>,
     ) -> Result<Dispatched> {
         trace!("chan dispatch");
@@ -392,7 +392,7 @@ impl Channels {
     pub async fn dispatch(
         &mut self,
         packet: Packet<'_>,
-        s: &mut TrafSend<'_>,
+        s: &mut TrafSend<'_, '_>,
         b: &mut Behaviour<'_>,
     ) -> Result<Dispatched> {
 
diff --git a/sshproto/src/cliauth.rs b/sshproto/src/cliauth.rs
index da61f5b..6acaab8 100644
--- a/sshproto/src/cliauth.rs
+++ b/sshproto/src/cliauth.rs
@@ -96,7 +96,7 @@ impl CliAuth {
     // May be called multiple times
     pub async fn start<'b>(
         &'b mut self,
-        s: &mut TrafSend<'_>,
+        s: &mut TrafSend<'_, '_>,
         b: &mut dyn CliBehaviour,
     ) -> Result<()> {
         if let AuthState::Unstarted = self.state {
@@ -182,7 +182,7 @@ impl CliAuth {
         auth60: &packets::Userauth60<'_>,
         sess_id: &SessId,
         parse_ctx: &mut ParseContext,
-        s: &mut TrafSend<'_>,
+        s: &mut TrafSend<'_, '_>,
     ) -> Result<()> {
         parse_ctx.cli_auth_type = None;
 
@@ -197,7 +197,7 @@ impl CliAuth {
         pkok: &UserauthPkOk<'_>,
         sess_id: &SessId,
         parse_ctx: &mut ParseContext,
-        s: &mut TrafSend<'_>,
+        s: &mut TrafSend<'_, '_>,
     ) -> Result<()> {
         // We are only sending keys one at a time so they shouldn't
         // get out of sync. In future we could change it to send
@@ -250,7 +250,7 @@ impl CliAuth {
         &'b mut self,
         failure: &packets::UserauthFailure<'_>,
         parse_ctx: &mut ParseContext,
-        s: &mut TrafSend<'_>,
+        s: &mut TrafSend<'_, '_>,
         b: &mut dyn CliBehaviour,
     ) -> Result<()> {
         parse_ctx.cli_auth_type = None;
diff --git a/sshproto/src/conn.rs b/sshproto/src/conn.rs
index 50c92f9..13f89e2 100644
--- a/sshproto/src/conn.rs
+++ b/sshproto/src/conn.rs
@@ -123,7 +123,7 @@ impl<'a> Conn<'a> {
     /// Updates `ConnState` and sends any packets required to progress the connection state.
     pub(crate) async fn progress<'b>(
         &mut self,
-        s: &mut TrafSend<'_>,
+        s: &mut TrafSend<'_, '_>,
         b: &mut Behaviour<'_>,
     ) -> Result<(), Error> {
         debug!("progress conn state {:?}", self.state);
@@ -174,7 +174,7 @@ impl<'a> Conn<'a> {
     /// after `handle_payload()` runs.
     pub(crate) async fn handle_payload<'p>(
         &mut self, payload: &'p [u8], seq: u32,
-        s: &mut TrafSend<'_>,
+        s: &mut TrafSend<'_, '_>,
         b: &mut Behaviour<'_>,
     ) -> Result<Dispatched, Error> {
         let r = sshwire::packet_from_bytes(payload, &self.parse_ctx);
@@ -190,7 +190,7 @@ impl<'a> Conn<'a> {
     }
 
     async fn dispatch_packet<'p>(
-        &mut self, packet: Packet<'p>, s: &mut TrafSend<'_>, b: &mut Behaviour<'_>,
+        &mut self, packet: Packet<'p>, s: &mut TrafSend<'_, '_>, b: &mut Behaviour<'_>,
     ) -> Result<Dispatched, Error> {
         // TODO: perhaps could consolidate packet allowed checks into a separate function
         // to run first?
@@ -350,6 +350,27 @@ impl<'a> Conn<'a> {
         };
         Ok(disp)
     }
+
+    /// creates an `Event` that borrows data from the payload. Some `Event` variants don't
+    /// require payload data, the payload is not required in that case.
+    /// Those variants are allowed to return `resp` packets from `dispatch()`
+    pub(crate) fn make_event<'p>(&mut self, payload: Option<&'p [u8]>, ev: EventMaker)
+            -> Result<Option<Event<'p>>> {
+        let p = payload.map(|pl| sshwire::packet_from_bytes(pl, &self.parse_ctx)).transpose()?;
+        let r = match ev {
+            EventMaker::Channel(ChanEventMaker::DataIn(_)) => {
+                // caller should have handled it instead
+                return Err(Error::bug())
+            }
+            EventMaker::Channel(cev) => {
+                let c = cev.make(p.trap()?);
+                c.map(|c| Event::Channel(c))
+            }
+            EventMaker::CliAuthed => Some(Event::CliAuthed),
+        };
+        Ok(r)
+    }
+
 }
 
 #[cfg(test)]
diff --git a/sshproto/src/kex.rs b/sshproto/src/kex.rs
index 92143ec..aceeb22 100644
--- a/sshproto/src/kex.rs
+++ b/sshproto/src/kex.rs
@@ -276,7 +276,7 @@ impl Kex {
     // consumes self.
     pub async fn handle_kexdhreply<'f>(
         self, p: &packets::KexDHReply<'f>, sess_id: &Option<SessId>,
-        s: &mut TrafSend<'_>,
+        s: &mut TrafSend<'_, '_>,
         b: &mut dyn CliBehaviour,
     ) -> Result<KexOutput> {
         if !self.algos.as_ref().trap()?.is_client {
diff --git a/sshproto/src/runner.rs b/sshproto/src/runner.rs
index 079c674..bbfc41d 100644
--- a/sshproto/src/runner.rs
+++ b/sshproto/src/runner.rs
@@ -84,22 +84,12 @@ impl<'a> Runner<'a> {
         Ok(r)
     }
 
-    pub async fn progress(&mut self, b: &mut Behaviour<'_>) -> Result<()> {
-        let done = self.progress_inner(b).await?;
-
-        if done {
-            self.wake();
-        }
-        Ok(())
-    }
-
-
     /// Drives connection progress, handling received payload and sending
     /// other packets as required. This must be polled/awaited regularly.
     /// Optionally returns `Event` which provides channel or session
     /// event to the application.
     /// [`done_payload()`] must be called after any `Ok` result.
-    pub async fn progress_inner(&'a mut self, behaviour: &mut Behaviour<'_>) -> Result<bool> {
+    pub async fn progress(&mut self, behaviour: &mut Behaviour<'_>) -> Result<()> {
         if let Some((payload, seq)) = self.traf_in.payload() {
             // Lifetimes here are a bit subtle.
             // `payload` has self.traffic lifetime, used until `handle_payload`
@@ -109,20 +99,19 @@ impl<'a> Runner<'a> {
             // After that progress() can perform more send_packet() itself.
 
             let mut s = self.traf_out.sender(&mut self.keys);
-
             let d = self.conn.handle_payload(payload, seq, &mut s, behaviour).await?;
 
             if let Some(d) = d.0 {
                 self.traf_in.handled_payload()?;
                 self.traf_in.set_channel_input(d)?;
-                false
             } else {
                 self.traf_in.done_payload()?;
                 self.conn.progress(&mut s, behaviour).await?;
-                true
+                self.wake();
             }
-
         }
+
+        Ok(())
     }
 
     pub fn done_payload(&mut self) -> Result<()> {
diff --git a/sshproto/src/traffic.rs b/sshproto/src/traffic.rs
index bc5bcb8..a404235 100644
--- a/sshproto/src/traffic.rs
+++ b/sshproto/src/traffic.rs
@@ -405,20 +405,20 @@ impl<'a> TrafOut<'a> {
     }
 
 
-    pub fn sender(&'a mut self, keys: &'a mut KeyState) -> TrafSend {
+    pub fn sender<'s>(&'s mut self, keys: &'s mut KeyState) -> TrafSend<'s, 'a> {
         TrafSend::new(self, keys)
     }
 
 }
 
 /// Convenience to pass TrafOut with keys
-pub(crate) struct TrafSend<'a> {
-    keys: &'a mut KeyState,
-    out: &'a mut TrafOut<'a>,
+pub(crate) struct TrafSend<'s, 'a> {
+    out: &'s mut TrafOut<'a>,
+    keys: &'s mut KeyState,
 }
 
-impl<'a> TrafSend<'a> {
-    fn new(out: &'a mut TrafOut<'a>, keys: &'a mut KeyState) -> Self {
+impl<'s, 'a> TrafSend<'s, 'a> {
+    fn new<'f>(out: &'s mut TrafOut<'a>, keys: &'s mut KeyState) -> Self {
         Self {
             out,
             keys,
@@ -442,3 +442,4 @@ impl<'a> TrafSend<'a> {
         self.out.can_output()
     }
 }
+
-- 
GitLab