From 6fda882f4dbe81338f175bac835ef4636b5df5d8 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Sun, 1 Nov 2026 23:02:15 +0900 Subject: [PATCH] update: upgrade to russh 0.65 and adopt its agent and signer APIs Refresh Cargo.lock and raise manifest floors to the latest releases. russh 0.65 carries six security fixes (EXT_INFO allocation, zero maximum packet size, nested certificate stack exhaustion, agent server issues). Handler::auth_publickey now takes VerifiedPublicKey, which only russh can construct, so the server policy moves to SshHandler::authenticate_publickey for direct testing. Use russh #794: bssh sends session-bind@openssh.com on direct-authentication agent connections and, with the forwarding flag set, on relayed forwarded-agent connections. Refusal is non-fatal as in OpenSSH; a timeout closes that connection. In agent-restrict, the four direct constrained phases now pass. The four multihop phases still fail because ssh-agent requires publickey-hostbound-v00@openssh.com, which russh cannot send. Use russh #788: a failed signer now leaves the session usable, so the authenticate_declinable workaround is removed and agent and local signer failures move on to the next identity. Refs #327 --- Cargo.lock | 163 +++++++------ Cargo.toml | 52 ++-- docs/architecture/ssh-client.md | 3 +- docs/openssh-compatibility-status.md | 12 +- src/server/handler.rs | 213 ++++++++-------- src/ssh/tokio_client/authentication.rs | 255 ++++++++++++++------ src/ssh/tokio_client/connection.rs | 35 ++- src/ssh/tokio_client/error.rs | 5 +- src/ssh/tokio_client/host_key_type.rs | 2 +- src/ssh/tokio_client/security_key_signer.rs | 85 +------ tests/agent_forwarding_live_test.rs | 20 ++ tests/server_sk_policy_test.rs | 4 +- 12 files changed, 486 insertions(+), 363 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3911901b8..72b085445 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -217,9 +217,9 @@ checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" [[package]] name = "aws-lc-rs" -version = "1.18.2" +version = "1.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "faac5829c2b74c28f830747e7818ccfb684261b5f48a1118b1e2a13d36dfab13" +checksum = "048006790404103c488e6c5616b3e7e702a71292aadda08e8f7698a9e0d2eb53" dependencies = [ "aws-lc-sys", "untrusted 0.7.1", @@ -228,9 +228,9 @@ dependencies = [ [[package]] name = "aws-lc-sys" -version = "0.46.0" +version = "0.47.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f1622d8446a2d4b2ce0c7eefc73dd43a99779028d5ee5c2dd8073a658ba8a2bc" +checksum = "8bfbe32ac36b3c428696ce315464181bc4029d689fd58e3f2a4e58c252242f52" dependencies = [ "cc", "cmake", @@ -426,7 +426,7 @@ dependencies = [ "sha1", "sha2 0.11.1", "shell-words", - "signal-hook 0.4.5", + "signal-hook 0.4.6", "smallvec", "socket2", "ssh-key", @@ -505,9 +505,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.9.0" +version = "1.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "877bf67ac8e52449eb2f9e98a344ea163beb70dd1f9eebe3cc5357dcfeaee4df" +checksum = "86e055fa6e3e40ad6187ba25b6855eeb635d5271f8e0f0e1e9a5826b8d75bea0" dependencies = [ "find-msvc-tools", "jobserver", @@ -635,9 +635,9 @@ checksum = "1c133bc6a41be0d194c306b5506d15e6feeea7b1d6604bd3f8310dfb2ca96486" [[package]] name = "clipboard-win" -version = "5.4.1" +version = "5.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bde03770d3df201d4fb868f2c9c59e66a3e4e2bd06692a0fe701e7103c7e84d4" +checksum = "052af7878466feccda12ba7a6a67d019d712f296809559b47c12c8cc196d3a9a" dependencies = [ "error-code", ] @@ -909,9 +909,9 @@ checksum = "460fbee9c2c2f33933d720630a6a0bac33ba7053db5344fac858d4b8952d77d5" [[package]] name = "crypto-bigint" -version = "0.7.5" +version = "0.7.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a52aa3fcda4e6302a9f48734f234d35d4721b96f8fe07d073f07ce9df4f0271" +checksum = "9c035cb64cba03dad2fb921f3e218785e8cf9a41790da5eb3a1400f62fa2be11" dependencies = [ "cpubits", "ctutils", @@ -976,9 +976,9 @@ dependencies = [ [[package]] name = "ctutils" -version = "0.4.5" +version = "0.4.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "38d28fb6aa7dd680b3ffd1a3e64e115c177bdba63c0ae7f9cd48129ff05c4fa0" +checksum = "a2376ffd6f96d8a9db59875b4b6b0237d6900b5baf64279714e05493ce64828a" dependencies = [ "cmov", "subtle", @@ -1085,9 +1085,9 @@ checksum = "5729f5117e208430e437df2f4843f5e5952997175992d1414f94c57d61e270b4" [[package]] name = "der" -version = "0.8.2" +version = "0.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a878c850e9e421b20262e9b41f9c860e4785fa07541c266b62ff9d1ef998a80a" +checksum = "6a996a108feb8b427a6e12fcf6af4aa83409b79d72c15337d7741368131ee004" dependencies = [ "const-oid", "der_derive", @@ -1268,9 +1268,9 @@ checksum = "0e9c71c2167ca323c882b99918929403426e2373ea17242ff5653e0d5e1058be" [[package]] name = "elliptic-curve" -version = "0.14.1" +version = "0.14.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9d65aa39b3a5c1c9c1b745c9a019234bb7a21b77abcb4f4d266d706e2d577d65" +checksum = "900f37e7e18d74a2347389af6cd69e3a30d5cc9e697f40004865679aa75e9c7c" dependencies = [ "base16ct", "crypto-bigint", @@ -2264,9 +2264,9 @@ dependencies = [ [[package]] name = "js-sys" -version = "0.3.106" +version = "0.3.107" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7883d941dae510fb2d978fc3fe018c71c9e2892fd38854de3e8b92c2e5ad9cc5" +checksum = "8dc09d2154098bbf4ef54eab6b117a3c24cbbffe94046b16ee6d6d4510584d3a" dependencies = [ "cfg-if", "futures-util", @@ -2391,9 +2391,9 @@ dependencies = [ [[package]] name = "log" -version = "0.4.34" +version = "0.4.35" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f9f8bd3e56ce4dfc153cf470fffbfa98c7620958b312ca5c3a4b8d5181fd13c6" +checksum = "5d15f86d12da44cd8d4ec3c73984537eb7f4cadcb9776a575a0a8a05ca11005c" [[package]] name = "lru" @@ -2498,9 +2498,9 @@ dependencies = [ [[package]] name = "ml-kem" -version = "0.3.3" +version = "0.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a9ba01ebcf90f6aff8e9b9f55bb69b19e6250e4ade5787d0040eaa27dd226238" +checksum = "84ca78b66f46050d49fa21693ae8adab2081d8101b65d8067316b6b70b64a0d1" dependencies = [ "hybrid-array", "kem", @@ -2564,9 +2564,9 @@ dependencies = [ [[package]] name = "module-lattice" -version = "0.2.3" +version = "0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c61b87c9683ab7cb1c6871d261ad5479b6b10ceb52c4352aaca3b5d35a8febe" +checksum = "c4a1e412c8ee83a1c678f427806219c49c96b2511b9f07497514f0e43fb26155" dependencies = [ "ctutils", "hybrid-array", @@ -2765,9 +2765,9 @@ checksum = "d6790f58c7ff633d8771f42965289203411a5e5c68388703c06e14f24770b41e" [[package]] name = "openssl-probe" -version = "0.2.1" +version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7c87def4c32ab89d880effc9e097653c8da5d6ef28e6b539d313baaacfbafcbe" +checksum = "7e036b411754132fd6b74e94b74e9a13c5cf606fa93d1885c35860d3060820b1" [[package]] name = "opentelemetry" @@ -2921,9 +2921,9 @@ dependencies = [ [[package]] name = "pageant" -version = "0.2.4" +version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f0786409b5aaa884655c9b4065681db0835db0cc185a6fb19fb17a5d38656866" +checksum = "e0bc922d9aa8cfcf598de27b3c6122cb2eeda9d6fe71527a22ebcb7f633540ff" dependencies = [ "base16ct", "byteorder", @@ -3032,21 +3032,20 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "pest" -version = "2.9.4" +version = "2.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fd47a460a9cbc3b3429c7e22bf0d76653e779319bae45a499ca010c9a990bb23" +checksum = "a8db75153506001cb36ed924f08cadc862eb3f8c4d0d534139f29074769d7660" dependencies = [ "memchr", - "psm", - "stacker", + "pest_stack_guard", "ucd-trie", ] [[package]] name = "pest_derive" -version = "2.9.4" +version = "2.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "203e46697708f7519ab1608393cf89239cafb46b8e6d08a8ffe359359b11ef44" +checksum = "20aa17e11fb35664b67d5214a0afac57a360d231b6ec53d6eba0700b27d933b4" dependencies = [ "pest", "pest_generator", @@ -3054,9 +3053,9 @@ dependencies = [ [[package]] name = "pest_generator" -version = "2.9.4" +version = "2.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e84ae6340a458d3d6cfc73cf328cf3cfe90b2484cd0de25eb66aa954615ed4d5" +checksum = "626ec5afccf44ccbfeb2b232316510bb55b4bf34d891afcb0fb07d2b17d7fe9d" dependencies = [ "pest", "pest_meta", @@ -3067,13 +3066,23 @@ dependencies = [ [[package]] name = "pest_meta" -version = "2.9.4" +version = "2.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b57f92f32b3dbce6598fd8f5cdf1f06a6072570b8103fa21728fa9036923e710" +checksum = "b3f45b6caa65c1b08186832745159c2529994146340a7bf44168095469720f5d" dependencies = [ "pest", ] +[[package]] +name = "pest_stack_guard" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97e6cd2217057fa86b6eaa512917e58089d875b957a31e35e7ab729977e0a5f2" +dependencies = [ + "psm", + "stacker", +] + [[package]] name = "phc" version = "0.6.1" @@ -3809,9 +3818,9 @@ dependencies = [ [[package]] name = "russh" -version = "0.64.1" +version = "0.65.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba61e87b9ec9a39a59a6bbed4c0b8ff7fb07073405b24a767a1f2c8560ac8f58" +checksum = "3bb77a4b259312b2abbc541bca1c13cb4e9b61c560a7fd894068c3b92e0c9dc3" dependencies = [ "aes", "aws-lc-rs", @@ -3933,9 +3942,9 @@ dependencies = [ [[package]] name = "rustix" -version = "1.1.5" +version = "1.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "891efababe418670775f199f0d233d84843c227a0949a883ce15b37c78d6629d" +checksum = "e5e76742e5294153d4dde1207dbec5bdd81eef6fabdc0d2118babadabfd858aa" dependencies = [ "bitflags 2.13.2", "errno", @@ -4324,9 +4333,9 @@ dependencies = [ [[package]] name = "signal-hook" -version = "0.4.5" +version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b29ffa27d3d59a049bb328258de100f8012848d1b08d444e08bbc02ac468435e" +checksum = "49f7447afd772ca79ad17ae0a3972a6b0df0cb4b80990fd82707593e9e012a74" dependencies = [ "libc", "signal-hook-registry", @@ -4405,9 +4414,9 @@ checksum = "0c790de23124f9ab44544d7ac05d60440adc586479ce501c1d6d7da3cd8c9cf5" [[package]] name = "smallvec" -version = "1.16.3" +version = "1.16.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b3dc8af474f516a851ff4bd12db780f948b9250ad37211e4eec0bccea54e01b" +checksum = "d0d5d4b1981b8bbbb7aed204cae5726d683d5d6ac7e74ec54122ea28f315e7c8" [[package]] name = "socket2" @@ -4947,13 +4956,14 @@ dependencies = [ [[package]] name = "tonic" -version = "0.14.6" +version = "0.14.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ac2a5518c70fa84342385732db33fb3f44bc4cc748936eb5833d2df34d6445ef" +checksum = "e2cdfc673aabd30fe8fee92e65837b644c37c5d47f0f97ae6d5e0f1237d15e1b" dependencies = [ "async-trait", "base64 0.22.1", "bytes", + "h2", "http", "http-body", "http-body-util", @@ -4965,6 +4975,7 @@ dependencies = [ "sync_wrapper", "tokio", "tokio-stream", + "tokio-util", "tower", "tower-layer", "tower-service", @@ -4973,9 +4984,9 @@ dependencies = [ [[package]] name = "tonic-prost" -version = "0.14.6" +version = "0.14.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "50849f68853be452acf590cde0b146665b8d507b3b8af17261df47e02c209ea0" +checksum = "eba464054e2226f182a34489b8b079bf4c6e3e7ab5fd7fc30e024f24aa92a484" dependencies = [ "bytes", "prost", @@ -4984,9 +4995,9 @@ dependencies = [ [[package]] name = "tonic-types" -version = "0.14.6" +version = "0.14.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "73ab1b02061f83d519bba3caa167f88f261ef05720ab8ebc954ade70de3348e8" +checksum = "ccbb6c31deb8e99dcdf87df19c5289b7838a600da74a8224baa671e52fe7c5ad" dependencies = [ "prost", "prost-types", @@ -5146,9 +5157,9 @@ dependencies = [ [[package]] name = "unicode-width" -version = "0.2.2" +version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b4ac048d71ede7ee76d585517add45da530660ef4390e49b098733c6e897f254" +checksum = "ca718471876992059f251e33aafc334346ada08042b7661dd1fe313a9bced557" [[package]] name = "unit-prefix" @@ -5198,9 +5209,12 @@ dependencies = [ [[package]] name = "utf8_iter" -version = "1.0.4" +version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b6c140620e7ffbb22c2dee59cafe6084a59b5ffc27a8859a5f0d494b5d52b6be" +checksum = "af3bd9d3e571c26fcce9d033b87ea5ab061b8d2ffe9b7a5675d810d68f03edf0" +dependencies = [ + "utf_types", +] [[package]] name = "utf8parse" @@ -5208,11 +5222,17 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" +[[package]] +name = "utf_types" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3a3b425286980a367278453f3b6d28cdcb134b574e5a35824a6ff5eebad50bb6" + [[package]] name = "uuid" -version = "1.28.0" +version = "1.29.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7cc1186384beb7dd8eedea376413fd654937285ea6c9cfbb928dc3043ea4b606" +checksum = "4a0e2d11620c50695fe12bbbfde96b3477fe606e7284c22d505c12ce51214514" dependencies = [ "atomic", "getrandom 0.4.3", @@ -5295,12 +5315,11 @@ dependencies = [ [[package]] name = "wasm-bindgen" -version = "0.2.129" +version = "0.2.130" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9bb54f33acc68fd454578d9820b0bde1a1a3d17aa17bb7b6595806d02886d409" +checksum = "a0181c14ddd6c7b8b0b0132a5f1ea2f32be0d893cd3ea97b6e3b28886b8a3d1c" dependencies = [ "cfg-if", - "once_cell", "rustversion", "wasm-bindgen-macro", "wasm-bindgen-shared", @@ -5308,9 +5327,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-futures" -version = "0.4.79" +version = "0.4.80" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3cbab34de2d982e9b48e18d216d04c4a6f641066ff19ffb699980f591ee3610e" +checksum = "5e14e0327cb5368ad60ccb22d04a9827a693c95a926d8d125e9fa39e0baec501" dependencies = [ "js-sys", "tokio", @@ -5319,9 +5338,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro" -version = "0.2.129" +version = "0.2.130" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2e29d0c35b16e224a7eeb5cd2d25e3e1968fbd65604117b44d3b789d00ee8535" +checksum = "6e931a02a39abdbf66966fc35902b5ed58344750a70885bed4ebb3d8eaf28ed9" dependencies = [ "quote", "wasm-bindgen-macro-support", @@ -5329,9 +5348,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro-support" -version = "0.2.129" +version = "0.2.130" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6f501a8bc3719dba86ef8ae4728879c08001bea749eb1333ac5b91e040e2a6b7" +checksum = "2ca4b54280e455bd831e33a6b8e734baf432fe63fbc35c42ac14ce67a043a53a" dependencies = [ "bumpalo", "proc-macro2", @@ -5342,18 +5361,18 @@ dependencies = [ [[package]] name = "wasm-bindgen-shared" -version = "0.2.129" +version = "0.2.130" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "23f0c9c52aa7cd7d77769a4cfe2a9adb1b331f489a41d912ce14513d5ab995c6" +checksum = "3b1f9c3798725016c6ee5f2f57869ec0831e8bbc0ab0310a86e787e97f41389e" dependencies = [ "unicode-ident", ] [[package]] name = "web-sys" -version = "0.3.106" +version = "0.3.107" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "88261b9deccee56594c11a3460c462c41f58d148598fe70ad77070126a68aba4" +checksum = "d7d179596ff5ec0f6de4db64d54011bde6289cc49603d3a6e4ce3f329207ab88" dependencies = [ "js-sys", "wasm-bindgen", diff --git a/Cargo.toml b/Cargo.toml index 0f1186843..abcbec712 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,58 +23,58 @@ edition = "2024" rust-version = "1.97" [dependencies] -bytes = "1.11.1" -tokio = { version = "1.52.3", features = ["full"] } -# Upstream russh. 0.64 includes the hostkeys-prove support and the post-auth +bytes = "1.12.1" +tokio = { version = "1.54.1", features = ["full"] } +# Upstream russh. 0.64+ includes the hostkeys-prove support and the post-auth # rekey limit fixes bssh previously carried in an internal fork. # `client::Handler::check_server_key` receives `PublicKeyOrCertificate`, and # host certificates are opt-in. bssh leaves `Preferred::host_key_certificates` # empty, so it never negotiates one; the handler in # src/ssh/tokio_client/connection.rs refuses certificates because bssh # verifies no CA signatures. -russh = "0.64.1" +russh = "0.65.0" # Upstream russh-sftp. bssh's pipelined client transfers live in # src/ssh/tokio_client/sftp_client.rs and its read-ahead / write-coalescing # server loop in src/server/sftp_loop.rs, both on russh-sftp's public API. russh-sftp = "3.0.1" -clap = { version = "4.6.1", features = ["derive", "env"] } -anyhow = "1.0.102" -thiserror = "2.0.18" +clap = { version = "4.6.7", features = ["derive", "env"] } +anyhow = "1.0.104" +thiserror = "2.0.21" tracing = "0.1.44" tracing-subscriber = { version = "0.3.23", features = ["env-filter"] } -serde = { version = "1.0.228", features = ["derive"] } +serde = { version = "1.0.229", features = ["derive"] } serde_yaml = "0.9" -futures = "0.3.32" -async-trait = "0.1.89" -indicatif = "0.18.4" -rpassword = "7.4.0" +futures = "0.3.34" +async-trait = "0.1.92" +indicatif = "0.18.6" +rpassword = "7.5.4" dirs = "7.0" -chrono = { version = "0.4.44", features = ["serde"] } -glob = "0.3.3" -whoami = "2.1.1" +chrono = { version = "0.4.45", features = ["serde"] } +glob = "0.3.4" +whoami = "2.1.3" sha1 = "0.11.0" -owo-colors = "4.3.0" -unicode-width = "0.2.2" +owo-colors = "4.4.0" +unicode-width = "0.2.3" terminal_size = "0.4.4" -zeroize = { version = "1.8.2", features = ["derive"] } +zeroize = { version = "1.9.1", features = ["derive"] } secrecy = { version = "0.10.3", features = ["serde"] } rustyline = "19.0.0" crossterm = "0.29" ratatui = "0.30" -regex = "1.12.3" +regex = "1.13.1" signal-hook = "0.4" nix = { version = "0.31", features = ["fs", "poll", "process", "signal", "term"] } -smallvec = "1.15.1" +smallvec = "1.16.4" lru = "0.19.0" -uuid = { version = "1.23.1", features = ["v4"] } -tokio-util = "0.7.18" +uuid = { version = "1.29.0", features = ["v4"] } +tokio-util = "0.7.20" socket2 = { version = "0.6", features = ["all"] } shell-words = "1.1.1" base64 = "0.23.1" hmac = "0.13.0" hkdf = "0.13.0" -sha2 = "0.11.0" -hickory-resolver = { version = "0.26.1", features = ["dnssec-ring"] } +sha2 = "0.11.1" +hickory-resolver = { version = "0.26.3", features = ["dnssec-ring"] } libc = "0.2" # sk-api (FIDO security key) middleware libraries are dlopened in process; # see src/ssh/security_key for why no ssh-sk-helper style subprocess exists. @@ -93,7 +93,7 @@ ipnetwork = "0.21" bcrypt = "0.19" argon2 = "0.6" rand = "0.10" -# Pinned to match russh's ssh-key (0.64.x uses =0.7.0-rc.11) so the workspace +# Pinned to match russh's ssh-key (0.65.x uses =0.7.0-rc.11) so the workspace # resolves a single ssh-key version instead of 0.6 + 0.7-rc side by side. ssh-key = { version = "=0.7.0-rc.11", features = ["std"] } async-compression = { version = "0.4", features = ["tokio", "gzip"] } @@ -128,7 +128,7 @@ tempfile = "3.27.0" mockito = "1.7.2" tokio-test = "0.4" serial_test = "4.0.1" -insta = "1.47" +insta = "1.49" criterion = { version = "0.8", features = ["html_reports"] } mockall = "0.15.0" diff --git a/docs/architecture/ssh-client.md b/docs/architecture/ssh-client.md index b205e7823..8bc6dcc8f 100644 --- a/docs/architecture/ssh-client.md +++ b/docs/architecture/ssh-client.md @@ -15,7 +15,8 @@ **Tokio Client Module Structure :** - `tokio_client/connection.rs` - Connection management (293 lines) - `tokio_client/authentication.rs` - Authentication methods (378 lines) -- `tokio_client/security_key_signer.rs` - russh `Signer` for FIDO `sk-*` identities: resolves `SecurityKeyProvider`, checks the provider before decrypting an `sk-*` key file, calls the provider on a blocking thread (serialized process-wide), prints the touch notice, prompts once for a PIN, and declines a failed signature so authentication continues with the next identity (`authenticate_declinable`) +- `tokio_client/authentication.rs` and `tokio_client/connection.rs` - agent session binding: every agent connection is bound with `session-bind@openssh.com` before use, with the forwarding flag clear for direct authentication and set for relayed forwarded-agent connections; an agent's refusal is not fatal, and a timeout abandons that connection +- `tokio_client/security_key_signer.rs` - russh `Signer` for FIDO `sk-*` identities: resolves `SecurityKeyProvider`, checks the provider before decrypting an `sk-*` key file, calls the provider on a blocking thread (serialized process-wide), prints the touch notice, prompts once for a PIN, and returns signing failures to russh, which abandons the request so authentication continues with the next identity - `ssh/security_key/` - provider interface (`SecurityKeyProvider` trait, `$VAR` resolution, `internal`/`none` handling), the `sk-api.h` `dlopen` binding (`external.rs`), the `sk-*` signature blob encoder (`signature.rs`), and the built-in provider (`internal/`: `ctaphid.rs` framing and channels over a `HidDevice` trait, `cbor.rs`, `ctap2.rs`, `u2f.rs`, `pin_protocol.rs` for PIN protocols 1 and 2, `device.rs` for one authenticator's `getAssertion`, `select.rs` for `sk_probe`-style device selection, `hidapi_backend.rs` as the only OS-facing file, and `mock.rs` as the scripted test authenticator). The built-in provider exists on Linux and macOS only; elsewhere `internal` fails with a "not available on this platform" error - `tokio_client/host_key_type.rs` - refuses a server host key or certificate whose type differs from the negotiated host-key algorithm (OpenSSH `kex_verify_host_key`), including `sk-*` types and ECDSA curves - `tokio_client/channel_manager.rs` - Channel operations (230 lines) diff --git a/docs/openssh-compatibility-status.md b/docs/openssh-compatibility-status.md index 96ac69645..a3abe0e90 100644 --- a/docs/openssh-compatibility-status.md +++ b/docs/openssh-compatibility-status.md @@ -19,7 +19,7 @@ This ledger records contracts still requiring implementation or verification, no | Dynamic forwarding | Local SOCKS4/4A/5; remote dynamic `-R [bind_address:]port`, destination restrictions and cancellation | Historical `dynamic-forward` fails on both platforms. Passing its local `-D` section does not establish remote dynamic support. | | Percent/environment | Directive-specific tokens and environment expansion, trusted `Match exec`, explicitly permitted shell `LocalCommand` | Historical `percent` fails on both platforms. Shell semantics must retain `PermitLocalCommand` gating and untrusted-input boundaries. | | Rekey | Actual byte/time-triggered exchange and truthful KEX/NEWKEYS diagnostics | `rekey` passes on Linux (114.3 s candidate, 114.5 s reference) after the client stopped padding every connection by 0.6 s; the manifest minimum is 180 s. macOS remains environmental (the reference client also exceeds the limit there). | -| Agent | Destination-constrained identities (`session-bind@openssh.com`) | Every `agent-restrict` phase that does not need a constrained key passes on Linux; the eight constrained phases fail until russh exposes the binding tuple. Tracked in #327. | +| Agent | Destination-constrained identities (`session-bind@openssh.com`) | bssh sends `session-bind@openssh.com` on agent and forwarded-agent connections, so destination-constrained keys work for direct authentication. The four multihop phases that sign through a forwarded agent still fail: ssh-agent requires a `publickey-hostbound-v00@openssh.com` request, which russh 0.65.0 cannot send. Tracked in #327. | | Security keys | `webauthn-sk-*` signatures and physical-authenticator validation | `hostkey-agent`, `keytype`, `limit-keytype` and `agent` pass on Linux with the regress `sk-dummy` middleware. The built-in `SecurityKeyProvider internal` (the default) signs over USB HID on Linux and macOS (#331) and is covered by mock-transport tests; a check against a physical authenticator is pending user validation. `webauthn-sk-ecdsa-sha2-nistp256@openssh.com` is not advertised because bssh cannot verify it (#345). `bssh-keygen` enrolls both SK key types and downloads resident credentials with `-K` through internal and external providers (#337); mock-device, native ABI, and OpenSSH key-file parsing tests cover these paths. | | Multiplexing | OpenSSH mux-v4 local protocol, raw `-O proxy`, complete channel reporting, allocated remote-port replies | `multiplex` passes on Linux (72.1 s) including TCP and Unix-domain `-O forward`/`-O cancel`, when the regress directory path fits `sockaddr_un`. Control sockets still use framed JSON; passenger reuse is not OpenSSH-master interoperability. | | Jump hosts | CLI/config parsing, SSH URI hops, `none`, multihop transport/authentication | Historical `proxyjump` fails on both platforms. Parser tests alone do not establish transport behavior. | @@ -35,7 +35,7 @@ The implemented control socket is bssh's private framed-JSON protocol. Its passe Unix-domain forwarding (`-L`/`-R` with a socket path on either side, including `-O forward`/`-O cancel`) never unlinks an existing path before binding and refuses to serve a bound path that is a symlink or not a socket; the listener socket is owner-only (mode 0600) without any mode change through the path name: on Linux the unbound socket is narrowed before `bind(2)` so the node is created 0600; elsewhere it is bound in a private `mkdtemp` directory beside the path, narrowed there with `fchmodat(AT_SYMLINK_NOFOLLOW)`, and moved into place without replacing anything (`renamex_np(RENAME_EXCL)` on macOS, `link(2)` then `unlink(2)` on other BSDs), so the node never appears at the path with a wider mode. Only a socket node is removed on shutdown. `StreamLocalBindUnlink` and `StreamLocalBindMask` are still parsed without a runtime consumer; the OpenSSH defaults (`no`, `0177`) are what the implementation applies. -Host certificates are accepted only after full validation: a `@cert-authority` line (in a known_hosts file or in the `KnownHostsCommand` output, which receives the certificate as `%K`/`%t` and runs once per check) naming the host (or `HostKeyAlias`) with the signing CA key, host certificate type, a principal matching the host name (principals may be case-sensitive wildcard patterns, as in OpenSSH, and are read only after the CA signature verifies), no critical options, a CA signature algorithm permitted by `CASignatureAlgorithms`, the validity window, a verified signature, and no revocation by `@revoked` lines in either source or by any `RevokedHostKeys` file (plain key lists; a KRL or unreadable file anywhere in the list fails closed). On a non-default port a CA line for the bare host name also matches, as OpenSSH's portless retry does. Without a matching CA the certified key is checked as a plain host key, but only if `HostKeyAlgorithms` permits its plain type, so a certificate-only list never falls back to known_hosts or first-use recording; a pinned key (`ServerCheckMethod::PublicKey`/`PublicKeyFile`) never consults CA lines; with a matching CA any other validation failure refuses the connection instead of downgrading, which is stricter than OpenSSH. `HostKeyAlgorithms` accepts certificate names and advertises the certificate form of every default algorithm; russh always sends certificate names ahead of plain names, so a mixed list cannot interleave them, and a certificate-only list yields no plain algorithm at all. The host key the server presents must have the type the key exchange negotiated, as `kex_verify_host_key` requires (`SSH_ERR_KEY_TYPE_MISMATCH`): `rsa-sha2-256` and `rsa-sha2-512` select an RSA key, a `*-cert-v01@openssh.com` name selects a certificate for that key type, and ECDSA curves must match, also for certificates. A mismatch fails the connection with `host key type ... does not match the negotiated host key algorithm ...` before any known_hosts or `StrictHostKeyChecking` decision; russh 0.64.1 verifies the exchange signature with whatever key arrives, so without this check a `HostKeyAlgorithms` restriction would not limit which trusted key type a server could use. russh does not hand the host key of a re-key to the client, so this check, like the known_hosts check, covers the initial key exchange only. `@cert-authority` host fields follow OpenSSH matching, including hashed (`|1|`) entries and `!pattern` negations that exclude a host from the CA's scope. A `RevokedHostKeys` path that cannot be expanded refuses every host key instead of disabling revocation, and `RevokedHostKeys` applies to certificates under `StrictHostKeyChecking=no` as it does to plain keys. +Host certificates are accepted only after full validation: a `@cert-authority` line (in a known_hosts file or in the `KnownHostsCommand` output, which receives the certificate as `%K`/`%t` and runs once per check) naming the host (or `HostKeyAlias`) with the signing CA key, host certificate type, a principal matching the host name (principals may be case-sensitive wildcard patterns, as in OpenSSH, and are read only after the CA signature verifies), no critical options, a CA signature algorithm permitted by `CASignatureAlgorithms`, the validity window, a verified signature, and no revocation by `@revoked` lines in either source or by any `RevokedHostKeys` file (plain key lists; a KRL or unreadable file anywhere in the list fails closed). On a non-default port a CA line for the bare host name also matches, as OpenSSH's portless retry does. Without a matching CA the certified key is checked as a plain host key, but only if `HostKeyAlgorithms` permits its plain type, so a certificate-only list never falls back to known_hosts or first-use recording; a pinned key (`ServerCheckMethod::PublicKey`/`PublicKeyFile`) never consults CA lines; with a matching CA any other validation failure refuses the connection instead of downgrading, which is stricter than OpenSSH. `HostKeyAlgorithms` accepts certificate names and advertises the certificate form of every default algorithm; russh always sends certificate names ahead of plain names, so a mixed list cannot interleave them, and a certificate-only list yields no plain algorithm at all. The host key the server presents must have the type the key exchange negotiated, as `kex_verify_host_key` requires (`SSH_ERR_KEY_TYPE_MISMATCH`): `rsa-sha2-256` and `rsa-sha2-512` select an RSA key, a `*-cert-v01@openssh.com` name selects a certificate for that key type, and ECDSA curves must match, also for certificates. A mismatch fails the connection with `host key type ... does not match the negotiated host key algorithm ...` before any known_hosts or `StrictHostKeyChecking` decision; russh 0.65.0 verifies the exchange signature with whatever key arrives, so without this check a `HostKeyAlgorithms` restriction would not limit which trusted key type a server could use. russh does not hand the host key of a re-key to the client, so this check, like the known_hosts check, covers the initial key exchange only. `@cert-authority` host fields follow OpenSSH matching, including hashed (`|1|`) entries and `!pattern` negations that exclude a host from the CA's scope. A `RevokedHostKeys` path that cannot be expanded refuses every host key instead of disabling revocation, and `RevokedHostKeys` applies to certificates under `StrictHostKeyChecking=no` as it does to plain keys. ## Security keys @@ -55,9 +55,9 @@ These exclusions bounded the original score-gated epic, not full compatibility. ## Dependency boundary: destination-constrained agents -`agent-restrict` requires OpenSSH `session-bind@openssh.com` before agent identity listing. russh 0.64.1 does not expose the complete proof tuple: the exact negotiated host-key/certificate blob, initial KEX session ID, and server KEX signature. Binding a plain key instead of the certificate blob loses certificate-principal constraints; replacing the initial proof after rekey is also incorrect. Direct authentication and forwarded agent connections require distinct binding modes. Constraints must never be dropped to make authentication succeed. +`agent-restrict` requires OpenSSH `session-bind@openssh.com` before agent identity listing. russh 0.65.0 (`Handle::agent_session_binding` and `AgentClient::bind_session`, upstream [#794](https://github.com/Eugeny/russh/pull/794)) exposes the proof tuple from the initial key exchange: the exact negotiated host-key/certificate blob, session ID, and server KEX signature. bssh binds each agent connection used for direct authentication with the forwarding flag clear and each relayed forwarded-agent connection with it set. As in `ssh_agent_bind_hostkey`, an agent's refusal is not fatal, and an enforcing agent still refuses constrained keys on an unbound connection. Constraints must never be dropped to make authentication succeed. -[PR #317](https://github.com/lablup/bssh/pull/317) removed the maintained russh fork after required APIs landed upstream; [PR #320](https://github.com/lablup/bssh/pull/320) similarly returned SFTP to upstream. Full constrained-agent support therefore needs an explicit dependency decision: preferably add a typed binding API upstream and consume a published release; alternatively use a temporary immutable revision of the official upstream after merge. A contributor-fork bridge or vendored implementation requires separate approval, a removal plan, and the same protocol tests. No such dependency change is implied by this ledger or by the existing agent-authentication fixes. +The remaining gap is also a dependency boundary. A signature made through a forwarded agent must come from a `publickey-hostbound-v00@openssh.com` userauth request carrying the server host key, or ssh-agent refuses it (`no hostkey recorded in signature for forwarded connection`). russh 0.65.0 always sends method `publickey` and bssh cannot change that from a `Signer`, so `multihop restricted`, `multihop username`, `multihop wildcard username`, and `multihop cycle restricted allow` stay failing. [PR #317](https://github.com/lablup/bssh/pull/317) removed the maintained russh fork after required APIs landed upstream, and [PR #320](https://github.com/lablup/bssh/pull/320) similarly returned SFTP to upstream, so the preferred route is again an upstream API consumed from a published release. A contributor-fork bridge or vendored implementation requires separate approval, a removal plan, and the same protocol tests. ## Follow-up validation checkpoints @@ -67,7 +67,7 @@ The following focused checks were performed on a Linux aarch64 host while closin - `multiplex`: passes in 72.1 s through the short OBJ path, including the TCP `forward` phase, Unix-domain `forward`, and the TCP and Unix `-O forward`/`-O cancel` phases. - `hostkey-agent`: every phase passes, including the two `sk-*` certificate phases that #328 added (the regress tree supplies `sk-dummy.so` through `SSH_SK_PROVIDER`). An explicit certificate-only `HostKeyAlgorithms` restriction is honored instead of being widened to the default certificate list; `certificate_only_host_key_algorithms_are_never_widened` in `connection_tests.rs` keeps it that way. - `keytype`, `limit-keytype` and `agent`: pass with the `sk-*` key types active. `keytype` signs user authentication with `ed25519-sk` and `ecdsa-sk` identity files through the provider. -- `agent-restrict`: through the short OBJ path, every phase passes except the eight that need `session-bind@openssh.com` (#327), including the certificate hosts `host_d` and `host_e` in the `w/o agent` and unrestricted phases. +- `agent-restrict`: through the short OBJ path, every phase passed except the eight that needed `session-bind@openssh.com` (#327; four remain after russh 0.65, see the dependency boundary above), including the certificate hosts `host_d` and `host_e` in the `w/o agent` and unrestricted phases. The earlier macOS checkpoints from the first #314 follow-up are kept below for history. @@ -77,7 +77,7 @@ The earlier macOS checkpoints from the first #314 follow-up are kept below for h - `proxyjump`: pinned harness passes (4.887 seconds). Five focused parser/chain tests also pass. - `scp3`: candidate now completes all legacy SCP and SFTP cases instead of hanging on the first pipe-backed session; candidate and reference encounter the same macOS symlink/unsafe temporary-path failure, so the pinned verdict remains environmental (7.519 seconds), not pass. An explicit legacy `scp -O -3` 1 MiB remote-to-remote copy succeeds with matching bytes. The cancellable pipe/socket stdin helper has five passing focused tests, including regular-file routing, buffered-socket draining and fail-closed `O_RDWR` FIFO handling. - `multiplex`: pinned test remains failed (78.3 seconds), with Unix-domain forwarding and control forward/cancel failures. Endpoint/negotiated-algorithm `conninfo`, check/exit/stop, status and TCP phases pass. Snapshot SHA-256: `bc9ca3e4f9d672d3416a9f3a8fe8a538d8db3e2f8b19f8028fbee6085551c58b`. Passing passenger-emulated proxy phases does not establish raw mux-v4 proxy support. -- `agent-restrict` (historical): unrestricted/offline-private-key phase passes; destination restrictions remain blocked on the upstream binding API. +- `agent-restrict` (historical): unrestricted/offline-private-key phase passes; destination restrictions were blocked on the upstream binding API, which russh 0.65.0 now provides. - `hostkey-agent` (historical): plain host-key algorithms pass; the certificate slice of that time was removed. Security regression coverage preserves trusted-file-only shell Match execution, bounded SOCKS negotiation, and first-obtained fail-closed `PermitRemoteOpen`. Host-certificate authority validation, certificate revocation and `CASignatureAlgorithms` policy are implemented and covered by unit tests in `src/ssh/tokio_client/host_certificate.rs` and `host_verification.rs`. diff --git a/src/server/handler.rs b/src/server/handler.rs index f02b4a57a..37ad8e6ec 100644 --- a/src/server/handler.rs +++ b/src/server/handler.rs @@ -245,112 +245,15 @@ impl SshHandler { .as_ref() .is_some_and(|s| s.auth_attempts >= self.config.max_auth_attempts) } -} - -impl russh::server::Handler for SshHandler { - type Error = anyhow::Error; - - /// Called when a new session channel is created. - fn channel_open_session( - &mut self, - channel: Channel, - reply: ChannelOpenHandle, - _session: &mut Session, - ) -> impl std::future::Future> + Send { - let channel_id = channel.id(); - tracing::debug!( - peer = ?self.peer_addr, - channel = ?channel_id, - "Channel opened for session" - ); - - // Store the channel itself so we can use it for subsystems like SFTP - self.channels - .insert(channel_id, ChannelState::with_channel(channel)); - async move { - reply.accept().await; - Ok(()) - } - } - /// Handle 'none' authentication. - /// - /// Always rejects and advertises available authentication methods. - fn auth_none( - &mut self, - user: &str, - ) -> impl std::future::Future> + Send { - tracing::debug!( - user = %user, - peer = ?self.peer_addr, - "Auth none attempt" - ); - - // If connection was rejected by IP access control, immediately reject - if self.rejected { - tracing::debug!( - peer = ?self.peer_addr, - "Rejecting auth for IP-blocked connection" - ); - return std::future::ready(Ok(Auth::Reject { - proceed_with_methods: None, - partial_success: false, - })) - .left_future(); - } - - // Create session info if not already created - let peer_addr = self.peer_addr; - let sessions = Arc::clone(&self.sessions); - let methods = self.allowed_methods(); - - // We need to handle session creation - let session_info_ref = &mut self.session_info; - - async move { - if session_info_ref.is_none() { - let mut sessions_guard = sessions.write().await; - if let Some(info) = sessions_guard.create_session(peer_addr) { - tracing::info!( - session_id = %info.id, - peer = ?peer_addr, - "New session created" - ); - *session_info_ref = Some(info); - } else { - tracing::warn!( - peer = ?peer_addr, - "Session limit reached, rejecting connection" - ); - return Ok(Auth::Reject { - proceed_with_methods: None, - partial_success: false, - }); - } - } - - // Reject with available methods - tracing::debug!( - methods = ?methods, - "Rejecting auth_none, advertising methods" - ); - - Ok(Auth::Reject { - proceed_with_methods: Some(methods), - partial_success: false, - }) - } - .right_future() - } - - /// Handle public key authentication. + /// Authorize a public key whose signature the transport has verified. /// /// Verifies the public key against the user's authorized_keys file. - fn auth_publickey( + pub fn authenticate_publickey( &mut self, user: &str, public_key: &ssh_key::PublicKey, - ) -> impl std::future::Future> + Send { + ) -> impl std::future::Future> + Send { tracing::debug!( user = %user, peer = ?self.peer_addr, @@ -578,6 +481,116 @@ impl russh::server::Handler for SshHandler { } } } +} + +impl russh::server::Handler for SshHandler { + type Error = anyhow::Error; + + /// Called when a new session channel is created. + fn channel_open_session( + &mut self, + channel: Channel, + reply: ChannelOpenHandle, + _session: &mut Session, + ) -> impl std::future::Future> + Send { + let channel_id = channel.id(); + tracing::debug!( + peer = ?self.peer_addr, + channel = ?channel_id, + "Channel opened for session" + ); + + // Store the channel itself so we can use it for subsystems like SFTP + self.channels + .insert(channel_id, ChannelState::with_channel(channel)); + async move { + reply.accept().await; + Ok(()) + } + } + + /// Handle 'none' authentication. + /// + /// Always rejects and advertises available authentication methods. + fn auth_none( + &mut self, + user: &str, + ) -> impl std::future::Future> + Send { + tracing::debug!( + user = %user, + peer = ?self.peer_addr, + "Auth none attempt" + ); + + // If connection was rejected by IP access control, immediately reject + if self.rejected { + tracing::debug!( + peer = ?self.peer_addr, + "Rejecting auth for IP-blocked connection" + ); + return std::future::ready(Ok(Auth::Reject { + proceed_with_methods: None, + partial_success: false, + })) + .left_future(); + } + + // Create session info if not already created + let peer_addr = self.peer_addr; + let sessions = Arc::clone(&self.sessions); + let methods = self.allowed_methods(); + + // We need to handle session creation + let session_info_ref = &mut self.session_info; + + async move { + if session_info_ref.is_none() { + let mut sessions_guard = sessions.write().await; + if let Some(info) = sessions_guard.create_session(peer_addr) { + tracing::info!( + session_id = %info.id, + peer = ?peer_addr, + "New session created" + ); + *session_info_ref = Some(info); + } else { + tracing::warn!( + peer = ?peer_addr, + "Session limit reached, rejecting connection" + ); + return Ok(Auth::Reject { + proceed_with_methods: None, + partial_success: false, + }); + } + } + + // Reject with available methods + tracing::debug!( + methods = ?methods, + "Rejecting auth_none, advertising methods" + ); + + Ok(Auth::Reject { + proceed_with_methods: Some(methods), + partial_success: false, + }) + } + .right_future() + } + + /// Handle public key authentication. + /// + /// russh has verified the request signature before calling this, and + /// `VerifiedPublicKey` cannot be constructed elsewhere, so the policy lives + /// in [`SshHandler::authenticate_publickey`]. + fn auth_publickey( + &mut self, + user: &str, + key: russh::server::VerifiedPublicKey<'_>, + ) -> impl std::future::Future> + Send { + self.authenticate_publickey(user, key.public_key) + } /// Handle password authentication. /// diff --git a/src/ssh/tokio_client/authentication.rs b/src/ssh/tokio_client/authentication.rs index de533fb0f..b8a47ef8d 100644 --- a/src/ssh/tokio_client/authentication.rs +++ b/src/ssh/tokio_client/authentication.rs @@ -564,6 +564,51 @@ where })? } +/// Send `session-bind@openssh.com` on one agent connection. +/// +/// Like `ssh_agent_bind_hostkey` in OpenSSH, an agent's refusal is not fatal: +/// agents without the extension keep working, and an agent enforcing +/// destination constraints refuses the constrained keys on an unbound +/// connection. A timeout is reported because a late reply would desynchronize +/// the stream, so the connection must not be used afterwards. +#[cfg(not(target_os = "windows"))] +pub(super) async fn bind_agent_session( + agent: &mut russh::keys::agent::client::AgentClient, + binding: Option<&russh::keys::agent::AgentSessionBinding>, + forwarding: bool, +) -> Result<(), super::Error> +where + S: russh::keys::agent::client::AgentStream + Unpin, +{ + let Some(binding) = binding else { + return Ok(()); + }; + bounded_agent_setup_operation("bind session", async { + match agent.bind_session(binding, forwarding).await { + Ok(()) => tracing::debug!(forwarding, "Bound SSH agent to the session host key"), + Err(error) => tracing::debug!(forwarding, %error, "SSH agent session binding failed"), + } + Ok(()) + }) + .await +} + +/// Connect to the agent and bind it for direct authentication, which the agent +/// requires before it lists or uses destination-constrained keys. +#[cfg(not(target_os = "windows"))] +async fn connect_bound_agent( + handle: &Handle, +) -> Result, super::Error> { + let mut agent = bounded_agent_setup_operation("connect", async { + russh::keys::agent::client::AgentClient::connect_env() + .await + .map_err(|_| super::Error::AgentConnectionFailed) + }) + .await?; + bind_agent_session(&mut agent, handle.agent_session_binding(), false).await?; + Ok(agent) +} + #[cfg(not(target_os = "windows"))] async fn bounded_agent_signer_operation( action: &'static str, @@ -838,12 +883,9 @@ async fn authenticate_private_key_with_policy( } } LocalSigner::SecurityKey(mut signer) => { - let decline = signer.decline(); - let result = super::security_key_signer::authenticate_declinable( - &decline, - handle.authenticate_publickey_with(username, public_key, None, &mut signer), - ) - .await?; + let result = handle + .authenticate_publickey_with(username, public_key, None, &mut signer) + .await?; if result.success() { return Ok(()); } @@ -1027,19 +1069,10 @@ async fn authenticate_certificate_file( allow_passphrase_prompt, ) .await?; - let decline = match &signer { - LocalSigner::SecurityKey(signer) => Some(signer.decline()), - LocalSigner::Software(_) => None, - }; for hash in hashes { - let exchange = - handle.authenticate_certificate_with(username, certificate.clone(), hash, &mut signer); - let result = match &decline { - Some(decline) => { - super::security_key_signer::authenticate_declinable(decline, exchange).await - } - None => exchange.await, - }?; + let result = handle + .authenticate_certificate_with(username, certificate.clone(), hash, &mut signer) + .await?; if result.success() { return Ok(()); } @@ -1101,12 +1134,7 @@ async fn authenticate_agent_public_key_file( public_key.algorithm() ))); } - let mut agent = bounded_agent_setup_operation("connect", async { - russh::keys::agent::client::AgentClient::connect_env() - .await - .map_err(|_| super::Error::AgentConnectionFailed) - }) - .await?; + let mut agent = connect_bound_agent(handle).await?; let identities = bounded_agent_setup_operation("request identities", async { agent .request_identities() @@ -1151,12 +1179,7 @@ async fn authenticate_agent_with_policy( ) -> Result<(), super::Error> { use russh::keys::agent::AgentIdentity; - let mut agent = bounded_agent_setup_operation("connect", async { - russh::keys::agent::client::AgentClient::connect_env() - .await - .map_err(|_| super::Error::AgentConnectionFailed) - }) - .await?; + let mut agent = connect_bound_agent(handle).await?; let identities = bounded_agent_setup_operation("request identities", async { agent .request_identities() @@ -1272,12 +1295,7 @@ async fn authenticate_agent_certificate_file( ))); } - let mut agent = bounded_agent_setup_operation("connect", async { - russh::keys::agent::client::AgentClient::connect_env() - .await - .map_err(|_| super::Error::AgentConnectionFailed) - }) - .await?; + let mut agent = connect_bound_agent(handle).await?; let identities = bounded_agent_setup_operation("request identities", async { agent .request_identities() @@ -1361,13 +1379,9 @@ pub(crate) async fn authenticate( } pub(super) fn is_retryable_rejection(error: &super::Error) -> bool { - // Agent setup fails before russh requests a signature, so the plan may - // safely continue. Signer failures are intentionally absent: russh may be - // waiting for `Msg::Signed`, and reusing that transport could then hang. - // `SecurityKeyUnavailable` is raised before the identity is offered. - // `SecurityKeySignFailed` is the exception among signer failures: the - // security-key signer declines without sending anything and - // `authenticate_declinable` releases the transport (see there). + // A failed signer leaves the transport usable: russh abandons the pending + // request, so the plan moves on as OpenSSH's `sign_and_send_pubkey` does. + // `AgentAuthError::Send` means the session itself is gone. matches!( error, super::Error::KeyboardInteractiveAuthFailed @@ -1382,6 +1396,9 @@ pub(super) fn is_retryable_rejection(error: &super::Error) -> bool { | super::Error::AgentConnectionFailed | super::Error::AgentRequestIdentitiesFailed | super::Error::AgentOperationTimeout { .. } + | super::Error::AgentSignerTimeout { .. } + | super::Error::AgentAuthError(russh::AgentAuthError::Key(_)) + | super::Error::LocalSignerFailed { .. } | super::Error::AgentNoIdentities | super::Error::AgentAuthenticationFailed ) @@ -1507,9 +1524,7 @@ async fn authenticate_single( AuthMethod::PublicKeyFile { key_file_path } => { let cpubk = russh::keys::load_public_key(key_file_path).map_err(super::Error::KeyInvalid)?; - let mut agent = russh::keys::agent::client::AgentClient::connect_env() - .await - .map_err(|_| super::Error::AgentConnectionFailed)?; + let mut agent = connect_bound_agent(handle).await?; let mut auth_identity_found = false; for identity in agent .request_identities() @@ -1540,9 +1555,7 @@ async fn authenticate_single( } #[cfg(not(target_os = "windows"))] AuthMethod::Agent => { - let mut agent = russh::keys::agent::client::AgentClient::connect_env() - .await - .map_err(|_| super::Error::AgentConnectionFailed)?; + let mut agent = connect_bound_agent(handle).await?; let identities = agent .request_identities() @@ -1676,7 +1689,7 @@ mod policy_execution_tests { async fn auth_publickey_offered( &mut self, _user: &str, - _public_key: &russh::keys::PublicKey, + _offer: russh::server::PublicKeyOffer<'_>, ) -> Result { Ok(Auth::Accept) } @@ -1684,11 +1697,11 @@ mod policy_execution_tests { async fn auth_publickey( &mut self, _user: &str, - public_key: &russh::keys::PublicKey, + key: russh::server::VerifiedPublicKey<'_>, ) -> Result { let name = self .key_names - .get(public_key.key_data()) + .get(key.public_key.key_data()) .cloned() .unwrap_or_else(|| "unknown-key".to_string()); self.attempts.lock().await.push(name.clone()); @@ -1918,19 +1931,11 @@ mod policy_execution_tests { identity, false, ); - let decline = signer.decline(); let outcome = tokio::time::timeout(std::time::Duration::from_secs(10), async { - let error = super::super::security_key_signer::authenticate_declinable( - &decline, - handle.authenticate_publickey_with( - "user", - sk_key.public_key().clone(), - None, - &mut signer, - ), - ) - .await - .unwrap_err(); + let error = handle + .authenticate_publickey_with("user", sk_key.public_key().clone(), None, &mut signer) + .await + .unwrap_err(); assert!(is_retryable_rejection(&error), "{error:?}"); assert!(error.to_string().contains("device not found"), "{error}"); @@ -1950,6 +1955,114 @@ mod policy_execution_tests { task.abort(); } + /// Read one framed agent request and answer it with `reply`. + async fn answer_agent_frame(agent: &mut tokio::io::DuplexStream, reply: u8) -> Vec { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + let mut length = [0u8; 4]; + agent.read_exact(&mut length).await.unwrap(); + let mut frame = vec![0u8; u32::from_be_bytes(length) as usize]; + agent.read_exact(&mut frame).await.unwrap(); + agent.write_all(&[0, 0, 0, 1, reply]).await.unwrap(); + frame + } + + fn read_string<'a>(frame: &mut &'a [u8]) -> &'a [u8] { + let (length, rest) = frame.split_at(4); + let (value, rest) = rest.split_at(u32::from_be_bytes(length.try_into().unwrap()) as usize); + *frame = rest; + value + } + + /// Both binding modes send the initial host key, session ID, and server + /// signature from the real key exchange, differing only in the final flag. + #[tokio::test] + async fn session_bind_carries_the_key_exchange_proof_in_both_modes() { + const SSH_AGENTC_EXTENSION: u8 = 27; + const SSH_AGENT_SUCCESS: u8 = 6; + + let (address, task) = start_server(ScriptedAuthServer { + attempts: Arc::new(Mutex::new(Vec::new())), + key_names: HashMap::new(), + accepted_key: None, + accepted_certificate: None, + }) + .await; + let handle = connect(address).await; + let binding = handle + .agent_session_binding() + .expect("initial key exchange") + .clone(); + + for forwarding in [false, true] { + let (client, mut agent) = tokio::io::duplex(4096); + let mut client = russh::keys::agent::client::AgentClient::connect(client); + let (result, frame) = tokio::join!( + bind_agent_session(&mut client, Some(&binding), forwarding), + answer_agent_frame(&mut agent, SSH_AGENT_SUCCESS) + ); + result.unwrap(); + + let (kind, mut body) = frame.split_first().unwrap(); + assert_eq!(*kind, SSH_AGENTC_EXTENSION); + assert_eq!(read_string(&mut body), b"session-bind@openssh.com"); + assert_eq!(read_string(&mut body), binding.host_key()); + assert_eq!(read_string(&mut body), &binding.session_id()[..]); + assert_eq!(read_string(&mut body), binding.signature()); + assert_eq!(body, [u8::from(forwarding)]); + } + task.abort(); + } + + /// An agent without the extension still serves unconstrained keys. + #[tokio::test] + async fn session_bind_refusal_is_not_fatal() { + const SSH_AGENT_FAILURE: u8 = 5; + + let (address, task) = start_server(ScriptedAuthServer { + attempts: Arc::new(Mutex::new(Vec::new())), + key_names: HashMap::new(), + accepted_key: None, + accepted_certificate: None, + }) + .await; + let handle = connect(address).await; + let (client, mut agent) = tokio::io::duplex(4096); + let mut client = russh::keys::agent::client::AgentClient::connect(client); + let (result, _) = tokio::join!( + bind_agent_session(&mut client, handle.agent_session_binding(), false), + answer_agent_frame(&mut agent, SSH_AGENT_FAILURE) + ); + result.unwrap(); + task.abort(); + } + + /// A stalled agent must not hang authentication, and the caller must see + /// the timeout because a late reply would desynchronize the connection. + #[tokio::test(start_paused = true)] + async fn session_bind_timeout_is_reported() { + let (address, task) = start_server(ScriptedAuthServer { + attempts: Arc::new(Mutex::new(Vec::new())), + key_names: HashMap::new(), + accepted_key: None, + accepted_certificate: None, + }) + .await; + let handle = connect(address).await; + let (client, _agent) = tokio::io::duplex(4096); + let mut client = russh::keys::agent::client::AgentClient::connect(client); + let error = bind_agent_session(&mut client, handle.agent_session_binding(), false) + .await + .unwrap_err(); + assert!(matches!( + error, + super::super::Error::AgentOperationTimeout { + action: "bind session", + .. + } + )); + task.abort(); + } + /// An encrypted `sk-*` identity without a usable provider is skipped from /// its public half, before any passphrase handling, as OpenSSH does. #[tokio::test] @@ -2280,7 +2393,7 @@ mod policy_execution_tests { #[cfg(not(target_os = "windows"))] #[tokio::test(start_paused = true)] - async fn agent_signer_operations_have_a_fatal_typed_timeout_bound() { + async fn agent_signer_operations_have_a_retryable_typed_timeout_bound() { let error = bounded_agent_signer_operation("sign certificate", async { std::future::pending::>().await }) @@ -2294,7 +2407,7 @@ mod policy_execution_tests { seconds: 5, } )); - assert!(!is_retryable_rejection(&error)); + assert!(is_retryable_rejection(&error)); assert!(error.is_ssh_client_failure()); } @@ -2417,13 +2530,13 @@ mod policy_execution_tests { seconds: 5, } )); - assert!(!is_retryable_rejection( + assert!(is_retryable_rejection( &super::super::Error::AgentSignerTimeout { action: "sign public key", seconds: 5, } )); - assert!(!is_retryable_rejection( + assert!(is_retryable_rejection( &super::super::Error::AgentAuthError(russh::AgentAuthError::Key( russh::keys::Error::AgentFailure, )) @@ -2434,7 +2547,7 @@ mod policy_execution_tests { assert!(is_retryable_rejection(&super::super::Error::KeyInvalid( russh::keys::Error::KeyIsCorrupt ))); - assert!(!is_retryable_rejection( + assert!(is_retryable_rejection( &super::super::Error::LocalSignerFailed { source: russh::keys::Error::KeyIsCorrupt, } @@ -2442,14 +2555,14 @@ mod policy_execution_tests { } #[test] - fn local_certificate_signer_wraps_post_request_failures_as_fatal() { + fn local_certificate_signer_wraps_post_request_failures_as_retryable() { let error = complete_local_signature( b"signed-userauth-request".to_vec(), Err(russh::keys::Error::KeyIsCorrupt), ) .unwrap_err(); - assert!(!is_retryable_rejection(&error)); + assert!(is_retryable_rejection(&error)); assert!(error.is_ssh_client_failure()); assert!(matches!( error, diff --git a/src/ssh/tokio_client/connection.rs b/src/ssh/tokio_client/connection.rs index c75892dc8..83e0a9180 100644 --- a/src/ssh/tokio_client/connection.rs +++ b/src/ssh/tokio_client/connection.rs @@ -1537,6 +1537,7 @@ impl ConnectionInfoState { pub(crate) struct AgentForwardingState { active_leases: Arc, socket_path: Arc>>, + session_binding: Arc>>, } impl AgentForwardingState { @@ -1551,6 +1552,21 @@ impl AgentForwardingState { self.active_leases.load(Ordering::Acquire) > 0 } + fn set_session_binding(&self, binding: Option) { + *self + .session_binding + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = binding; + } + + #[cfg(unix)] + fn session_binding(&self) -> Option { + self.session_binding + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone() + } + fn set_socket_path(&self, socket_path: Option) { *self .socket_path @@ -2312,6 +2328,8 @@ impl Client { &self, channel: &russh::Channel, ) -> Result { + self.agent_forwarding + .set_session_binding(self.connection_handle.agent_session_binding().cloned()); let lease = self.agent_forwarding.acquire(); if let Err(source) = channel.agent_forward(true).await { return Err(self.session_error_or(super::Error::CommandExecution { @@ -3705,7 +3723,7 @@ impl Handler for ClientHandler { reply.reject(russh::ChannelOpenFailure::ConnectFailed).await; return Ok(()); }; - let mut agent = match tokio::net::UnixStream::connect(&socket_path).await { + let agent = match tokio::net::UnixStream::connect(&socket_path).await { Ok(agent) => agent, Err(error) => { tracing::warn!( @@ -3719,7 +3737,22 @@ impl Handler for ClientHandler { }; reply.accept().await; + let binding = forwarding.session_binding(); tokio::spawn(async move { + // Bound here, not above, so a slow agent cannot stall the + // session task. Requests wait in the channel until then. + let mut agent = russh::keys::agent::client::AgentClient::connect(agent); + if let Err(error) = super::authentication::bind_agent_session( + &mut agent, + binding.as_ref(), + true, + ) + .await + { + tracing::warn!(%error, "Closing the forwarded SSH agent channel"); + return; + } + let mut agent = agent.into_inner(); let mut stream = channel.into_stream(); if let Err(error) = tokio::io::copy_bidirectional(&mut stream, &mut agent).await { diff --git a/src/ssh/tokio_client/error.rs b/src/ssh/tokio_client/error.rs index 9ded6f4ce..e3c70b0fa 100644 --- a/src/ssh/tokio_client/error.rs +++ b/src/ssh/tokio_client/error.rs @@ -58,9 +58,8 @@ pub enum Error { reason: String, }, /// The security-key provider failed after the server accepted the key. - /// The signer declines without sending a signature (see - /// `security_key_signer::authenticate_declinable`), so authentication - /// continues with the next identity on the same transport. + /// russh abandons the request without sending a signature, so + /// authentication continues with the next identity on the same transport. #[error("security key signing failed for {path}: {reason}")] SecurityKeySignFailed { path: std::path::PathBuf, diff --git a/src/ssh/tokio_client/host_key_type.rs b/src/ssh/tokio_client/host_key_type.rs index 5edb2252e..d544f5683 100644 --- a/src/ssh/tokio_client/host_key_type.rs +++ b/src/ssh/tokio_client/host_key_type.rs @@ -19,7 +19,7 @@ //! (`sshkey_type_from_name`), and, for plain ECDSA, whose curve differs, with //! `SSH_ERR_KEY_TYPE_MISMATCH`. Signature-algorithm names map to their key //! type: `rsa-sha2-256` and `rsa-sha2-512` select an RSA key, and -//! `rsa-sha2-512-cert-v01@openssh.com` an RSA certificate. russh 0.64.1 +//! `rsa-sha2-512-cert-v01@openssh.com` an RSA certificate. russh 0.65.0 //! verifies the exchange signature with whatever key the server sends, so //! without this check a `HostKeyAlgorithms` restriction would not limit which //! trusted key type the server may use. diff --git a/src/ssh/tokio_client/security_key_signer.rs b/src/ssh/tokio_client/security_key_signer.rs index bd5f06a85..7b6857c07 100644 --- a/src/ssh/tokio_client/security_key_signer.rs +++ b/src/ssh/tokio_client/security_key_signer.rs @@ -20,17 +20,12 @@ //! //! A provider failure (device absent, PIN required in `BatchMode`, provider //! error) skips the identity the way a failed `identity_sign` does in -//! OpenSSH: the signer hands russh the request back unsigned, which russh -//! then does not send, and [`authenticate_declinable`] stops waiting for a -//! server reply that will never come. The transport stays usable for the -//! next identity. +//! OpenSSH: the signer returns an error, russh abandons that request without +//! sending anything, and the transport stays usable for the next identity. -use std::future::Future; use std::io::IsTerminal; use std::path::{Path, PathBuf}; -use std::sync::{Arc, LazyLock, Mutex}; - -use tokio::sync::Notify; +use std::sync::{Arc, LazyLock}; use russh::keys::ssh_encoding::Encode as _; use russh::keys::ssh_key::HashAlg; @@ -155,66 +150,6 @@ pub(super) async fn open_security_key_provider( static PROVIDER_CALL_LOCK: LazyLock>> = LazyLock::new(|| Arc::new(tokio::sync::Mutex::new(()))); -/// Records a signature the signer declined to produce. -#[derive(Debug, Default)] -pub(super) struct SignatureDecline { - error: Mutex>, - notify: Notify, -} - -impl SignatureDecline { - fn record(&self, error: super::Error) { - *self - .error - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) = Some(error); - self.notify.notify_one(); - } - - fn take(&self) -> Option { - self.error - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .take() - } -} - -/// Drive one russh public-key or certificate exchange that signs with a -/// [`SecurityKeySigner`], ending it as soon as the signer declines. -/// -/// When the signer declines, it has returned the request unsigned. russh's -/// session loop compares the returned buffer with the request it handed out -/// and, finding it unchanged, sends nothing; the `Msg::Signed` that carries -/// it is queued on russh's bounded handle channel in the same poll, because -/// authentication holds the handle exclusively and that channel is otherwise -/// empty. The handle-side future then waits for a server reply that will -/// never arrive, so it is dropped here (its receive is cancellation-safe) and -/// the recorded signing error is returned instead. The server never saw a -/// signed request, so the next identity can be offered on the same transport, -/// as OpenSSH's `sign_and_send_pubkey` failure path does. -pub(super) async fn authenticate_declinable( - decline: &SignatureDecline, - exchange: F, -) -> Result -where - F: Future>, -{ - tokio::pin!(exchange); - let result = tokio::select! { - biased; - result = &mut exchange => result, - () = decline.notify.notified() => Err(decline - .take() - .unwrap_or_else(|| super::Error::AuthenticationPolicy( - "security key signature declined".to_string(), - ))), - }; - match decline.take() { - Some(error) => Err(error), - None => result, - } -} - /// Resolve the provider for an `sk-*` identity file before its private half /// is decrypted, as OpenSSH skips such keys from their public half when no /// provider is configured. The public key of an OpenSSH-format private key is @@ -249,7 +184,6 @@ pub(super) struct SecurityKeySigner { identity: Arc, /// `BatchMode no`: a touch notice may be printed and a PIN prompted. interactive: bool, - decline: Arc, } impl SecurityKeySigner { @@ -262,16 +196,9 @@ impl SecurityKeySigner { provider, identity: Arc::new(identity), interactive, - decline: Arc::new(SignatureDecline::default()), } } - /// Where a declined signature is reported; pass it to - /// [`authenticate_declinable`] around the russh exchange. - pub(super) fn decline(&self) -> Arc { - Arc::clone(&self.decline) - } - pub(super) async fn sign_blob(&self, data: Vec) -> Result, super::Error> { let failed = |reason: String| super::Error::SecurityKeySignFailed { path: self.identity.path.clone(), @@ -372,7 +299,7 @@ impl russh::Signer for SecurityKeySigner { ) -> Result, Self::Error> { let signed = match self.sign_blob(to_sign.clone()).await { Ok(blob) => { - let mut signed = to_sign.clone(); + let mut signed = to_sign; blob.as_slice() .encode(&mut signed) .map(|()| signed) @@ -395,9 +322,7 @@ impl russh::Signer for SecurityKeySigner { self.identity.path.display() ), } - self.decline.record(error); - // Unchanged length: russh sends nothing for this request. - Ok(to_sign) + Err(error) } } } diff --git a/tests/agent_forwarding_live_test.rs b/tests/agent_forwarding_live_test.rs index 6a968e7a3..af8a09775 100644 --- a/tests/agent_forwarding_live_test.rs +++ b/tests/agent_forwarding_live_test.rs @@ -213,6 +213,26 @@ async fn agent_channel_bridges_only_after_forwarding_is_requested() { .await .expect("client did not connect to fake SSH agent") .expect("accept fake SSH agent connection"); + + // The client binds the relayed connection before any forwarded byte. + let mut length = [0; 4]; + timeout(TEST_TIMEOUT, stream.read_exact(&mut length)) + .await + .expect("session bind timed out") + .expect("read session bind length"); + let mut bind = vec![0; u32::from_be_bytes(length) as usize]; + stream + .read_exact(&mut bind) + .await + .expect("read session bind"); + assert_eq!(bind[0], 27, "SSH_AGENTC_EXTENSION"); + assert_eq!(&bind[5..29], b"session-bind@openssh.com"); + assert_eq!(bind.last(), Some(&1), "forwarded connections set the flag"); + stream + .write_all(&[0, 0, 0, 1, 6]) + .await + .expect("acknowledge session bind"); + let mut request = vec![0; AGENT_REQUEST.len()]; timeout(TEST_TIMEOUT, stream.read_exact(&mut request)) .await diff --git a/tests/server_sk_policy_test.rs b/tests/server_sk_policy_test.rs index 02d12cc75..40578d4a0 100644 --- a/tests/server_sk_policy_test.rs +++ b/tests/server_sk_policy_test.rs @@ -19,7 +19,7 @@ use russh::keys::ssh_key::{ PublicKey, public::{Ed25519PublicKey, KeyData, SkEcdsaSha2NistP256, SkEd25519}, }; -use russh::server::{Auth, Handler}; +use russh::server::Auth; use tokio::sync::RwLock; fn keys() -> [PublicKey; 3] { @@ -59,7 +59,7 @@ async fn authenticate(key: &PublicKey, lines: &[String]) -> bool { Arc::new(RwLock::new(SessionManager::new())), ); matches!( - handler.auth_publickey("alice", key).await.unwrap(), + handler.authenticate_publickey("alice", key).await.unwrap(), Auth::Accept ) }