From f7d3404ca8bddaadf2a06c58b52be4715090a839 Mon Sep 17 00:00:00 2001 From: silas Date: Tue, 7 Jul 2026 15:55:17 +0800 Subject: [PATCH 1/4] fix(agent): build rsa signing key from components, not ssh-key TryFrom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ssh-key`'s `TryFrom<&RsaKeypair> for rsa::RsaPrivateKey` passes `p` twice and omits `q`: vec![ rsa::BigUint::try_from(&key.private.p)?, rsa::BigUint::try_from(&key.private.p)?, // should be .q ] So `from_components` gets primes `[p, p]`, `Πprimes = p² ≠ n`, and the internal `validate()` fails with `InvalidModulus` → surfaced as `Error::Crypto`. This breaks EVERY RSA identity: `Session::sign` returns `AgentError` and ssh reports "agent refused operation". (Only Ed25519 was covered by tests, so the RSA path was never exercised.) The bug is present in the entire 0.6.x line, including 0.6.7 (the latest stable, 2024-10-15). It is fixed upstream by RustCrypto/SSH#274, first published in ssh-key 0.7.0-pre.1 (2024-08-13) and carried through the 0.7.0-rc.* prereleases — but 0.7.0 has no stable release yet, and #274 is a breaking rewrite (private RSA fields, BigUint→Uint, subtle→ctutils, edition 2024, MSRV 1.85, crypto-bigint 0.7-pre / rsa rc). Bumping to a prerelease to fix a one-line typo isn't worth the blast radius. Instead, build the key from its Mpint components directly with the correct `p` and `q` and call `rsa::RsaPrivateKey::from_components` (which itself validates + precomputes). Verified against the affected key: pq==n, de≡1 mod p-1/q-1, from_components accepts it, and `ssh-add -T` now signs (rsa-sha2-256/512, ssh-rsa). Works on the pinned stable ssh-key 0.6.7. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/server_macos/ssh_agent.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/server_macos/ssh_agent.rs b/src/server_macos/ssh_agent.rs index 30d1a6c..19ab864 100644 --- a/src/server_macos/ssh_agent.rs +++ b/src/server_macos/ssh_agent.rs @@ -648,9 +648,29 @@ fn sign_data_with_privkey( KeypairData::Rsa(ref key) => { use rsa::pkcs1v15::SigningKey; use rsa::signature::{RandomizedSigner, SignatureEncoding}; + use rsa::BigUint; use ssh_agent_lib::proto::signature; - let private_key: rsa::RsaPrivateKey = key.try_into().map_err(AgentError::other)?; + // Build the rsa key from its components directly rather than via + // ssh-key's `TryFrom<&RsaKeypair> for rsa::RsaPrivateKey`: that + // conversion (ssh-key 0.6.7) rejects otherwise-valid OpenSSH RSA + // keys with `Error::Crypto`, while `from_components` (which itself + // validates + precomputes) accepts the exact same n/e/d/p/q. An + // Mpint stores a signed big-endian integer; `as_positive_bytes` + // strips the leading sign byte so `from_bytes_be` sees the raw + // magnitude. + let mp = |m: &ssh_key::Mpint| { + m.as_positive_bytes() + .map(BigUint::from_bytes_be) + .ok_or_else(|| agent_err(anyhow::anyhow!("RSA component is not a positive integer"))) + }; + let private_key = rsa::RsaPrivateKey::from_components( + mp(&key.public.n)?, + mp(&key.public.e)?, + mp(&key.private.d)?, + vec![mp(&key.private.p)?, mp(&key.private.q)?], + ) + .map_err(AgentError::other)?; let mut rng = rand::thread_rng(); if flags & signature::RSA_SHA2_512 != 0 { From 45433623fba889181b5b38d191ee848e70197b52 Mon Sep 17 00:00:00 2001 From: qiqi Date: Tue, 7 Jul 2026 16:36:01 +0800 Subject: [PATCH 2/4] test(agent): RSA sign/verify regression test for from_components fix Covers the ssh-key 0.6.7 TryFrom bug that broke every RSA identity (passes p twice, omits q -> InvalidModulus -> Error::Crypto). Parses a fixed 2048-bit OpenSSH RSA key and asserts sign_data_with_privkey produces rsa-sha2-256 / rsa-sha2-512 / ssh-rsa signatures that verify under the key's own modulus. macOS-gated like the module it guards. Co-authored-by: Claude --- src/server_macos/ssh_agent.rs | 93 +++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/src/server_macos/ssh_agent.rs b/src/server_macos/ssh_agent.rs index 19ab864..2707b05 100644 --- a/src/server_macos/ssh_agent.rs +++ b/src/server_macos/ssh_agent.rs @@ -2371,6 +2371,99 @@ mod tests { .expect("signature must verify under the key's own public key"); } + // Regression test for the ssh-key 0.6.7 `TryFrom<&RsaKeypair> for + // rsa::RsaPrivateKey` bug (passes `p` twice, omits `q` → `Πprimes = p² ≠ n` + // → `validate()` fails with `InvalidModulus` → `Error::Crypto`). Before the + // `from_components`-with-correct-p/q fix, this `.expect("sign")` panicked and + // every RSA identity was unusable ("agent refused operation"). We build the + // key from a fixed 2048-bit OpenSSH RSA key (no slow keygen), sign under + // each SHA2 variant OpenSSH requests, and verify each signature under the + // key's own public modulus so a correct-but-wrong-key regression can't slip + // through either. + #[test] + fn sign_data_with_privkey_rsa_signs_and_verifies() { + use rsa::signature::Verifier; + use rsa::{BigUint, RsaPublicKey}; + use ssh_agent_lib::proto::signature; + use ssh_key::private::PrivateKey; + + // Throwaway key generated solely for this test (never used elsewhere). + const RSA_TEST_KEY: &str = "\ +-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAABFwAAAAdzc2gtcn +NhAAAAAwEAAQAAAQEAv1DuBUp4HN3VKYb7f/4iMGxx3pC0r3AEql67eYoT+J5dm21EuOQE +pSz7+B2YnylnQ0ypMEsp0bOn99IlBeG++66Rp18Ypp6RUS8v27gNd4ZoI22gXDpQ3t/EIf +v5FYMNhqHYS9OWCSxwY0pVASVdQkKSV7LX+cD/Q0TwFFe+0pvweXd085T1L/IgD/fvroIb +3ckJ22ENGkdAH6wlcYqsTvxk2avZCRLifUZ+O3htBdfRa7MIuKBqD+QZyouXmkEPwiKkFo +NINzUCJmC19GoiaLwfW0OL2Md60ogmA+HO44k3pGcP2kWPJSO21wfmkLMMv6kLqHSDKUDP +1L+qFkxYXQAAA9DW6UJQ1ulCUAAAAAdzc2gtcnNhAAABAQC/UO4FSngc3dUphvt//iIwbH +HekLSvcASqXrt5ihP4nl2bbUS45ASlLPv4HZifKWdDTKkwSynRs6f30iUF4b77rpGnXxim +npFRLy/buA13hmgjbaBcOlDe38Qh+/kVgw2GodhL05YJLHBjSlUBJV1CQpJXstf5wP9DRP +AUV77Sm/B5d3TzlPUv8iAP9++ughvdyQnbYQ0aR0AfrCVxiqxO/GTZq9kJEuJ9Rn47eG0F +19Frswi4oGoP5BnKi5eaQQ/CIqQWg0g3NQImYLX0aiJovB9bQ4vYx3rSiCYD4c7jiTekZw +/aRY8lI7bXB+aQswy/qQuodIMpQM/Uv6oWTFhdAAAAAwEAAQAAAQAPdTtKH6Z9VJou0Qp8 +oLzD81su+7uxqigiWOWmcBblgWw4TPeexcOvUedo+IE2qPqAOE86SPRvzmeNsUPPChqrjM +MVhixAeDLvH5QrGV+zLt+2rxqkIQ0cOPHIuip5x709ydFnbQjkJFxPVXfxUALNQgI/hkKH +mkW1unn4ds+DBjT4JDlNQUsP/u9JLEnQtcsJT4tNrNT3TjqIc6L/MEf84POPI8WDwh/oJr +3txJCSSbDcvZe8e+Dks7Te81efsFqCfVOJdSO2HlYjIT2C60E/jh8+AC5CnXrG8uxw0LVe +Efm47qAy9Jey7CtqtqdWZ/lHYz09xo2jREbAba1Tt4RJAAAAgCSkmvIoXNd4atHlem/xkk ++RdaGh7kLmzuRSnFGSUZbHOxniXTetE80sr4PLpRfmATUw7ARLMYBpMGfVT09hF1FVHhLP +2bZwWNvyQeID3Mf9DdjxDojMrosTBFSDzTcy4vO+tzj2tSQvl64gfIAamhyhkMwziubbX5 +xZukhlVfp3AAAAgQD6567L89SEqjrduzyISvkDSdIfhQ/CLCt5e95YJ9kc4jrnHLYx83SJ +dwxb2C9I3QDoOskhDxhxMW3kL1xiIm9WcgBJzkimRImX8DpniNsEc03UEK5ovQxaOXHtV4 +7GtxAz6Lk6t1maFY5vskFBL6ZiqAcrYtFI18Xs+fKnOS97NQAAAIEAwzN6JnfM5bJ4uRkO +8ej2jLwDvtol/iOhG5X09snhK/iSuF1HztKhJ+eCM3o2t6fj30uy+UrdnArTZCrIl7EiMm +zlOx88YIANh/5p3Kf6SlADCbea0TJ9bpUQf3BhkzN6cE8RF04Uc3VtvUG1CKcyBr2Tj0TW +d0EI4yKGPuCZ5YkAAAAWdnQtcnNhLXJlZ3Jlc3Npb24tdGVzdAECAwQF +-----END OPENSSH PRIVATE KEY----- +"; + + let privkey = PrivateKey::from_openssh(RSA_TEST_KEY).expect("parse RSA test key"); + let data = b"sign@vt rsa regression payload"; + + // Public modulus for verification, rebuilt from the ssh public key's + // Mpint components the same way the signing core rebuilds the private key. + let mp = |m: &ssh_key::Mpint| { + BigUint::from_bytes_be(m.as_positive_bytes().expect("positive component")) + }; + let pubkey = match privkey.public_key().key_data() { + KeyData::Rsa(k) => RsaPublicKey::new(mp(&k.n), mp(&k.e)).expect("rsa pubkey"), + _ => unreachable!("embedded key is RSA"), + }; + + // rsa-sha2-256 + let sig = + sign_data_with_privkey(&privkey, data, signature::RSA_SHA2_256).expect("sign 256"); + assert_eq!(sig.algorithm().to_string(), "rsa-sha2-256"); + rsa::pkcs1v15::VerifyingKey::::new(pubkey.clone()) + .verify( + data, + &rsa::pkcs1v15::Signature::try_from(sig.as_bytes()).expect("sig256"), + ) + .expect("rsa-sha2-256 signature must verify"); + + // rsa-sha2-512 + let sig = + sign_data_with_privkey(&privkey, data, signature::RSA_SHA2_512).expect("sign 512"); + assert_eq!(sig.algorithm().to_string(), "rsa-sha2-512"); + rsa::pkcs1v15::VerifyingKey::::new(pubkey.clone()) + .verify( + data, + &rsa::pkcs1v15::Signature::try_from(sig.as_bytes()).expect("sig512"), + ) + .expect("rsa-sha2-512 signature must verify"); + + // legacy ssh-rsa (SHA-1) when no SHA2 flag is set + let sig = sign_data_with_privkey(&privkey, data, 0).expect("sign sha1"); + assert_eq!(sig.algorithm().to_string(), "ssh-rsa"); + rsa::pkcs1v15::VerifyingKey::::new(pubkey) + .verify( + data, + &rsa::pkcs1v15::Signature::try_from(sig.as_bytes()).expect("sig1"), + ) + .expect("ssh-rsa signature must verify"); + } + #[test] fn test_ssh_key_entry_serde_roundtrip() { let entries = vec![ From d2e5637a81d1ba2d0543f5891dbc82c3308177d6 Mon Sep 17 00:00:00 2001 From: qiqi Date: Tue, 7 Jul 2026 16:40:52 +0800 Subject: [PATCH 3/4] ci: run tests on Linux + macOS on every PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a GitHub Actions CI workflow triggered by default on pull_request (all base branches) and pushes to main: - test matrix over ubuntu-latest + macos-latest (cargo test); macOS is what exercises the cfg(target_os="macos") server_macos module incl. the RSA sign/verify regression test - Linux type-check parity with `just check` (host + linux-gnu boundary) - musl-static release build to catch musl link regressions pre-release Windows is intentionally excluded — vt is unix-only (setsid/O_NOFOLLOW/ forwarded SSH agent). Co-authored-by: Claude --- .github/workflows/ci.yml | 75 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..1a00d6a --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,75 @@ +name: CI + +# Default-triggers on every PR (all base branches) plus pushes to main. +on: + pull_request: + push: + branches: [main] + +# One in-flight run per ref: a new push to a PR cancels the stale run. +concurrency: + group: ci-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + # Unit + integration tests on every platform vt ships to. macos-latest is the + # one that actually exercises the `cfg(target_os = "macos")` module + # (server_macos: SSH agent, keychain, the RSA sign/verify regression test) — + # that code and its `rsa`/`sha1` deps don't compile on Linux at all. + test: + name: test (${{ matrix.os }}) + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v4 + + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@stable + + - uses: Swatinem/rust-cache@v2 + with: + key: ${{ matrix.os }} + + - name: Test + run: cargo test --locked --all-targets + + # Mirror `just check`: host + the x86_64-unknown-linux-gnu boundary that + # CLAUDE.md requires to stay green. Only meaningful on Linux. + - name: Type-check host + linux-gnu boundary + if: matrix.os == 'ubuntu-latest' + run: | + rustup target add x86_64-unknown-linux-gnu + cargo check --locked + cargo check --locked --target x86_64-unknown-linux-gnu + + # The shipping artifact is a musl-static binary (see justfile `build-musl`). + # Build it in CI so musl-specific link regressions (the ring/CRT segfault the + # justfile warns about) surface on PRs, not at release time. + build-musl: + name: build (linux musl-static) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@stable + with: + targets: x86_64-unknown-linux-musl + + - uses: Swatinem/rust-cache@v2 + with: + key: musl + + - name: Install musl tools + run: sudo apt-get update && sudo apt-get install -y musl-tools + + # CC lets ring (via rustls) compile its C/asm for musl; do NOT set a musl + # LINKER — rustc's self-contained musl link already bundles the CRT. + - name: Build (musl-static) + run: CC_x86_64_unknown_linux_musl=musl-gcc cargo build --release --bin vt --target x86_64-unknown-linux-musl From 114f9888a5838eb4ff853c513ccd9dbacfb72079 Mon Sep 17 00:00:00 2001 From: qiqi Date: Tue, 7 Jul 2026 16:46:24 +0800 Subject: [PATCH 4/4] test(agent): scope RSA regression test to SHA-2 (ssh-rsa/SHA-1 unbuildable) CI (macos-latest) caught that the ssh-rsa (SHA-1, flags==0) branch of sign_data_with_privkey can't build a Signature: ssh-key 0.6.7 Signature::new only accepts Algorithm::Rsa { hash: Some(_) }, so a plain ssh-rsa returns Encoding(Length). That's a pre-existing limitation orthogonal to the from_components fix. Assert only the reachable SHA-2 paths (which fully exercise the fix) and document the SHA-1 gap. Co-authored-by: Claude --- src/server_macos/ssh_agent.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/server_macos/ssh_agent.rs b/src/server_macos/ssh_agent.rs index 2707b05..80b074e 100644 --- a/src/server_macos/ssh_agent.rs +++ b/src/server_macos/ssh_agent.rs @@ -2446,22 +2446,21 @@ d0EI4yKGPuCZ5YkAAAAWdnQtcnNhLXJlZ3Jlc3Npb24tdGVzdAECAwQF let sig = sign_data_with_privkey(&privkey, data, signature::RSA_SHA2_512).expect("sign 512"); assert_eq!(sig.algorithm().to_string(), "rsa-sha2-512"); - rsa::pkcs1v15::VerifyingKey::::new(pubkey.clone()) + rsa::pkcs1v15::VerifyingKey::::new(pubkey) .verify( data, &rsa::pkcs1v15::Signature::try_from(sig.as_bytes()).expect("sig512"), ) .expect("rsa-sha2-512 signature must verify"); - // legacy ssh-rsa (SHA-1) when no SHA2 flag is set - let sig = sign_data_with_privkey(&privkey, data, 0).expect("sign sha1"); - assert_eq!(sig.algorithm().to_string(), "ssh-rsa"); - rsa::pkcs1v15::VerifyingKey::::new(pubkey) - .verify( - data, - &rsa::pkcs1v15::Signature::try_from(sig.as_bytes()).expect("sig1"), - ) - .expect("ssh-rsa signature must verify"); + // NOTE: the legacy ssh-rsa (SHA-1, `flags == 0`) branch is intentionally + // NOT asserted here. ssh-key 0.6.7 `Signature::new` only accepts RSA + // signatures with `Algorithm::Rsa { hash: Some(_) }`; a plain `ssh-rsa` + // (hash: None) hits the catch-all arm and returns `Encoding(Length)`, so + // that branch of `sign_data_with_privkey` can never succeed on this + // ssh-key version. That is a pre-existing limitation orthogonal to the + // from_components fix (which the SHA-2 paths above fully exercise), and + // SHA-1 ssh-rsa is deprecated (OpenSSH disables it by default ≥ 8.8). } #[test]