From d396f87f93f9b4692ce9ef52fa58d2312bf44a75 Mon Sep 17 00:00:00 2001
From: Matt Johnston <matt@ucc.asn.au>
Date: Mon, 15 Aug 2022 22:14:35 +0800
Subject: [PATCH] Add sess_req_shell() etc

---
 sshproto/Cargo.toml             |  1 +
 sshproto/src/async_behaviour.rs | 64 ++++++++++++++++++++++++---------
 sshproto/src/behaviour.rs       | 36 +++++++++++++++----
 sshproto/src/block_behaviour.rs | 56 ++++++++++++++++++++++-------
 sshproto/src/channel.rs         | 42 ++++++++++++++--------
 sshproto/src/conn.rs            |  6 ++--
 sshproto/src/kex.rs             | 38 ++++++++++++++++++--
 sshproto/src/lib.rs             |  2 +-
 sshproto/src/packets.rs         |  2 ++
 9 files changed, 190 insertions(+), 57 deletions(-)

diff --git a/sshproto/Cargo.toml b/sshproto/Cargo.toml
index af94f5d..4b584e9 100644
--- a/sshproto/Cargo.toml
+++ b/sshproto/Cargo.toml
@@ -55,6 +55,7 @@ anyhow = { version = "1.0" }
 pretty-hex = "0.3"
 simplelog = { version = "0.12", features = ["test"] }
 proptest = "1.0"
+async-trait = { version = "0.1" }
 
 serde_json = "1.0"
 
diff --git a/sshproto/src/async_behaviour.rs b/sshproto/src/async_behaviour.rs
index f64e874..f7e5f8f 100644
--- a/sshproto/src/async_behaviour.rs
+++ b/sshproto/src/async_behaviour.rs
@@ -12,6 +12,8 @@ use async_trait::async_trait;
 
 use crate::{*, conn::RespPackets};
 use packets::{ForwardedTcpip,DirectTcpip};
+use channel::ChanOpened;
+use sshnames::*;
 use behaviour::*;
 
 pub(crate) enum AsyncCliServ<'a> {
@@ -49,9 +51,9 @@ impl<'a> AsyncCliServ<'a> {
 // #[async_trait(?Send)]
 #[async_trait]
 pub trait AsyncCliBehaviour: Sync+Send {
-    /// Provide the username to use for authentication. Will only be called once
+    /// Provide the user to use for authentication. Will only be called once
     /// per session.
-    /// If the username needs to change a new connection should be made
+    /// If the user needs to change a new connection should be made
     /// – servers often have limits on authentication attempts.
     ///
     async fn username(&mut self) -> BhResult<ResponseString>;
@@ -62,7 +64,7 @@ pub trait AsyncCliBehaviour: Sync+Send {
 
     /// Get a password to use for authentication returning `Ok(true)`.
     /// Return `Ok(false)` to skip password authentication
-    // TODO: having the hostname and username is useful to build a prompt?
+    // TODO: having the hostname and user is useful to build a prompt?
     // or we could provide a full prompt as Args
     #[allow(unused)]
     async fn auth_password(&mut self, pwbuf: &mut ResponseString) -> BhResult<bool> {
@@ -95,43 +97,71 @@ pub trait AsyncCliBehaviour: Sync+Send {
     }
     // TODO: postauth channel callbacks
 
-    // TODO: do we want this to be async? probably not.
-    fn open_tcp_forwarded(&self, chan: u32,
-        t: &ForwardedTcpip) -> channel::ChanOpened;
+    #[allow(unused)]
+    fn open_tcp_forwarded(&self, chan: u32, t: &ForwardedTcpip) -> ChanOpened {
+        ChanOpened::Failure(ChanFail::SSH_OPEN_UNKNOWN_CHANNEL_TYPE)
+    }
 
-    fn open_tcp_direct(&self, chan: u32,
-        t: &DirectTcpip) -> channel::ChanOpened;
+    #[allow(unused)]
+    fn open_tcp_direct(&self, chan: u32, t: &DirectTcpip) -> ChanOpened {
+        ChanOpened::Failure(ChanFail::SSH_OPEN_UNKNOWN_CHANNEL_TYPE)
+    }
 }
 
 // #[async_trait(?Send)]
 #[async_trait]
 pub trait AsyncServBehaviour: Sync+Send {
-    async fn hostkeys(&self) -> BhResult<&[&sign::SignKey]>;
+    // TODO: load keys on demand?
+    // at present `async` isn't very useful here, since it can't load keys
+    // on demand. perhaps it should have a callback to get key types,
+    // then later request a single key.
+    // Also could make it take a closure to call with the key, lets it just
+    // be loaded on the stack rather than kept in memory for the whole lifetime.
+    async fn hostkeys(&self) -> BhResult<&[sign::SignKey]>;
 
     // TODO: or return a slice of enums
-    fn have_auth_password(&self, username: &str) -> bool;
-    fn have_auth_pubkey(&self, username: &str) -> bool;
+    fn have_auth_password(&self, user: &str) -> bool;
+    fn have_auth_pubkey(&self, user: &str) -> bool;
 
 
     #[allow(unused)]
     // TODO: change password
-    async fn auth_password(&self, username: &str, password: &str) -> bool {
+    async fn auth_password(&self, user: &str, password: &str) -> bool {
         false
     }
 
     /// Returns true if the pubkey can be used to log in.
     /// TODO: allow returning pubkey restriction options
     #[allow(unused)]
-    async fn auth_pubkey(&self, username: &str, pubkey: &sign::SignKey) -> bool {
+    async fn auth_pubkey(&self, user: &str, pubkey: &sign::SignKey) -> bool {
         false
     }
 
     /// Returns whether a session can be opened
     fn open_session(&self, chan: u32) -> channel::ChanOpened;
 
-    fn open_tcp_forwarded(&self, chan: u32,
-        t: &ForwardedTcpip) -> channel::ChanOpened;
+    #[allow(unused)]
+    fn open_tcp_forwarded(&self, chan: u32, t: &ForwardedTcpip) -> ChanOpened {
+        ChanOpened::Failure(ChanFail::SSH_OPEN_UNKNOWN_CHANNEL_TYPE)
+    }
+
+    #[allow(unused)]
+    fn open_tcp_direct(&self, chan: u32, t: &DirectTcpip) -> ChanOpened {
+        ChanOpened::Failure(ChanFail::SSH_OPEN_UNKNOWN_CHANNEL_TYPE)
+    }
+
+    #[allow(unused)]
+    fn sess_req_shell(&self, chan: u32) -> bool {
+        false
+    }
+
+    #[allow(unused)]
+    fn sess_req_exec(&self, chan: u32, cmd: &str) -> bool {
+        false
+    }
 
-    fn open_tcp_direct(&self, chan: u32,
-        t: &ForwardedTcpip) -> channel::ChanOpened;
+    #[allow(unused)]
+    fn sess_pty(&self, chan: u32, pty: &Pty) -> bool {
+        false
+    }
 }
diff --git a/sshproto/src/behaviour.rs b/sshproto/src/behaviour.rs
index a2c2259..f0ad108 100644
--- a/sshproto/src/behaviour.rs
+++ b/sshproto/src/behaviour.rs
@@ -257,15 +257,15 @@ pub struct ServBehaviour<'a> {
 
 #[cfg(feature = "std")]
 impl<'a> ServBehaviour<'a> {
-    pub(crate) async fn hostkeys(&self) -> BhResult<&[&sign::SignKey]> {
+    pub(crate) async fn hostkeys(&self) -> BhResult<&[sign::SignKey]> {
         self.inner.hostkeys().await
     }
 
-    pub(crate) fn have_auth_password(&self, username: &str) -> bool {
-        self.inner.have_auth_password(username)
+    pub(crate) fn have_auth_password(&self, user: &str) -> bool {
+        self.inner.have_auth_password(user)
     }
-    pub(crate) fn have_auth_pubkey(&self, username: &str) -> bool {
-        self.inner.have_auth_pubkey(username)
+    pub(crate) fn have_auth_pubkey(&self, user: &str) -> bool {
+        self.inner.have_auth_pubkey(user)
     }
 
     // fn authmethods(&self) -> [AuthMethod];
@@ -288,11 +288,23 @@ impl<'a> ServBehaviour<'a> {
         t: &DirectTcpip) -> channel::ChanOpened {
         self.inner.open_tcp_direct(chan, t)
     }
+
+    pub(crate) fn sess_req_shell(&self, chan: u32) -> bool {
+        self.inner.sess_req_shell(chan)
+    }
+
+    pub(crate) fn sess_req_exec(&self, chan: u32, cmd: &str) -> bool {
+        self.inner.sess_req_exec(chan, cmd)
+    }
+
+    pub(crate) fn sess_pty(&self, chan: u32, pty: &Pty) -> bool {
+        self.inner.sess_pty(chan, pty)
+    }
 }
 
 #[cfg(not(feature = "std"))]
 impl<'a> ServBehaviour<'a> {
-    pub(crate) async fn hostkeys(&self) -> BhResult<&[&sign::SignKey]> {
+    pub(crate) fn hostkeys(&self) -> BhResult<&[sign::SignKey]> {
         self.inner.hostkeys()
     }
 
@@ -310,6 +322,18 @@ impl<'a> ServBehaviour<'a> {
         t: &DirectTcpip) -> channel::ChanOpened {
         self.inner.open_tcp_direct(chan, t)
     }
+
+    pub(crate) fn sess_req_shell(&self, chan: u32) -> bool {
+        self.inner.sess_req_shell(chan)
+    }
+
+    pub(crate) fn sess_req_exec(&self, chan: u32, cmd: &str) -> bool {
+        self.inner.sess_req_exec(chan, cmd)
+    }
+
+    pub(crate) fn sess_pty(&self, chan: u32, pty: &Pty) -> bool {
+        self.inner.sess_pty(chan, pty)
+    }
 }
 
 /// A stack-allocated string to store responses for usernames or passwords.
diff --git a/sshproto/src/block_behaviour.rs b/sshproto/src/block_behaviour.rs
index 85e8757..29dfc9e 100644
--- a/sshproto/src/block_behaviour.rs
+++ b/sshproto/src/block_behaviour.rs
@@ -10,6 +10,7 @@ use {
 use crate::{conn::RespPackets, *};
 use packets::{ForwardedTcpip,DirectTcpip};
 use behaviour::*;
+use sshnames::*;
 
 pub(crate) enum BlockCliServ<'a> {
     Client(&'a mut dyn BlockCliBehaviour),
@@ -84,29 +85,58 @@ pub trait BlockCliBehaviour {
     }
     // TODO: postauth channel callbacks
 
-    fn open_tcp_forwarded(&self, chan: u32,
-        t: &ForwardedTcpip) -> channel::ChanOpened;
+    #[allow(unused)]
+    fn open_tcp_forwarded(&self, chan: u32, t: &ForwardedTcpip) -> ChanOpened {
+        ChanOpened::Failure(ChanFail::SSH_OPEN_UNKNOWN_CHANNEL_TYPE)
+    }
 
-    fn open_tcp_direct(&self, chan: u32,
-        t: &DirectTcpip) -> channel::ChanOpened;
+    #[allow(unused)]
+    fn open_tcp_direct(&self, chan: u32, t: &DirectTcpip) -> ChanOpened {
+        ChanOpened::Failure(ChanFail::SSH_OPEN_UNKNOWN_CHANNEL_TYPE)
+    }
 }
 
 pub trait BlockServBehaviour {
-    fn hostkeys(&self) -> BhResult<&[&sign::SignKey]>;
+    fn hostkeys(&self) -> BhResult<&[sign::SignKey]>;
 
-    fn have_auth_password(&self, username: &str) -> bool;
-    fn have_auth_pubkey(&self, username: &str) -> bool;
+    fn have_auth_password(&self, user: &str) -> bool;
+    fn have_auth_pubkey(&self, user: &str) -> bool;
 
     // fn authmethods(&self) -> [AuthMethod];
 
-    fn auth_password(&self, user: &str, password: &str) -> bool;
+    fn auth_password(&self, user: &str, password: &str) -> bool {
+        false
+    }
+
+    fn auth_pubkey(&self, user: &str, pubkey: &sign::SignKey) -> bool {
+        false
+    }
 
     /// Returns whether a session can be opened
-    fn open_session(&self, chan: u32) -> channel::ChanOpened;
+    fn open_session(&self, chan: u32) -> ChanOpened;
+
+    #[allow(unused)]
+    fn open_tcp_forwarded(&self, chan: u32, t: &ForwardedTcpip) -> ChanOpened {
+        ChanOpened::Failure(ChanFail::SSH_OPEN_UNKNOWN_CHANNEL_TYPE)
+    }
 
-    fn open_tcp_forwarded(&self, chan: u32,
-        t: &ForwardedTcpip) -> channel::ChanOpened;
+    #[allow(unused)]
+    fn open_tcp_direct(&self, chan: u32, t: &DirectTcpip) -> ChanOpened {
+        ChanOpened::Failure(ChanFail::SSH_OPEN_UNKNOWN_CHANNEL_TYPE)
+    }
+
+    #[allow(unused)]
+    fn sess_req_shell(&self, chan: u32) -> bool {
+        false
+    }
 
-    fn open_tcp_direct(&self, chan: u32,
-        t: &DirectTcpip) -> channel::ChanOpened;
+    #[allow(unused)]
+    fn sess_req_exec(&self, chan: u32, cmd: &str) -> bool {
+        false
+    }
+
+    #[allow(unused)]
+    fn sess_pty(&self, chan: u32, pty: &Pty) -> bool {
+        false
+    }
 }
diff --git a/sshproto/src/channel.rs b/sshproto/src/channel.rs
index 9522b8e..eac61b8 100644
--- a/sshproto/src/channel.rs
+++ b/sshproto/src/channel.rs
@@ -276,6 +276,24 @@ impl Channels {
         Ok(())
     }
 
+    pub fn dispatch_request(&mut self,
+        p: &packets::ChannelRequest,
+        resp: &mut RespPackets<'_>,
+        b: &mut Behaviour<'_>,
+        ) -> Result<()> {
+            let ch = match self.get(p.num) {
+                Ok(ch) => ch,
+                Err(Error::BadChannel) => {
+                    debug!("request {p:?} channel is unknown");
+                    return Ok(())
+                },
+                Err(e) => unreachable!(),
+            };
+
+
+            Ok(())
+    }
+
     /// Incoming packet handling
     // TODO: protocol errors etc should perhaps be less fatal,
     // ssh implementations are usually imperfect.
@@ -288,8 +306,8 @@ impl Channels {
         trace!("chan dispatch");
         let r = match packet {
             Packet::ChannelOpen(p) => {
-                self.dispatch_open(&p, resp, b)
-                .map(|_| None)
+                self.dispatch_open(&p, resp, b)?;
+                Ok(None)
             }
 
             Packet::ChannelOpenConfirmation(p) => {
@@ -322,7 +340,7 @@ impl Channels {
                     // TODO: or just warn?
                     Err(Error::SSHProtoError)
                 } else {
-                    self.remove(p.num);
+                    self.remove(p.num)?;
                     // TODO event
                     Ok(None)
                 }
@@ -333,7 +351,7 @@ impl Channels {
                 Ok(None)
             }
             Packet::ChannelData(p) => {
-                let ch = self.get(p.num)?;
+                self.get(p.num)?;
                 // TODO check we are expecting input
                 if self.pending_input.is_some() {
                     return Err(Error::bug())
@@ -343,7 +361,7 @@ impl Channels {
                 Ok(Some(ChanEventMaker::DataIn(di)))
             }
             Packet::ChannelDataExt(p) => {
-                let ch = self.get(p.num)?;
+                self.get(p.num)?;
                 // TODO check we are expecting input and ext is valid.
                 if self.pending_input.is_some() {
                     return Err(Error::bug())
@@ -354,7 +372,7 @@ impl Channels {
                 Ok(Some(ChanEventMaker::DataIn(di)))
             }
             Packet::ChannelEof(p) => {
-                let _ch = self.get(p.num)?;
+                self.get(p.num)?;
                 Ok(Some(ChanEventMaker::Eof { num: p.num }))
             }
             Packet::ChannelClose(_p) => {
@@ -363,15 +381,8 @@ impl Channels {
                 Ok(None)
             }
             Packet::ChannelRequest(p) => {
-                match self.get(p.num) {
-                    Ok(ch) => Ok(Some(ChanEventMaker::Req)),
-                    Err(ch) => {
-                        if p.want_reply {
-                            // TODO respond with an error
-                        }
-                        Ok(None)
-                    }
-                }
+                self.dispatch_request(&p, resp, b)?;
+                Ok(None)
             }
             Packet::ChannelSuccess(_p) => {
                 trace!("channel success, TODO");
@@ -416,6 +427,7 @@ pub struct ModePair {
     pub arg: u32,
 }
 
+// TODO: name confused with packets::Pty
 #[derive(Debug)]
 pub struct Pty {
     // or could we put String into packets::Pty and serialize modes there...
diff --git a/sshproto/src/conn.rs b/sshproto/src/conn.rs
index cc64030..de77fe7 100644
--- a/sshproto/src/conn.rs
+++ b/sshproto/src/conn.rs
@@ -78,7 +78,9 @@ enum ConnState {
     /// Binary packet protocol has started, KexInit not yet received
     PreKex,
     /// At any time between receiving KexInit and NewKeys.
-    /// Can occur multiple times during a session, at later key exchanges
+    ///
+    /// Can occur multiple times during a session, at later key exchanges.
+    /// Non-kex packets are not allowed during that time
     InKex {
         done_auth: bool,
         output: Option<kex::KexOutput>,
@@ -236,7 +238,7 @@ impl<'a> Conn<'a> {
                             let kex =
                                 core::mem::replace(&mut self.kex, kex::Kex::new()?);
                             *output = Some(kex.handle_kexdhinit(&p, &self.sess_id)?);
-                            let reply = output.as_ref().trap()?.make_kexdhreply()?;
+                            let reply = output.as_ref().trap()?.make_kexdhreply(&b.server()?)?;
                             resp.push(reply.into()).trap()?;
                         }
                     }
diff --git a/sshproto/src/kex.rs b/sshproto/src/kex.rs
index fc82b50..59b9e18 100644
--- a/sshproto/src/kex.rs
+++ b/sshproto/src/kex.rs
@@ -492,7 +492,7 @@ impl<'a> KexOutput {
     }
 
     // server only
-    pub fn make_kexdhreply(&'a self) -> Result<Packet<'a>> {
+    pub fn make_kexdhreply(&'a self, b: &ServBehaviour) -> Result<Packet<'a>> {
         let q_s = BinString(self.pubkey.as_ref().trap()?);
         // TODO real signature
         let k_s = Blob(PubKey::Ed25519(packets::Ed25519PubKey{ key: BinString(&[]) }));
@@ -550,6 +550,9 @@ impl KexCurve25519 {
 
 #[cfg(test)]
 mod tests {
+    use async_trait::async_trait;
+    use pretty_hex::PrettyHex;
+
     use crate::encrypt;
     use crate::error::Error;
     use crate::ident::RemoteVersion;
@@ -557,7 +560,7 @@ mod tests {
     use crate::packets::{Packet,ParseContext};
     use crate::*;
     use crate::doorlog::init_test_log;
-    use pretty_hex::PrettyHex;
+
 
     use super::SSH_NAME_CURVE25519;
 
@@ -607,6 +610,28 @@ mod tests {
         sshwire::packet_from_bytes(&out_buf[..l], &ctx).unwrap()
     }
 
+    struct TestServBehaviour {
+        keys: Vec<SignKey>,
+    }
+
+    impl BlockServBehaviour for TestServBehaviour {
+        fn hostkeys(&self) -> BhResult<&[SignKey]> {
+            Ok(self.keys.as_slice())
+        }
+
+        fn have_auth_pubkey(&self, userame: &str) -> bool {
+            false
+        }
+
+        fn have_auth_password(&self, userame: &str) -> bool {
+            false
+        }
+
+        fn open_session(&self, chan: u32) -> ChanOpened {
+            ChanOpened::Success
+        }
+    }
+
     #[test]
     fn test_agree_kex() {
         init_test_log();
@@ -620,6 +645,13 @@ mod tests {
         let mut cli_version = RemoteVersion::new();
         cli_version.consume("SSH-2.0-door\r\n".as_bytes()).unwrap();
 
+        let mut sb = TestServBehaviour {
+            // TODO: put keys in
+            keys: vec![]
+        };
+        let mut sb = Behaviour::new_blocking_server(&mut sb);
+        let sb = &sb.server().unwrap();
+
         let mut cli = kex::Kex::new().unwrap();
         let mut serv = kex::Kex::new().unwrap();
 
@@ -636,7 +668,7 @@ mod tests {
         let ci = cli.make_kexdhinit().unwrap();
         let ci = if let Packet::KexDHInit(k) = ci { k } else { panic!() };
         let sout = serv.handle_kexdhinit(&ci, &None).unwrap();
-        let kexreply = sout.make_kexdhreply().unwrap();
+        let kexreply = sout.make_kexdhreply(sb).unwrap();
 
         let kexreply =
             if let Packet::KexDHReply(k) = kexreply { k } else { panic!() };
diff --git a/sshproto/src/lib.rs b/sshproto/src/lib.rs
index c5ff43e..5bbcaba 100644
--- a/sshproto/src/lib.rs
+++ b/sshproto/src/lib.rs
@@ -57,5 +57,5 @@ pub use conn::RespPackets;
 pub use sign::SignKey;
 pub use packets::PubKey;
 pub use error::{Error,Result};
-pub use channel::{ChanMsg, ChanMsgDetails, ChanEvent, Pty};
+pub use channel::{ChanMsg, ChanMsgDetails, ChanEvent, Pty, ChanOpened};
 pub use conn::Event;
diff --git a/sshproto/src/packets.rs b/sshproto/src/packets.rs
index 2b79ce8..1b3292f 100644
--- a/sshproto/src/packets.rs
+++ b/sshproto/src/packets.rs
@@ -498,6 +498,8 @@ pub struct Exec<'a> {
     pub command: TextString<'a>,
 }
 
+/// The contents of a `"pty-req"` request. Note that most calls use [`channel::Pty`]
+/// rather than this struct.
 #[derive(Debug, SSHEncode, SSHDecode)]
 pub struct Pty<'a> {
     pub term: TextString<'a>,
-- 
GitLab