Skip to content

HSM refactor#61

Merged
brainstorm merged 92 commits intobrainstorm:mainfrom
Autofix:ssh-stamp-hsm
Feb 22, 2026
Merged

HSM refactor#61
brainstorm merged 92 commits intobrainstorm:mainfrom
Autofix:ssh-stamp-hsm

Conversation

@Autofix
Copy link
Copy Markdown
Collaborator

@Autofix Autofix commented Feb 20, 2026

This PR is for the big refactor required by the HSM (Hierachical State Machine). Fixes #25

Summary of changes:

  1. main.rs now contains the HSM which steps through each state so we can be certain of what has been run and configured at each point. Each state calls out to other functions and will only proceed to the next state once a valid result has been returned.
  2. Currently the device will software reset if any error is encountered.
  3. UART is only established after making an ssh connection to first read the required UART pins from ssh environment variables.
  4. pins.rs has been removed and the pin config is instead stored as u8 in config.rs. Pins are only setup once after reading the value from the config. This occurs in the uart embassy task which has been moved from main.rs to buffered_uart.rs
  5. src/storage.rs renamed to store.rs to avoid naming conflict with workspace.

Additionally this PR is built upon:

brainstorm and others added 30 commits June 1, 2025 16:12
…assumptions around mutexes and rename Fl/FlashCo fig struct names
…onfigure the UART's pins at runtime (possible?) and also pass the config object around (safely/mutex-ing) too
…t a CriticalSectionRawMutex for the SSHConfig. Next up: figure out non-unsafe() ways to steal() pins for the UART pins
…channel, needs more work/thought... also PeripheralRef is not present in 1.0.0-beta.1
Co-authored-by: Marko Malenic <mmalenic1@gmail.com>
… call is now required. The partition checking/handling functionalities might be relevant to @jubeormk1 too, btw
…with the UART operation, need to debug further
…X/TX/CTS/RTS... etc.. pin assignment and at-runtime validation et al

Co-authored-by: Marko Malenic <mmalenic1@gmail.com>
…erialization/deserialization "on the wire" of SSHConfig, unfortunately /cc @mmalenic
Introduced custom Errors
Introduced env vars and started refactoring accordingly.

Config and peripherals are borrow checker challenges.

Co-authored-by: Marko Malenic <mmalenic1@gmail.com>
….. PinConfig no longer needs a new() method but OTOH resolve_pin() requires a bit more rework (or remove it altogether?)
Co-authored-by: Marko Malenic <mmalenic1@gmail.com>
… PinConfig::new()... Also channel cannot send at the end of uart_task() because there's an infinite loop right before.

Co-authored-by: Marko Malenic <mmalenic1@gmail.com>
Use channels to send individual pins around... probably we should refactor to Signal instead since message size N=1.

Co-authored-by: Marko Malenic <mmalenic1@gmail.com>
… lifetimes and futures.

Co-authored-by: Marko Malenic <mmalenic1@gmail.com>
WARNING: use --release
  We *strongly* recommend using release profile when building esp-hal.  The dev profile can potentially be one or more orders of magnitude slower than release, and may cause issues with timing-senstive peripherals and/or devices.
@Autofix Autofix requested a review from brainstorm February 21, 2026 02:38
@brainstorm brainstorm changed the title HSM refactor (Issue 25) HSM refactor Feb 21, 2026
Copy link
Copy Markdown
Owner

@brainstorm brainstorm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the suggestions! 🚀

Over to you @jubeormk1 (and @mmalenic, but only if you feel like it ;))

…l warn us if and when something breaks. Helps with not having to bump minor versions continuously, see https://words.filippo.io/dependabot/ for a gist of the idea
@brainstorm
Copy link
Copy Markdown
Owner

brainstorm commented Feb 21, 2026

@jubeormk1 @Autofix @mmalenic ☝🏻 As of commit 8fcdafd, we have no hardcoded hostkey, it gets generated on-device and stored on FlashConfig. Subsequent re-flashes of code-only do not overwrite that private key (but it's still possible to exfiltrate it via flash dump, of course).

From now on you'll see the typical SSH prompt when testing on-device:

The authenticity of host '192.168.4.1 (192.168.4.1)' can't be established.
ED25519 key fingerprint is SHA256:zTQSC+4Fofl/j4vmg2jBhUDFwONeB+PKMSdke1bYzdQ.
This key is not known by any other names.
Are you sure you want to continue connecting (yes/no/[fingerprint])?

This is expected/normal, we should not support private key generation off-device (and then shuttling it over env vars, as previously discussed).

I've tested in on-hardware (esp32c6) and seems to work, but I need to setup the serial side test jig and/or I'll revisit Julio's remote testing jig as soon as this PR is merged.

Still doesn't fix the pub auth itself (more work required), but at least we longer have hardcoded privkeys in code...

@brainstorm
Copy link
Copy Markdown
Owner

brainstorm commented Feb 21, 2026

HSM nit/design question: the device gets reset after 3 unsuccessful password auth attempts while using the following SSH client options to force password auth:

$ ssh -o PubkeyAuthentication=no -o PreferredAuthentications=password zssh@192.168.4.1

See the events and reboot happening thereafter:

(...)
Entering connection_loop and prog_loop is next...
[src/serve.rs:233] "Preparing bridge" = "Preparing bridge"
ServEvent::Hostkeys
ServEvent::FirstAuth
ServEvent::PasswordAuth
ServEvent::PasswordAuth
ServEvent::PasswordAuth
WARN - rx complete ()
ESP-ROM:esp32c6-20220919
Build:Sep 19 2022
rst:0x3 (LP_SW_HPSYS),boot:0xc (SPI_FAST_FLASH_BOOT)
(...)

I'll try to fix it myself as I'm implementing password auth logic now (issue #20), just raising the issue, we should just close the connection instead of rebooting the whole device.

@Autofix
Copy link
Copy Markdown
Collaborator Author

Autofix commented Feb 21, 2026

I'll try to fix it myself as I'm implementing password auth logic now (issue #20), just raising the issue, we should just close the connection instead of rebooting the whole device.

I agree. My next step on implementing the HSM is managing the return/fail cases rather than software resets, so I can have a look into that next. I'll write a follow up of the remaining HSM parts in issue #25 after this PR is merged.

@@ -0,0 +1,28 @@
use esp_hal::hmac::Hmac;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Difficult to follow which Hmac is which, could we name esp_hal::hmac::Hmac as EspHmac to avoid the type confusion?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree they should be changed but @brainstorm will need to answer this.
Probably it shouldn't be done with this PR.

pub mod net;
pub mod rng;
// TODO: Specialise for Espressif, tricky since it seems to require burning eFuses?:
// https://github.com/esp-rs/esp-hal/blob/main/examples/src/bin/hmac.rs
Copy link
Copy Markdown
Collaborator

@jubeormk1 jubeormk1 Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably shouldn't be done with this PR.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it.

Thanks for taking your time to provide the default pins.

# SPDX-License-Identifier: GPL-3.0-or-later

[toolchain]
#channel = "nightly"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove this line

[workspace.dependencies]

sunset = { git = "https://github.com/mkj/sunset", default-features = false, features = [
sunset = { git = "https://github.com/jubeormk1/sunset", branch = "dev/sftp-start", default-features = false, features = [
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point, you do not need to point to my fork/branch I would keep it out but it is ot a big deal. Same for sunset-async and sunset-sshwire-derive

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this because you still have not merged your ssh-stamp sftp branch into main? We can leave it for now if you think it will still take some time to merge your sunset fork.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @Autofix, I think it's important to have the OTA (even if it's on a personal fork for now) so that it's part the sec audit... hopefully on a not-so-distant future PR you can get the sunset OTA bits merged upstream before the beginning of March?

Cargo.toml Outdated

[features]
ipv6 = []
default = ["esp32c6", "sftp-ota"]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sftp-ota feature at this point is a placeholder. Maybe I left it there, but looking at it I don't think that it should be default

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think calling --no-default-features with every build command disables this anyway. I'll remove the entire line.

@brainstorm brainstorm merged commit a3c80d5 into brainstorm:main Feb 22, 2026
8 checks passed
Copy link
Copy Markdown
Collaborator

@jubeormk1 jubeormk1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi!

It was fun reviewing this. Thanks for considering my comments and well done in general

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HSM refactor (Previously FSMs + SansIO refactor)

3 participants