From 2317963fbea3542a73de5ccfa889652fa7522974 Mon Sep 17 00:00:00 2001
From: Matt Johnston <matt@ucc.asn.au>
Date: Sat, 30 Dec 2023 18:09:24 +0800
Subject: [PATCH] Improve some docs and public symbols

Some unnecessary members are no longer exported, and some docs improved.
---
 async/src/agent.rs                 |  7 ++++--
 async/src/cmdline_client.rs        | 14 +++++++++--
 async/src/lib.rs                   |  6 ++---
 embassy/demos/common/src/server.rs |  6 ++---
 embassy/src/embassy_channel.rs     |  2 +-
 src/auth.rs                        |  4 +---
 src/behaviour.rs                   |  3 +++
 src/error.rs                       |  2 ++
 src/lib.rs                         | 37 +++++++++++++++---------------
 src/runner.rs                      | 13 ++++++++---
 src/sshnames.rs                    |  4 ++--
 11 files changed, 60 insertions(+), 38 deletions(-)

diff --git a/async/src/agent.rs b/async/src/agent.rs
index de3380b..814eff9 100644
--- a/async/src/agent.rs
+++ b/async/src/agent.rs
@@ -14,10 +14,9 @@ use sunset_sshwire_derive::*;
 
 use crate::*;
 use sunset::sshwire;
-use sunset::{PubKey, AuthSigMsg, Signature};
+use sunset::{PubKey, AuthSigMsg, Signature,OwnedSig, SignKey};
 use sshwire::{WireError, WireResult, BinString, TextString, Blob, SSHSink, SSHSource, SSHDecode, SSHEncode};
 use sshwire::{SSHEncodeEnum, SSHDecodeEnum};
-use sunset::sign::{OwnedSig, SignKey};
 use sunset::sshnames::*;
 
 // Must be sufficient for the list of all public keys
@@ -105,12 +104,16 @@ impl<'de: 'a, 'a> SSHDecode<'de> for AgentIdentitiesAnswer<'a> {
     }
 }
 
+/// A SSH Agent client
 pub struct AgentClient {
     conn: UnixStream,
     buf: Vec<u8>,
 }
 
 impl AgentClient {
+    /// Create a new client
+    ///
+    /// `path` is a Unix socket to a ssh-agent, such as that from `$SSH_AUTH_SOCK`.
     pub async fn new(path: impl AsRef<Path>) -> Result<Self, std::io::Error> {
         let conn = UnixStream::connect(path).await?;
         Ok(Self {
diff --git a/async/src/cmdline_client.rs b/async/src/cmdline_client.rs
index 7b45c75..f7d67ef 100644
--- a/async/src/cmdline_client.rs
+++ b/async/src/cmdline_client.rs
@@ -50,6 +50,9 @@ enum Msg {
     Exited,
 }
 
+/// A commandline client session
+///
+/// This opens a single channel and presents it to the stdin/stdout terminal.
 pub struct CmdlineClient {
     cmd: SessionCommand<String>,
     want_pty: bool,
@@ -101,6 +104,11 @@ impl CmdlineClient {
         }
     }
 
+    /// Splits a `CmdlineClient` into hooks and the runner.
+    ///
+    /// `CmdlineRunner` should be awaited until the session completes.
+    /// `CmdlineHooks` can be used to exit early (and may in future provide
+    /// other functionality).
     pub fn split(&mut self) -> (CmdlineHooks, CmdlineRunner) {
 
         let pty = self.make_pty();
@@ -306,8 +314,10 @@ impl<'a> CmdlineRunner<'a> {
         Ok(())
     }
 
-    /// Runs the `CmdlineClient` session. Requests a shell or command, performs
-    /// channel IO.
+    /// Runs the `CmdlineClient` session to completion.
+    ///
+    /// Performs authentication, requests a shell or command, performs channel IO.
+    /// Will return `Ok` after the session ends normally, or an error.
     pub async fn run(&mut self, cli: &'a SSHClient<'a, CmdlineHooks<'a>>) -> Result<()> {
         // chanio is only set once a channel is opened below
         let chanio = Fuse::terminated();
diff --git a/async/src/lib.rs b/async/src/lib.rs
index 8702a45..80ed969 100644
--- a/async/src/lib.rs
+++ b/async/src/lib.rs
@@ -11,11 +11,11 @@ mod agent;
 #[cfg(unix)]
 mod fdio;
 #[cfg(unix)]
-pub use fdio::{stdin, stdout, stderr_out};
+use fdio::{stdin, stdout, stderr_out};
 
-pub use pty::{raw_pty, RawPtyGuard};
+use pty::{raw_pty, RawPtyGuard};
 
-pub use cmdline_client::CmdlineClient;
+pub use cmdline_client::{CmdlineClient, CmdlineRunner, CmdlineHooks};
 
 pub use agent::AgentClient;
 
diff --git a/embassy/demos/common/src/server.rs b/embassy/demos/common/src/server.rs
index 16b6da1..3cd858f 100644
--- a/embassy/demos/common/src/server.rs
+++ b/embassy/demos/common/src/server.rs
@@ -70,10 +70,10 @@ async fn session<S: DemoServer>(socket: &mut TcpSocket<'_>, config: &SunsetMutex
     let src = socket.remote_endpoint().unwrap();
     info!("Connection from {}:{}", src.addr, src.port);
 
-    let s = S::new(init);
+    let demo = S::new(init);
 
     let conf = config.lock().await.clone();
-    let app = ServerApp::new(&s, conf)?;
+    let app = ServerApp::new(&demo, conf)?;
     let app = Mutex::<NoopRawMutex, _>::new(app);
 
     let mut ssh_rxbuf = [0; 2000];
@@ -81,7 +81,7 @@ async fn session<S: DemoServer>(socket: &mut TcpSocket<'_>, config: &SunsetMutex
     let serv = SSHServer::new(&mut ssh_rxbuf, &mut ssh_txbuf)?;
     let serv = &serv;
 
-    let session = s.run(serv);
+    let session = demo.run(serv);
 
     let (mut rsock, mut wsock) = socket.split();
 
diff --git a/embassy/src/embassy_channel.rs b/embassy/src/embassy_channel.rs
index 0b20e21..a9e3b83 100644
--- a/embassy/src/embassy_channel.rs
+++ b/embassy/src/embassy_channel.rs
@@ -72,7 +72,7 @@ impl<'a, C: CliBehaviour, S: ServBehaviour> Write for ChanIO<'a, C, S> {
 
 // Public wrappers for In only
 
-/// An standard bidirectional SSH channel
+/// A standard bidirectional SSH channel
 #[derive(Debug)]
 pub struct ChanInOut<'a, C: CliBehaviour, S: ServBehaviour>(ChanIO<'a, C, S>);
 
diff --git a/src/auth.rs b/src/auth.rs
index 66bbed6..90f9c76 100644
--- a/src/auth.rs
+++ b/src/auth.rs
@@ -19,8 +19,6 @@ use kex::SessId;
 
 /// The message to be signed in a pubkey authentication message,
 /// RFC4252 Section 7.
-///
-/// The UserauthRequest's signature field None.
 #[derive(Debug)]
 pub struct AuthSigMsg<'a> {
     pub(crate) sess_id: BinString<'a>,
@@ -40,7 +38,7 @@ impl SSHEncode for &AuthSigMsg<'_> {
 }
 
 impl<'a> AuthSigMsg<'a> {
-    pub fn new(u: &'a packets::UserauthRequest<'a>, sess_id: &'a SessId) -> Self {
+    pub(crate) fn new(u: &'a packets::UserauthRequest<'a>, sess_id: &'a SessId) -> Self {
         auth::AuthSigMsg {
             sess_id: BinString(sess_id.as_ref()),
             u,
diff --git a/src/behaviour.rs b/src/behaviour.rs
index d223dc1..08fe38f 100644
--- a/src/behaviour.rs
+++ b/src/behaviour.rs
@@ -149,6 +149,9 @@ pub trait CliBehaviour {
     ///
     /// `key` is a key previously returned from `next_authkey()`,
     /// it will be one of the `Agent...` variants.
+    ///
+    /// The client can call [`msg.enc()`](crate::sshwire::SSHEncode::enc()) to obtain the
+    /// message to use for agent signing.
     #[allow(unused)]
     async fn agent_sign(&mut self, key: &sign::SignKey, msg: &AuthSigMsg<'_>) -> BhResult<sign::OwnedSig> {
         Err(BhError::Fail)
diff --git a/src/error.rs b/src/error.rs
index de25bf5..2b3ad4d 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -14,6 +14,7 @@ use crate::channel::ChanNum;
 
 // TODO: can we make Snafu not require Debug?
 
+/// The Sunset error type.
 #[non_exhaustive]
 #[derive(Snafu, Debug)]
 #[snafu(context(suffix(false)))]
@@ -204,6 +205,7 @@ impl embedded_io::Error for Error {
     }
 }
 
+/// A Sunset-specific Result type.
 pub type Result<T, E = Error> = core::result::Result<T, E>;
 
 pub trait TrapBug<T> {
diff --git a/src/lib.rs b/src/lib.rs
index b433b6a..33d1018 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -13,17 +13,25 @@
 // XXX unused_imports only during dev churn
 #![allow(unused_imports)]
 
-// XXX decide what is public
-pub mod conn;
-pub mod encrypt;
+pub mod sshwire;
+pub mod packets;
+pub mod sshnames;
+pub mod config;
+// exported so that UnusedCli can be used
+pub mod behaviour;
+// exported so that some Channel error variants can be created with .fail().
+// perhaps the ones of interest should be expored separately.
 pub mod error;
-pub mod ident;
-pub mod kex;
-pub mod test;
-pub mod namelist;
+// perhaps don't need this, users could just use getrandom?
 pub mod random;
-pub mod sshnames;
-pub mod sign;
+
+mod conn;
+mod encrypt;
+mod ident;
+mod kex;
+mod test;
+mod namelist;
+mod sign;
 
 mod client;
 mod cliauth;
@@ -31,24 +39,15 @@ mod cliauth;
 mod server;
 mod servauth;
 
-// mod bhtokio;
-// mod bhnostd;
-
-pub mod sunsetlog;
+mod sunsetlog;
 mod auth;
 mod channel;
 mod runner;
-// TODO only public for UnusedCli etc.
-pub mod behaviour;
 mod termmodes;
 mod ssh_chapoly;
 mod traffic;
 mod noasync;
 
-pub mod packets;
-pub mod sshwire;
-pub mod config;
-pub mod prelude;
 
 // Application API
 pub use behaviour::{Behaviour, ServBehaviour, CliBehaviour,
diff --git a/src/runner.rs b/src/runner.rs
index 5f57501..b74031c 100644
--- a/src/runner.rs
+++ b/src/runner.rs
@@ -22,6 +22,10 @@ use conn::{Conn, Dispatched};
 // was completed. The `ChanHandle` is consumed by `Runner::channel_done()`.
 // Internally sunset uses `ChanNum`, which is just a newtype around u32.
 
+/// A SSH session instance
+///
+/// An application provides network or channel data to `Runner` method calls,
+/// and provides customisation callbacks via `CliBehaviour` or `ServBehaviour`.
 pub struct Runner<'a, C: CliBehaviour, S: ServBehaviour> {
     conn: Conn<C, S>,
 
@@ -222,6 +226,7 @@ impl<'a, C: CliBehaviour, S: ServBehaviour> Runner<'a, C, S> {
     }
 
     /// Send data from this application out the wire.
+    ///
     /// Returns `Ok(len)` consumed, `Err(Error::ChannelEof)` on EOF,
     /// or other errors.
     pub fn channel_send(
@@ -254,6 +259,7 @@ impl<'a, C: CliBehaviour, S: ServBehaviour> Runner<'a, C, S> {
     }
 
     /// Receive data coming from the wire into this application.
+    ///
     /// Returns `Ok(len)` received, `Err(Error::ChannelEof)` on EOF,
     /// or other errors. Ok(0) indicates no data available, ie pending.
     /// TODO: EOF is unimplemented
@@ -286,7 +292,7 @@ impl<'a, C: CliBehaviour, S: ServBehaviour> Runner<'a, C, S> {
         Ok(len)
     }
 
-    /// Receives input data, either dt or normal.
+    /// Receives input data, either normal or extended.
     pub fn channel_input_either(
         &mut self,
         chan: &ChanHandle,
@@ -307,7 +313,7 @@ impl<'a, C: CliBehaviour, S: ServBehaviour> Runner<'a, C, S> {
 
 
     /// Discards any channel input data pending for `chan`, regardless of whether
-    /// normal or `dt`.
+    /// normal or extended.
     pub fn discard_channel_input(&mut self, chan: &ChanHandle) -> Result<()> {
         let len = self.traf_in.discard_channel_input(chan.0);
         let wind_adjust = self.conn.channels.finished_input(chan.0, len)?;
@@ -321,7 +327,7 @@ impl<'a, C: CliBehaviour, S: ServBehaviour> Runner<'a, C, S> {
     /// Indicates when channel data is ready.
     ///
     /// When channel data is ready, returns a tuple
-    /// `Some((channel, dt, len))`
+    /// `Some((channel, data, len))`
     /// `len` is the amount of data ready remaining to read, will always be non-zero.
     /// Note that this returns a `ChanNum` index rather than a `ChanHandle` (which would
     /// be owned by the caller already.
@@ -363,6 +369,7 @@ impl<'a, C: CliBehaviour, S: ServBehaviour> Runner<'a, C, S> {
     }
 
     /// Returns `true` if the channel and `dt` are currently valid for writing.
+    ///
     /// Note that they may not be ready to send output.
     pub fn valid_channel_send(&self, chan: &ChanHandle, dt: ChanData) -> bool {
         self.conn.channels.valid_send(chan.0, dt)
diff --git a/src/sshnames.rs b/src/sshnames.rs
index d86fc5e..915ae94 100644
--- a/src/sshnames.rs
+++ b/src/sshnames.rs
@@ -80,7 +80,7 @@ pub enum ChanFail {
 
 /// SSH agent message numbers
 ///
-/// [draft-miller-ssh-agent-04](https://tools.ietf.org/html/draft-miller-ssh-agent-04)
+/// [draft-miller-ssh-agent](https://datatracker.ietf.org/doc/html/draft-miller-ssh-agent-11#section-5.1)
 #[allow(non_camel_case_types)]
 #[derive(Debug, Copy, Clone)]
 pub enum AgentMessageNum {
@@ -93,5 +93,5 @@ pub enum AgentMessageNum {
 
 }
 
-/// [draft-miller-ssh-agent-04](https://datatracker.ietf.org/doc/html/draft-miller-ssh-agent-04)
+/// [draft-miller-ssh-agent](https://datatracker.ietf.org/doc/html/draft-miller-ssh-agent-11#section-5.3)
 pub const SSH_AGENT_FLAG_RSA_SHA2_256: u32 = 0x02;
-- 
GitLab