small fixes

lib/cluelessh-transport/Cargo.toml

@@ -22,6 +22,8 @@ x25519-dalek = "2.0.1"
 tracing.workspace = true
 base64 = "0.22.1"
 secrecy = "0.8.0"
+hex = "0.4.3"
+serde = { version = "1.0.209", features = ["derive"] }
 
 [dev-dependencies]
 hex-literal = "0.4.1"

lib/cluelessh-transport/src/client.rs

@@ -4,10 +4,11 @@ use tracing::{debug, info, trace};
 
 use crate::{
     crypto::{
-        self, AlgorithmName, EncodedSshSignature, EncryptionAlgorithm, HostKeyVerifyAlgorithm, KeyExchangeSecret, SharedSecret, SupportedAlgorithms
+        self, AlgorithmName, EncodedSshSignature, EncryptionAlgorithm, HostKeyVerifyAlgorithm,
+        KeyExchangeSecret, SharedSecret, SupportedAlgorithms,
     },
     packet::{Packet, PacketTransport, ProtocolIdentParser},
-    peer_error, Msg, Result, SshRng, SshStatus,
+    peer_error, Msg, Result, SessionId, SshRng, SshStatus,
 };
 use cluelessh_format::{numbers, NameList, Reader, Writer};
 
@@ -50,10 +51,10 @@ enum ClientState {
         encryption_server_to_client: EncryptionAlgorithm,
     },
     ServiceRequest {
-        session_identifier: [u8; 32],
+        session_id: SessionId,
     },
     Open {
-        session_identifier: [u8; 32],
+        session_id: SessionId,
     },
 }
 
@@ -310,10 +311,10 @@ impl ClientConnection {
                         .queue_packet(Packet::new_msg_service_request(b"ssh-userauth"));
 
                     self.state = ClientState::ServiceRequest {
-                        session_identifier: *h,
+                        session_id: SessionId(*h),
                     };
                 }
-                ClientState::ServiceRequest { session_identifier } => {
+                ClientState::ServiceRequest { session_id } => {
                     let mut accept = packet.payload_parser();
                     let packet_type = accept.u8()?;
                     if packet_type != numbers::SSH_MSG_SERVICE_ACCEPT {
@@ -326,7 +327,7 @@ impl ClientConnection {
 
                     debug!("Connection has been opened successfully");
                     self.state = ClientState::Open {
-                        session_identifier: *session_identifier,
+                        session_id: *session_id,
                     };
                 }
                 ClientState::Open { .. } => {
@@ -349,9 +350,9 @@ impl ClientConnection {
         self.packet_transport.queue_packet(packet);
     }
 
-    pub fn is_open(&self) -> Option<[u8; 32]> {
+    pub fn is_open(&self) -> Option<SessionId> {
         match self.state {
-            ClientState::Open { session_identifier } => Some(session_identifier),
+            ClientState::Open { session_id } => Some(session_id),
             _ => None,
         }
     }

lib/cluelessh-transport/src/crypto.rs

@@ -7,7 +7,7 @@ use sha2::Digest;
 
 use crate::{
     packet::{EncryptedPacket, MsgKind, Packet, RawPacket},
-    peer_error, Msg, Result, SshRng,
+    peer_error, Msg, Result, SessionId, SshRng,
 };
 
 pub type SharedSecret = secrecy::Secret<SharedSecretInner>;
@@ -308,7 +308,7 @@ impl SupportedAlgorithms {
 }
 
 pub(crate) struct Session {
-    session_id: [u8; 32],
+    session_id: SessionId,
     from_peer: Tunnel,
     to_peer: Tunnel,
 }
@@ -326,6 +326,7 @@ pub(crate) trait Keys: Send + Sync + 'static {
     fn encrypt_packet_to_msg(&mut self, packet: Packet, packet_number: u64) -> Msg;
 
     fn additional_mac_len(&self) -> usize;
+    // TODO: actually rekey...
     fn rekey(
         &mut self,
         h: [u8; 32],
@@ -362,7 +363,7 @@ impl Keys for Plaintext {
 
 impl Session {
     pub(crate) fn new(
-        h: [u8; 32],
+        h: SessionId,
         k: &SharedSecret,
         encryption_client_to_server: EncryptionAlgorithm,
         encryption_server_to_client: EncryptionAlgorithm,
@@ -370,7 +371,7 @@ impl Session {
     ) -> Self {
         Self::from_keys(
             h,
-            h,
+            h.0,
             k,
             encryption_client_to_server,
             encryption_server_to_client,
@@ -380,7 +381,7 @@ impl Session {
 
     /// <https://datatracker.ietf.org/doc/html/rfc4253#section-7.2>
     fn from_keys(
-        session_id: [u8; 32],
+        session_id: SessionId,
         h: [u8; 32],
         k: &SharedSecret,
         alg_c2s: EncryptionAlgorithm,
@@ -462,7 +463,7 @@ fn derive_key(
     k: &SharedSecret,
     h: [u8; 32],
     letter: &str,
-    session_id: [u8; 32],
+    session_id: SessionId,
     key_size: usize,
 ) -> Vec<u8> {
     let sha2len = sha2::Sha256::output_size();
@@ -476,7 +477,7 @@ fn derive_key(
 
         if i == 0 {
             hash.update(letter.as_bytes());
-            hash.update(session_id);
+            hash.update(session_id.0);
         } else {
             hash.update(&output[..(i * sha2len)]);
         }

lib/cluelessh-transport/src/lib.rs

@@ -3,8 +3,13 @@ pub mod crypto;
 pub mod packet;
 pub mod server;
 
+use std::fmt::Debug;
+
 use cluelessh_format::ParseError;
 pub use packet::Msg;
+use serde::{Deserialize, Serialize};
+
+// TODO: extensions
 
 #[derive(Debug)]
 pub enum SshStatus {
@@ -17,6 +22,17 @@ pub enum SshStatus {
     PeerError(String),
 }
 
+#[derive(Clone, Copy, Serialize, Deserialize)]
+pub struct SessionId(pub [u8; 32]);
+
+impl Debug for SessionId {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_tuple("SessionId")
+            .field(&hex::encode(self.0))
+            .finish()
+    }
+}
+
 pub type Result<T, E = SshStatus> = std::result::Result<T, E>;
 
 impl From<ParseError> for SshStatus {

lib/cluelessh-transport/src/packet.rs

@@ -6,8 +6,8 @@ use std::mem;
 use tracing::{debug, trace};
 
 use crate::crypto::{self, EncryptionAlgorithm, Keys, Plaintext, Session, SharedSecret};
-use crate::peer_error;
 use crate::Result;
+use crate::{peer_error, SessionId};
 use cluelessh_format::numbers;
 use cluelessh_format::{NameList, Reader, Writer};
 
@@ -67,7 +67,8 @@ impl PacketTransport {
     }
 
     fn recv_bytes_step(&mut self, bytes: &[u8]) -> Result<Option<usize>> {
-        // TODO: This might not work if we buffer two packets where one changes keys in between?
+        // This would not work if we buffer two packets where one changes keys in between,
+        // but SSH_MSG_NEWKEYS messages guarantee that this cannot happen.
 
         let result =
             self.recv_next_packet
@@ -125,7 +126,7 @@ impl PacketTransport {
             is_server,
         ) {
             self.keys = Box::new(Session::new(
-                h,
+                SessionId(h),
                 k,
                 encryption_client_to_server,
                 encryption_server_to_client,

lib/cluelessh-transport/src/server.rs

@@ -7,18 +7,14 @@ use crate::crypto::{
 use crate::packet::{
     KeyExchangeEcDhInitPacket, KeyExchangeInitPacket, Packet, PacketTransport, ProtocolIdentParser,
 };
-use crate::Result;
 use crate::{peer_error, Msg, SshRng, SshStatus};
+use crate::{Result, SessionId};
 use cluelessh_format::numbers;
 use cluelessh_format::{NameList, Reader, Writer};
 use cluelessh_keys::private::PlaintextPrivateKey;
 use cluelessh_keys::signature::Signature;
 use tracing::{debug, info, trace};
 
-// This is definitely who we are.
-// TODO: dont make cluelesshd do this
-pub const SERVER_IDENTIFICATION: &[u8] = b"SSH-2.0-OpenSSH_9.7\r\n";
-
 pub struct ServerConnection {
     state: ServerState,
     packet_transport: PacketTransport,
@@ -31,6 +27,7 @@ pub struct ServerConnection {
 
 #[derive(Debug, Clone, Default)]
 pub struct ServerConfig {
+    pub server_identification: Vec<u8>,
     pub host_keys: Vec<cluelessh_keys::public::PublicKey>,
 }
 
@@ -69,10 +66,10 @@ enum ServerState {
         encryption_server_to_client: EncryptionAlgorithm,
     },
     ServiceRequest {
-        session_ident: [u8; 32],
+        session_id: SessionId,
     },
     Open {
-        session_ident: [u8; 32],
+        session_id: SessionId,
     },
 }
 
@@ -87,7 +84,7 @@ pub struct KeyExchangeParameters {
 }
 
 pub struct KeyExchangeResponse {
-    pub hash: [u8; 32],
+    pub hash: SessionId,
     pub server_ephemeral_public_key: Vec<u8>,
     pub shared_secret: SharedSecret,
     pub signature: Signature,
@@ -111,7 +108,7 @@ impl ServerConnection {
             ident_parser.recv_bytes(bytes);
             if let Some(client_identification) = ident_parser.get_peer_ident() {
                 self.packet_transport
-                    .queue_send_protocol_info(SERVER_IDENTIFICATION.to_vec());
+                    .queue_send_protocol_info(self.config.server_identification.clone());
                 self.state = ServerState::KeyExchangeInit {
                     client_identification,
                 };
@@ -311,10 +308,11 @@ impl ServerConnection {
                         *encryption_server_to_client,
                         true,
                     );
-                    self.state = ServerState::ServiceRequest { session_ident: *h };
+                    self.state = ServerState::ServiceRequest {
+                        session_id: SessionId(*h),
+                    };
                 }
-                ServerState::ServiceRequest { session_ident } => {
-                    // TODO: this should probably move out of here? unsure.
+                ServerState::ServiceRequest { session_id } => {
                     if packet.payload.first() != Some(&numbers::SSH_MSG_SERVICE_REQUEST) {
                         return Err(peer_error!("did not send SSH_MSG_SERVICE_REQUEST"));
                     }
@@ -335,7 +333,7 @@ impl ServerConnection {
                         },
                     });
                     self.state = ServerState::Open {
-                        session_ident: *session_ident,
+                        session_id: *session_id,
                     };
                 }
                 ServerState::Open { .. } => {
@@ -346,9 +344,9 @@ impl ServerConnection {
         Ok(())
     }
 
-    pub fn is_open(&self) -> Option<[u8; 32]> {
+    pub fn is_open(&self) -> Option<SessionId> {
         match self.state {
-            ServerState::Open { session_ident } => Some(session_ident),
+            ServerState::Open { session_id } => Some(session_id),
             _ => None,
         }
     }
@@ -365,7 +363,7 @@ impl ServerConnection {
                 ..
             } => Some(KeyExchangeParameters {
                 client_ident: client_identification.clone(),
-                server_ident: SERVER_IDENTIFICATION.to_vec(),
+                server_ident: self.config.server_identification.to_vec(),
                 client_kexinit: client_kexinit.clone(),
                 server_kexinit: server_kexinit.clone(),
                 eph_client_public_key: client_ephemeral_public_key.clone(),
@@ -392,7 +390,7 @@ impl ServerConnection {
 
                 self.packet_transport.queue_packet(packet);
                 self.state = ServerState::NewKeys {
-                    hash: response.hash,
+                    hash: response.hash.0,
                     shared_secret: response.shared_secret.clone(),
                     encryption_client_to_server: *encryption_client_to_server,
                     encryption_server_to_client: *encryption_server_to_client,
@@ -437,7 +435,7 @@ pub fn do_key_exchange(
     );
 
     Ok(KeyExchangeResponse {
-        hash,
+        hash: SessionId(hash),
         server_ephemeral_public_key,
         shared_secret,
         signature: private.private_key.sign(&hash),