security: Testing harness, TOFU fix, and transport hardening#19
Merged
security: Testing harness, TOFU fix, and transport hardening#19
Conversation
Add the security testing and verification harness called for in Phase 5 of the hardening plan, fix the TOFU handshake regression, and correct an overly-aggressive UDP frame size limit that caused client hangs. TOFU handshake fix (moshpit/src/runtime.rs): - Replace dialoguer::Confirm (which dropped the y/n keypress in raw mode) with dialoguer::Input requiring the user to type "yes" explicitly, matching OpenSSH UX. Wrap in tokio::task::block_in_place so the async runtime continues processing while waiting for terminal input. Auth/KEX negative unit tests (libmoshpit/src/kex/reader.rs): - Add 8 hermetic unit tests for check_known_hosts and check_authorized_keys: host-key match and MITM-mismatch detection, TOFU accept/reject/no-callback paths, authorized_keys key match/mismatch, and bad file permissions (0o644 → rejected). Tests use TempDir for filesystem isolation and a shared Mutex to serialize HOME mutations across parallel test threads. Frame parser fuzz targets (libmoshpit/fuzz/): - Add cargo-fuzz harness crate with two libFuzzer targets: fuzz_frame for Frame::parse and fuzz_encframe for EncryptedFrame::parse. Both assert no panics on arbitrary byte input; FrameTooLarge and Ok(None) are the expected outcomes for random data. Corpus directories committed. CI fuzz smoke-test (.github/workflows/audit.yml): - Add fuzz-smoke job that runs both fuzz targets for 30 seconds on every PR and scheduled run using nightly Rust + cargo-fuzz. Crash artifacts are uploaded on failure. UDP sequence property tests (libmoshpit/src/udp/reader.rs): - Add proptest dev-dependency and 5 property tests sweeping arbitrary sequence numbers: in-order delivery, single-gap reordering, MAX_SEQ_JUMP DoS rejection, replay rejection, and recv_buffer size bounds. Fix MAX_ENCFRAME_LENGTH regression (libmoshpit/src/frames/encframe.rs): - Raise MAX_ENCFRAME_LENGTH from 2 KB to 64 KB. The 2 KB limit silently dropped ScreenState frames (which carry vt100::Screen::contents_formatted output, typically 4-15 KB) causing the client to freeze and the server to spin at 100% retrying the screen sync. 64 KB matches the IPv4 UDP MTU ceiling and still provides meaningful DoS protection.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19 +/- ##
==========================================
+ Coverage 54.35% 63.41% +9.06%
==========================================
Files 33 32 -1
Lines 4686 5082 +396
==========================================
+ Hits 2547 3223 +676
+ Misses 2139 1859 -280 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Add `libmoshpit/fuzz/fuzz_targets/regression_frame.rs` and `regression_encframe.rs` to capture fuzzer-discovered edge cases. - Include a specific regression test for `regression_crash_6720028039`, which was identified as an OOM issue in `Frame::parse`. - Fix OOM vulnerability in both `Frame::parse` and `EncryptedFrame::parse` by enforcing a 64KB memory limit during bincode decoding. This prevents malicious payloads from triggering large allocations. - Integrate the new fuzzer corpus generated during verification. - Update root `.gitignore` to recursively ignore all `target/` directories (fixing accidental staging of fuzz build artifacts).
The periodic 50 ms screen-state sync task previously woke every tick,
held the vt100 emulator lock, formatted the entire screen into a byte
buffer, and hashed it — all to detect whether the screen had changed.
This caused continuous measurable CPU usage even when the terminal was
completely idle.
Replace the hash approach with an Arc<AtomicU64> dirty_counter stored
in SessionRecord. The counter is incremented (Ordering::Relaxed) by:
- spawn_pty_reader: once per MTU-sized PTY output chunk
- spawn_pty: once per terminal Resize event
The sync task now does a single atomic load per 50 ms tick. If the
counter has not advanced since the last tick the task continues
immediately with zero lock contention, no allocation, and no emulator
formatting. Formatting only occurs when the screen has actually changed.
Also add unit tests for keygen module and fix associated clippy lints:
- keygen/cli.rs: fix Cli::command() test ambiguity with explicit trait
- keygen/runtime.rs: needless_pass_by_value, uninlined_format_args,
manual_string_new
- keygen/main.rs: mark main with #[cfg_attr(coverage_nightly, coverage(off))]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #18
This pull request introduces several improvements to the
moshpit-keygentool and its supporting libraries, focusing on enhanced test coverage, improved key generation UX, and expanded CI/CD and fuzz testing infrastructure. The most significant changes include a major refactor and test addition for the key generation logic, the introduction of a fuzzing suite forlibmoshpit, and updates to CI workflows for more robust automation and artifact handling.Key changes:
Key Generation Refactor and Testing
keygen/src/runtime.rsto modularize user prompts, file path handling, and key writing, improving maintainability and testability. Added comprehensive unit tests for these components. [1] [2]keygen/src/cli.rsto ensure correct argument handling and improve reliability.#[cfg_attr(coverage_nightly, coverage(off))]to main entry points to improve code coverage reporting accuracy. [1] [2]Fuzzing Infrastructure
libmoshpit-fuzzcrate with fuzz targets and regression tests for core library components, along with initial corpus and artifact placeholder files. This enables automated fuzz testing for robustness. [1] [2] [3] [4] [5]proptestas a dev-dependency inlibmoshpitfor property-based testing.CI/CD and Workflow Enhancements
.github/workflows/audit.ymlto run short fuzzing sessions and upload crash artifacts on failure, helping catch regressions early..github/workflows/release.ymlfor better security and to comply with GitHub Actions best practices. [1] [2] [3] [4] [5]Dependency and Version Updates
libmoshpitandmoshpit-keygenversions to0.3.0and updated internal dependencies accordingly. [1] [2] [3]zeroizeas a dependency to both the workspace andlibmoshpitfor secure memory handling. [1] [2]These changes collectively improve the reliability, security, and maintainability of the codebase, while laying the groundwork for more robust automated testing and release processes.