From dea326b879b93d511b353cd6e622f8f4a3fefc6d Mon Sep 17 00:00:00 2001 From: brainstorm Date: Tue, 24 Feb 2026 19:51:03 +0100 Subject: [PATCH 01/21] Skeleton for user-less password auth (all users are allowed because we don't really have (need?) users but password must be right). Cleanup of remainders of dynamic UART pins allocations, minor import organization --- src/config.rs | 4 +-- src/errors.rs | 3 +- src/serve.rs | 91 +++++++++++++++++++++++-------------------------- src/settings.rs | 2 +- 4 files changed, 47 insertions(+), 53 deletions(-) diff --git a/src/config.rs b/src/config.rs index bcd1521..45e3196 100644 --- a/src/config.rs +++ b/src/config.rs @@ -20,7 +20,7 @@ use sunset::{ sshwire::{SSHDecode, SSHEncode, SSHSink, SSHSource, WireError, WireResult}, }; -use crate::settings::{DEFAULT_SSID, DEFAULT_UART_RX_PIN, DEFAULT_UART_TX_PIN, KEY_SLOTS}; +use crate::settings::{DEFAULT_SSID, DEFAULT_UART_RX_PIN, DEFAULT_UART_TX_PIN, KEY_SLOTS, PASSWORD_AUTHENTICATION}; #[derive(Debug, PartialEq)] pub struct SSHStampConfig { @@ -88,7 +88,7 @@ impl SSHStampConfig { Ok(SSHStampConfig { hostkey, - password_authentication: true, + password_authentication: PASSWORD_AUTHENTICATION, admin_pw: None, admin_keys: Default::default(), wifi_ssid, diff --git a/src/errors.rs b/src/errors.rs index 8b69a87..7633a1b 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -5,8 +5,9 @@ pub type Result = result::Result; #[derive(Debug, Snafu)] pub enum Error { + #[snafu(display("Invalid PIN provided"))] InvalidPin, #[snafu(display("Flash storage error"))] FlashStorageError, -} +} \ No newline at end of file diff --git a/src/serve.rs b/src/serve.rs index e8011e0..c1b5772 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -2,31 +2,55 @@ // // SPDX-License-Identifier: GPL-3.0-or-later -use crate::config::SSHStampConfig; +// Espressif HAL +use esp_hal::system::software_reset; + +// Crate +use crate::errors::Error; +use crate::config::{PwHash, SSHStampConfig}; use crate::settings::UART_BUFFER_SIZE; +use crate::espressif::buffered_uart::UART_SIGNAL; use crate::store; +use storage::flash; + use core::option::Option::{self, None, Some}; use core::result::Result; -use storage::flash; +use heapless::String; + // Embassy -use embassy_sync::blocking_mutex::raw::NoopRawMutex; +use embassy_sync::blocking_mutex::raw::{ NoopRawMutex }; use embassy_sync::channel::Channel; -use embassy_sync::mutex::Mutex; -// use embedded_storage::Storage; -use esp_hal::system::software_reset; -use heapless::String; +use embassy_sync::mutex::{ Mutex }; + +// Sunset use sunset_async::SunsetMutex; -// use sunset::sshwire::SSHEncode; -use crate::espressif::buffered_uart::UART_SIGNAL; -use esp_println::{dbg, println}; -use sunset::{ChanHandle, ServEvent, error}; +use sunset::{ChanHandle, ServEvent, event::ServPasswordAuth, error}; use sunset_async::{ProgressHolder, SSHServer}; +use esp_println::{dbg, println}; + pub enum SessionType { Bridge(ChanHandle), Sftp(ChanHandle), } +pub async fn handle_password_auth(a: ServPasswordAuth<'_, '_>, passwd_hash: Option) -> Result<(), Error> { + let password = match a.password() { + Ok(u) => u, + Err(_) => return Ok(()), + }; + + let p = passwd_hash.unwrap(); + + if p.check(password) { + a.allow()? + } else { + a.reject()? + } + + Ok(()) +} + pub async fn connection_loop( serv: &SSHServer<'_>, chan_pipe: &Channel, @@ -36,7 +60,7 @@ pub async fn connection_loop( let mut session: Option = None; println!("Entering connection_loop and prog_loop is next..."); - let mut config_changed: bool = false; + let config_changed: bool = false; loop { let mut ph = ProgressHolder::new(); // dbg!("Waiting for ssh server event"); @@ -97,12 +121,14 @@ pub async fn connection_loop( ServEvent::Hostkeys(h) => { println!("ServEvent::Hostkeys"); let config_guard = config.lock().await; + // Just take it from config as private hostkey is generated on first boot. h.hostkeys(&[&config_guard.hostkey])?; } ServEvent::PasswordAuth(a) => { - println!("ServEvent::PasswordAuth"); - a.allow()?; - } + let config_guard = config.lock().await; + let admin_pw = config_guard.admin_pw.clone(); + handle_password_auth(a, admin_pw).await.unwrap(); + }, ServEvent::PubkeyAuth(a) => { println!("ServEvent::PubkeyAuth"); a.allow()?; @@ -130,37 +156,6 @@ pub async fn connection_loop( // config.change(c): Apply the config change to the relevant subsystem. // i.e: if UART_TX_PIN or UART_RX_PIN, we update the PinChannel with with_channel() to change pins live. match a.name()? { - "SAVE_CONFIG" => { - if a.value()? == "1" { - dbg!("Triggering config save..."); - todo!("Implement config save to flash"); - } - } - // If the env var is UART_TX_PIN or UART_RX_PIN - "UART_TX_PIN" => { - let val = a.value()?; - dbg!("Updating UART TX pin to ", val); - if let Ok(pin_num) = val.parse::() { - let mut config_lock = config.lock().await; - config_lock.uart_pins.tx = pin_num; - config_changed = true; - dbg!("TX pin updated"); - } else { - dbg!("Invalid TX pin value"); - } - } - "UART_RX_PIN" => { - let val = a.value()?; - dbg!("Updating UART RX pin to ", val); - if let Ok(pin_num) = val.parse::() { - let mut config_lock = config.lock().await; - config_lock.uart_pins.rx = pin_num; - config_changed = true; - dbg!("RX pin updated"); - } else { - dbg!("Invalid RX pin value"); - } - } _ => { dbg!("Unknown/unsupported ENV var"); } @@ -183,9 +178,7 @@ pub async fn connection_loop( println!("Expected caller to handle event"); error::BadUsage.fail()? } - ServEvent::PollAgain => { - // println!("ServEvent::PollAgain"); - } + ServEvent::PollAgain => {} _ => (), } } diff --git a/src/settings.rs b/src/settings.rs index 0b66f87..d2c9721 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -8,7 +8,7 @@ use core::net::Ipv4Addr; pub(crate) const DEFAULT_SSID: &str = "ssh-stamp"; //pub(crate) const SSH_SERVER_ID: &str = "SSH-2.0-ssh-stamp-0.1"; pub(crate) const KEY_SLOTS: usize = 1; // TODO: Document whether this a "reasonable default"? Justify why? -//pub(crate) const PASSWORD_AUTHENTICATION: bool = true; +pub(crate) const PASSWORD_AUTHENTICATION: bool = false; pub(crate) const DEFAULT_IP: &Ipv4Addr = &Ipv4Addr::new(192, 168, 4, 1); pub const UART_BUFFER_SIZE: usize = 4096; From 8987e305b7755371d0813ef32befe619fabadcd8 Mon Sep 17 00:00:00 2001 From: brainstorm Date: Sat, 28 Feb 2026 20:39:36 +0100 Subject: [PATCH 02/21] Simplify password check, place it inline on the event itself --- src/serve.rs | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/src/serve.rs b/src/serve.rs index c1b5772..d0f1ea1 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -6,8 +6,7 @@ use esp_hal::system::software_reset; // Crate -use crate::errors::Error; -use crate::config::{PwHash, SSHStampConfig}; +use crate::config::SSHStampConfig; use crate::settings::UART_BUFFER_SIZE; use crate::espressif::buffered_uart::UART_SIGNAL; use crate::store; @@ -24,7 +23,7 @@ use embassy_sync::mutex::{ Mutex }; // Sunset use sunset_async::SunsetMutex; -use sunset::{ChanHandle, ServEvent, event::ServPasswordAuth, error}; +use sunset::{ChanHandle, ServEvent, error}; use sunset_async::{ProgressHolder, SSHServer}; use esp_println::{dbg, println}; @@ -34,23 +33,6 @@ pub enum SessionType { Sftp(ChanHandle), } -pub async fn handle_password_auth(a: ServPasswordAuth<'_, '_>, passwd_hash: Option) -> Result<(), Error> { - let password = match a.password() { - Ok(u) => u, - Err(_) => return Ok(()), - }; - - let p = passwd_hash.unwrap(); - - if p.check(password) { - a.allow()? - } else { - a.reject()? - } - - Ok(()) -} - pub async fn connection_loop( serv: &SSHServer<'_>, chan_pipe: &Channel, @@ -125,9 +107,16 @@ pub async fn connection_loop( h.hostkeys(&[&config_guard.hostkey])?; } ServEvent::PasswordAuth(a) => { - let config_guard = config.lock().await; - let admin_pw = config_guard.admin_pw.clone(); - handle_password_auth(a, admin_pw).await.unwrap(); + if let Ok(password) = a.password() { + let mut config_guard = config.lock().await; + if config_guard.check_admin_pw(password) { + a.allow()? + } else { + a.reject()? + } + } else { + a.reject()? + } }, ServEvent::PubkeyAuth(a) => { println!("ServEvent::PubkeyAuth"); From c0450c11e51563dd19f5e0a28e5e359877e7fad8 Mon Sep 17 00:00:00 2001 From: brainstorm Date: Sat, 28 Feb 2026 20:39:50 +0100 Subject: [PATCH 03/21] fmt, clippy and gone are the README fat warnings about known issues. --- README.md | 16 +++++----------- src/config.rs | 4 +++- src/errors.rs | 3 +-- src/serve.rs | 21 ++++++++++++--------- 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index aff59ca..ea8660b 100644 --- a/README.md +++ b/README.md @@ -6,20 +6,10 @@ SPDX-License-Identifier: GPL-3.0-or-later # SSH Stamp -Sponsored by: - -![nlnet_zero_commons][nlnet_zero_commons] - -# ⚠️ WARNING: Pre-alpha PoC quality, DO NOT use in production. Currently contains highly unsafe business logic auth issues (both password and key management handlers need to be fixed). - -# ⚠️ WARNING: Do not file CVEs reports since deficiencies are very much known at this point in time and they'll be worked on soon as part of [this NLNet SSH-Stamp research and development grant][nlnet-grant] ;) - -Expect panics, lost bytes on the UART and other tricky UX issues, we are working on it, pull-requests are accepted too! +Your everyday SSH secured serial access. ## Description -Your everyday SSH secured serial access. - The **SSH Stamp** is a secure wireless to UART bridge implemented in Rust (no_std, no_alloc and no_unsafe whenever possible) with simplicity and robustness as its main design tenets. @@ -149,6 +139,10 @@ cargo install cargo-cyclonedx cargo cyclonedx -f json --manifest-path ./docs/ ``` +Sponsored by: + +![nlnet_zero_commons][nlnet_zero_commons] + [nlnet-grant]: https://nlnet.nl/project/SSH-Stamp/ [openwrt_mediatek_no_monitor]: https://github.com/openwrt/openwrt/issues/16279 [nlnet_zero_commons]: ./docs/nlnet/zero_commons_logo.svg diff --git a/src/config.rs b/src/config.rs index 45e3196..9401a59 100644 --- a/src/config.rs +++ b/src/config.rs @@ -20,7 +20,9 @@ use sunset::{ sshwire::{SSHDecode, SSHEncode, SSHSink, SSHSource, WireError, WireResult}, }; -use crate::settings::{DEFAULT_SSID, DEFAULT_UART_RX_PIN, DEFAULT_UART_TX_PIN, KEY_SLOTS, PASSWORD_AUTHENTICATION}; +use crate::settings::{ + DEFAULT_SSID, DEFAULT_UART_RX_PIN, DEFAULT_UART_TX_PIN, KEY_SLOTS, PASSWORD_AUTHENTICATION, +}; #[derive(Debug, PartialEq)] pub struct SSHStampConfig { diff --git a/src/errors.rs b/src/errors.rs index 7633a1b..8b69a87 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -5,9 +5,8 @@ pub type Result = result::Result; #[derive(Debug, Snafu)] pub enum Error { - #[snafu(display("Invalid PIN provided"))] InvalidPin, #[snafu(display("Flash storage error"))] FlashStorageError, -} \ No newline at end of file +} diff --git a/src/serve.rs b/src/serve.rs index d0f1ea1..a26746f 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -7,8 +7,8 @@ use esp_hal::system::software_reset; // Crate use crate::config::SSHStampConfig; -use crate::settings::UART_BUFFER_SIZE; use crate::espressif::buffered_uart::UART_SIGNAL; +use crate::settings::UART_BUFFER_SIZE; use crate::store; use storage::flash; @@ -17,13 +17,13 @@ use core::result::Result; use heapless::String; // Embassy -use embassy_sync::blocking_mutex::raw::{ NoopRawMutex }; +use embassy_sync::blocking_mutex::raw::NoopRawMutex; use embassy_sync::channel::Channel; -use embassy_sync::mutex::{ Mutex }; +use embassy_sync::mutex::Mutex; // Sunset -use sunset_async::SunsetMutex; use sunset::{ChanHandle, ServEvent, error}; +use sunset_async::SunsetMutex; use sunset_async::{ProgressHolder, SSHServer}; use esp_println::{dbg, println}; @@ -117,7 +117,7 @@ pub async fn connection_loop( } else { a.reject()? } - }, + } ServEvent::PubkeyAuth(a) => { println!("ServEvent::PubkeyAuth"); a.allow()?; @@ -144,10 +144,13 @@ pub async fn connection_loop( // // config.change(c): Apply the config change to the relevant subsystem. // i.e: if UART_TX_PIN or UART_RX_PIN, we update the PinChannel with with_channel() to change pins live. - match a.name()? { - _ => { - dbg!("Unknown/unsupported ENV var"); - } + // match a.name()? { + // _ => { + // dbg!("Unknown/unsupported ENV var"); + // } + a.name()?; + { + dbg!("Unsupported environment variable"); } // config.save(a): Potentially an optional special environment variable SAVE_CONFIG=1 From 045aad81bd4a6e0e898082099deb58c9bee08e9d Mon Sep 17 00:00:00 2001 From: brainstorm Date: Sun, 1 Mar 2026 21:19:29 +0100 Subject: [PATCH 04/21] [ci skip] Ignore usernames and rename admin_pw/keys for password and keys. SECURITY: Allow FirstAuth unconditionally by default for now until we find a better provisioning mechanism --- src/config.rs | 35 +++++++++++++++++------------------ src/serve.rs | 37 +++++++++++++++++++------------------ src/settings.rs | 3 ++- 3 files changed, 38 insertions(+), 37 deletions(-) diff --git a/src/config.rs b/src/config.rs index 9401a59..0630184 100644 --- a/src/config.rs +++ b/src/config.rs @@ -21,18 +21,17 @@ use sunset::{ }; use crate::settings::{ - DEFAULT_SSID, DEFAULT_UART_RX_PIN, DEFAULT_UART_TX_PIN, KEY_SLOTS, PASSWORD_AUTHENTICATION, + DEFAULT_SSID, DEFAULT_UART_RX_PIN, DEFAULT_UART_TX_PIN, KEY_SLOTS, PASSWORD_AUTH, }; #[derive(Debug, PartialEq)] pub struct SSHStampConfig { - //pub first_boot: bool, pub hostkey: SignKey, /// Authentication pub password_authentication: bool, - pub admin_pw: Option, - pub admin_keys: [Option; KEY_SLOTS], + pub password: Option, + pub pubkeys: [Option; KEY_SLOTS], /// WiFi pub wifi_ssid: String<32>, @@ -90,9 +89,9 @@ impl SSHStampConfig { Ok(SSHStampConfig { hostkey, - password_authentication: PASSWORD_AUTHENTICATION, - admin_pw: None, - admin_keys: Default::default(), + password_authentication: PASSWORD_AUTH, + password: None, + pubkeys: Default::default(), wifi_ssid, wifi_pw, mac, @@ -103,13 +102,13 @@ impl SSHStampConfig { }) } - pub fn set_admin_pw(&mut self, pw: Option<&str>) -> Result<()> { - self.admin_pw = pw.map(PwHash::new).transpose()?; + pub fn set_password(&mut self, pw: Option<&str>) -> Result<()> { + self.password = pw.map(PwHash::new).transpose()?; Ok(()) } - pub fn check_admin_pw(&mut self, pw: &str) -> bool { - if let Some(ref p) = self.admin_pw { + pub fn check_password(&mut self, pw: &str) -> bool { + if let Some(ref p) = self.password { p.check(pw) } else { false @@ -258,9 +257,9 @@ impl SSHEncode for SSHStampConfig { // Authentication self.password_authentication.enc(s)?; - enc_option(&self.admin_pw, s)?; + enc_option(&self.password, s)?; - for k in self.admin_keys.iter() { + for k in self.pubkeys.iter() { enc_option(k, s)?; } @@ -289,9 +288,9 @@ impl<'de> SSHDecode<'de> for SSHStampConfig { // Authentication let password_authentication = SSHDecode::dec(s)?; - let admin_pw = dec_option(s)?; - let mut admin_keys = [None; KEY_SLOTS]; - for k in admin_keys.iter_mut() { + let password = dec_option(s)?; + let mut pubkeys= [None; KEY_SLOTS]; + for k in pubkeys.iter_mut() { *k = dec_option(s)?; } @@ -313,8 +312,8 @@ impl<'de> SSHDecode<'de> for SSHStampConfig { Ok(Self { hostkey, password_authentication, - admin_pw, - admin_keys, + password, + pubkeys, wifi_ssid, wifi_pw, mac, diff --git a/src/serve.rs b/src/serve.rs index a26746f..446854e 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -8,7 +8,7 @@ use esp_hal::system::software_reset; // Crate use crate::config::SSHStampConfig; use crate::espressif::buffered_uart::UART_SIGNAL; -use crate::settings::UART_BUFFER_SIZE; +use crate::settings::{PASSWORD_AUTH, PUBKEY_AUTH, UART_BUFFER_SIZE}; use crate::store; use storage::flash; @@ -38,7 +38,7 @@ pub async fn connection_loop( chan_pipe: &Channel, config: &SunsetMutex, ) -> Result<(), sunset::Error> { - let username = Mutex::::new(String::<20>::new()); + //let username = Mutex::::new(String::<20>::new()); let mut session: Option = None; println!("Entering connection_loop and prog_loop is next..."); @@ -93,12 +93,16 @@ pub async fn connection_loop( a.fail()?; } } - ServEvent::FirstAuth(ref a) => { + ServEvent::FirstAuth(mut a) => { println!("ServEvent::FirstAuth"); // record the username - if username.lock().await.push_str(a.username()?).is_err() { - println!("Too long username") - } + // if username.lock().await.push_str(a.username()?).is_err() { + // println!("Too long username") + // } + a.enable_password_auth(PASSWORD_AUTH)?; + a.enable_pubkey_auth(PUBKEY_AUTH)?; + a.allow()?; // SECURITY: Controversial (but necessary?) + // to provision client pubkeys to device? } ServEvent::Hostkeys(h) => { println!("ServEvent::Hostkeys"); @@ -109,7 +113,7 @@ pub async fn connection_loop( ServEvent::PasswordAuth(a) => { if let Ok(password) = a.password() { let mut config_guard = config.lock().await; - if config_guard.check_admin_pw(password) { + if config_guard.check_password(password) { a.allow()? } else { a.reject()? @@ -118,9 +122,15 @@ pub async fn connection_loop( a.reject()? } } - ServEvent::PubkeyAuth(a) => { + ServEvent::PubkeyAuth(_a) => { println!("ServEvent::PubkeyAuth"); - a.allow()?; + // let mut config_guard = config.lock().await; + // let client_pubkey = a.pubkey()?; + // if config_guard.pubkeys.iter().any(|k| k.as_ref() == Some(&client_pubkey)) { + // a.allow()?; + // } else { + // a.reject()?; + // } } ServEvent::OpenSession(a) => { println!("ServEvent::OpenSession"); @@ -139,15 +149,6 @@ pub async fn connection_loop( dbg!(a.name()?); dbg!(a.value()?); - // TODO: Logic to serialise/validate env vars? I.e: - // a.name.validate(); // Checks the input variable, sanitizes, assigns a target subsystem - // - // config.change(c): Apply the config change to the relevant subsystem. - // i.e: if UART_TX_PIN or UART_RX_PIN, we update the PinChannel with with_channel() to change pins live. - // match a.name()? { - // _ => { - // dbg!("Unknown/unsupported ENV var"); - // } a.name()?; { dbg!("Unsupported environment variable"); diff --git a/src/settings.rs b/src/settings.rs index d2c9721..7132a66 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -8,7 +8,8 @@ use core::net::Ipv4Addr; pub(crate) const DEFAULT_SSID: &str = "ssh-stamp"; //pub(crate) const SSH_SERVER_ID: &str = "SSH-2.0-ssh-stamp-0.1"; pub(crate) const KEY_SLOTS: usize = 1; // TODO: Document whether this a "reasonable default"? Justify why? -pub(crate) const PASSWORD_AUTHENTICATION: bool = false; +pub(crate) const PASSWORD_AUTH: bool = false; +pub(crate) const PUBKEY_AUTH: bool = true; pub(crate) const DEFAULT_IP: &Ipv4Addr = &Ipv4Addr::new(192, 168, 4, 1); pub const UART_BUFFER_SIZE: usize = 4096; From 90933222498e825bec9f262364d2899e5eed8962 Mon Sep 17 00:00:00 2001 From: brainstorm Date: Tue, 3 Mar 2026 15:05:08 +0100 Subject: [PATCH 05/21] Implement storage of Ed25519 pubkeys into SSHStampConfig over ssh env vars and PubKeyAuth checking, testing on hardware needed... --- src/config.rs | 32 +++++++++++++++++++++++++++++-- src/serve.rs | 52 +++++++++++++++++++++++++++++++++++---------------- 2 files changed, 66 insertions(+), 18 deletions(-) diff --git a/src/config.rs b/src/config.rs index 0630184..1c9a578 100644 --- a/src/config.rs +++ b/src/config.rs @@ -14,7 +14,7 @@ use sha2::Sha256; use subtle::ConstantTimeEq; use sunset::packets::Ed25519PubKey; -use sunset::{KeyType, Result, sshwire}; +use sunset::{Error, KeyType, Result, sshwire}; use sunset::{ SignKey, sshwire::{SSHDecode, SSHEncode, SSHSink, SSHSource, WireError, WireResult}, @@ -121,6 +121,34 @@ impl SSHStampConfig { s } + pub(crate) fn add_pubkey(&mut self, key_str: &str) -> Result<(), Error> { + // Accept OpenSSH public key format (e.g. "ssh-ed25519 AAAA...") and + // validate it is an Ed25519 key. Insert into the first empty slot or + // overwrite slot 0 if none empty. + let openssh = ssh_key::PublicKey::from_openssh(key_str) + .map_err(|_| Error::msg("Unsupported public key format"))?; + + match openssh.key_data() { + ssh_key::public::KeyData::Ed25519(k) => { + let bytes = k.0; // [u8; 32] + let newk = Ed25519PubKey { + key: sunset::sshwire::Blob(bytes), + }; + + for slot in self.pubkeys.iter_mut() { + if slot.is_none() { + *slot = Some(newk); + return Ok(()); + } + } + + // No empty slot: overwrite the first one. + self.pubkeys[0] = Some(newk); + Ok(()) + } + _ => Err(Error::msg("Not an Ed25519 public key")), + } + } // pub fn config_change(&mut self, conf: SSHConfig) -> Result<()> { // ServEvent::ConfigChange(); // } @@ -289,7 +317,7 @@ impl<'de> SSHDecode<'de> for SSHStampConfig { // Authentication let password_authentication = SSHDecode::dec(s)?; let password = dec_option(s)?; - let mut pubkeys= [None; KEY_SLOTS]; + let mut pubkeys = [None; KEY_SLOTS]; for k in pubkeys.iter_mut() { *k = dec_option(s)?; } diff --git a/src/serve.rs b/src/serve.rs index 446854e..9e4727b 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -14,12 +14,11 @@ use storage::flash; use core::option::Option::{self, None, Some}; use core::result::Result; -use heapless::String; +use log::warn; // Embassy use embassy_sync::blocking_mutex::raw::NoopRawMutex; use embassy_sync::channel::Channel; -use embassy_sync::mutex::Mutex; // Sunset use sunset::{ChanHandle, ServEvent, error}; @@ -95,14 +94,14 @@ pub async fn connection_loop( } ServEvent::FirstAuth(mut a) => { println!("ServEvent::FirstAuth"); - // record the username + // SECURITY: We have all and no users, let's confuse the audit tools? :) // if username.lock().await.push_str(a.username()?).is_err() { // println!("Too long username") // } a.enable_password_auth(PASSWORD_AUTH)?; a.enable_pubkey_auth(PUBKEY_AUTH)?; a.allow()?; // SECURITY: Controversial (but necessary?) - // to provision client pubkeys to device? + // to provision client pubkeys to device? } ServEvent::Hostkeys(h) => { println!("ServEvent::Hostkeys"); @@ -122,15 +121,27 @@ pub async fn connection_loop( a.reject()? } } - ServEvent::PubkeyAuth(_a) => { + ServEvent::PubkeyAuth(a) => { println!("ServEvent::PubkeyAuth"); - // let mut config_guard = config.lock().await; - // let client_pubkey = a.pubkey()?; - // if config_guard.pubkeys.iter().any(|k| k.as_ref() == Some(&client_pubkey)) { - // a.allow()?; - // } else { - // a.reject()?; - // } + let config_guard = config.lock().await; + let client_pubkey = a.pubkey()?; + + let allowed = config_guard.pubkeys.iter().any(|slot| { + if let Some(stored) = slot.as_ref() { + match &client_pubkey { + sunset::packets::PubKey::Ed25519(presented) => stored == presented, + _ => false, + } + } else { + false + } + }); + + if allowed { + a.allow()?; + } else { + a.reject()?; + } } ServEvent::OpenSession(a) => { println!("ServEvent::OpenSession"); @@ -149,16 +160,25 @@ pub async fn connection_loop( dbg!(a.name()?); dbg!(a.value()?); - a.name()?; - { - dbg!("Unsupported environment variable"); + match a.name()? { + "pub_key" => { + let mut config_guard = config.lock().await; + if config_guard.add_pubkey(a.value()?).is_ok() { + a.succeed()?; + } else { + a.fail()?; + } + } + _ => { + warn!("Unsupported environment variable"); + } } // config.save(a): Potentially an optional special environment variable SAVE_CONFIG=1 // that serialises current config to flash // Only save once all ENV requests have been recorded? - a.succeed()?; + //a.succeed()?; } ServEvent::SessionPty(a) => { println!("ServEvent::SessionPty"); From 68f2df315dd305e6329d20e0f799067b4ee4860a Mon Sep 17 00:00:00 2001 From: brainstorm Date: Tue, 3 Mar 2026 22:51:50 +0100 Subject: [PATCH 06/21] [ci skip] add_pubkey() failing on from_openssh() method... --- src/config.rs | 25 +++++++++++++++---------- src/errors.rs | 8 ++++++++ src/serve.rs | 12 ++++++++++-- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/src/config.rs b/src/config.rs index 1c9a578..7a33fd2 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,3 +1,4 @@ +use core::fmt::Debug; use core::net::Ipv4Addr; #[cfg(feature = "ipv6")] use core::net::Ipv6Addr; @@ -6,20 +7,20 @@ use embassy_net::{Ipv4Cidr, StaticConfigV4}; use embassy_net::{Ipv6Cidr, StaticConfigV6}; use heapless::String; -use esp_println::dbg; - use bcrypt; use hmac::{Hmac, Mac}; +use log::{ info, warn, debug }; use sha2::Sha256; use subtle::ConstantTimeEq; use sunset::packets::Ed25519PubKey; -use sunset::{Error, KeyType, Result, sshwire}; +use sunset::{KeyType, Result, sshwire}; use sunset::{ SignKey, sshwire::{SSHDecode, SSHEncode, SSHSink, SSHSource, WireError, WireResult}, }; +use crate::errors::Error; use crate::settings::{ DEFAULT_SSID, DEFAULT_UART_RX_PIN, DEFAULT_UART_TX_PIN, KEY_SLOTS, PASSWORD_AUTH, }; @@ -125,8 +126,12 @@ impl SSHStampConfig { // Accept OpenSSH public key format (e.g. "ssh-ed25519 AAAA...") and // validate it is an Ed25519 key. Insert into the first empty slot or // overwrite slot 0 if none empty. - let openssh = ssh_key::PublicKey::from_openssh(key_str) - .map_err(|_| Error::msg("Unsupported public key format"))?; + + info!("Checking pubkey string passed through ENV: {}", key_str); + + let openssh = ssh_key::PublicKey::from_openssh(key_str)?; + + info!("Public key format valid, continuing to parse"); match openssh.key_data() { ssh_key::public::KeyData::Ed25519(k) => { @@ -135,6 +140,7 @@ impl SSHStampConfig { key: sunset::sshwire::Blob(bytes), }; + info!("Parsed Ed25519 public key, adding to config"); for slot in self.pubkeys.iter_mut() { if slot.is_none() { *slot = Some(newk); @@ -142,16 +148,15 @@ impl SSHStampConfig { } } + warn!("Public key slots full, overwriting the first one"); + // TODO SECURITY: Remove this fallback after FirstAuth. // No empty slot: overwrite the first one. self.pubkeys[0] = Some(newk); Ok(()) } - _ => Err(Error::msg("Not an Ed25519 public key")), + _ => Err(Error::BadKey), } } - // pub fn config_change(&mut self, conf: SSHConfig) -> Result<()> { - // ServEvent::ConfigChange(); - // } } fn random_mac() -> Result<[u8; 6]> { @@ -209,7 +214,7 @@ fn enc_ipv4_config(v: &Option, s: &mut dyn SSHSink) -> WireResul v.is_some().enc(s)?; if let Some(v) = v { v.address.address().to_bits().enc(s)?; - dbg!("enc_ipv4_config: prefix", &v.address.prefix_len()); + debug!("enc_ipv4_config: prefix {}", &v.address.prefix_len()); v.address.prefix_len().enc(s)?; // to u32 let gw = v.gateway.map(|a| a.to_bits()); diff --git a/src/errors.rs b/src/errors.rs index 8b69a87..6baddc6 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -9,4 +9,12 @@ pub enum Error { InvalidPin, #[snafu(display("Flash storage error"))] FlashStorageError, + BadKey, + OpenSSHParseError, +} + +impl From for Error { + fn from(_e: ssh_key::Error) -> Error { + Error::OpenSSHParseError + } } diff --git a/src/serve.rs b/src/serve.rs index 9e4727b..a43a5e4 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -14,7 +14,7 @@ use storage::flash; use core::option::Option::{self, None, Some}; use core::result::Result; -use log::warn; +use log::{info, warn}; // Embassy use embassy_sync::blocking_mutex::raw::NoopRawMutex; @@ -161,16 +161,24 @@ pub async fn connection_loop( dbg!(a.value()?); match a.name()? { - "pub_key" => { + "LANG" => { + // Ignore, but succeed to avoid client-side warnings + // This env variable will always be sent by OpenSSH client. + a.succeed()?; + }, + "SSH_PUBKEY" => { let mut config_guard = config.lock().await; if config_guard.add_pubkey(a.value()?).is_ok() { + info!("Added new pubkey from ENV"); a.succeed()?; } else { + warn!("Failed to add new pubkey from ENV"); a.fail()?; } } _ => { warn!("Unsupported environment variable"); + a.fail()?; } } From c49c995451fdfb4cb566bd77cd194d4ba5732b2f Mon Sep 17 00:00:00 2001 From: brainstorm Date: Thu, 5 Mar 2026 20:18:16 +0100 Subject: [PATCH 07/21] Cleanup after more stringent imports and vars policy --- src/config.rs | 3 +-- src/serve.rs | 20 +++++++------------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/config.rs b/src/config.rs index aee9746..41d615f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,4 +1,4 @@ -use log::{debug, info}; +use log::{debug, info, warn}; use core::net::Ipv4Addr; #[cfg(feature = "ipv6")] @@ -10,7 +10,6 @@ use heapless::String; use bcrypt; use hmac::{Hmac, Mac}; -use log::{ info, warn, debug }; use sha2::Sha256; use subtle::ConstantTimeEq; diff --git a/src/serve.rs b/src/serve.rs index cc5802b..a0fc640 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: GPL-3.0-or-later -use log::{debug, info}; +use log::{debug, info, warn}; use crate::config::SSHStampConfig; use crate::espressif::buffered_uart::UART_SIGNAL; @@ -12,23 +12,18 @@ use storage::flash; use core::option::Option::{self, None, Some}; use core::result::Result; -use log::{info, warn}; // Embassy use embassy_sync::blocking_mutex::raw::NoopRawMutex; use embassy_sync::channel::Channel; -use embassy_sync::mutex::Mutex; -// use embedded_storage::Storage; use esp_hal::system::software_reset; -use heapless::String; -use sunset_async::SunsetMutex; -// use sunset::sshwire::SSHEncode; -use crate::espressif::buffered_uart::UART_SIGNAL; + +// Sunset use sunset::{ChanHandle, ServEvent, error}; use sunset_async::SunsetMutex; use sunset_async::{ProgressHolder, SSHServer}; -use esp_println::{dbg, println}; +use esp_println::println; pub enum SessionType { Bridge(ChanHandle), @@ -49,7 +44,6 @@ pub async fn connection_loop( let mut ph = ProgressHolder::new(); let ev = serv.progress(&mut ph).await?; // debug!(&ev); - #[allow(unreachable_patterns)] match ev { // #[cfg(feature = "sftp-ota")] ServEvent::SessionSubsystem(a) => { @@ -73,6 +67,7 @@ pub async fn connection_loop( if let Some(ch) = session.take() { // Save config after connection successful (SessionEnv completed) if config_changed { + config_changed = false; // TODO: Avoid unnecessary "does not neet to be mutable" warnings for now let config_guard = config.lock().await; let Some(flash_storage_guard) = flash::get_flash_n_buffer() else { panic!("Could not acquire flash storage lock"); @@ -167,7 +162,7 @@ pub async fn connection_loop( // Ignore, but succeed to avoid client-side warnings // This env variable will always be sent by OpenSSH client. a.succeed()?; - }, + } "SSH_PUBKEY" => { let mut config_guard = config.lock().await; if config_guard.add_pubkey(a.value()?).is_ok() { @@ -197,14 +192,13 @@ pub async fn connection_loop( ServEvent::SessionExec(a) => { a.fail()?; } - ServEvent::Defunct | ServEvent::SessionShell(_) => { + ServEvent::Defunct => { info!("Expected caller to handle event"); error::BadUsage.fail()? } ServEvent::PollAgain => { // info!("ServEvent::PollAgain"); } - _ => (), } } } From dfd01aeec4be5c15f3a1bae2ad0e55373ab750e5 Mon Sep 17 00:00:00 2001 From: brainstorm Date: Thu, 5 Mar 2026 21:59:47 +0100 Subject: [PATCH 08/21] Fixed input pubkey validation (from_str instead of from_openssh)... next up is making sure that pubkey is only added on FirstAuth and FirstBoot --- src/config.rs | 5 +++-- src/serve.rs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/config.rs b/src/config.rs index 41d615f..e9adcc2 100644 --- a/src/config.rs +++ b/src/config.rs @@ -3,6 +3,7 @@ use log::{debug, info, warn}; use core::net::Ipv4Addr; #[cfg(feature = "ipv6")] use core::net::Ipv6Addr; +use core::str::FromStr; use embassy_net::{Ipv4Cidr, StaticConfigV4}; #[cfg(feature = "ipv6")] use embassy_net::{Ipv6Cidr, StaticConfigV6}; @@ -128,7 +129,7 @@ impl SSHStampConfig { info!("Checking pubkey string passed through ENV: {}", key_str); - let openssh = ssh_key::PublicKey::from_openssh(key_str)?; + let openssh = ssh_key::PublicKey::from_str(key_str)?; info!("Public key format valid, continuing to parse"); @@ -148,7 +149,7 @@ impl SSHStampConfig { } warn!("Public key slots full, overwriting the first one"); - // TODO SECURITY: Remove this fallback after FirstAuth. + // TODO SECURITY: Remove this fallback after FirstAuth ON FIRST BOOT ONLY. // No empty slot: overwrite the first one. self.pubkeys[0] = Some(newk); Ok(()) diff --git a/src/serve.rs b/src/serve.rs index a0fc640..c55ba80 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -90,7 +90,7 @@ pub async fn connection_loop( } } ServEvent::FirstAuth(mut a) => { - println!("ServEvent::FirstAuth"); + info!("ServEvent::FirstAuth"); // SECURITY: We have all and no users, let's confuse the audit tools? :) // if username.lock().await.push_str(a.username()?).is_err() { // println!("Too long username") From 9a9ab6a37277f9c74a2e01517ee5aad786d8f0f9 Mon Sep 17 00:00:00 2001 From: brainstorm Date: Fri, 6 Mar 2026 20:18:26 +0100 Subject: [PATCH 09/21] Simplify further: remove all password-related logic and ONLY support Public Key authentication, support only most secure scenarios --- src/config.rs | 147 ++++++------------------------------------------ src/serve.rs | 50 +++++++++------- src/settings.rs | 4 +- 3 files changed, 49 insertions(+), 152 deletions(-) diff --git a/src/config.rs b/src/config.rs index e9adcc2..eb14456 100644 --- a/src/config.rs +++ b/src/config.rs @@ -9,30 +9,21 @@ use embassy_net::{Ipv4Cidr, StaticConfigV4}; use embassy_net::{Ipv6Cidr, StaticConfigV6}; use heapless::String; -use bcrypt; -use hmac::{Hmac, Mac}; -use sha2::Sha256; -use subtle::ConstantTimeEq; - use sunset::packets::Ed25519PubKey; -use sunset::{KeyType, Result, sshwire}; +use sunset::{KeyType, Result}; use sunset::{ SignKey, sshwire::{SSHDecode, SSHEncode, SSHSink, SSHSource, WireError, WireResult}, }; use crate::errors::Error; -use crate::settings::{ - DEFAULT_SSID, DEFAULT_UART_RX_PIN, DEFAULT_UART_TX_PIN, KEY_SLOTS, PASSWORD_AUTH, -}; +use crate::settings::{DEFAULT_SSID, DEFAULT_UART_RX_PIN, DEFAULT_UART_TX_PIN, KEY_SLOTS}; #[derive(Debug, PartialEq)] pub struct SSHStampConfig { pub hostkey: SignKey, - /// Authentication - pub password_authentication: bool, - pub password: Option, + /// Authentication: only pubkey-based auth supported pub pubkeys: [Option; KEY_SLOTS], /// WiFi @@ -49,6 +40,8 @@ pub struct SSHStampConfig { pub ipv6_static: Option, /// UART pub uart_pins: UartPins, + /// True until the device has been provisioned for the first time. + pub first_boot: bool, } #[derive(Debug, PartialEq)] @@ -69,7 +62,7 @@ impl Default for UartPins { impl SSHStampConfig { /// Bump this when the format changes - pub const CURRENT_VERSION: u8 = 6; + pub const CURRENT_VERSION: u8 = 8; /// Creates a new config with default parameters. /// @@ -77,7 +70,6 @@ impl SSHStampConfig { pub fn new() -> Result { let hostkey = SignKey::generate(KeyType::Ed25519, None)?; - // TODO: Those env events come from system's std::env / core::env (if any)... so it shouldn't be unsafe() let wifi_ssid = Self::default_ssid(); let mac = random_mac()?; let wifi_pw = None; @@ -88,10 +80,11 @@ impl SSHStampConfig { uart_pins.rx, uart_pins.tx ); + // No password config fields nor logic: only pubkey auth supported. + // Leave password fields out (except wifi one). + Ok(SSHStampConfig { hostkey, - password_authentication: PASSWORD_AUTH, - password: None, pubkeys: Default::default(), wifi_ssid, wifi_pw, @@ -100,21 +93,11 @@ impl SSHStampConfig { #[cfg(feature = "ipv6")] ipv6_static: None, uart_pins, + first_boot: true, }) } - pub fn set_password(&mut self, pw: Option<&str>) -> Result<()> { - self.password = pw.map(PwHash::new).transpose()?; - Ok(()) - } - - pub fn check_password(&mut self, pw: &str) -> bool { - if let Some(ref p) = self.password { - p.check(pw) - } else { - false - } - } + // Password functions removed; pubkey-only auth supported. pub(crate) fn default_ssid() -> String<32> { let mut s = String::<32>::new(); @@ -288,10 +271,6 @@ impl SSHEncode for SSHStampConfig { fn enc(&self, s: &mut dyn SSHSink) -> WireResult<()> { enc_signkey(&self.hostkey, s)?; - // Authentication - self.password_authentication.enc(s)?; - enc_option(&self.password, s)?; - for k in self.pubkeys.iter() { enc_option(k, s)?; } @@ -308,6 +287,9 @@ impl SSHEncode for SSHStampConfig { self.uart_pins.rx.enc(s)?; self.uart_pins.tx.enc(s)?; + // Persist first-boot marker + self.first_boot.enc(s)?; + Ok(()) } } @@ -319,9 +301,6 @@ impl<'de> SSHDecode<'de> for SSHStampConfig { { let hostkey = dec_signkey(s)?; - // Authentication - let password_authentication = SSHDecode::dec(s)?; - let password = dec_option(s)?; let mut pubkeys = [None; KEY_SLOTS]; for k in pubkeys.iter_mut() { *k = dec_option(s)?; @@ -342,10 +321,10 @@ impl<'de> SSHDecode<'de> for SSHStampConfig { let tx: u8 = SSHDecode::dec(s)?; let uart_pins = UartPins { rx, tx }; + let first_boot = SSHDecode::dec(s)?; + Ok(Self { hostkey, - password_authentication, - password, pubkeys, wifi_ssid, wifi_pw, @@ -354,97 +333,7 @@ impl<'de> SSHDecode<'de> for SSHStampConfig { #[cfg(feature = "ipv6")] ipv6_static, uart_pins, + first_boot, }) } -} - -/// Stores a bcrypt password hash. -/// -/// We use bcrypt because it seems the best password hashing option where -/// memory hardness isn't possible (the rp2040 is smaller than CPU or GPU memory). -/// -/// The cost is currently set to 6, taking ~500ms on a 125mhz rp2040. -/// Time converges to roughly 8.6ms * 2**cost -/// -/// Passwords are pre-hashed to avoid bcrypt's 72 byte limit. -/// rust-bcrypt allows nulls in passwords. -/// We use an hmac rather than plain hash to avoid password shucking -/// (an attacker bcrypts known hashes from some other breach, then -/// brute forces the weaker hash for any that match). -//#[derive(Clone, SSHEncode, SSHDecode, PartialEq)] -#[derive(Clone, PartialEq)] -pub struct PwHash { - salt: [u8; 16], - hash: [u8; 24], - cost: u8, -} - -impl PwHash { - const COST: u8 = 6; - /// `pw` must not be empty. - pub fn new(pw: &str) -> Result { - if pw.is_empty() { - return sunset::error::BadUsage.fail(); - } - - let mut salt = [0u8; 16]; - sunset::random::fill_random(&mut salt)?; - let prehash = Self::prehash(pw, &salt); - let cost = Self::COST; - let hash = bcrypt::bcrypt(cost as u32, salt, &prehash); - Ok(Self { salt, hash, cost }) - } - - pub fn check(&self, pw: &str) -> bool { - if pw.is_empty() { - return false; - } - let prehash = Self::prehash(pw, &self.salt); - let check_hash = bcrypt::bcrypt(self.cost as u32, self.salt, &prehash); - check_hash.ct_eq(&self.hash).into() - } - - fn prehash(pw: &str, salt: &[u8]) -> [u8; 32] { - // OK unwrap: can't fail, accepts any length - // TODO: Generalise, not only Espressif esp_hal - let mut prehash = Hmac::::new_from_slice(salt).unwrap(); - prehash.update(pw.as_bytes()); - prehash.finalize().into_bytes().into() - } -} - -impl core::fmt::Debug for PwHash { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - f.debug_struct("PwHash").finish_non_exhaustive() - } -} - -impl SSHEncode for PwHash { - fn enc(&self, s: &mut dyn SSHSink) -> WireResult<()> { - self.salt.enc(s)?; - self.hash.enc(s)?; - self.cost.enc(s) - } -} - -impl<'de> SSHDecode<'de> for PwHash { - fn dec(s: &mut S) -> WireResult - where - S: SSHSource<'de>, - { - let salt = <[u8; 16]>::dec(s)?; - let hash = <[u8; 24]>::dec(s)?; - let cost = u8::dec(s)?; - Ok(PwHash { salt, hash, cost }) - } -} - -pub fn roundtrip_config() { - // default config - let c1 = SSHStampConfig::new().unwrap(); - let mut buf = [0u8; 1000]; - let l = sshwire::write_ssh(&mut buf, &c1).unwrap(); - let v = &buf[..l]; - let c2: SSHStampConfig = sshwire::read_ssh(v, None).unwrap(); - assert_eq!(c1, c2); -} +} \ No newline at end of file diff --git a/src/serve.rs b/src/serve.rs index c55ba80..d9e18e5 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -6,7 +6,7 @@ use log::{debug, info, warn}; use crate::config::SSHStampConfig; use crate::espressif::buffered_uart::UART_SIGNAL; -use crate::settings::{PASSWORD_AUTH, PUBKEY_AUTH, UART_BUFFER_SIZE}; +use crate::settings::{PUBKEY_AUTH, UART_BUFFER_SIZE}; use crate::store; use storage::flash; @@ -53,7 +53,7 @@ pub async fn connection_loop( debug_assert!(ch.num() == a.channel()); a.succeed()?; - info!("We got SFTP subsystem"); + info!("SFTP subsystem is up and running"); let _ = chan_pipe.try_send(SessionType::Sftp(ch)); } else { a.fail()?; @@ -91,14 +91,21 @@ pub async fn connection_loop( } ServEvent::FirstAuth(mut a) => { info!("ServEvent::FirstAuth"); - // SECURITY: We have all and no users, let's confuse the audit tools? :) - // if username.lock().await.push_str(a.username()?).is_err() { - // println!("Too long username") - // } - a.enable_password_auth(PASSWORD_AUTH)?; - a.enable_pubkey_auth(PUBKEY_AUTH)?; - a.allow()?; // SECURITY: Controversial (but necessary?) - // to provision client pubkeys to device? + // Allow the "first auth" behaviour only on first-boot-like configs. + // Consider the device in first-boot state when there is no password + // and no stored client pubkeys. + let config_guard = config.lock().await; + + if config_guard.first_boot{ + // SECURITY: We have no users; enable pubkey auth so the + // provisioner can add a key. + a.enable_pubkey_auth(PUBKEY_AUTH)?; + a.allow()?; // SECURITY: Controversial (but necessary?) + } else { + // Not first boot: do not auto-allow; reject the first-auth helper. + info!("FirstAuth received but not first-boot; rejecting"); + a.reject()?; + } } ServEvent::Hostkeys(h) => { info!("ServEvent::Hostkeys"); @@ -107,16 +114,8 @@ pub async fn connection_loop( h.hostkeys(&[&config_guard.hostkey])?; } ServEvent::PasswordAuth(a) => { - if let Ok(password) = a.password() { - let mut config_guard = config.lock().await; - if config_guard.check_password(password) { - a.allow()? - } else { - a.reject()? - } - } else { - a.reject()? - } + // Password auth is not supported; always reject. + a.reject()?; } ServEvent::PubkeyAuth(a) => { println!("ServEvent::PubkeyAuth"); @@ -165,9 +164,18 @@ pub async fn connection_loop( } "SSH_PUBKEY" => { let mut config_guard = config.lock().await; - if config_guard.add_pubkey(a.value()?).is_ok() { + // Only allow adding a pubkey via ENV on first-boot-like configs. + + if !config_guard.first_boot { + warn!("SSH_PUBKEY env received but not first-boot; rejecting"); + a.fail()?; + } else if config_guard.add_pubkey(a.value()?).is_ok() { info!("Added new pubkey from ENV"); a.succeed()?; + // Mark that config has changed and clear first_boot so + // future connections are not treated as first-boot. + config_guard.first_boot = false; + config_changed = true; } else { warn!("Failed to add new pubkey from ENV"); a.fail()?; diff --git a/src/settings.rs b/src/settings.rs index 1c2e07c..8178cad 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -8,14 +8,14 @@ use core::net::Ipv4Addr; pub(crate) const DEFAULT_SSID: &str = "ssh-stamp"; //pub(crate) const SSH_SERVER_ID: &str = "SSH-2.0-ssh-stamp-0.1"; pub(crate) const KEY_SLOTS: usize = 1; // TODO: Document whether this a "reasonable default"? Justify why? -pub(crate) const PASSWORD_AUTH: bool = false; pub(crate) const PUBKEY_AUTH: bool = true; pub(crate) const DEFAULT_IP: &Ipv4Addr = &Ipv4Addr::new(192, 168, 4, 1); -pub const UART_BUFFER_SIZE: usize = 4096; // UART settings //pub(crate) const BAUD_RATE: u32 = 115200; //pub(crate) const UART_SETTINGS: &str = "8N1"; +pub const UART_BUFFER_SIZE: usize = 4096; + cfg_if::cfg_if!( // if #[cfg(feature = "esp32")] { if #[cfg(feature = "esp32")] { From a6ace13ff1c63108d50ae8ab1cc772d07fbe514e Mon Sep 17 00:00:00 2001 From: brainstorm Date: Fri, 6 Mar 2026 21:25:43 +0100 Subject: [PATCH 10/21] Change env var to SSH_STAMP_PUBKEY /cc @jubeormk1 --- src/config.rs | 7 +++---- src/serve.rs | 6 +++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/config.rs b/src/config.rs index eb14456..69233d6 100644 --- a/src/config.rs +++ b/src/config.rs @@ -80,7 +80,7 @@ impl SSHStampConfig { uart_pins.rx, uart_pins.tx ); - // No password config fields nor logic: only pubkey auth supported. + // No password config fields nor logic: only pubkey auth supported. // Leave password fields out (except wifi one). Ok(SSHStampConfig { @@ -132,8 +132,7 @@ impl SSHStampConfig { } warn!("Public key slots full, overwriting the first one"); - // TODO SECURITY: Remove this fallback after FirstAuth ON FIRST BOOT ONLY. - // No empty slot: overwrite the first one. + // SECURITY: Allow this on FirstAuth ON FIRST BOOT ONLY. self.pubkeys[0] = Some(newk); Ok(()) } @@ -336,4 +335,4 @@ impl<'de> SSHDecode<'de> for SSHStampConfig { first_boot, }) } -} \ No newline at end of file +} diff --git a/src/serve.rs b/src/serve.rs index d9e18e5..d74c709 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -96,7 +96,7 @@ pub async fn connection_loop( // and no stored client pubkeys. let config_guard = config.lock().await; - if config_guard.first_boot{ + if config_guard.first_boot { // SECURITY: We have no users; enable pubkey auth so the // provisioner can add a key. a.enable_pubkey_auth(PUBKEY_AUTH)?; @@ -162,12 +162,12 @@ pub async fn connection_loop( // This env variable will always be sent by OpenSSH client. a.succeed()?; } - "SSH_PUBKEY" => { + "SSH_STAMP_PUBKEY" => { let mut config_guard = config.lock().await; // Only allow adding a pubkey via ENV on first-boot-like configs. if !config_guard.first_boot { - warn!("SSH_PUBKEY env received but not first-boot; rejecting"); + warn!("SSH_STAMP_PUBKEY env received but not first-boot; rejecting"); a.fail()?; } else if config_guard.add_pubkey(a.value()?).is_ok() { info!("Added new pubkey from ENV"); From c38cc7af65961847717a3858fd3f7ebe51fb642d Mon Sep 17 00:00:00 2001 From: brainstorm Date: Fri, 6 Mar 2026 21:43:00 +0100 Subject: [PATCH 11/21] Deny password auth overall, allow pubkey_auth selectively --- src/serve.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/serve.rs b/src/serve.rs index eed4552..aa68347 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -104,14 +104,19 @@ pub async fn connection_loop( // and no stored client pubkeys. let config_guard = config.lock().await; + // Disable password auth method regardless. + a.enable_password_auth(false)?; if config_guard.first_boot { // SECURITY: We have no users; enable pubkey auth so the // provisioner can add a key. a.enable_pubkey_auth(PUBKEY_AUTH)?; - a.allow()?; // SECURITY: Controversial (but necessary?) + a.allow()?; // SECURITY: Controversial (but necessary to provision?) } else { // Not first boot: do not auto-allow; reject the first-auth helper. - info!("FirstAuth received but not first-boot; rejecting"); + info!( + "FirstAuth received but not first-boot; rejecting any pubkey auth attempts" + ); + a.enable_pubkey_auth(false)?; a.reject()?; } } From 692a852291138c984da50be273cfcc456905479b Mon Sep 17 00:00:00 2001 From: brainstorm Date: Sat, 7 Mar 2026 13:58:15 +0100 Subject: [PATCH 12/21] [ci skip] We're off-by-one on the presented pubkey (see last 89 vs 114)... INFO - Stored pubkey: [37, 176, 11, 96, 150, 182, 247, 189, 220, 173, 1, 185, 181, 116, 129, 254, 248, 146, 252, 195, 27, 213, 140, 229, 39, 215, 0, 7, 43, 251, 151, 89] INFO - Presented pubkey: [37, 176, 11, 96, 150, 182, 247, 189, 220, 173, 1, 185, 181, 116, 129, 254, 248, 146, 252, 195, 27, 213, 140, 229, 39, 215, 0, 7, 43, 251, 151, 114] --- src/serve.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/serve.rs b/src/serve.rs index aa68347..1f5322b 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -23,8 +23,6 @@ use sunset::{ChanHandle, ServEvent, error}; use sunset_async::SunsetMutex; use sunset_async::{ProgressHolder, SSHServer}; -use esp_println::println; - pub enum SessionType { Bridge(ChanHandle), #[cfg(feature = "sftp-ota")] @@ -114,9 +112,10 @@ pub async fn connection_loop( } else { // Not first boot: do not auto-allow; reject the first-auth helper. info!( - "FirstAuth received but not first-boot; rejecting any pubkey auth attempts" + "FirstAuth received but not first-boot, allowing pubkey auth but rejecting + additions of new public keys on already provisioned device" ); - a.enable_pubkey_auth(false)?; + a.enable_pubkey_auth(PUBKEY_AUTH)?; a.reject()?; } } @@ -131,22 +130,25 @@ pub async fn connection_loop( a.reject()?; } ServEvent::PubkeyAuth(a) => { - println!("ServEvent::PubkeyAuth"); + info!("ServEvent::PubkeyAuth"); let config_guard = config.lock().await; let client_pubkey = a.pubkey()?; - let allowed = config_guard.pubkeys.iter().any(|slot| { + if config_guard.pubkeys.iter().any(|slot| { if let Some(stored) = slot.as_ref() { match &client_pubkey { - sunset::packets::PubKey::Ed25519(presented) => stored == presented, + sunset::packets::PubKey::Ed25519(presented) => { + info!("Stored pubkey: {:?}", stored.key.0); + info!("Presented pubkey: {:?}", presented.key.0); + + stored == presented + }, _ => false, } } else { false } - }); - - if allowed { + }) { a.allow()?; } else { a.reject()?; @@ -182,6 +184,7 @@ pub async fn connection_loop( if !config_guard.first_boot { warn!("SSH_STAMP_PUBKEY env received but not first-boot; rejecting"); a.fail()?; + continue; } else if config_guard.add_pubkey(a.value()?).is_ok() { info!("Added new pubkey from ENV"); a.succeed()?; From 5a5447d54db11e8ae0698ddf59428c43fdf3322c Mon Sep 17 00:00:00 2001 From: brainstorm Date: Sat, 7 Mar 2026 14:21:01 +0100 Subject: [PATCH 13/21] A few too many software_reset() on the HSM when auth goes south for my taste, but will do for now... most obvious auth flows covered, but needs much closer inspection and refinement, specially at the HSM flow level /cc @Autofix --- src/config.rs | 7 +++++-- src/serve.rs | 36 ++++++++++++++++++++---------------- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/config.rs b/src/config.rs index 69233d6..8a8559f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -110,9 +110,12 @@ impl SSHStampConfig { // validate it is an Ed25519 key. Insert into the first empty slot or // overwrite slot 0 if none empty. - info!("Checking pubkey string passed through ENV: {}", key_str); + info!( + "Checking pubkey string passed through ENV: {}", + key_str.trim() + ); - let openssh = ssh_key::PublicKey::from_str(key_str)?; + let openssh = ssh_key::PublicKey::from_str(key_str.trim())?; info!("Public key format valid, continuing to parse"); diff --git a/src/serve.rs b/src/serve.rs index 1f5322b..27af046 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -134,24 +134,26 @@ pub async fn connection_loop( let config_guard = config.lock().await; let client_pubkey = a.pubkey()?; - if config_guard.pubkeys.iter().any(|slot| { - if let Some(stored) = slot.as_ref() { - match &client_pubkey { - sunset::packets::PubKey::Ed25519(presented) => { - info!("Stored pubkey: {:?}", stored.key.0); - info!("Presented pubkey: {:?}", presented.key.0); + match client_pubkey { + sunset::packets::PubKey::Ed25519(presented) => { + let matched = config_guard + .pubkeys + .iter() + .any(|slot| slot.as_ref().is_some_and(|stored| *stored == presented)); - stored == presented - }, - _ => false, + if matched { + a.allow()?; + } else { + info!("No matching pubkey slot found"); + a.reject()?; + software_reset(); // TODO: Handle better HSM-flow-wise. } - } else { - false } - }) { - a.allow()?; - } else { - a.reject()?; + _ => { + // Only Ed25519 keys supported + a.reject()?; + software_reset(); // TODO: Handle better HSM-flow-wise. + } } } ServEvent::OpenSession(a) => { @@ -184,7 +186,7 @@ pub async fn connection_loop( if !config_guard.first_boot { warn!("SSH_STAMP_PUBKEY env received but not first-boot; rejecting"); a.fail()?; - continue; + break Ok(()); // TODO: Do better HSM-flow-wise } else if config_guard.add_pubkey(a.value()?).is_ok() { info!("Added new pubkey from ENV"); a.succeed()?; @@ -192,6 +194,8 @@ pub async fn connection_loop( // future connections are not treated as first-boot. config_guard.first_boot = false; config_changed = true; + // Don't immediately allow the new user/key to establish bridge but reboot first? + //software_reset(); // TODO: Do better HSM-flow-wise. } else { warn!("Failed to add new pubkey from ENV"); a.fail()?; From 766bf8aa365d55d9ae9af7ec62834b8f6b452e2b Mon Sep 17 00:00:00 2001 From: jubeormk1 Date: Mon, 9 Mar 2026 16:06:37 +1100 Subject: [PATCH 14/21] feature: No unauthenticated Shell or Subsystem With this change, only a Shell or Subsystem (sftp-ota) can be started unless the user has set a authentication key. This way we only allow secure uses of ssh-stamp by default --- src/serve.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/serve.rs b/src/serve.rs index 27af046..62d5f9a 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -47,6 +47,16 @@ pub async fn connection_loop( // #[cfg(feature = "sftp-ota")] ServEvent::SessionSubsystem(a) => { info!("ServEvent::SessionSubsystem"); + { + let mut config_guard = config.lock().await; + if config_guard.first_boot { + warn!("Unauthenticated SessionSubsystem rejected"); + a.fail()?; + // TODO: Handle this gracefully + // TODO: Provide a message back to the client and the close the session? + software_reset(); + } + } if a.command()?.to_lowercase().as_str() == "sftp" { if let Some(ch) = session.take() { debug_assert!(ch.num() == a.channel()); @@ -70,6 +80,16 @@ pub async fn connection_loop( } ServEvent::SessionShell(a) => { info!("ServEvent::SessionShell"); + { + let mut config_guard = config.lock().await; + if config_guard.first_boot { + warn!("Unauthenticated SessionShell rejected"); + a.fail()?; + // TODO: Handle this gracefully + // TODO: Provide a message back to the client and the close the session? + software_reset(); + } + } if let Some(ch) = session.take() { // Save config after connection successful (SessionEnv completed) if config_changed { From fb38bf16ce15f98c49f7b6b5c06c7bdaef6fc0a2 Mon Sep 17 00:00:00 2001 From: jubeormk1 Date: Mon, 9 Mar 2026 16:18:32 +1100 Subject: [PATCH 15/21] Fixing CI failure: Removing mut in config guard Apologies, I should have checked the warnings. --- src/serve.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/serve.rs b/src/serve.rs index 62d5f9a..fde7a83 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -48,7 +48,7 @@ pub async fn connection_loop( ServEvent::SessionSubsystem(a) => { info!("ServEvent::SessionSubsystem"); { - let mut config_guard = config.lock().await; + let config_guard = config.lock().await; if config_guard.first_boot { warn!("Unauthenticated SessionSubsystem rejected"); a.fail()?; @@ -81,7 +81,7 @@ pub async fn connection_loop( ServEvent::SessionShell(a) => { info!("ServEvent::SessionShell"); { - let mut config_guard = config.lock().await; + let config_guard = config.lock().await; if config_guard.first_boot { warn!("Unauthenticated SessionShell rejected"); a.fail()?; From 9023eb3a9fe7e29adcc6c1ac868737b53ee1ed0f Mon Sep 17 00:00:00 2001 From: brainstorm Date: Mon, 9 Mar 2026 20:52:28 +0100 Subject: [PATCH 16/21] Address @jubeormk1 PR review points, except the HSM ones as they'll require a bit more feedback from @Autofix, I reckon? --- src/main.rs | 3 --- src/serve.rs | 21 +++++++++------------ src/settings.rs | 1 - 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/src/main.rs b/src/main.rs index 2be296c..e593bb3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -98,9 +98,6 @@ async fn main(spawner: Spawner) -> ! { panic!("Could not acquire flash storage lock"); }; let mut flash_storage = flash_storage_guard.lock().await; - // TODO: Migrate this function/test to embedded-test. - // Quick roundtrip test for SSHStampConfig - // ssh_stamp::config::roundtrip_config(); ssh_stamp::store::load_or_create(&mut flash_storage).await } .expect("Could not load or create SSHStampConfig"); diff --git a/src/serve.rs b/src/serve.rs index fde7a83..bbc6821 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -2,11 +2,11 @@ // // SPDX-License-Identifier: GPL-3.0-or-later -use log::{debug, info, warn}; +use log::{debug, info, trace, warn}; use crate::config::SSHStampConfig; use crate::espressif::buffered_uart::UART_SIGNAL; -use crate::settings::{PUBKEY_AUTH, UART_BUFFER_SIZE}; +use crate::settings::UART_BUFFER_SIZE; use crate::store; use storage::flash; @@ -34,7 +34,6 @@ pub async fn connection_loop( chan_pipe: &Channel, config: &SunsetMutex, ) -> Result<(), sunset::Error> { - //let username = Mutex::::new(String::<20>::new()); let mut session: Option = None; debug!("Entering connection_loop and prog_loop is next..."); @@ -42,7 +41,8 @@ pub async fn connection_loop( loop { let mut ph = ProgressHolder::new(); let ev = serv.progress(&mut ph).await?; - // debug!(&ev); + + trace!("{:?}", &ev); match ev { // #[cfg(feature = "sftp-ota")] ServEvent::SessionSubsystem(a) => { @@ -99,9 +99,6 @@ pub async fn connection_loop( panic!("Could not acquire flash storage lock"); }; let mut flash_storage = flash_storage_guard.lock().await; - // TODO: Migrate this function/test to embedded-test. - // Quick roundtrip test for SSHStampConfig - // ssh_stamp::config::roundtrip_config(); let _result = store::save(&mut flash_storage, &config_guard).await; } debug_assert!(ch.num() == a.channel()); @@ -124,10 +121,11 @@ pub async fn connection_loop( // Disable password auth method regardless. a.enable_password_auth(false)?; + + // SECURITY: We have no users; enable pubkey auth so the + // provisioner can add a key. + a.enable_pubkey_auth(true)?; if config_guard.first_boot { - // SECURITY: We have no users; enable pubkey auth so the - // provisioner can add a key. - a.enable_pubkey_auth(PUBKEY_AUTH)?; a.allow()?; // SECURITY: Controversial (but necessary to provision?) } else { // Not first boot: do not auto-allow; reject the first-auth helper. @@ -135,7 +133,6 @@ pub async fn connection_loop( "FirstAuth received but not first-boot, allowing pubkey auth but rejecting additions of new public keys on already provisioned device" ); - a.enable_pubkey_auth(PUBKEY_AUTH)?; a.reject()?; } } @@ -146,7 +143,7 @@ pub async fn connection_loop( h.hostkeys(&[&config_guard.hostkey])?; } ServEvent::PasswordAuth(a) => { - // Password auth is not supported; always reject. + warn!("Password auth is not supported, use public key auth instead."); a.reject()?; } ServEvent::PubkeyAuth(a) => { diff --git a/src/settings.rs b/src/settings.rs index 8178cad..f3bdef4 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -8,7 +8,6 @@ use core::net::Ipv4Addr; pub(crate) const DEFAULT_SSID: &str = "ssh-stamp"; //pub(crate) const SSH_SERVER_ID: &str = "SSH-2.0-ssh-stamp-0.1"; pub(crate) const KEY_SLOTS: usize = 1; // TODO: Document whether this a "reasonable default"? Justify why? -pub(crate) const PUBKEY_AUTH: bool = true; pub(crate) const DEFAULT_IP: &Ipv4Addr = &Ipv4Addr::new(192, 168, 4, 1); // UART settings From 39772d2568527e9b3912218bd4c3be2fa0bb209d Mon Sep 17 00:00:00 2001 From: jubeormk1 Date: Tue, 10 Mar 2026 14:32:39 +1100 Subject: [PATCH 17/21] fix: Splitting channel fixes issue closing the session on channel EOF Before now we where cloning the stdio for the shell channel. We used one stdio to read and the other (stdio2) for writing. This causes some internal issue in sunset and trying to read on a channel that has received an EOF would not return Ok(0) or error. Splitting the stdio instead of cloning fixes the behavior. for the shake of exploring the situation I have tried the next combinations: - cloning stdio, reading from the cloned stdio: No Ok(0) - cloning stdio and splitting it. Using split stdio for read/write: No Ok(0) In summary, when cloned, we loose the Ok(0) expected after the channel has received EOF This time I have checked the warnings so the CI should be happy ;-) --- src/serve.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/serve.rs b/src/serve.rs index bbc6821..d325964 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -271,7 +271,6 @@ pub async fn ssh_disable() -> () { use crate::espressif::buffered_uart::BufferedUart; use crate::serial::serial_bridge; -use sunset_async::ChanInOut; pub async fn handle_ssh_client<'a, 'b>( uart_buff: &'a BufferedUart, @@ -284,10 +283,9 @@ pub async fn handle_ssh_client<'a, 'b>( match session_type { SessionType::Bridge(ch) => { info!("Handling bridge session"); - let stdio: ChanInOut<'_> = ssh_server.stdio(ch).await?; - let stdio2 = stdio.clone(); + let (stdin, stdout) = ssh_server.stdio(ch).await?.split(); info!("Starting bridge"); - serial_bridge(stdio, stdio2, uart_buff).await? + serial_bridge(stdin, stdout, uart_buff).await? } #[cfg(feature = "sftp-ota")] SessionType::Sftp(ch) => { From 9ef5d396781e0c3373dbed89b9010ed15ce6e297 Mon Sep 17 00:00:00 2001 From: brainstorm Date: Tue, 10 Mar 2026 20:55:43 +0100 Subject: [PATCH 18/21] First pass at addressing @jubeormk1 PR feedback while https://github.com/Autofix/ssh-stamp/commit/6fe6b2ad29d2b8f58562dd70a5719946d29d3c89 gets fully addressed by @Autofix, wip/broken ATM --- src/espressif/net.rs | 2 -- src/serve.rs | 39 +++++++++++++++++++-------------------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/espressif/net.rs b/src/espressif/net.rs index 899c23d..73135ea 100644 --- a/src/espressif/net.rs +++ b/src/espressif/net.rs @@ -103,8 +103,6 @@ pub async fn ap_stack_disable() -> () { pub async fn tcp_socket_disable() -> () { // drop tcp stack info!("TCP socket disabled"); - // TODO: Correctly disable/restart tcp socket and/or send messsage to user over SSH - software_reset(); } pub async fn accept_requests<'a>( diff --git a/src/serve.rs b/src/serve.rs index d325964..31d312a 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -43,20 +43,21 @@ pub async fn connection_loop( let ev = serv.progress(&mut ph).await?; trace!("{:?}", &ev); + + // Guard for authentication state + let is_authed = false; + match ev { - // #[cfg(feature = "sftp-ota")] ServEvent::SessionSubsystem(a) => { info!("ServEvent::SessionSubsystem"); - { - let config_guard = config.lock().await; - if config_guard.first_boot { - warn!("Unauthenticated SessionSubsystem rejected"); - a.fail()?; - // TODO: Handle this gracefully - // TODO: Provide a message back to the client and the close the session? - software_reset(); - } + if is_authed { + info!("User authenticated, session granted"); + a.succeed(); + } else { + info!("User not authenticated, session denied"); + a.fail(); } + if a.command()?.to_lowercase().as_str() == "sftp" { if let Some(ch) = session.take() { debug_assert!(ch.num() == a.channel()); @@ -80,16 +81,14 @@ pub async fn connection_loop( } ServEvent::SessionShell(a) => { info!("ServEvent::SessionShell"); - { - let config_guard = config.lock().await; - if config_guard.first_boot { - warn!("Unauthenticated SessionShell rejected"); - a.fail()?; - // TODO: Handle this gracefully - // TODO: Provide a message back to the client and the close the session? - software_reset(); - } + if is_authed { + info!("User authenticated, shell granted"); + a.succeed(); + } else { + info!("User not authenticated, shell denied"); + a.fail(); } + if let Some(ch) = session.take() { // Save config after connection successful (SessionEnv completed) if config_changed { @@ -160,6 +159,7 @@ pub async fn connection_loop( if matched { a.allow()?; + is_authed = true; } else { info!("No matching pubkey slot found"); a.reject()?; @@ -212,7 +212,6 @@ pub async fn connection_loop( config_guard.first_boot = false; config_changed = true; // Don't immediately allow the new user/key to establish bridge but reboot first? - //software_reset(); // TODO: Do better HSM-flow-wise. } else { warn!("Failed to add new pubkey from ENV"); a.fail()?; From 9757277aa14882162b7b6db06f4d3078e709c858 Mon Sep 17 00:00:00 2001 From: brainstorm Date: Wed, 11 Mar 2026 21:05:37 +0100 Subject: [PATCH 19/21] evert "First pass at addressing @jubeormk1 PR feedback while https://github.com/Autofix/ssh-stamp/commit/6fe6b2ad29d2b8f58562dd70a5719946d29d3c89 gets fully addressed by @Autofix, wip/broken ATM" This reverts commit 9ef5d396781e0c3373dbed89b9010ed15ce6e297. --- src/espressif/net.rs | 2 ++ src/serve.rs | 39 ++++++++++++++++++++------------------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/espressif/net.rs b/src/espressif/net.rs index 73135ea..899c23d 100644 --- a/src/espressif/net.rs +++ b/src/espressif/net.rs @@ -103,6 +103,8 @@ pub async fn ap_stack_disable() -> () { pub async fn tcp_socket_disable() -> () { // drop tcp stack info!("TCP socket disabled"); + // TODO: Correctly disable/restart tcp socket and/or send messsage to user over SSH + software_reset(); } pub async fn accept_requests<'a>( diff --git a/src/serve.rs b/src/serve.rs index 31d312a..d325964 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -43,21 +43,20 @@ pub async fn connection_loop( let ev = serv.progress(&mut ph).await?; trace!("{:?}", &ev); - - // Guard for authentication state - let is_authed = false; - match ev { + // #[cfg(feature = "sftp-ota")] ServEvent::SessionSubsystem(a) => { info!("ServEvent::SessionSubsystem"); - if is_authed { - info!("User authenticated, session granted"); - a.succeed(); - } else { - info!("User not authenticated, session denied"); - a.fail(); + { + let config_guard = config.lock().await; + if config_guard.first_boot { + warn!("Unauthenticated SessionSubsystem rejected"); + a.fail()?; + // TODO: Handle this gracefully + // TODO: Provide a message back to the client and the close the session? + software_reset(); + } } - if a.command()?.to_lowercase().as_str() == "sftp" { if let Some(ch) = session.take() { debug_assert!(ch.num() == a.channel()); @@ -81,14 +80,16 @@ pub async fn connection_loop( } ServEvent::SessionShell(a) => { info!("ServEvent::SessionShell"); - if is_authed { - info!("User authenticated, shell granted"); - a.succeed(); - } else { - info!("User not authenticated, shell denied"); - a.fail(); + { + let config_guard = config.lock().await; + if config_guard.first_boot { + warn!("Unauthenticated SessionShell rejected"); + a.fail()?; + // TODO: Handle this gracefully + // TODO: Provide a message back to the client and the close the session? + software_reset(); + } } - if let Some(ch) = session.take() { // Save config after connection successful (SessionEnv completed) if config_changed { @@ -159,7 +160,6 @@ pub async fn connection_loop( if matched { a.allow()?; - is_authed = true; } else { info!("No matching pubkey slot found"); a.reject()?; @@ -212,6 +212,7 @@ pub async fn connection_loop( config_guard.first_boot = false; config_changed = true; // Don't immediately allow the new user/key to establish bridge but reboot first? + //software_reset(); // TODO: Do better HSM-flow-wise. } else { warn!("Failed to add new pubkey from ENV"); a.fail()?; From a58357614305811b23dd0f9b06db589afcd280dc Mon Sep 17 00:00:00 2001 From: jubeormk1 Date: Thu, 12 Mar 2026 17:10:38 +1100 Subject: [PATCH 20/21] Fixing issue in 9ef5d396781e0c3373dbed89b9010ed15ce6e297 The software_reset() calls were masking an issue handling the borrow of `a` in the ev match for `ServEvent::SessionSubsystem(a)` and `ServEvent::SessionShell(a)` The changes in this commit make the borrow exclusive using a simple if else if else cases handling. As addition I have reduced the scope of the config_guard to check the first_boot to a minimum Handling the auth on first boot and subsequent boots. The behavior is: - First boot: - Good key provided: Store key and open session - Other: No session. bye - Not first boot (client pub key in config): - Correct key: session open - Other: No session. bye --- src/serve.rs | 69 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 41 insertions(+), 28 deletions(-) diff --git a/src/serve.rs b/src/serve.rs index d325964..c5d018d 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -10,6 +10,7 @@ use crate::settings::UART_BUFFER_SIZE; use crate::store; use storage::flash; +use core::fmt::Debug; use core::option::Option::{self, None, Some}; use core::result::Result; @@ -23,6 +24,7 @@ use sunset::{ChanHandle, ServEvent, error}; use sunset_async::SunsetMutex; use sunset_async::{ProgressHolder, SSHServer}; +#[derive(Debug)] pub enum SessionType { Bridge(ChanHandle), #[cfg(feature = "sftp-ota")] @@ -38,6 +40,10 @@ pub async fn connection_loop( debug!("Entering connection_loop and prog_loop is next..."); let mut config_changed: bool = false; + + // Will be set in `ev` PubkeyAuth is accepted and cleared once the channel is sent down chan_pipe + let mut auth_checked = false; + loop { let mut ph = ProgressHolder::new(); let ev = serv.progress(&mut ph).await?; @@ -47,24 +53,24 @@ pub async fn connection_loop( // #[cfg(feature = "sftp-ota")] ServEvent::SessionSubsystem(a) => { info!("ServEvent::SessionSubsystem"); - { - let config_guard = config.lock().await; - if config_guard.first_boot { - warn!("Unauthenticated SessionSubsystem rejected"); - a.fail()?; - // TODO: Handle this gracefully - // TODO: Provide a message back to the client and the close the session? - software_reset(); - } - } - if a.command()?.to_lowercase().as_str() == "sftp" { + + if !auth_checked { + warn!("Unauthenticated SessionSubsystem rejected"); + a.fail()?; + // TODO: Handle this gracefully + // TODO: Provide a message back to the client and the close the session? + //software_reset(); + } else if a.command()?.to_lowercase().as_str() == "sftp" { if let Some(ch) = session.take() { debug_assert!(ch.num() == a.channel()); #[cfg(feature = "sftp-ota")] { a.succeed()?; info!("We got SFTP subsystem"); - let _ = chan_pipe.try_send(SessionType::Sftp(ch)); + match chan_pipe.try_send(SessionType::Sftp(ch)) { + Ok(_) => auth_checked = false, + Err(e) => error!("Could not send the channel: {:?}", e), + }; } #[cfg(not(feature = "sftp-ota"))] { @@ -74,23 +80,18 @@ pub async fn connection_loop( } else { a.fail()?; } - } else { - a.fail()?; } } ServEvent::SessionShell(a) => { info!("ServEvent::SessionShell"); - { - let config_guard = config.lock().await; - if config_guard.first_boot { - warn!("Unauthenticated SessionShell rejected"); - a.fail()?; - // TODO: Handle this gracefully - // TODO: Provide a message back to the client and the close the session? - software_reset(); - } - } - if let Some(ch) = session.take() { + + if !auth_checked { + warn!("Unauthenticated SessionShell rejected"); + a.fail()?; + // TODO: Handle this gracefully + // TODO: Provide a message back to the client and the close the session? + //software_reset(); + } else if let Some(ch) = session.take() { // Save config after connection successful (SessionEnv completed) if config_changed { config_changed = false; // TODO: Avoid unnecessary "does not neet to be mutable" warnings for now @@ -107,7 +108,10 @@ pub async fn connection_loop( // Signal for uart task to configure pins and run. Value is irrelevant. UART_SIGNAL.signal(1); info!("Connection loop: UART_SIGNAL sent"); - let _ = chan_pipe.try_send(SessionType::Bridge(ch)); + match chan_pipe.try_send(SessionType::Bridge(ch)) { + Ok(_) => auth_checked = false, + Err(e) => log::error!("Could not send the channel: {:?}", e), + }; } else { a.fail()?; } @@ -160,6 +164,7 @@ pub async fn connection_loop( if matched { a.allow()?; + auth_checked = true; } else { info!("No matching pubkey slot found"); a.reject()?; @@ -211,6 +216,7 @@ pub async fn connection_loop( // future connections are not treated as first-boot. config_guard.first_boot = false; config_changed = true; + auth_checked = true; // Don't immediately allow the new user/key to establish bridge but reboot first? //software_reset(); // TODO: Do better HSM-flow-wise. } else { @@ -231,8 +237,15 @@ pub async fn connection_loop( //a.succeed()?; } ServEvent::SessionPty(a) => { - info!("ServEvent::SessionPty"); - a.succeed()?; + let first_boot = { config.lock().await.first_boot }; + + if auth_checked || first_boot { + info!("ServEvent::SessionPty: Session granted"); + a.succeed()?; + } else { + info!("ServEvent::SessionPty: No auth not session"); + a.fail()?; + } } ServEvent::SessionExec(a) => { a.fail()?; From 10b9d0bcacaf0a990a32fcda959cefa6be4bf7cd Mon Sep 17 00:00:00 2001 From: jubeormk1 Date: Thu, 12 Mar 2026 17:28:09 +1100 Subject: [PATCH 21/21] Removing unnecessary software_resets keeping one in main The software resets where masking borrowing issues. I have removed them and added a WIP message. In the future it would be desirable having the FSM dealing with the depth of the system closed instead of reverting each state until it get back to main. Maybe a return value in main.rs::bridge_connected select3 requesting the right level of steps back? - session closed/bad auth: could stay in that function - config changes that imply hardware: Reset or better granularity - server finishes: Not sure. Reset until decided --- src/espressif/buffered_uart.rs | 7 ++----- src/espressif/net.rs | 10 ++++------ src/main.rs | 5 +++-- src/serve.rs | 17 ++++++++--------- 4 files changed, 17 insertions(+), 22 deletions(-) diff --git a/src/espressif/buffered_uart.rs b/src/espressif/buffered_uart.rs index 9ae63d7..84231cb 100644 --- a/src/espressif/buffered_uart.rs +++ b/src/espressif/buffered_uart.rs @@ -16,7 +16,6 @@ use embassy_sync::{blocking_mutex::raw::CriticalSectionRawMutex, pipe::Pipe}; use esp_hal::Async; use esp_hal::gpio::AnyPin; use esp_hal::peripherals::UART1; -use esp_hal::system::software_reset; use esp_hal::uart::{Config, RxConfig, Uart}; use portable_atomic::{AtomicUsize, Ordering}; use static_cell::StaticCell; @@ -133,17 +132,15 @@ impl Default for BufferedUart { pub async fn uart_buffer_disable() -> () { // disable uart buffer - info!("UART buffer disabled"); + info!("UART buffer disabled: WIP"); // TODO: Correctly disable/restart UART buffer and/or send messsage to user over SSH - software_reset(); } // use embassy_sync::blocking_mutex::raw::CriticalSectionRawMutex; pub async fn uart_disable() -> () { // disable uart - info!("UART disabled"); + info!("UART disabled: WIP"); // TODO: Correctly disable/restart UART and/or send messsage to user over SSH - software_reset(); } /// UART pins for the buffered UART task. diff --git a/src/espressif/net.rs b/src/espressif/net.rs index 899c23d..0aa136e 100644 --- a/src/espressif/net.rs +++ b/src/espressif/net.rs @@ -21,7 +21,6 @@ use embassy_net::{Stack, StackResources, tcp::TcpSocket}; use embassy_time::{Duration, Timer}; use esp_hal::peripherals::WIFI; use esp_hal::rng::Rng; -use esp_hal::system::software_reset; use esp_radio::Controller; use esp_radio::wifi::WifiEvent; use esp_radio::wifi::{AccessPointConfig, ModeConfig, WifiApState, WifiController}; @@ -95,16 +94,14 @@ pub async fn if_up( pub async fn ap_stack_disable() -> () { // drop ap_stack - info!("AP Stack disabled"); + info!("AP Stack disabled: WIP"); // TODO: Correctly disable/restart AP Stack and/or send messsage to user over SSH - software_reset(); } pub async fn tcp_socket_disable() -> () { // drop tcp stack - info!("TCP socket disabled"); + info!("TCP socket disabled: WIP"); // TODO: Correctly disable/restart tcp socket and/or send messsage to user over SSH - software_reset(); } pub async fn accept_requests<'a>( @@ -186,7 +183,8 @@ pub async fn wifi_controller_disable() -> () { // drop wifi controller // esp_wifi::deinit_unchecked() // wifi_controller.deinit_unchecked() - software_reset(); + info!("Disabling wifi: WIP"); + //software_reset(); } use esp_radio::wifi::WifiDevice; diff --git a/src/main.rs b/src/main.rs index e593bb3..43692bd 100644 --- a/src/main.rs +++ b/src/main.rs @@ -50,7 +50,7 @@ cfg_if::cfg_if! { pub async fn peripherals_disable() -> () { // drop peripherals - software_reset(); + info!("Disabling peripherals: WIP"); } pub struct SshStampInit<'a> { @@ -188,7 +188,8 @@ async fn main(spawner: Spawner) -> ! { } peripherals_disable().await; - // loop{} + // loop {} + log::warn!("End of Main... Reset!!"); software_reset(); } diff --git a/src/serve.rs b/src/serve.rs index c5d018d..b9da76f 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -17,7 +17,6 @@ use core::result::Result; // Embassy use embassy_sync::blocking_mutex::raw::NoopRawMutex; use embassy_sync::channel::Channel; -use esp_hal::system::software_reset; // Sunset use sunset::{ChanHandle, ServEvent, error}; @@ -168,13 +167,13 @@ pub async fn connection_loop( } else { info!("No matching pubkey slot found"); a.reject()?; - software_reset(); // TODO: Handle better HSM-flow-wise. + //software_reset(); // TODO: Handle better HSM-flow-wise. } } _ => { // Only Ed25519 keys supported a.reject()?; - software_reset(); // TODO: Handle better HSM-flow-wise. + //software_reset(); // TODO: Handle better HSM-flow-wise. } } } @@ -263,9 +262,9 @@ pub async fn connection_loop( pub async fn connection_disable() -> () { // disable connection loop - info!("Connection loop disabled"); + info!("Connection loop disabled: WIP"); // TODO: Correctly disable/restart Conection loop and/or send messsage to user over SSH - software_reset(); + // software_reset(); } pub async fn ssh_wait_for_initialisation<'server>( @@ -277,9 +276,9 @@ pub async fn ssh_wait_for_initialisation<'server>( pub async fn ssh_disable() -> () { // drop ssh server - info!("SSH Server disabled"); + info!("SSH Server disabled: WIP"); // TODO: Correctly disable/restart SSH Server and/or send messsage to user over SSH - software_reset(); + // software_reset(); } use crate::espressif::buffered_uart::BufferedUart; @@ -316,7 +315,7 @@ pub async fn handle_ssh_client<'a, 'b>( pub async fn bridge_disable() -> () { // disable bridge - info!("Bridge disabled"); + info!("Bridge disabled: WIP"); // TODO: Correctly disable/restart bridge and/or send message to user over SSH - software_reset(); + // software_reset(); }