From e8c0ad25da69c43dc28708e984204bbbacbb4375 Mon Sep 17 00:00:00 2001
From: Matt Johnston <matt@ucc.asn.au>
Date: Tue, 26 Jul 2022 23:31:38 +0800
Subject: [PATCH] A few comments

---
 async/Cargo.toml                |  1 +
 async/src/async_channel.rs      |  1 +
 async/src/fdio.rs               |  1 +
 async/src/pty.rs                |  1 +
 docs/design.md                  |  2 +-
 sshproto/src/async_behaviour.rs |  1 +
 sshproto/src/behaviour.rs       | 11 ++++++++++-
 sshproto/src/block_behaviour.rs |  1 +
 sshproto/src/kex.rs             |  2 --
 sshproto/src/packets.rs         |  4 ++--
 sshproto/src/sshwire.rs         | 13 +++++++++++--
 sshwire_derive/src/lib.rs       |  2 ++
 12 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/async/Cargo.toml b/async/Cargo.toml
index eb07034..d411642 100644
--- a/async/Cargo.toml
+++ b/async/Cargo.toml
@@ -22,6 +22,7 @@ argh = "0.1"
 # "net" for AsyncFd on unix
 tokio = { version = "1.19", features = ["sync", "net"] }
 # require alpha for https://github.com/rust-lang/futures-rs/pull/2571
+# need later than 0.3.21
 futures = { git = "https://github.com/rust-lang/futures-rs", revision = "8b0f812f53ada0d0aeb74abc32be22ab9dafae05" }
 async-trait = "0.1"
 moro = "0.4"
diff --git a/async/src/async_channel.rs b/async/src/async_channel.rs
index 0adc4c7..ad745f4 100644
--- a/async/src/async_channel.rs
+++ b/async/src/async_channel.rs
@@ -1,3 +1,4 @@
+//! Presents SSH channels as async
 #[allow(unused_imports)]
 use log::{debug, error, info, log, trace, warn};
 
diff --git a/async/src/fdio.rs b/async/src/fdio.rs
index 814b60c..f257243 100644
--- a/async/src/fdio.rs
+++ b/async/src/fdio.rs
@@ -1,3 +1,4 @@
+//! Helpers for async file descriptor IO
 #[allow(unused_imports)]
 use log::{debug, error, info, log, trace, warn};
 
diff --git a/async/src/pty.rs b/async/src/pty.rs
index ec2f413..4876d35 100644
--- a/async/src/pty.rs
+++ b/async/src/pty.rs
@@ -1,3 +1,4 @@
+//! Code to manipulate PTYs
 #[allow(unused_imports)]
 use log::{debug, error, info, log, trace, warn};
 
diff --git a/docs/design.md b/docs/design.md
index f539546..cba75ba 100644
--- a/docs/design.md
+++ b/docs/design.md
@@ -69,5 +69,5 @@ between async and non-async traits, hiding that from the main code. Eventually `
 
 ## Async
 
-The majority of packet dispatch handling isn't async, it just returns Ready straight away. Becaues of that we just have a Tokio `Mutex` which occassionally
+The majority of packet dispatch handling isn't async, it just returns Ready straight away. Because of that we just have a Tokio `Mutex` which occassionally
 holds the mutex across the `.await` boundary - it should seldom be contended.
diff --git a/sshproto/src/async_behaviour.rs b/sshproto/src/async_behaviour.rs
index e6ef038..558b98f 100644
--- a/sshproto/src/async_behaviour.rs
+++ b/sshproto/src/async_behaviour.rs
@@ -100,4 +100,5 @@ pub trait AsyncCliBehaviour: Sync+Send {
 // #[async_trait(?Send)]
 #[async_trait]
 pub trait AsyncServBehaviour: Sync+Send {
+    async fn hostkeys(&self) -> BhResult<&[&sign::SignKey]>;
 }
diff --git a/sshproto/src/behaviour.rs b/sshproto/src/behaviour.rs
index b964177..914148e 100644
--- a/sshproto/src/behaviour.rs
+++ b/sshproto/src/behaviour.rs
@@ -112,7 +112,6 @@ pub struct CliBehaviour<'a> {
     pub inner: &'a mut (dyn async_behaviour::AsyncCliBehaviour + Send),
     #[cfg(not(feature = "std"))]
     pub inner: &'a mut dyn block_behaviour::BlockCliBehaviour,
-    // pub phantom: core::marker::PhantomData<&'a ()>,
 }
 
 // wraps everything in AsyncCliBehaviour
@@ -190,8 +189,18 @@ pub struct ServBehaviour<'a> {
 
 #[cfg(feature = "std")]
 impl<'a> ServBehaviour<'a> {
+    pub(crate) async fn hostkeys(&self) -> BhResult<&[&sign::SignKey]> {
+        self.inner.hostkeys().await
+    }
+}
 
+#[cfg(not(feature = "std"))]
+impl<'a> ServBehaviour<'a> {
+    pub(crate) async fn hostkeys(&self) -> BhResult<&[&sign::SignKey]> {
+        self.inner.hostkeys()
+    }
 }
+
 /// A stack-allocated string to store responses for usernames or passwords.
 // 100 bytes is an arbitrary size.
 pub type ResponseString = heapless::String<100>;
diff --git a/sshproto/src/block_behaviour.rs b/sshproto/src/block_behaviour.rs
index 16e2b0c..b63ab13 100644
--- a/sshproto/src/block_behaviour.rs
+++ b/sshproto/src/block_behaviour.rs
@@ -86,4 +86,5 @@ pub trait BlockCliBehaviour {
 }
 
 pub trait BlockServBehaviour {
+    fn hostkeys(&self) -> BhResult<&[&sign::SignKey]>;
 }
diff --git a/sshproto/src/kex.rs b/sshproto/src/kex.rs
index cee9f13..ef3b8e4 100644
--- a/sshproto/src/kex.rs
+++ b/sshproto/src/kex.rs
@@ -27,7 +27,6 @@ use behaviour::{CliBehaviour, Behaviour, ServBehaviour};
 const MAX_SESSID: usize = 32;
 pub type SessId = heapless::Vec<u8, MAX_SESSID>;
 
-// #[cfg(test)]
 use pretty_hex::PrettyHex;
 
 const EMPTY_LOCALNAMES: LocalNames = LocalNames(&[]);
@@ -404,7 +403,6 @@ impl SharedSecret {
 
     fn make_kexdhinit(&self) -> Result<Packet> {
         let q_c = self.pubkey();
-        trace!("pubkey ours {:?}", self.pubkey().hex_dump());
         let q_c = BinString(q_c);
         Ok(packets::KexDHInit { q_c }.into())
     }
diff --git a/sshproto/src/packets.rs b/sshproto/src/packets.rs
index 6d63ae4..586704d 100644
--- a/sshproto/src/packets.rs
+++ b/sshproto/src/packets.rs
@@ -1,6 +1,6 @@
 //! SSH protocol packets. A [`Packet`] can be encoded/decoded to the
 //! SSH Binary Packet Protocol using [`crate::sshwire`].
-//!
+
 use core::borrow::BorrowMut;
 use core::cell::Cell;
 use core::fmt;
@@ -592,7 +592,7 @@ impl ParseContext {
     }
 }
 
-// we have repeated `match` statements for the various packet types, use a macro
+/// We have repeated `match` statements for the various packet types, use a macro
 macro_rules! messagetypes {
     (
         $( ( $message_num:literal, $SpecificPacketVariant:ident, $SpecificPacketType:ty, $SSH_MESSAGE_NAME:ident ), )*
diff --git a/sshproto/src/sshwire.rs b/sshproto/src/sshwire.rs
index dbc0a74..1196d98 100644
--- a/sshproto/src/sshwire.rs
+++ b/sshproto/src/sshwire.rs
@@ -1,3 +1,7 @@
+//! SSH wire format reading/writing.
+//! Used in conjunction with [`sshwire_derive`] and the [`packet`](crate::packets) format
+//! definitions.
+
 #[allow(unused_imports)]
 use {
     crate::error::{Error, Result, TrapBug},
@@ -15,7 +19,7 @@ use ascii::{AsAsciiStr, AsciiChar, AsciiStr};
 use crate::*;
 use packets::{Packet, ParseContext};
 
-
+/// A generic destination for serializing, used similarly to `serde::Serializer`
 pub trait SSHSink {
     fn push(&mut self, v: &[u8]) -> WireResult<()>;
     fn ctx(&self) -> Option<&ParseContext> {
@@ -23,12 +27,14 @@ pub trait SSHSink {
     }
 }
 
+/// A generic source for a packet, used similarly to `serde::Deserializer`
 pub trait SSHSource<'de> {
     fn take(&mut self, len: usize) -> WireResult<&'de [u8]>;
     fn pos(&self) -> usize;
     fn ctx(&self) -> &ParseContext;
 }
 
+/// Encodes the type in SSH wire format
 pub trait SSHEncode {
     fn enc<S>(&self, s: &mut S) -> WireResult<()> where S: SSHSink;
 }
@@ -117,6 +123,7 @@ where
     Ok(s.pos)
 }
 
+/// Hashes the SSH wire format representation of `value`, with a `u32` length prefix.
 pub fn hash_ser_length<T>(hash_ctx: &mut impl digest::DynDigest,
     value: &T) -> Result<()>
 where
@@ -127,6 +134,7 @@ where
     hash_ser(hash_ctx, None, value)
 }
 
+/// Hashes the SSH wire format representation of `value`
 pub fn hash_ser<T>(hash_ctx: &mut impl digest::DynDigest,
     parse_ctx: Option<&ParseContext>,
     value: &T) -> Result<()>
@@ -331,7 +339,7 @@ impl<'de> SSHDecode<'de> for TextString<'de> {
     }
 }
 
-/// A wrapper for a u32 prefixed data structure `B`, such as a public key blob
+/// A wrapper for a `u32` length prefixed data structure `B`, such as a public key blob
 pub struct Blob<B>(pub B);
 
 impl<B> AsRef<B> for Blob<B> {
@@ -372,6 +380,7 @@ impl<'de, B: SSHDecode<'de>> SSHDecode<'de> for Blob<B> {
         let pos1 = s.pos();
         let inner = SSHDecode::dec(s)?;
         let pos2 = s.pos();
+        // Sanity check the length matched
         if (pos2 - pos1) == len as usize {
             Ok(Blob(inner))
         } else {
diff --git a/sshwire_derive/src/lib.rs b/sshwire_derive/src/lib.rs
index 73d7249..4b354ff 100644
--- a/sshwire_derive/src/lib.rs
+++ b/sshwire_derive/src/lib.rs
@@ -1,3 +1,5 @@
+//! Used in conjunction with `sshwire.rs` and `packets.rs`
+
 use std::collections::HashSet;
 
 use proc_macro::Delimiter;
-- 
GitLab