From 375af256b286abdf7cf6b7c2bc06d9a3352cfbae Mon Sep 17 00:00:00 2001
From: Matt Johnston <matt@ucc.asn.au>
Date: Sun, 21 Aug 2022 12:46:46 +0800
Subject: [PATCH] Get rid of async_behaviour

---
 async/src/client.rs             |   7 +-
 sshproto/src/async_behaviour.rs | 167 ------------
 sshproto/src/behaviour.rs       | 440 +++++++++-----------------------
 sshproto/src/block_behaviour.rs | 142 -----------
 sshproto/src/cliauth.rs         |  18 +-
 sshproto/src/client.rs          |  12 +-
 sshproto/src/conn.rs            |  10 +-
 sshproto/src/kex.rs             |  27 +-
 sshproto/src/lib.rs             |   8 +-
 9 files changed, 151 insertions(+), 680 deletions(-)
 delete mode 100644 sshproto/src/async_behaviour.rs
 delete mode 100644 sshproto/src/block_behaviour.rs

diff --git a/async/src/client.rs b/async/src/client.rs
index d84a64c..4a00dad 100644
--- a/async/src/client.rs
+++ b/async/src/client.rs
@@ -21,7 +21,7 @@ use crate::async_door::*;
 use crate::async_channel::*;
 
 use door_sshproto as door;
-use door::{Behaviour, AsyncCliBehaviour, Runner, Result};
+use door::{Behaviour, CliBehaviour, Runner, Result};
 use door::sshnames::SSH_EXTENDED_DATA_STDERR;
 use door::config::*;
 
@@ -47,13 +47,12 @@ impl<'a> SSHClient<'a> {
     /// (This output can't be returned directly since it refers
     /// to contents of `Self` and would hit lifetime issues).
     pub async fn progress<F, R>(&mut self,
-        behaviour: &mut (dyn AsyncCliBehaviour+Send),
+        b: &mut (dyn CliBehaviour+Send),
         f: F)
         -> Result<Option<R>>
         where F: FnOnce(door::Event) -> Result<Option<R>> {
 
-        let mut b = Behaviour::new_async_client(behaviour);
-        self.door.progress(&mut b, f).await
+        self.door.progress(b, f).await
     }
 
     // TODO: return a Channel object that gives events like WinChange or exit status
diff --git a/sshproto/src/async_behaviour.rs b/sshproto/src/async_behaviour.rs
deleted file mode 100644
index fc8395b..0000000
--- a/sshproto/src/async_behaviour.rs
+++ /dev/null
@@ -1,167 +0,0 @@
-// std only, see comments in behaviour.rs
-#![cfg(feature = "std")]
-
-
-#[allow(unused_imports)]
-use {
-    crate::error::{Error, Result, TrapBug},
-    log::{debug, error, info, log, trace, warn},
-};
-
-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> {
-    Client(&'a mut (dyn AsyncCliBehaviour + Send)),
-    Server(&'a mut (dyn AsyncServBehaviour + Send)),
-}
-
-impl<'a> AsyncCliServ<'a> {
-    pub fn client(&mut self) -> Result<CliBehaviour> {
-        let c = match self {
-            Self::Client(c) => c,
-            _ => Error::bug_msg("Not client")?,
-        };
-        let c = CliBehaviour {
-            inner: *c,
-        };
-        Ok(c)
-    }
-
-    pub fn server(&mut self) -> Result<ServBehaviour> {
-        let c = match self {
-            Self::Server(c) => c,
-            _ => Error::bug_msg("Not server")?,
-        };
-        let c = ServBehaviour {
-            inner: *c,
-        };
-        Ok(c)
-    }
-}
-
-// Send+Sync bound here is required for trait objects since there are
-// default implementations of some methods.
-// https://docs.rs/async-trait/latest/async_trait/index.html#dyn-traits
-// #[async_trait(?Send)]
-#[async_trait]
-pub trait AsyncCliBehaviour: Sync+Send {
-    /// Provide the user to use for authentication. Will only be called once
-    /// per session.
-    /// 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>;
-
-    /// Whether to accept a hostkey for the server. The implementation
-    /// should compare the key with the key expected for the hostname used.
-    async fn valid_hostkey(&mut self, key: &PubKey) -> BhResult<bool>;
-
-    /// Get a password to use for authentication returning `Ok(true)`.
-    /// Return `Ok(false)` to skip password authentication
-    // 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> {
-        Ok(false)
-    }
-
-    /// Get the next private key to authenticate with. Will not be called
-    /// again once returning `HookError::Skip`
-    /// The default implementation returns `HookError::Skip`
-    async fn next_authkey(&mut self) -> BhResult<Option<sign::SignKey>> {
-        Ok(None)
-    }
-
-    /// Called after authentication has succeeded
-    // TODO: perhaps this should be an eventstream not a behaviour?
-    async fn authenticated(&mut self);
-
-    /// Show a banner sent from a server. Arguments are provided
-    /// by the server so could be hazardous, they should be escaped with
-    /// [`banner.escape_default()`](core::str::escape_default) or similar.
-    /// Language may be empty, is provided by the server.
-
-    /// This is a `Behaviour` method rather than an [`Event`] because
-    /// it must be displayed prior to other authentication
-    /// functions. `Events` may be handled asynchronously so wouldn't
-    /// guarantee that.
-    #[allow(unused)]
-    async fn show_banner(&self, banner: &str, language: &str) {
-        info!("Got banner:\n{:?}", banner.escape_default());
-    }
-    // TODO: postauth channel callbacks
-
-    #[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)
-    }
-}
-
-// #[async_trait(?Send)]
-#[async_trait]
-pub trait AsyncServBehaviour: Sync+Send {
-    // 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(&mut self) -> BhResult<&[sign::SignKey]>;
-
-    // TODO: or return a slice of enums
-    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(&mut 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(&mut self, user: &str, pubkey: &sign::SignKey) -> bool {
-        false
-    }
-
-    /// Returns whether a session can be opened
-    fn open_session(&mut self, chan: u32) -> channel::ChanOpened;
-
-    #[allow(unused)]
-    fn open_tcp_forwarded(&mut self, chan: u32, t: &ForwardedTcpip) -> ChanOpened {
-        ChanOpened::Failure(ChanFail::SSH_OPEN_UNKNOWN_CHANNEL_TYPE)
-    }
-
-    #[allow(unused)]
-    fn open_tcp_direct(&mut self, chan: u32, t: &DirectTcpip) -> ChanOpened {
-        ChanOpened::Failure(ChanFail::SSH_OPEN_UNKNOWN_CHANNEL_TYPE)
-    }
-
-    #[allow(unused)]
-    fn sess_req_shell(&mut self, chan: u32) -> bool {
-        false
-    }
-
-    #[allow(unused)]
-    fn sess_req_exec(&mut self, chan: u32, cmd: &str) -> bool {
-        false
-    }
-
-    #[allow(unused)]
-    fn sess_pty(&mut self, chan: u32, pty: &Pty) -> bool {
-        false
-    }
-}
diff --git a/sshproto/src/behaviour.rs b/sshproto/src/behaviour.rs
index d9861b1..ebbe27e 100644
--- a/sshproto/src/behaviour.rs
+++ b/sshproto/src/behaviour.rs
@@ -5,18 +5,11 @@ use {
 };
 
 use snafu::prelude::*;
-use core::task::{Waker,Poll};
-use core::future::Future;
-use core::mem;
-use core::fmt;
-
-use heapless::spsc::{Queue,Producer,Consumer};
-
-use crate::*;
-use packets::{self,Packet,ForwardedTcpip,DirectTcpip};
-use runner::{self,Runner};
-use channel::ChanMsg;
-use conn::RespPackets;
+
+use crate::{*, conn::RespPackets};
+use packets::{ForwardedTcpip,DirectTcpip};
+use channel::ChanOpened;
+use sshnames::*;
 use sshwire::TextString;
 
 // TODO: "Bh" is an ugly abbreviation. Naming is hard.
@@ -31,58 +24,32 @@ pub enum BhError {
     Fail,
 }
 
-// TODO: once async functions in traits work with no_std, this can all be reworked
-// to probably have Behaviour as a trait not a struct.
+/// A stack-allocated string to store responses for usernames or passwords.
+// 100 bytes is an arbitrary size.
+// TODO this might get replaced with something better
+pub type ResponseString = heapless::String<100>;
+
+// TODO: once async functions in traits work with no_std, some of the trait
+// methods could become async.
 //  Tracking Issue for static async fn in traits
 // https://github.com/rust-lang/rust/issues/91611
 
-// Even without no_std async functions in traits we could probably make Behaviour
-// a type alias to 'impl AsyncBehaviour" on std, and a wrapper struct on no_std.
-// That will require
-// Permit impl Trait in type aliases
-// https://github.com/rust-lang/rust/issues/63063
-
 // TODO: another interim option would to split the async trait methods
-// into a separate trait (which impls the non-async trait) for a bit more
-// DRY.
-
-pub struct Behaviour<'a> {
-    #[cfg(feature = "std")]
-    inner: async_behaviour::AsyncCliServ<'a>,
-    #[cfg(not(feature = "std"))]
-    inner: block_behaviour::BlockCliServ<'a>,
+// into a separate trait (which impls the non-async trait)
+
+pub enum Behaviour<'a> {
+    Client(&'a mut (dyn CliBehaviour + Send)),
+    Server(&'a mut (dyn ServBehaviour + Send)),
 }
 
-#[cfg(feature = "std")]
 impl<'a> Behaviour<'a> {
-    pub fn new_async_client(b: &'a mut (dyn AsyncCliBehaviour + Send)) -> Self {
-        Self {
-            inner: async_behaviour::AsyncCliServ::Client(b),
-        }
+    // TODO take and store a value not a reference
+    pub fn new_client(b: &'a mut (dyn CliBehaviour + Send)) -> Self {
+        Self::Client(b)
     }
 
-    pub fn new_async_server(b: &'a mut (dyn AsyncServBehaviour + Send)) -> Self {
-        Self {
-            inner: async_behaviour::AsyncCliServ::Server(b),
-        }
-    }
-
-    // TODO: or should we just pass CliBehaviour and ServBehaviour through runner,
-    // don't switch here at all
-    pub(crate) fn client(&mut self) -> Result<CliBehaviour> {
-        self.inner.client()
-    }
-
-    pub(crate) fn server(&mut self) -> Result<ServBehaviour> {
-        self.inner.server()
-    }
-
-    pub(crate) fn is_client(&self) -> bool {
-        matches!(self.inner, async_behaviour::AsyncCliServ::Client(_))
-    }
-
-    pub(crate) fn is_server(&self) -> bool {
-        !self.is_client()
+    pub fn new_server(b: &'a mut (dyn ServBehaviour + Send)) -> Self {
+        Self::Server(b)
     }
 
     /// Calls either client or server
@@ -104,320 +71,141 @@ impl<'a> Behaviour<'a> {
             self.server().unwrap().open_tcp_direct(chan, t)
         }
     }
-}
-
-#[cfg(not(feature = "std"))]
-impl<'a> Behaviour<'a>
-{
-    pub fn new_blocking_client(b: &'a mut dyn BlockCliBehaviour) -> Self {
-        Self {
-            inner: block_behaviour::BlockCliServ::Client(b),
-        }
-    }
-
-    pub fn new_blocking_server(b: &'a mut dyn BlockServBehaviour) -> Self {
-        Self {
-            inner: block_behaviour::BlockCliServ::Server(b),
-        }
-    }
-
-    // TODO: or should we just pass CliBehaviour and ServBehaviour through runner,
-    // don't switch here at all
-    pub(crate) fn client(&mut self) -> Result<CliBehaviour> {
-        self.inner.client()
-    }
 
-    pub(crate) fn server(&mut self) -> Result<ServBehaviour> {
-        self.inner.server()
-    }
-
-    pub(crate) fn is_client(&mut self) -> bool {
-        matches!(self.inner, block_behaviour::BlockCliServ::Client(_))
+    pub(crate) fn is_client(&self) -> bool {
+        matches!(self, Self::Client(_))
     }
 
-    pub(crate) fn is_server(&mut self) -> bool {
+    pub(crate) fn is_server(&self) -> bool {
         !self.is_client()
     }
 
-    /// Calls either client or server
-    pub(crate) fn open_tcp_forwarded(&mut self, chan: u32,
-        t: &ForwardedTcpip) -> channel::ChanOpened {
-        if self.is_client() {
-            self.client().unwrap().open_tcp_forwarded(chan, t)
-        } else {
-            self.server().unwrap().open_tcp_forwarded(chan, t)
+
+    pub(crate) fn client(&mut self) -> Result<&mut dyn CliBehaviour> {
+        match self {
+            Self::Client(c) => Ok(*c),
+            _ => Error::bug_msg("Not client"),
         }
     }
 
-    /// Calls either client or server
-    pub(crate) fn open_tcp_direct(&mut self, chan: u32,
-        t: &DirectTcpip) -> channel::ChanOpened {
-        if self.is_client() {
-            self.client().unwrap().open_tcp_direct(chan, t)
-        } else {
-            self.server().unwrap().open_tcp_direct(chan, t)
+    pub(crate) fn server(&mut self) -> Result<&mut dyn ServBehaviour> {
+        match self {
+            Self::Server(c) => Ok(*c),
+            _ => Error::bug_msg("Not server"),
         }
     }
 }
 
-pub struct CliBehaviour<'a> {
-    #[cfg(feature = "std")]
-    pub inner: &'a mut (dyn async_behaviour::AsyncCliBehaviour + Send),
-    #[cfg(not(feature = "std"))]
-    pub inner: &'a mut dyn block_behaviour::BlockCliBehaviour,
-}
-
-// wraps everything in AsyncCliBehaviour
-#[cfg(feature = "std")]
-impl<'a> CliBehaviour<'a> {
-    pub(crate) async fn username(&mut self) -> BhResult<ResponseString>{
-        self.inner.username().await
-    }
-
-    pub(crate) async fn valid_hostkey<'f>(&mut self, key: &PubKey<'f>) -> BhResult<bool> {
-        self.inner.valid_hostkey(key).await
-    }
-
+// `Sync+Send` here is to allow for future changes to make async.
+pub trait CliBehaviour: Sync+Send {
+    /// Provide the user to use for authentication. Will only be called once
+    /// per session.
+    /// If the user needs to change a new connection should be made
+    /// – servers often have limits on authentication attempts.
+    ///
+    fn username(&mut self) -> BhResult<ResponseString>;
+
+    /// Whether to accept a hostkey for the server. The implementation
+    /// should compare the key with the key expected for the hostname used.
+    fn valid_hostkey(&mut self, key: &PubKey) -> BhResult<bool>;
+
+    /// Get a password to use for authentication returning `Ok(true)`.
+    /// Return `Ok(false)` to skip password authentication
+    // TODO: having the hostname and user is useful to build a prompt?
+    // or we could provide a full prompt as Args
     #[allow(unused)]
-    pub(crate) async fn auth_password(&mut self, pwbuf: &mut ResponseString) -> BhResult<bool> {
-        self.inner.auth_password(pwbuf).await
+    fn auth_password(&mut self, pwbuf: &mut ResponseString) -> BhResult<bool> {
+        Ok(false)
     }
 
-    pub(crate) async fn next_authkey(&mut self) -> BhResult<Option<sign::SignKey>> {
-        self.inner.next_authkey().await
+    /// Get the next private key to authenticate with. Will not be called
+    /// again once returning `HookError::Skip`
+    /// The default implementation returns `HookError::Skip`
+    fn next_authkey(&mut self) -> BhResult<Option<sign::SignKey>> {
+        Ok(None)
     }
 
-    pub(crate) async fn authenticated(&mut self) {
-        self.inner.authenticated().await
-    }
+    /// Called after authentication has succeeded
+    // TODO: perhaps this should be an eventstream not a behaviour?
+    fn authenticated(&mut self);
 
-    pub(crate) async fn show_banner(&self, banner: TextString<'_>, language: TextString<'_>) -> Result<()> {
-        let banner = banner.as_str().map_err(|e| { warn!("Bad banner {:?}", banner); e})?;
-        let language = language.as_str()?;
-        self.inner.show_banner(banner, language).await;
-        Ok(())
-    }
-
-    pub(crate) fn open_tcp_forwarded(&self, chan: u32,
-        t: &ForwardedTcpip) -> channel::ChanOpened {
-        self.inner.open_tcp_forwarded(chan, t)
-    }
-
-    pub(crate) fn open_tcp_direct(&self, chan: u32,
-        t: &DirectTcpip) -> channel::ChanOpened {
-        self.inner.open_tcp_direct(chan, t)
-    }
-}
-
-// no_std blocking variant
-#[cfg(not(feature = "std"))]
-impl<'a> CliBehaviour<'a> {
-    pub(crate) async fn username(&mut self) -> BhResult<ResponseString>{
-        self.inner.username()
-    }
-
-    pub(crate) async fn valid_hostkey<'f>(&mut self, key: &PubKey<'f>) -> BhResult<bool> {
-        self.inner.valid_hostkey(key)
-    }
+    /// Show a banner sent from a server. Arguments are provided
+    /// by the server so could be hazardous, they should be escaped with
+    /// [`banner.escape_default()`](core::str::escape_default) or similar.
+    /// Language may be empty, is provided by the server.
 
+    /// This is a `Behaviour` method rather than an [`Event`] because
+    /// it must be displayed prior to other authentication
+    /// functions. `Events` may be handled asynchronously so wouldn't
+    /// guarantee that.
     #[allow(unused)]
-    pub(crate) async fn auth_password(&mut self, pwbuf: &mut ResponseString) -> BhResult<bool> {
-        self.inner.auth_password(pwbuf)
-    }
-
-    pub(crate) async fn next_authkey(&mut self) -> BhResult<Option<sign::SignKey>> {
-        self.inner.next_authkey()
-    }
-
-    pub(crate) async fn authenticated(&mut self) {
-        self.inner.authenticated()
-    }
-
-    // TODO: make ascii/utf8 a feature
-    pub(crate) async fn show_banner(&self, banner: TextString<'_>, language: TextString<'_>) -> Result<()> {
-        let banner = banner.as_ascii().map_err(|e| { warn!("Bad banner {:?}", banner); e})?;
-        let language = language.as_ascii()?;
-        self.inner.show_banner(banner, language);
-        Ok(())
+    fn show_banner(&self, banner: TextString, language: TextString) {
     }
+    // TODO: postauth channel callbacks
 
-    pub(crate) fn open_tcp_forwarded(&self, chan: u32,
-        t: &ForwardedTcpip) -> channel::ChanOpened {
-        self.inner.open_tcp_forwarded(chan, t)
+    #[allow(unused)]
+    fn open_tcp_forwarded(&self, chan: u32, t: &ForwardedTcpip) -> ChanOpened {
+        ChanOpened::Failure(ChanFail::SSH_OPEN_UNKNOWN_CHANNEL_TYPE)
     }
 
-    pub(crate) fn open_tcp_direct(&self, chan: u32,
-        t: &DirectTcpip) -> channel::ChanOpened {
-        self.inner.open_tcp_direct(chan, t)
+    #[allow(unused)]
+    fn open_tcp_direct(&self, chan: u32, t: &DirectTcpip) -> ChanOpened {
+        ChanOpened::Failure(ChanFail::SSH_OPEN_UNKNOWN_CHANNEL_TYPE)
     }
 }
 
-pub struct ServBehaviour<'a> {
-    #[cfg(feature = "std")]
-    pub inner: &'a mut dyn async_behaviour::AsyncServBehaviour,
-    #[cfg(not(feature = "std"))]
-    pub inner: &'a mut dyn block_behaviour::BlockServBehaviour,
-}
-
-#[cfg(feature = "std")]
-impl<'a> ServBehaviour<'a> {
-    pub(crate) async fn hostkeys(&mut self) -> BhResult<&[sign::SignKey]> {
-        self.inner.hostkeys().await
-    }
-
-    pub(crate) fn have_auth_password(&self, user: &str) -> bool {
-        self.inner.have_auth_password(user)
-    }
-    pub(crate) fn have_auth_pubkey(&self, user: &str) -> bool {
-        self.inner.have_auth_pubkey(user)
-    }
+pub trait ServBehaviour: Sync+Send {
+    // 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.
+    fn hostkeys(&mut self) -> BhResult<&[sign::SignKey]>;
 
-    // fn authmethods(&self) -> [AuthMethod];
+    // TODO: or return a slice of enums
+    fn have_auth_password(&self, user: &str) -> bool;
+    fn have_auth_pubkey(&self, user: &str) -> bool;
 
-    pub(crate) async fn auth_password(&mut self, user: &str, password: &str) -> bool {
-        self.inner.auth_password(user, password).await
-    }
 
-    /// Returns whether a session channel can be opened
-    pub(crate) fn open_session(&mut self, chan: u32) -> channel::ChanOpened {
-        self.inner.open_session(chan)
-    }
-
-    pub(crate) fn open_tcp_forwarded(&mut self, chan: u32,
-        t: &ForwardedTcpip) -> channel::ChanOpened {
-        self.inner.open_tcp_forwarded(chan, t)
-    }
-
-    pub(crate) fn open_tcp_direct(&mut self, chan: u32,
-        t: &DirectTcpip) -> channel::ChanOpened {
-        self.inner.open_tcp_direct(chan, t)
-    }
-
-    pub(crate) fn sess_req_shell(&mut self, chan: u32) -> bool {
-        self.inner.sess_req_shell(chan)
-    }
-
-    pub(crate) fn sess_req_exec(&mut self, chan: u32, cmd: &str) -> bool {
-        self.inner.sess_req_exec(chan, cmd)
-    }
-
-    pub(crate) fn sess_pty(&mut self, chan: u32, pty: &Pty) -> bool {
-        self.inner.sess_pty(chan, pty)
+    #[allow(unused)]
+    // TODO: change password
+    fn auth_password(&mut self, user: &str, password: &str) -> bool {
+        false
     }
-}
 
-#[cfg(not(feature = "std"))]
-impl<'a> ServBehaviour<'a> {
-    pub(crate) fn hostkeys(&self) -> BhResult<&[sign::SignKey]> {
-        self.inner.hostkeys()
+    /// Returns true if the pubkey can be used to log in.
+    /// TODO: allow returning pubkey restriction options
+    #[allow(unused)]
+    fn auth_pubkey(&mut self, user: &str, pubkey: &sign::SignKey) -> bool {
+        false
     }
 
-    /// Returns whether a session channel can be opened
-    pub(crate) fn open_session(&self, chan: u32) -> channel::ChanOpened {
-        self.inner.open_session(chan)
-    }
+    /// Returns whether a session can be opened
+    fn open_session(&mut self, chan: u32) -> channel::ChanOpened;
 
-    pub(crate) fn open_tcp_forwarded(&self, chan: u32,
-        t: &ForwardedTcpip) -> channel::ChanOpened {
-        self.inner.open_tcp_forwarded(chan, t)
+    #[allow(unused)]
+    fn open_tcp_forwarded(&mut self, chan: u32, t: &ForwardedTcpip) -> ChanOpened {
+        ChanOpened::Failure(ChanFail::SSH_OPEN_UNKNOWN_CHANNEL_TYPE)
     }
 
-    pub(crate) fn open_tcp_direct(&self, chan: u32,
-        t: &DirectTcpip) -> channel::ChanOpened {
-        self.inner.open_tcp_direct(chan, t)
+    #[allow(unused)]
+    fn open_tcp_direct(&mut self, chan: u32, t: &DirectTcpip) -> ChanOpened {
+        ChanOpened::Failure(ChanFail::SSH_OPEN_UNKNOWN_CHANNEL_TYPE)
     }
 
-    pub(crate) fn sess_req_shell(&self, chan: u32) -> bool {
-        self.inner.sess_req_shell(chan)
+    #[allow(unused)]
+    fn sess_req_shell(&mut self, chan: u32) -> bool {
+        false
     }
 
-    pub(crate) fn sess_req_exec(&self, chan: u32, cmd: &str) -> bool {
-        self.inner.sess_req_exec(chan, cmd)
+    #[allow(unused)]
+    fn sess_req_exec(&mut self, chan: u32, cmd: &str) -> bool {
+        false
     }
 
-    pub(crate) fn sess_pty(&self, chan: u32, pty: &Pty) -> bool {
-        self.inner.sess_pty(chan, pty)
+    #[allow(unused)]
+    fn sess_pty(&mut self, chan: u32, pty: &Pty) -> bool {
+        false
     }
 }
-
-/// A stack-allocated string to store responses for usernames or passwords.
-// 100 bytes is an arbitrary size.
-pub type ResponseString = heapless::String<100>;
-
-// // TODO sketchy api
-// pub enum BhQuery<'a> {
-//     Username(ResponseString),
-//     ValidHostkey(PubKey<'a>),
-//     Password(PubKey<'a>),
-// }
-
-// pub enum BhCommand {
-//     Session(),
-// }
-
-// // not derived since it can hold passwords etc
-// impl fmt::Debug for BhQuery<'_> {
-//     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-//         write!(f, "Query ...todo...")
-//     }
-// }
-
-// pub struct HookAskFut<'a> {
-//     mbox: &'a mut HookMailbox,
-// }
-
-// impl<'a> core::future::Future for HookAskFut<'a> {
-//     type Output = HookQuery;
-//     fn poll(self: core::pin::Pin<&mut Self>, cx: &mut core::task::Context<'_>) -> core::task::Poll<Self::Output> {
-//         let m = &mut self.get_mut().mbox;
-//         if let Some(reply) = m.reply.take() {
-//             Poll::Ready(reply)
-//         } else {
-//             m.reply_waker.take().map(|w| {
-//                 // this shouldn't happen?
-//                 warn!("existing waker");
-//                 w.wake()
-//             });
-//             m.reply_waker = Some(cx.waker().clone());
-//             Poll::Pending
-//         }
-//     }
-// }
-
-// impl core::future::Future for HookMailbox {
-//     type Output = HookQuery;
-//     fn poll(self: core::pin::Pin<&mut Self>, cx: &mut core::task::Context<'_>) -> core::task::Poll<Self::Output> {
-//         let m = self.get_mut();
-//         if let Some(reply) = m.reply.take() {
-//             Poll::Ready(reply)
-//         } else {
-//             m.reply_waker.take().map(|w| {
-//                 // this shouldn't happen?
-//                 warn!("existing waker");
-//                 w.wake()
-//             });
-//             m.reply_waker = Some(cx.waker().clone());
-//             Poll::Pending
-//         }
-//     }
-// }
-
-
-// struct HookCon<'a> {
-//     c: Consumer<'a, Query, 2>,
-// }
-
-// impl<'a> Future for HookCon<'a> {
-//     type Output = Query;
-//     fn poll(self: core::pin::Pin<&mut Self>, cx: &mut core::task::Context<'_>) -> Poll<Self::Output> {
-//         if let Some(r) = self.c.dequeue() {
-//             Poll::Ready(r)
-//         } else {
-//             // TODO
-//             // assert!(self.waker.is_none());
-//             // self.waker = Some(cx.waker().clone());
-//             Poll::Pending
-//         }
-//     }
-// }
-
diff --git a/sshproto/src/block_behaviour.rs b/sshproto/src/block_behaviour.rs
deleted file mode 100644
index 29dfc9e..0000000
--- a/sshproto/src/block_behaviour.rs
+++ /dev/null
@@ -1,142 +0,0 @@
-// see comments in behaviour.rs
-#![cfg(not(feature = "std"))]
-
-#[allow(unused_imports)]
-use {
-    crate::error::{Error, Result, TrapBug},
-    log::{debug, error, info, log, trace, warn},
-};
-
-use crate::{conn::RespPackets, *};
-use packets::{ForwardedTcpip,DirectTcpip};
-use behaviour::*;
-use sshnames::*;
-
-pub(crate) enum BlockCliServ<'a> {
-    Client(&'a mut dyn BlockCliBehaviour),
-    Server(&'a mut dyn BlockServBehaviour),
-}
-
-impl BlockCliServ<'_>
-{
-    pub fn client(&mut self) -> Result<CliBehaviour> {
-        let c = match self {
-            Self::Client(c) => c,
-            _ => Error::bug_msg("Not client")?,
-        };
-        let c = CliBehaviour {
-            inner: *c,
-        };
-        Ok(c)
-    }
-
-    pub fn server(&mut self) -> Result<ServBehaviour> {
-        let c = match self {
-            Self::Server(c) => c,
-            _ => Error::bug_msg("Not server")?,
-        };
-        let c = ServBehaviour {
-            inner: *c,
-        };
-        Ok(c)
-    }
-}
-
-pub trait BlockCliBehaviour {
-    /// Provide the username to use for authentication. Will only be called once
-    /// per session.
-    /// If the username needs to change a new connection should be made
-    /// – servers often have limits on authentication attempts.
-    ///
-    fn username(&mut self) -> BhResult<ResponseString>;
-
-    /// Whether to accept a hostkey for the server. The implementation
-    /// should compare the key with the key expected for the hostname used.
-    fn valid_hostkey(&mut self, key: &PubKey) -> BhResult<bool>;
-
-    /// 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?
-    // or we could provide a full prompt as Args
-    // TODO: should just return an Option<ResponseString>.
-    #[allow(unused)]
-    fn auth_password(&mut self, pwbuf: &mut ResponseString) -> BhResult<bool> {
-        Ok(false)
-    }
-
-    /// Get the next private key to authenticate with. Will not be called
-    /// again once returning `HookError::Skip`
-    /// The default implementation returns `HookError::Skip`
-    fn next_authkey(&mut self) -> BhResult<Option<sign::SignKey>> {
-        Ok(None)
-    }
-
-    /// Called after authentication has succeeded
-    // TODO: perhaps this should be an eventstream not a behaviour?
-    fn authenticated(&mut self);
-
-    /// Show a banner sent from a server. Arguments are provided
-    /// by the server so could be hazardous, they should be escaped with
-    /// [`banner.escape_default()`](core::str::escape_default) or similar.
-    /// Language may be empty, is provided by the server.
-    #[allow(unused)]
-    fn show_banner(&self, banner: &str, language: &str) {
-        info!("Got banner:\n{:?}", banner.escape_default());
-    }
-    // TODO: postauth channel callbacks
-
-    #[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)
-    }
-}
-
-pub trait BlockServBehaviour {
-    fn hostkeys(&self) -> BhResult<&[sign::SignKey]>;
-
-    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 {
-        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) -> 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
-    }
-
-    #[allow(unused)]
-    fn sess_pty(&self, chan: u32, pty: &Pty) -> bool {
-        false
-    }
-}
diff --git a/sshproto/src/cliauth.rs b/sshproto/src/cliauth.rs
index 61e8d4d..cd517ab 100644
--- a/sshproto/src/cliauth.rs
+++ b/sshproto/src/cliauth.rs
@@ -97,11 +97,11 @@ impl CliAuth {
     pub async fn start<'b>(
         &'b mut self,
         resp: &mut RespPackets<'b>,
-        mut b: CliBehaviour<'_>,
+        b: &mut dyn CliBehaviour,
     ) -> Result<()> {
         if let AuthState::Unstarted = self.state {
             self.state = AuthState::MethodQuery;
-            self.username = b.username().await?;
+            self.username = b.username()?;
 
             let p: Packet = packets::ServiceRequest {
                 name: SSH_SERVICE_USERAUTH,
@@ -120,10 +120,10 @@ impl CliAuth {
 
     async fn make_password_req(
         &mut self,
-        b: &mut CliBehaviour<'_>,
+        b: &mut dyn CliBehaviour,
     ) -> Result<Option<Req>> {
         let mut pw = ResponseString::new();
-        match b.auth_password(&mut pw).await {
+        match b.auth_password(&mut pw) {
             Err(_) => Err(Error::BehaviourError { msg: "No password returned" }),
             Ok(r) if r => Ok(Some(Req::Password(pw))),
             Ok(_) => Ok(None),
@@ -132,9 +132,9 @@ impl CliAuth {
 
     async fn make_pubkey_req(
         &mut self,
-        b: &mut CliBehaviour<'_>,
+        b: &mut dyn CliBehaviour,
     ) -> Result<Option<Req>> {
-        let k = b.next_authkey().await.map_err(|_| {
+        let k = b.next_authkey().map_err(|_| {
             self.try_pubkey = false;
             Error::BehaviourError { msg: "next_pubkey failed TODO" }
         })?;
@@ -251,7 +251,7 @@ impl CliAuth {
     pub async fn failure<'b>(
         &'b mut self,
         failure: &packets::UserauthFailure<'_>,
-        b: &mut CliBehaviour<'_>,
+        b: &mut dyn CliBehaviour,
         resp: &mut RespPackets<'b>,
         parse_ctx: &mut ParseContext,
     ) -> Result<()> {
@@ -292,10 +292,10 @@ impl CliAuth {
         Ok(())
     }
 
-    pub async fn success(&mut self, b: &mut CliBehaviour<'_>) -> Result<()> {
+    pub fn success(&mut self, b: &mut dyn CliBehaviour) -> Result<()> {
         // TODO: check current state? Probably just informational
         self.state = AuthState::Idle;
-        let _ = b.authenticated().await;
+        let _ = b.authenticated();
         // TODO errors
         Ok(())
     }
diff --git a/sshproto/src/client.rs b/sshproto/src/client.rs
index e295a1b..e2d61ff 100644
--- a/sshproto/src/client.rs
+++ b/sshproto/src/client.rs
@@ -28,19 +28,17 @@ impl Client {
 
     // pub fn check_hostkey(hostkey: )
 
-    pub(crate) async fn auth_success(&mut self, resp: &mut RespPackets<'_>,
+    pub(crate) fn auth_success(&mut self, resp: &mut RespPackets<'_>,
         parse_ctx: &mut ParseContext,
-        b: &mut CliBehaviour<'_>) -> Result<()> {
+        b: &mut dyn CliBehaviour) -> Result<()> {
 
         parse_ctx.cli_auth_type = None;
         let p: Packet = packets::ServiceRequest { name: SSH_SERVICE_CONNECTION }.into();
         resp.push(p.into()).trap()?;
-        self.auth.success(b).await
+        self.auth.success(b)
     }
 
-    pub(crate) async fn banner(&mut self, banner: &packets::UserauthBanner<'_>, b: &mut CliBehaviour<'_>) {
-        if let Err(e) = b.show_banner(banner.message, banner.lang).await {
-            warn!("Banner not shown: {e}")
-        }
+    pub(crate) fn banner(&mut self, banner: &packets::UserauthBanner<'_>, b: &mut dyn CliBehaviour) {
+        b.show_banner(banner.message, banner.lang)
     }
 }
diff --git a/sshproto/src/conn.rs b/sshproto/src/conn.rs
index 556f8e9..4c5b830 100644
--- a/sshproto/src/conn.rs
+++ b/sshproto/src/conn.rs
@@ -242,7 +242,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_mut().trap()?.make_kexdhreply(&mut b.server()?).await?;
+                            let reply = output.as_mut().trap()?.make_kexdhreply(b.server()?).await?;
                             resp.push(reply.into()).trap()?;
                         }
                     }
@@ -258,7 +258,7 @@ impl<'a> Conn<'a> {
                             } else {
                                 let kex =
                                     core::mem::replace(&mut self.kex, kex::Kex::new()?);
-                                *output = Some(kex.handle_kexdhreply(&p, &self.sess_id, &mut b.client()?).await?);
+                                *output = Some(kex.handle_kexdhreply(&p, &self.sess_id, b.client()?).await?);
                                 resp.push(Packet::NewKeys(packets::NewKeys {}).into()).trap()?;
                             }
                         } else {
@@ -313,7 +313,7 @@ impl<'a> Conn<'a> {
             Packet::UserauthFailure(p) => {
                 // TODO: client only
                 if let ClientServer::Client(cli) = &mut self.cliserv {
-                    cli.auth.failure(&p, &mut b.client()?, &mut resp, &mut self.parse_ctx).await?;
+                    cli.auth.failure(&p, b.client()?, &mut resp, &mut self.parse_ctx).await?;
                 } else {
                     debug!("Received UserauthFailure as a server");
                     return Err(Error::SSHProtoError)
@@ -324,7 +324,7 @@ impl<'a> Conn<'a> {
                 if let ClientServer::Client(cli) = &mut self.cliserv {
                     if matches!(self.state, ConnState::PreAuth) {
                         self.state = ConnState::Authed;
-                        cli.auth_success(&mut resp, &mut self.parse_ctx, &mut b.client()?).await?;
+                        cli.auth_success(&mut resp, &mut self.parse_ctx, b.client()?)?;
                         event = Some(EventMaker::CliAuthed);
                     } else {
                         debug!("Received UserauthSuccess unrequested")
@@ -337,7 +337,7 @@ impl<'a> Conn<'a> {
             Packet::UserauthBanner(p) => {
                 // TODO: client only
                 if let ClientServer::Client(cli) = &mut self.cliserv {
-                    cli.banner(&p, &mut b.client()?).await;
+                    cli.banner(&p, b.client()?);
                 } else {
                     debug!("Received banner as a server");
                     return Err(Error::SSHProtoError)
diff --git a/sshproto/src/kex.rs b/sshproto/src/kex.rs
index cc6db09..88543e8 100644
--- a/sshproto/src/kex.rs
+++ b/sshproto/src/kex.rs
@@ -267,7 +267,7 @@ impl Kex {
     // consumes self.
     pub async fn handle_kexdhreply<'f>(
         self, p: &packets::KexDHReply<'f>, sess_id: &Option<SessId>,
-        b: &mut CliBehaviour<'_>,
+        b: &mut dyn CliBehaviour,
     ) -> Result<KexOutput> {
         if !self.algos.as_ref().trap()?.is_client {
             return Err(Error::bug());
@@ -409,7 +409,7 @@ impl SharedSecret {
     // client only
     async fn handle_kexdhreply<'f>(
         mut kex: Kex, p: &packets::KexDHReply<'f>, sess_id: &Option<SessId>,
-        b: &mut CliBehaviour<'_>
+        b: &mut dyn CliBehaviour
     ) -> Result<KexOutput> {
         // let mut algos = kex.algos.take().trap()?;
         let mut algos = kex.algos.trap()?;
@@ -424,7 +424,7 @@ impl SharedSecret {
 
         algos.hostsig.verify(&p.k_s.0, kex_out.h.as_ref(), &p.sig.0)?;
         debug!("Hostkey signature is valid");
-        if matches!(b.valid_hostkey(&p.k_s.0).await, Ok(true)) {
+        if matches!(b.valid_hostkey(&p.k_s.0), Ok(true)) {
             Ok(kex_out)
         } else {
             Err(Error::BehaviourError { msg: "Host key rejected" })
@@ -495,11 +495,11 @@ impl<'a> KexOutput {
     }
 
     // server only
-    pub async fn make_kexdhreply<'b>(&'a mut self, b: &'a mut ServBehaviour<'_>) -> Result<Packet<'a>> {
+    pub async fn make_kexdhreply<'b>(&'a mut self, b: &'a mut dyn ServBehaviour) -> Result<Packet<'a>> {
         let q_s = BinString(self.kex_pub.as_ref().trap()?);
 
         // hostkeys list must contain the signature type
-        let key = b.hostkeys().await?.iter().find(|k| k.can_sign(&self.sig_type)).trap()?;
+        let key = b.hostkeys()?.iter().find(|k| k.can_sign(&self.sig_type)).trap()?;
         let k_s = Blob(key.pubkey());
         self.sig = Some(key.sign(&self.h.as_slice(), None)?);
         let sig: Signature = self.sig.as_ref().unwrap().into();
@@ -621,8 +621,8 @@ mod tests {
         keys: Vec<SignKey>,
     }
 
-    impl BlockServBehaviour for TestServBehaviour {
-        fn hostkeys(&self) -> BhResult<&[SignKey]> {
+    impl ServBehaviour for TestServBehaviour {
+        fn hostkeys(&mut self) -> BhResult<&[SignKey]> {
             Ok(self.keys.as_slice())
         }
 
@@ -634,7 +634,7 @@ mod tests {
             false
         }
 
-        fn open_session(&self, chan: u32) -> ChanOpened {
+        fn open_session(&mut self, chan: u32) -> ChanOpened {
             ChanOpened::Success
         }
     }
@@ -656,8 +656,8 @@ mod tests {
             // TODO: put keys in
             keys: vec![]
         };
-        let mut sb = Behaviour::new_blocking_server(&mut sb);
-        let sb = &sb.server().unwrap();
+        let mut sb = Behaviour::new_server(&mut sb);
+        let sb = sb.server().unwrap();
 
         let mut cli = kex::Kex::new().unwrap();
         let mut serv = kex::Kex::new().unwrap();
@@ -675,10 +675,11 @@ 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(sb).unwrap();
+        // let kexreply = sout.make_kexdhreply(sb);
+
+        // let kexreply =
+        //     if let Packet::KexDHReply(k) = kexreply { k } else { panic!() };
 
-        let kexreply =
-            if let Packet::KexDHReply(k) = kexreply { k } else { panic!() };
         // TODO need host signatures for it to succeed
         // let cout = cli.handle_kexdhreply(&kexreply, &None).unwrap();
 
diff --git a/sshproto/src/lib.rs b/sshproto/src/lib.rs
index df31681..9a98b0c 100644
--- a/sshproto/src/lib.rs
+++ b/sshproto/src/lib.rs
@@ -37,8 +37,6 @@ mod channel;
 mod runner;
 mod behaviour;
 mod termmodes;
-mod async_behaviour;
-mod block_behaviour;
 mod ssh_chapoly;
 
 pub mod packets;
@@ -46,11 +44,7 @@ pub mod sshwire;
 pub mod config;
 
 // Application API
-pub use behaviour::{Behaviour, BhError, BhResult, ResponseString};
-#[cfg(feature = "std")]
-pub use async_behaviour::{AsyncCliBehaviour,AsyncServBehaviour};
-#[cfg(not(feature = "std"))]
-pub use block_behaviour::{BlockCliBehaviour,BlockServBehaviour};
+pub use behaviour::{Behaviour, ServBehaviour, CliBehaviour, BhError, BhResult, ResponseString};
 
 pub use runner::Runner;
 pub use conn::RespPackets;
-- 
GitLab