From a3da9fda6910c20cacba37a10334b1c707d7e4c3 Mon Sep 17 00:00:00 2001 From: Chili Palmer Date: Tue, 25 Aug 2026 22:07:47 +0200 Subject: [PATCH] Complete SSH transport release audit (#118) --- .gitea/workflows/security-audit.yml | 22 ++ AGENTS.md | 6 + Cargo.toml | 2 +- DEPENDENCIES.md | 11 +- README.md | 3 + apple/AppStore/DISTRIBUTION.md | 7 + apple/TESTING.md | 2 + crates/storage/src/git/ssh_push_tests.rs | 91 +++++---- crates/storage/src/git/ssh_tests.rs | 238 +++++++++------------- crates/storage/src/ssh.rs | 147 +++++++++++-- crates/storage/tests/git_embedded.rs | 7 +- crates/storage/tests/ssh_release_audit.rs | 53 +++++ docs/cli-parity.md | 2 + docs/configuration.md | 3 +- docs/desktop-audit.md | 4 + docs/git-synchronization.md | 6 +- docs/secure-secret-storage.md | 3 + docs/ssh-transport-audit.md | 94 +++++++++ 18 files changed, 489 insertions(+), 212 deletions(-) create mode 100644 .gitea/workflows/security-audit.yml create mode 100644 crates/storage/tests/ssh_release_audit.rs create mode 100644 docs/ssh-transport-audit.md diff --git a/.gitea/workflows/security-audit.yml b/.gitea/workflows/security-audit.yml new file mode 100644 index 0000000..2a66c19 --- /dev/null +++ b/.gitea/workflows/security-audit.yml @@ -0,0 +1,22 @@ +name: Dependency security audit + +on: + push: + branches: + - main + pull_request: + schedule: + - cron: "17 4 * * 1" + +jobs: + rustsec: + runs-on: linux-arm64 + steps: + - name: Check out the repository + uses: actions/checkout@v4 + + - name: Install the reviewed cargo-audit release + run: cargo install cargo-audit --locked --version 0.22.2 + + - name: Reject new RustSec vulnerabilities + run: cargo audit --ignore RUSTSEC-2023-0071 diff --git a/AGENTS.md b/AGENTS.md index 64f390f..c871247 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,6 +13,12 @@ entering Git transport code. Builds without the `ssh` feature must still parse SSH endpoints and return a typed unsupported-transport error before connection or repository mutation. +The SSH feature must keep its algorithm allowlist, strict known-host checking, +single configured identity, bounded channel/diagnostic limits, and ambiguous +push outcome rules in `crates/storage`. Do not add OpenSSH configuration, +proxy commands, password or keyboard-interactive authentication, host-key +bypasses, or frontend transport policy. + The CLI, Ratatui, Iced, Swift, SwiftUI, AutoFill, and watchOS code may collect input, invoke the Rust API, and present Rust-provided state. They must not duplicate storage rules, derive domain state from display strings, or directly diff --git a/Cargo.toml b/Cargo.toml index 9d7fc60..9b8a054 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,7 +52,7 @@ reqwest = { version = "0.13", default-features = false, features = ["blocking", rfd = { version = "0.17", default-features = false } rqrr = { version = "0.10", default-features = false } rpassword = "7.5" -russh = { version = "0.63.1", default-features = false, features = ["ring", "rsa"] } +russh = { version = "=0.63.1", default-features = false, features = ["ring", "rsa"] } ratatui = { version = "0.30", default-features = false, features = ["crossterm_0_29", "layout-cache", "macros", "underline-color", "unstable-rendered-line-info"] } security-framework = "3.7" secret-service = { version = "5.1", default-features = false, features = ["rt-tokio-crypto-rust"] } diff --git a/DEPENDENCIES.md b/DEPENDENCIES.md index 5bad378..73bb860 100644 --- a/DEPENDENCIES.md +++ b/DEPENDENCIES.md @@ -23,7 +23,7 @@ The current direct dependencies are: | [rfd 0.17](https://crates.io/crates/rfd/0.17.2), [ashpd 0.13](https://crates.io/crates/ashpd/0.13.13) | Native macOS/Windows folder picker and Linux XDG portal folder picker | MIT | | [gix 0.86](https://crates.io/crates/gix/0.86.0), [gix-config 0.59](https://crates.io/crates/gix-config/0.59.0) | Embedded Git objects, index, references, fetch, and merge | MIT OR Apache-2.0 | | [reqwest 0.13](https://crates.io/crates/reqwest/0.13.4) | HTTPS smart-Git transport with Rustls | MIT OR Apache-2.0 | -| [russh 0.63](https://crates.io/crates/russh/0.63.1), [Tokio 1.53](https://crates.io/crates/tokio/1.53.1) | Optional pure-Rust SSH transport, key/agent authentication, and protocol runtime | Apache-2.0 OR MIT; MIT | +| [russh 0.63.1](https://crates.io/crates/russh/0.63.1), [Tokio 1.53](https://crates.io/crates/tokio/1.53.1) | Optional pure-Rust SSH transport, key/agent authentication, and protocol runtime | Apache-2.0; MIT | | [flate2 1.1](https://crates.io/crates/flate2/1.1.9), [sha1 0.10](https://crates.io/crates/sha1/0.10.7) | Git pack compression and checksums | MIT OR Apache-2.0 | | [pgp 0.20](https://crates.io/crates/pgp/0.20.0) | Embedded OpenPGP key import, encryption, decryption, and signatures | MIT OR Apache-2.0 | | [keepass 0.13](https://crates.io/crates/keepass/0.13.20) | Pure-Rust KeePass KDBX 3/4 decryption and parsing for direct imports | MIT | @@ -52,8 +52,8 @@ decision. | Alternative GPG implementation | [`sequoia-openpgp` 2.4](https://crates.io/crates/sequoia-openpgp/2.4.1) | LGPL-2.0-or-later | Hold in reserve. Its default Nettle backend is native; its Rust backend exists, but the LGPL adds distribution work we can avoid. | | GnuPG integration | [`gpgme` 0.11](https://crates.io/crates/gpgme/0.11.0) | LGPL-2.1 | Reject: native GPGME/GnuPG integration and GPG engine processes violate the portability and no-process requirements. | | Local Git plus HTTPS fetch/push | [`gix` 0.86](https://crates.io/crates/gix/0.86.0) | MIT OR Apache-2.0 | Selected with default features off and `blocking-http-transport-reqwest-rust-tls`; accept HTTPS remotes only, supply credentials directly, and use the storage-owned receive-pack implementation for push. | -| SSH client and agent protocol | [`russh` 0.63](https://crates.io/crates/russh/0.63.1) | Apache-2.0 OR MIT | Selected behind the optional `ssh` feature with default features off and Ring/RSA enabled. Storage restricts host and authentication signatures to modern algorithms, selects exactly one configured key or agent fingerprint, and never invokes OpenSSH or another process. | -| Git FFI fallback | [`git2` 0.21](https://crates.io/crates/git2/0.21.0) | MIT OR Apache-2.0 | Reject for now; it links libgit2 and is unnecessary for the HTTPS-only scope. | +| SSH client and agent protocol | [`russh` 0.63.1](https://crates.io/crates/russh/0.63.1) | Apache-2.0 | Exactly pinned behind the optional `ssh` feature with default features off and Ring/RSA enabled. Storage owns an explicit modern algorithm allowlist, selects exactly one configured key or agent fingerprint, and never invokes OpenSSH or another process. | +| Git FFI fallback | [`git2` 0.21](https://crates.io/crates/git2/0.21.0) | MIT OR Apache-2.0 | Reject; it links libgit2 and is unnecessary for the embedded pure-Rust HTTPS and SSH transports. | | Server/application credentials | [`keyring-core` 1.0](https://crates.io/crates/keyring-core/1.0.0), [`apple-native-keyring-store` 1.0](https://crates.io/crates/apple-native-keyring-store/1.0.2), [`windows-native-keyring-store` 1.1](https://crates.io/crates/windows-native-keyring-store/1.1.0), [`zbus-secret-service-keyring-store` 1.0](https://crates.io/crates/zbus-secret-service-keyring-store/1.0.0) | MIT OR Apache-2.0 | Selected behind target-specific dependencies. Apple supports legacy Keychain plus protected-data user presence, Windows uses Credential Manager, and Linux uses Secret Service with the Tokio/Rust-crypto feature. | | Secret values in memory | [`secrecy` 0.10](https://crates.io/crates/secrecy/0.10.3), [`zeroize` 1.9](https://crates.io/crates/zeroize/1.9.0) | MIT OR Apache-2.0 | `zeroize` selected for the storage-owned redacted byte type; consider `secrecy` only when typed exposure controls add value. | | Password generation | [`rand`](https://crates.io/crates/rand) | MIT OR Apache-2.0 | Preferred using the operating-system CSPRNG. | @@ -79,7 +79,10 @@ decryption timing. The SSH dependency is used for signatures, while the existing OpenPGP dependency can decrypt RSA material, so the finding remains a tracked risk rather than an ignored or suppressed advisory. The audit also reports allowed unmaintained/unsound warnings in transitive UI and utility -dependencies; none are hidden by an audit configuration. +dependencies. The scheduled Gitea audit passes only the named no-fix RSA +advisory on the command line and fails every other vulnerability; the full +manual audit remains unfiltered. The lifecycle, algorithm, feature-graph, and +size evidence is recorded in `docs/ssh-transport-audit.md`. ## Release packaging diff --git a/README.md b/README.md index d2c36c1..2e87325 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,9 @@ The shared TOML schema, path rules, editor precedence, and Git remote formats are documented in [`docs/configuration.md`](docs/configuration.md). Embedded Git, HTTPS synchronization, merge behavior, and commit signing are documented in [`docs/git-synchronization.md`](docs/git-synchronization.md). +The complete SSH lifecycle, failure, algorithm, dependency, target, and release +matrix is documented in +[`docs/ssh-transport-audit.md`](docs/ssh-transport-audit.md). Native credential storage, opaque secret references, user-presence policy, and bounded caching are documented in [`docs/secure-secret-storage.md`](docs/secure-secret-storage.md). diff --git a/apple/AppStore/DISTRIBUTION.md b/apple/AppStore/DISTRIBUTION.md index 5c630f4..00bc095 100644 --- a/apple/AppStore/DISTRIBUTION.md +++ b/apple/AppStore/DISTRIBUTION.md @@ -4,6 +4,13 @@ Use `distribution/` only as ignored local staging. Never store Apple credentials signing material, review credentials, GPG material, application tokens, or private diagnostics in the repository or release logs. +The Apple Rust bridge intentionally excludes the optional storage `ssh` +feature. Before archiving, confirm `cargo tree -p ironstorage-apple -i russh -e +features` reports no matching package. Do not enable SSH for an Apple archive +without a separate Apple host-trust/key-interaction design; the CLI, TUI, and +desktop release artifacts enable SSH independently. The reviewed contract is +recorded in `../../docs/ssh-transport-audit.md`. + ## Release identity - iPhone application: `de.rfc1437.ironstorage` diff --git a/apple/TESTING.md b/apple/TESTING.md index 69d4c4e..4cee56e 100644 --- a/apple/TESTING.md +++ b/apple/TESTING.md @@ -26,6 +26,8 @@ Git UI, non-TOTP Watch features, and production AutoFill behavior. The Apple Rust bridge explicitly compiles storage without the optional `ssh` feature; typed SSH configuration therefore fails as unsupported before any connection until a future Apple target opts in. +The cross-target and artifact evidence for this exclusion is maintained in +`../docs/ssh-transport-audit.md`. ## Security boundary audit diff --git a/crates/storage/src/git/ssh_push_tests.rs b/crates/storage/src/git/ssh_push_tests.rs index c0cfe1a..c52e0e7 100644 --- a/crates/storage/src/git/ssh_push_tests.rs +++ b/crates/storage/src/git/ssh_push_tests.rs @@ -545,66 +545,69 @@ fn assert_pack_request(request: &[u8], old: Option<&str>, new: &str) { #[test] fn new_branch_fast_forward_and_already_current_push_are_confirmed() { - for (old_kind, behavior) in [ - (None, ReceiveBehavior::Success), - (Some("base"), ReceiveBehavior::Success), - ] { + for scp_like in [false, true] { + for old_kind in [None, Some("base")] { + let temporary = tempfile::tempdir().expect("temporary directory"); + let local = local_repository(&temporary.path().join("local")); + let base = commit(&local, ".gpg-id", b"ALICE\n", "Initialize"); + let new = commit(&local, "entry.gpg", b"ciphertext", "Add entry"); + let old = old_kind.map(|_| base.clone()); + let user_key = key(); + let server = start_server( + user_key.public_key().clone(), + None, + old.clone(), + ReceiveBehavior::Success, + false, + 1, + ); + let remote = remote(temporary.path(), &server, &user_key, scp_like); + let mut local = local; + local.add_remote("origin", remote.url()).expect("remote"); + let outcome = local + .push(&remote, Some("main"), &Credentials) + .expect("push"); + assert_eq!(outcome.new_id(), new); + assert_eq!(tracking_id(&local), Some(new.clone())); + server.join.join().expect("server"); + let observed = server.observed.lock().expect("observed"); + let expected = if scp_like { + b"git-receive-pack 'team/store.git'".to_vec() + } else { + b"git-receive-pack '/team/store.git'".to_vec() + }; + assert_eq!(observed.commands, [expected]); + assert_eq!(observed.receive_requests.len(), 1); + assert_pack_request(&observed.receive_requests[0], old.as_deref(), &new); + } + } + + for scp_like in [false, true] { let temporary = tempfile::tempdir().expect("temporary directory"); let local = local_repository(&temporary.path().join("local")); - let base = commit(&local, ".gpg-id", b"ALICE\n", "Initialize"); - let new = commit(&local, "entry.gpg", b"ciphertext", "Add entry"); - let old = old_kind.map(|_| base.clone()); + let head = commit(&local, ".gpg-id", b"ALICE\n", "Initialize"); let user_key = key(); let server = start_server( user_key.public_key().clone(), None, - old.clone(), - behavior, + Some(head.clone()), + ReceiveBehavior::Success, false, 1, ); - let remote = remote(temporary.path(), &server, &user_key, false); + let remote = remote(temporary.path(), &server, &user_key, scp_like); let mut local = local; local.add_remote("origin", remote.url()).expect("remote"); - let outcome = local + local .push(&remote, Some("main"), &Credentials) - .expect("push"); - assert_eq!(outcome.new_id(), new); - assert_eq!(tracking_id(&local), Some(new.clone())); + .expect("current push"); + assert_eq!(tracking_id(&local), Some(head)); server.join.join().expect("server"); - let observed = server.observed.lock().expect("observed"); assert_eq!( - observed.commands, - [b"git-receive-pack '/team/store.git'".to_vec()] + server.observed.lock().expect("observed").receive_requests, + [b"0000".to_vec()] ); - assert_eq!(observed.receive_requests.len(), 1); - assert_pack_request(&observed.receive_requests[0], old.as_deref(), &new); } - - let temporary = tempfile::tempdir().expect("temporary directory"); - let local = local_repository(&temporary.path().join("local")); - let head = commit(&local, ".gpg-id", b"ALICE\n", "Initialize"); - let user_key = key(); - let server = start_server( - user_key.public_key().clone(), - None, - Some(head.clone()), - ReceiveBehavior::Success, - false, - 1, - ); - let remote = remote(temporary.path(), &server, &user_key, false); - let mut local = local; - local.add_remote("origin", remote.url()).expect("remote"); - local - .push(&remote, Some("main"), &Credentials) - .expect("current push"); - assert_eq!(tracking_id(&local), Some(head)); - server.join.join().expect("server"); - assert_eq!( - server.observed.lock().expect("observed").receive_requests, - [b"0000".to_vec()] - ); } #[test] diff --git a/crates/storage/src/git/ssh_tests.rs b/crates/storage/src/git/ssh_tests.rs index a57279d..0cda5d6 100644 --- a/crates/storage/src/git/ssh_tests.rs +++ b/crates/storage/src/git/ssh_tests.rs @@ -377,155 +377,111 @@ fn remote_with_url(root: &Path, server: &Server, identity: &PrivateKey, url: Str } #[test] -fn upload_pack_drives_discovery_clone_fetch_fast_forward_merge_and_conflict() { - let temporary = tempfile::tempdir().expect("temporary directory"); - let source_root = temporary.path().join("source"); - fs::create_dir(&source_root).expect("source directory"); - let source_store = Repository::open(&source_root).expect("source store"); - let source = GitRepository::init(&source_store, identity()).expect("source Git"); - commit(&source, ".gpg-id", b"ALICE\n", "Initialize recipients"); - commit(&source, "shared.gpg", b"base", "Add shared entry"); - let user_key = key(); - let server = start_server( - user_key.public_key().clone(), - populated_fixture(&source), - Behavior::Normal, - 5, - ); - let remote = remote(temporary.path(), &server, &user_key, "/team/store.git"); +fn both_ssh_url_forms_drive_discovery_clone_fetch_fast_forward_merge_and_conflict() { + for scp_like in [false, true] { + let temporary = tempfile::tempdir().expect("temporary directory"); + let source_root = temporary.path().join("source"); + fs::create_dir(&source_root).expect("source directory"); + let source_store = Repository::open(&source_root).expect("source store"); + let source = GitRepository::init(&source_store, identity()).expect("source Git"); + commit(&source, ".gpg-id", b"ALICE\n", "Initialize recipients"); + commit(&source, "shared.gpg", b"base", "Add shared entry"); + let user_key = key(); + let server = start_server( + user_key.public_key().clone(), + populated_fixture(&source), + Behavior::Normal, + 6, + ); + let remote = if scp_like { + remote_with_url( + temporary.path(), + &server, + &user_key, + "git@127.0.0.1:team/store.git".to_owned(), + ) + } else { + remote(temporary.path(), &server, &user_key, "/team/store.git") + }; - let branches = GitRepository::discover_remote_branches( - temporary.path(), - identity(), - &remote, - &Credentials, - &GitOperationControl::default(), - ) - .expect("discover branches"); - assert_eq!(branches, ["feature", "main"]); - - let destination = temporary.path().join("clone"); - let clone = GitRepository::clone_into(&destination, identity(), &remote, &Credentials) - .expect("clone over SSH"); - assert_eq!( - fs::read(destination.join("shared.gpg")).expect("entry"), - b"base" - ); - assert!( - clone - .repository - .try_find_reference("refs/tags/v1") - .expect("tag lookup") - .is_some() - ); - - commit(&source, "remote-one.gpg", b"remote", "Remote fast-forward"); - *server.fixture.lock().expect("fixture") = populated_fixture(&source); - assert_eq!( - clone - .pull(&remote, Some("main"), &Credentials) - .expect("fast-forward pull"), - PullOutcome::FastForward - ); - - commit(&clone, "local.gpg", b"local", "Local change"); - commit( - &source, - "remote-two.gpg", - b"remote", - "Remote parallel change", - ); - *server.fixture.lock().expect("fixture") = populated_fixture(&source); - assert_eq!( - clone - .pull(&remote, Some("main"), &Credentials) - .expect("merge pull"), - PullOutcome::Merged - ); - - commit(&clone, "shared.gpg", b"local conflict", "Local conflict"); - commit(&source, "shared.gpg", b"remote conflict", "Remote conflict"); - *server.fixture.lock().expect("fixture") = populated_fixture(&source); - let before = fs::read(destination.join("shared.gpg")).expect("local entry"); - assert!(matches!( - clone.pull(&remote, Some("main"), &Credentials), - Err(GitError::MergeConflicts { .. }) - )); - assert_eq!( - fs::read(destination.join("shared.gpg")).expect("entry"), - before - ); - - server.join.join().expect("server"); - let commands = server.commands.lock().expect("commands"); - assert_eq!(commands.len(), 5); - assert!( - commands - .iter() - .all(|command| command == b"git-upload-pack '/team/store.git'") - ); -} - -#[test] -fn scp_remote_drives_discovery_clone_fetch_and_pull() { - let temporary = tempfile::tempdir().expect("temporary directory"); - let source_root = temporary.path().join("source"); - fs::create_dir(&source_root).expect("source directory"); - let source_store = Repository::open(&source_root).expect("source store"); - let source = GitRepository::init(&source_store, identity()).expect("source Git"); - commit(&source, ".gpg-id", b"ALICE\n", "Initialize recipients"); - let user_key = key(); - let server = start_server( - user_key.public_key().clone(), - populated_fixture(&source), - Behavior::Normal, - 4, - ); - let remote = remote_with_url( - temporary.path(), - &server, - &user_key, - "git@127.0.0.1:team/store.git".to_owned(), - ); - - assert_eq!( - GitRepository::discover_remote_branches( + let branches = GitRepository::discover_remote_branches( temporary.path(), identity(), &remote, &Credentials, &GitOperationControl::default(), ) - .expect("discover branches"), - ["feature", "main"] - ); - let destination = temporary.path().join("clone"); - let clone = GitRepository::clone_into(&destination, identity(), &remote, &Credentials) - .expect("clone over scp-like SSH remote"); - commit(&source, "remote.gpg", b"remote", "Remote fast-forward"); - *server.fixture.lock().expect("fixture") = populated_fixture(&source); - assert!( - clone - .fetch(&remote, &Credentials) - .expect("fetch over scp-like SSH remote") - .received_pack() - ); - assert_eq!( - clone - .pull(&remote, Some("main"), &Credentials) - .expect("pull over scp-like SSH remote"), - PullOutcome::FastForward - ); + .expect("discover branches"); + assert_eq!(branches, ["feature", "main"]); - server.join.join().expect("server"); - assert!( - server - .commands - .lock() - .expect("commands") - .iter() - .all(|command| command == b"git-upload-pack 'team/store.git'") - ); + let destination = temporary.path().join("clone"); + let clone = GitRepository::clone_into(&destination, identity(), &remote, &Credentials) + .expect("clone over SSH"); + assert_eq!( + fs::read(destination.join("shared.gpg")).expect("entry"), + b"base" + ); + assert!( + clone + .repository + .try_find_reference("refs/tags/v1") + .expect("tag lookup") + .is_some() + ); + + commit(&source, "remote-one.gpg", b"remote", "Remote fast-forward"); + *server.fixture.lock().expect("fixture") = populated_fixture(&source); + assert!( + clone + .fetch(&remote, &Credentials) + .expect("fetch over SSH") + .received_pack() + ); + assert_eq!( + clone + .pull(&remote, Some("main"), &Credentials) + .expect("fast-forward pull"), + PullOutcome::FastForward + ); + + commit(&clone, "local.gpg", b"local", "Local change"); + commit( + &source, + "remote-two.gpg", + b"remote", + "Remote parallel change", + ); + *server.fixture.lock().expect("fixture") = populated_fixture(&source); + assert_eq!( + clone + .pull(&remote, Some("main"), &Credentials) + .expect("merge pull"), + PullOutcome::Merged + ); + + commit(&clone, "shared.gpg", b"local conflict", "Local conflict"); + commit(&source, "shared.gpg", b"remote conflict", "Remote conflict"); + *server.fixture.lock().expect("fixture") = populated_fixture(&source); + let before = fs::read(destination.join("shared.gpg")).expect("local entry"); + assert!(matches!( + clone.pull(&remote, Some("main"), &Credentials), + Err(GitError::MergeConflicts { .. }) + )); + assert_eq!( + fs::read(destination.join("shared.gpg")).expect("entry"), + before + ); + + server.join.join().expect("server"); + let commands = server.commands.lock().expect("commands"); + assert_eq!(commands.len(), 6); + let expected = if scp_like { + b"git-upload-pack 'team/store.git'".as_slice() + } else { + b"git-upload-pack '/team/store.git'".as_slice() + }; + assert!(commands.iter().all(|command| command == expected)); + } } #[test] diff --git a/crates/storage/src/ssh.rs b/crates/storage/src/ssh.rs index 1841ffc..f45fecb 100644 --- a/crates/storage/src/ssh.rs +++ b/crates/storage/src/ssh.rs @@ -14,11 +14,12 @@ use cap_std::{ambient_authority, fs::Dir}; use cap_tempfile::TempFile; use hmac::{Hmac, Mac as _}; use russh::{ - ChannelMsg, Disconnect, client, + ChannelMsg, Disconnect, Preferred, cipher, client, compression, kex, keys::{ HashAlg, PrivateKey, PublicKey, agent::client::AgentClient, key::PrivateKeyWithHashAlg, ssh_key::Algorithm, }, + mac, }; use sha1::Sha1; @@ -140,19 +141,7 @@ impl SshSession { port, known_hosts, }; - let mut config = client::Config { - inactivity_timeout: Some(CONNECTION_TIMEOUT), - ..client::Config::default() - }; - config.preferred.key = Cow::Owned( - config - .preferred - .key - .iter() - .filter(|algorithm| !matches!(algorithm, Algorithm::Rsa { hash: None })) - .cloned() - .collect(), - ); + let config = client_config(); let handle = runtime.block_on(async { controlled( tokio::time::timeout( @@ -300,6 +289,61 @@ impl SshSession { } } +fn client_config() -> client::Config { + client::Config { + inactivity_timeout: Some(CONNECTION_TIMEOUT), + preferred: Preferred { + kex: Cow::Owned(vec![ + kex::MLKEM768X25519_SHA256, + kex::CURVE25519, + kex::CURVE25519_PRE_RFC_8731, + kex::DH_GEX_SHA256, + kex::DH_G18_SHA512, + kex::DH_G17_SHA512, + kex::DH_G16_SHA512, + kex::DH_G15_SHA512, + kex::DH_G14_SHA256, + kex::EXTENSION_SUPPORT_AS_CLIENT, + kex::EXTENSION_OPENSSH_STRICT_KEX_AS_CLIENT, + ]), + host_key_certificates: Cow::Borrowed(&[]), + key: Cow::Owned(vec![ + Algorithm::Ed25519, + Algorithm::Ecdsa { + curve: russh::keys::ssh_key::EcdsaCurve::NistP256, + }, + Algorithm::Ecdsa { + curve: russh::keys::ssh_key::EcdsaCurve::NistP384, + }, + Algorithm::Ecdsa { + curve: russh::keys::ssh_key::EcdsaCurve::NistP521, + }, + Algorithm::Rsa { + hash: Some(HashAlg::Sha512), + }, + Algorithm::Rsa { + hash: Some(HashAlg::Sha256), + }, + ]), + cipher: Cow::Owned(vec![ + cipher::CHACHA20_POLY1305, + cipher::AES_256_GCM, + cipher::AES_256_CTR, + cipher::AES_192_CTR, + cipher::AES_128_CTR, + ]), + mac: Cow::Owned(vec![ + mac::HMAC_SHA512_ETM, + mac::HMAC_SHA256_ETM, + mac::HMAC_SHA512, + mac::HMAC_SHA256, + ]), + compression: Cow::Owned(vec![compression::NONE]), + }, + ..client::Config::default() + } +} + async fn open_command_channel( handle: &client::Handle, command: Vec, @@ -983,7 +1027,7 @@ mod tests { use russh::{ keys::{ - PrivateKey, PublicKey, + HashAlg, PrivateKey, PublicKey, agent::client::AgentClient, ssh_key::{Algorithm, LineEnding}, }, @@ -997,7 +1041,8 @@ mod tests { }; use super::{ - GitService, SshSession, git_service_command, persist_confirmed_host, ssh_host_key, + GitService, SshSession, client_config, git_service_command, persist_confirmed_host, + ssh_host_key, }; struct Passphrase(Option<&'static [u8]>); @@ -1143,13 +1188,19 @@ mod tests { Algorithm::Ecdsa { curve: russh::keys::ssh_key::EcdsaCurve::NistP256, }, + Algorithm::Ecdsa { + curve: russh::keys::ssh_key::EcdsaCurve::NistP384, + }, + Algorithm::Ecdsa { + curve: russh::keys::ssh_key::EcdsaCurve::NistP521, + }, Algorithm::Rsa { hash: Some(russh::keys::HashAlg::Sha512), }, ] { let temporary = tempfile::tempdir().expect("temporary directory"); - let identity = key(algorithm); - let host_key = key(Algorithm::Ed25519); + let identity = key(algorithm.clone()); + let host_key = key(algorithm); let identity_path = temporary.path().join("identity"); let known_hosts = temporary.path().join("known_hosts"); write_key(&identity_path, &identity); @@ -1172,6 +1223,47 @@ mod tests { } } + #[test] + fn client_algorithm_policy_excludes_legacy_ssh_primitives() { + let config = client_config(); + assert!( + config + .preferred + .kex + .iter() + .all(|name| !name.as_ref().contains("sha1")) + ); + assert!(config.preferred.host_key_certificates.is_empty()); + assert!(config.preferred.key.iter().all(|algorithm| matches!( + algorithm, + Algorithm::Ed25519 + | Algorithm::Ecdsa { .. } + | Algorithm::Rsa { + hash: Some(HashAlg::Sha256 | HashAlg::Sha512) + } + ))); + assert!(config.preferred.cipher.iter().all(|name| { + let name = name.as_ref(); + !name.contains("cbc") && name != "none" + })); + assert!( + config + .preferred + .mac + .iter() + .all(|name| !name.as_ref().contains("sha1")) + ); + assert_eq!( + config + .preferred + .compression + .iter() + .map(AsRef::as_ref) + .collect::>(), + ["none"] + ); + } + #[test] fn encrypted_key_requires_the_matching_protected_passphrase() { let temporary = tempfile::tempdir().expect("temporary directory"); @@ -1331,7 +1423,7 @@ mod tests { } #[test] - fn unavailable_network_is_distinct_from_ssh_protocol_failure() { + fn dns_and_network_failures_are_distinct_from_ssh_protocol_failure() { let temporary = tempfile::tempdir().expect("temporary directory"); let identity = key(Algorithm::Ed25519); let identity_path = temporary.path().join("identity"); @@ -1350,6 +1442,23 @@ mod tests { .expect_err("unavailable network"), GitError::NetworkUnavailable ); + + let dns_remote = GitRemote::ssh_with_authentication( + "origin", + "ssh://git@does-not-exist.invalid/team/store.git", + SshRemoteAuthentication::key_file(identity_path, known_hosts) + .expect("SSH authentication"), + ) + .expect("SSH remote"); + assert_eq!( + SshSession::connect( + &dns_remote, + &Passphrase(None), + &GitOperationControl::default(), + ) + .expect_err("unavailable DNS name"), + GitError::NetworkUnavailable + ); } #[test] diff --git a/crates/storage/tests/git_embedded.rs b/crates/storage/tests/git_embedded.rs index 29de1bb..3997029 100644 --- a/crates/storage/tests/git_embedded.rs +++ b/crates/storage/tests/git_embedded.rs @@ -208,12 +208,17 @@ fn ssh_remotes_are_typed_but_unavailable_before_transport_or_mutation() -> TestR #[cfg(feature = "ssh")] #[test] -fn ssh_feature_allows_repository_remote_configuration() -> TestResult { +fn ssh_feature_allows_both_remote_forms_through_add_set_and_get() -> TestResult { let temporary = tempfile::tempdir()?; let store = Repository::open(temporary.path())?; let mut git = GitRepository::init(&store, identity())?; git.add_remote("origin", "git@example.test:team/store.git")?; assert_eq!(git.remote_url("origin")?, "git@example.test:team/store.git"); + git.set_remote_url("origin", "ssh://git@example.test/team/store.git")?; + assert_eq!( + git.remote_url("origin")?, + "ssh://git@example.test/team/store.git" + ); Ok(()) } diff --git a/crates/storage/tests/ssh_release_audit.rs b/crates/storage/tests/ssh_release_audit.rs new file mode 100644 index 0000000..08044c0 --- /dev/null +++ b/crates/storage/tests/ssh_release_audit.rs @@ -0,0 +1,53 @@ +const WORKSPACE: &str = include_str!("../../../Cargo.toml"); +const STORAGE_MANIFEST: &str = include_str!("../Cargo.toml"); +const APPLE_MANIFEST: &str = include_str!("../../apple/Cargo.toml"); +const CLI_MANIFEST: &str = include_str!("../../../apps/cli/Cargo.toml"); +const TUI_MANIFEST: &str = include_str!("../../../apps/tui/Cargo.toml"); +const DESKTOP_MANIFEST: &str = include_str!("../../../apps/desktop/Cargo.toml"); +const SSH_SOURCE: &str = include_str!("../src/ssh.rs"); +const GIT_SOURCE: &str = include_str!("../src/git.rs"); +const AUDIT: &str = include_str!("../../../docs/ssh-transport-audit.md"); +const AUDIT_WORKFLOW: &str = include_str!("../../../.gitea/workflows/security-audit.yml"); + +#[test] +fn release_feature_and_advisory_boundaries_stay_explicit() { + assert!(WORKSPACE.contains( + "russh = { version = \"=0.63.1\", default-features = false, features = [\"ring\", \"rsa\"] }" + )); + assert!(STORAGE_MANIFEST.contains("ssh = [\"dep:russh\", \"dep:tokio\"]")); + for manifest in [CLI_MANIFEST, TUI_MANIFEST, DESKTOP_MANIFEST] { + assert!(manifest.contains("features = [\"ssh\"]")); + } + assert!(APPLE_MANIFEST.contains("default-features = false, features = [\"full\"]")); + assert!(!APPLE_MANIFEST.contains("features = [\"ssh\"]")); + assert!(AUDIT_WORKFLOW.contains("cargo-audit --locked --version 0.22.2")); + assert!(AUDIT_WORKFLOW.contains("cargo audit --ignore RUSTSEC-2023-0071")); + assert!(AUDIT.contains("RUSTSEC-2023-0071")); +} + +#[test] +fn production_ssh_transport_has_no_process_unsafe_or_proxy_escape_hatch() { + let process_command = ["process", "::Command"].concat(); + let command_constructor = ["Command", "::new("].concat(); + let unsafe_block = ["unsafe", " {"].concat(); + for (name, source) in [("ssh.rs", SSH_SOURCE), ("git.rs", GIT_SOURCE)] { + let production = source.split("#[cfg(test)]").next().unwrap_or(source); + for forbidden in [ + process_command.as_str(), + command_constructor.as_str(), + unsafe_block.as_str(), + "ProxyCommand", + "proxy_command", + "russh_config", + ] { + assert!( + !production.contains(forbidden), + "{name} contains forbidden production token {forbidden}" + ); + } + } + assert!(SSH_SOURCE.contains("check_server_key")); + assert!(SSH_SOURCE.contains("verify_known_host")); + assert!(SSH_SOURCE.contains("MAX_SSH_DIAGNOSTIC_BYTES")); + assert!(SSH_SOURCE.contains("MAX_CHANNEL_CHUNK")); +} diff --git a/docs/cli-parity.md b/docs/cli-parity.md index e2229b5..39b1d90 100644 --- a/docs/cli-parity.md +++ b/docs/cli-parity.md @@ -83,6 +83,8 @@ are selected. Platform secret-store implementations may compile operating- system IPC/runtime support, but IronStorage never calls dependency APIs that spawn a helper. Direct dependency licenses and the remaining project-license release decision are recorded in `DEPENDENCIES.md`. +The final pure-Rust SSH lifecycle, limitation, advisory, and release graph is +recorded in `ssh-transport-audit.md`. Run the executable gate from the workspace root: diff --git a/docs/configuration.md b/docs/configuration.md index c7949f4..67d1351 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -109,7 +109,8 @@ without `ssh`, keeping the iPhone, AutoFill, and Watch dependency graph HTTPS-only until an Apple target opts in. The optional storage `ssh` feature contains `russh` 0.63.1 and Tokio; `russh` default features are disabled and the Ring backend plus RSA key support -are selected explicitly. +are selected explicitly. The reviewed release, algorithm allowlist, advisory, +target, and size evidence is recorded in `ssh-transport-audit.md`. For SSH, URI paths are absolute, scp-like paths without a leading slash are relative to the remote account, and `~`/`~user` paths retain tilde-expansion diff --git a/docs/desktop-audit.md b/docs/desktop-audit.md index 8799031..3a089f1 100644 --- a/docs/desktop-audit.md +++ b/docs/desktop-audit.md @@ -113,6 +113,10 @@ remain native-host smoke checks because CI cannot emulate those OS services. | Background and window lifecycle | Generation counters reject stale asynchronous results. Lock cancels Git/clipboard work and clears OTP, QR, URI, entry, and editor state. Close and quit use the same dirty guard. | | Repository and domain ownership | The executable source audit rejects repository/Git construction, process launch, OTP/QR parsing, filesystem writes, unsafe blocks, and insecure HTTP literals in production desktop modules. HTTPS and SSH endpoint parsing, host trust, authentication, and protocol behavior remain in `crates/storage`; desktop only presents typed state and native confirmation or masked-passphrase prompts. The folder picker may read only a user-selected QR image; all password-store I/O remains in `crates/storage`. | +`ssh-transport-audit.md` records the complete two-URL transport lifecycle and +the dependency, algorithm, target, artifact, and deliberate-limit evidence +shared by desktop, TUI, and CLI. + Run the complete repository gate after the desktop-specific checks: ```sh diff --git a/docs/git-synchronization.md b/docs/git-synchronization.md index 2aa18f7..3a70e4b 100644 --- a/docs/git-synchronization.md +++ b/docs/git-synchronization.md @@ -56,7 +56,8 @@ Encrypted-key passphrases come from `SecretBytes` in the operating-system secret store. Identity attempts are bounded and deterministic; IronStorage does not spray keys, use passwords or keyboard-interactive authentication, read OpenSSH configuration, start/probe an agent process, or run proxy/helper -commands. SHA-1 host signatures and `ssh-rsa` authentication are excluded. +commands. The explicit client allowlist excludes SHA-1 key exchange/MAC, +RSA/SHA-1 signatures, DSA, CBC, `none`, compression, and host certificates. Cancellation interrupts connection and authentication without changing Git, known hosts, or secure storage. @@ -122,3 +123,6 @@ quoting, cancellation, and rollback without an external Git or SSH executable. Frontend tests cover typed transport display, prompt masking and cancellation, retry routing, conflicts, authentication failures, and successful HTTPS regression paths without duplicating protocol logic. + +The final two-URL lifecycle, adversarial failure, dependency, target, and +artifact matrix is maintained in `ssh-transport-audit.md`. diff --git a/docs/secure-secret-storage.md b/docs/secure-secret-storage.md index 40759c0..8c22d19 100644 --- a/docs/secure-secret-storage.md +++ b/docs/secure-secret-storage.md @@ -57,3 +57,6 @@ other failures never create or replace a record. Prompts are hidden or masked, and secret values are excluded from arguments, history, normal output, debug models, notifications, and the clipboard. Tests inject a memory backend and never access a developer or CI user keyring. + +The SSH release audit in `ssh-transport-audit.md` ties this passphrase contract +to the complete host-trust, retry, failure, frontend, and artifact matrix. diff --git a/docs/ssh-transport-audit.md b/docs/ssh-transport-audit.md new file mode 100644 index 0000000..a3ffe09 --- /dev/null +++ b/docs/ssh-transport-audit.md @@ -0,0 +1,94 @@ +# Pure-Rust Git-over-SSH release audit + +Reviewed 2026-08-25. This is the final milestone-07 matrix for the optional +storage `ssh` feature. The production transport is a Russh session connected +directly to the configured server; it opens only Git upload-pack or +receive-pack channels and never delegates to a local Git, SSH, shell, transport +helper, or proxy process. + +## Remote lifecycle matrix + +The two spellings enter one `RemoteEndpoint::Ssh` contract. URI paths remain +absolute; scp-like paths remain account-relative. The tests below run a +pure-Rust in-process SSH server with synthetic keys and Git objects. + +| Surface | `ssh://` | scp-like | Executable evidence | +| --- | --- | --- | --- | +| Parse, add, set, get, reject helpers | yes | yes | `ssh_remote_endpoints_parse_to_one_typed_contract`, `ssh_feature_allows_both_remote_forms_through_add_set_and_get`, configuration rejection tests | +| Branch discovery and clone | yes | yes | `both_ssh_url_forms_drive_discovery_clone_fetch_fast_forward_merge_and_conflict` | +| Explicit fetch and fast-forward pull | yes | yes | same full-lifecycle test, including chunked advertisements and packs | +| Three-way merge and typed conflict pull | yes | yes | same full-lifecycle test; conflicts preserve the local worktree | +| New, fast-forward, and already-current push | yes | yes | `new_branch_fast_forward_and_already_current_push_are_confirmed` | +| Pull-then-push synchronization | yes | yes | `synchronization_pulls_then_pushes_over_both_ssh_url_forms` | +| Progress, cancellation, and retry | transport-neutral | transport-neutral | controlled Git tests, slow upload/push cancellation, unknown-host confirm/retry, encrypted-key reject/retry, and frontend interaction tests | +| CLI, TUI, and desktop presentation | typed SSH | typed SSH | frontend selection, masked prompt, cancellation, authentication-error, conflict, progress, and HTTPS regression tests | + +The in-process server implements the same `git-upload-pack ''` and +`git-receive-pack ''` exec-channel boundary used by a Gitea SSH endpoint. +It validates both service commands, packet-line advertisements, chunked pack +streams, receive status, and non-fast-forward behavior without a Git or +OpenSSH installation. A real Gitea smoke test is optional and must use a +disposable repository and synthetic key; it adds no protocol coverage and must +never use or print a developer key. + +## Authentication and failure matrix + +| Risk or compatibility case | Enforced result and evidence | +| --- | --- | +| Key files | Ed25519, ECDSA P-256/P-384/P-521, and RSA/SHA-2 host and user keys authenticate; encrypted OpenSSH keys require the matching protected passphrase. Identity and known-host files are bounded and symlinks are rejected. | +| Existing agent | The agent is opt-in, never launched, and one configured SHA-256 fingerprint is selected from at most 64 identities. Missing socket, missing identity, and rejection are distinct typed failures. | +| Host identity | Exact, hashed, default-port, and bracketed non-default-port entries are supported. Unknown keys stop before authentication and require explicit persistence; changed or revoked keys stop before authentication and cannot be replaced through confirmation. Confirmation is bound to the configured host and port. | +| Algorithms | The client owns an explicit allowlist: ML-KEM/X25519, Curve25519, SHA-2/SHA-512 finite-field KEX; Ed25519, ECDSA, and RSA/SHA-2 host/auth signatures; ChaCha20-Poly1305 or AES-GCM/CTR; SHA-2 MACs; no compression. DSA, RSA/SHA-1, SHA-1 KEX/MAC, CBC, `none`, and host certificates are excluded. A unit test rejects policy regression. | +| Paths and commands | Absolute, relative, and `~user` paths are preserved as one shell-quoted argument for exactly one built-in Git service. Quotes, whitespace, metacharacters, environment-looking text, and leading options cannot select another command. | +| Network and server failures | Reserved-name DNS failure and refused connections are typed as network unavailable. Channel rejection, non-zero exit, signal, malformed packet/pack/status, early EOF, and disconnect are separately exercised. Remote diagnostics are control-sanitized and capped at 8 KiB. | +| Slow/chunked I/O and cancellation | Channel chunks are capped at 32 KiB with queues of eight chunks. Connection and service completion time out after 30 seconds. Slow upload cancels without a destination; push cancellation after transmission returns unknown outcome and never advances tracking or blindly replays. | +| Rollback and ambiguity | Clone uses a private sibling and atomically installs only a complete repository. Fetch/pull failures preserve refs/worktree. Push accepts success only after unpack, ref status, zero exit, and clean close; ambiguous completion requires a fresh fetch. Sync never pushes after a failed or conflicted pull. | +| Secrets | Key/passphrase bytes use zeroizing storage types, never configuration, history, arguments, diagnostics, notifications, or clipboard. A prompted passphrase is fingerprint-bound and persisted only after successful authentication and Git work. | + +## Dependencies, advisories, and size + +Russh is exactly pinned to `0.63.1` (Apache-2.0), requires Rust 1.85, and is +built without default features using only Ring and RSA support. Tokio is pinned +by the lockfile at 1.53.1 (MIT) with only I/O, macros, network, multi-threaded +runtime, synchronization, and time features. The selected licenses are +compatible with IronStorage's MIT release; `DEPENDENCIES.md` records the full +decision. + +`cargo audit` 0.22.2 reports RUSTSEC-2023-0071 for `rsa` 0.10.0-rc.18 through +Russh and for `rsa` 0.9.10 through the pre-existing OpenPGP/packaging graph. +RustSec lists no patched release. The finding is not suppressed from the +manual audit: it remains documented here and in `DEPENDENCIES.md`. The Gitea +security workflow ignores only that named no-fix advisory so every other new +vulnerability still fails pushes, pull requests, and the weekly audit. + +On the reviewed release build, storage `full` without SSH contains 422 unique +packages and a 15,961,360-byte optimized rlib. `full,ssh` contains 489 packages +and an 18,376,592-byte rlib: 67 packages and 2,415,232 bytes (15.1%) added at +the library artifact. Final executable size varies by target and linker, so +release package sizes are recorded with the issue verification rather than +treated as a stable source invariant. + +## Target and packaging contract + +The CLI, TUI, and desktop manifests explicitly enable `ssh`; the tagged release +workflow builds those ordinary artifacts for ARM64/x86-64 Linux and macOS and +x86-64 Windows. Target-specific `cargo tree -e features` checks must show Russh +through storage `ssh` on all three platforms. The macOS application package +must contain all three SSH-enabled binaries. + +The Apple bridge explicitly selects storage `full` with default features off +and does not select `ssh`. Both iPhone simulator and device static libraries +therefore exclude Russh while retaining typed SSH endpoint parsing and the +pre-connection unsupported-transport error. Apple archives must preserve that +graph until an Apple target explicitly opts in and adds a platform-specific +host/key interaction design. + +## Deliberate OpenSSH differences + +IronStorage does not read arbitrary OpenSSH configuration, `ProxyCommand`, +`Match`, URL rewrites, separate push URLs, host certificates, password or +keyboard-interactive authentication, agent forwarding, arbitrary remote +commands, local transport helpers, or local-path remotes. These are explicit +security boundaries, not partial implementations. HTTPS smart Git remains the +unchanged alternative transport and shares the same embedded repository, +merge, rollback, progress, and cancellation contracts.