diff --git a/architecture/notes/nebula_app/ssh_session/2026-10-03-changed-host-key-confirmation.md b/architecture/notes/nebula_app/ssh_session/2026-10-03-changed-host-key-confirmation.md new file mode 100644 index 000000000..bb7f93baa --- /dev/null +++ b/architecture/notes/nebula_app/ssh_session/2026-10-03-changed-host-key-confirmation.md @@ -0,0 +1,74 @@ +# Changed SSH host-key confirmation + +## Status + +Proposed for review. + +## Context + +Issue #252 requests an explicit update when a known SSH server changes identity. +The existing russh check rejects changed keys, without a confirmation path. + +## Evidence + +russh 0.62.2 checks literal/hashed host records and reports `KeyChanged` for an +incoming algorithm with a different recorded key. Its line index excludes +comments; marker records are not interpreted as ordinary host entries. +`ssh-key::known_hosts::Entry` supplies the existing semantic parser. + +## Decision + +Keep the handshake refused until the existing bounded prompt receives explicit +trust and the changed record is saved successfully. Show host/port, old SHA256 +fingerprints, the new SHA256 fingerprint and the identity-change warning. + +A presented key already trusted in the same snapshot remains trusted even when +another ordinary record retains an older key. This is not a reason to update. +Use the library parser, preserve unrelated original lines, and split only the +literal target from multi-host records. Preserve hashed names, comments, line +endings and other algorithms. Marked matching records and changed wildcard or +negated patterns fail closed; Pebrel does not implement certificate trust or +silently convert revocation into ordinary trust. + +Confirmation owns a file snapshot. The short application writer lock and two +byte comparisons reject concurrent edits observed before replacement. Stage a +regular sibling file, preserve file permissions, sync and use existing atomic +replacement on Unix and DACL-preserving replacement on Windows. Reject symlinks rather than replacing a user's indirection. + +## Rejected alternatives + +- Automatically trusting a changed server bypasses identity verification. +- Deleting a reported line can remove other hosts and uses an unreliable index. +- Appending a literal record leaves the stale key authoritative. +- Reimplementing the complete OpenSSH parser adds unnecessary protocol policy. + +## Consequences + +Canceled, unattended and unavailable UI paths cannot update trust. Pattern and +marked records require external known_hosts administration. Invalid matching keys +fail closed; records without a valid matching pattern remain uninterpreted. Independent OpenSSH writers do not share Pebrel's lock: the comparison +is not a cross-tool atomic compare-and-swap. Windows uses `ReplaceFileW` without ACL/merge-error bypass flags, preserving +the target DACL. An explicit backup protects failure cases that move the original +file; recovery never overwrites a concurrent destination and retains the backup +if automatic recovery cannot complete. + +## Validation + +Active regressions cover isolated files, shared host lines, hashed names, ports, +other algorithms, comments/endings, markers, patterns and changed snapshots. +Unrelated malformed keys and unknown algorithms cannot block a trusted host. +Windows regressions compare a protected target DACL against a broader parent +and verify replacement failure retains the original file. +A real loopback SSH service rejects the old record, remains refused after +cancellation, and connects after explicit confirmation saves the new key. +The GPUI cancellation path and English/Chinese fingerprint formatting are covered. +All execution is delegated to GitHub Actions; local automated execution is prohibited. + +## Supersedes + +None. + +## Revisit when + +Safe wildcard replacement, certificate-authority policy, stronger cross-tool +coordination is requested. diff --git a/nebula_app/i18n/en-US.json b/nebula_app/i18n/en-US.json index 9e992bbbe..1a673b6df 100644 --- a/nebula_app/i18n/en-US.json +++ b/nebula_app/i18n/en-US.json @@ -1748,6 +1748,9 @@ "changed": "The package changed after inspection. Choose the file again before installing." } }, + "ssh.changedHost.title": "SSH host identity changed", + "ssh.changedHost.description": "{endpoint}\n\nPreviously saved SHA256 fingerprint(s):\n{previous}\n\nNew SHA256 fingerprint:\n{fingerprint}\n\nThis may mean the server was replaced or an attacker is impersonating it. Verify the new fingerprint through a trusted channel. Updating replaces this host’s saved key; cancel keeps the previous trust record and refuses the connection.", + "ssh.changedHost.confirm": "Update key and connect", "ssh.ports.button": "Ports", "ssh.ports.title": "SSH port forwarding", "ssh.ports.empty": "No forwarded ports", diff --git a/nebula_app/i18n/zh-CN.json b/nebula_app/i18n/zh-CN.json index b3c8067d5..e304978be 100644 --- a/nebula_app/i18n/zh-CN.json +++ b/nebula_app/i18n/zh-CN.json @@ -1748,6 +1748,9 @@ "changed": "主题包在检查后发生了变化。请重新选择文件，再确认安装。" } }, + "ssh.changedHost.title": "SSH 主机身份已变更", + "ssh.changedHost.description": "{endpoint}\n\n之前保存的 SHA256 指纹：\n{previous}\n\n新的 SHA256 指纹：\n{fingerprint}\n\n这可能表示服务器已更换，也可能有攻击者冒充主机。请通过可信渠道核对新指纹。更新将替换此主机保存的密钥；取消将保留原信任记录并拒绝连接。", + "ssh.changedHost.confirm": "更新密钥并连接", "ssh.ports.button": "端口", "ssh.ports.title": "SSH 端口转发", "ssh.ports.empty": "暂无端口转发", diff --git a/nebula_app/src/gpui_shell/workspace/ssh_dialog.rs b/nebula_app/src/gpui_shell/workspace/ssh_dialog.rs index bb4234f75..258156a06 100644 --- a/nebula_app/src/gpui_shell/workspace/ssh_dialog.rs +++ b/nebula_app/src/gpui_shell/workspace/ssh_dialog.rs @@ -29,6 +29,19 @@ pub(super) fn show(request: Arc, window: &mut Window, cx: &mut App) { false, false, ), + PromptKind::ChangedHostKey { host, port, previous, fingerprint } => ( + language.text(crate::i18n::Message::SshChangedHostTitle), + language.format( + crate::i18n::Message::SshChangedHostDescription, + &[ + ("endpoint", &format!("{host}:{port}")), + ("previous", previous), + ("fingerprint", fingerprint), + ], + ), + false, + false, + ), PromptKind::Secret { label, allow_save } => { (language.pick("SSH 身份验证", "SSH authentication"), label.clone(), true, *allow_save) }, @@ -65,6 +78,8 @@ pub(super) fn show(request: Arc, window: &mut Window, cx: &mut App) { description.clone(), if is_secret { language.pick("继续", "Continue") + } else if matches!(&request.kind, PromptKind::ChangedHostKey { .. }) { + language.text(crate::i18n::Message::SshChangedHostConfirm) } else { language.pick("信任并连接", "Trust and connect") }, @@ -133,6 +148,43 @@ mod tests { ); } + #[gpui::test] + fn changed_host_dialog_cancel_preserves_refusal(cx: &mut TestAppContext) { + cx.update(|cx| { + gpui_component::init(cx); + cx.set_reduce_motion(true); + }); + let (_, cx) = cx.add_window_view(|window, cx| { + let view = cx.new(|_| DialogProbe); + Root::new(view, window, cx) + }); + let (request, mut response) = Prompt::for_test(PromptKind::ChangedHostKey { + host: "fixture.example".into(), + port: 2200, + previous: "SHA256:previous".into(), + fingerprint: "SHA256:new".into(), + }); + cx.update(|window, cx| { + show(request, window, cx); + let _ = window.draw(cx); + }); + click(cx, "confirm-dialog-cancel"); + assert!(matches!(response.try_recv(), Ok(PromptResponse::Cancel))); + for language in [crate::i18n::UiLanguage::EnUs, crate::i18n::UiLanguage::ZhCn] { + let description = language.format( + crate::i18n::Message::SshChangedHostDescription, + &[ + ("endpoint", "fixture.example:2200"), + ("previous", "SHA256:previous"), + ("fingerprint", "SHA256:new"), + ], + ); + for value in ["fixture.example:2200", "SHA256:previous", "SHA256:new"] { + assert!(description.contains(value)); + } + } + } + #[gpui::test] fn ssh_dialog_cancel_does_not_accept_an_unknown_host(cx: &mut TestAppContext) { cx.update(|cx| { diff --git a/nebula_app/src/platform/mod.rs b/nebula_app/src/platform/mod.rs index e4f1b2564..f2b3c7932 100644 --- a/nebula_app/src/platform/mod.rs +++ b/nebula_app/src/platform/mod.rs @@ -53,6 +53,7 @@ mod ssh_config_probe_tests; pub mod startup; #[cfg(unix)] pub(crate) mod tray_native; +pub(crate) mod trust_file; pub(crate) mod update_installation; #[cfg(feature = "gpui-shell")] pub(crate) mod video_underlay; diff --git a/nebula_app/src/platform/trust_file.rs b/nebula_app/src/platform/trust_file.rs new file mode 100644 index 000000000..13f19db09 --- /dev/null +++ b/nebula_app/src/platform/trust_file.rs @@ -0,0 +1,17 @@ +//! Replacement of an existing trust file must preserve its access controls. +use std::io; +use std::path::Path; + +#[cfg(windows)] +mod windows; + +pub(crate) fn replace(source: &Path, destination: &Path) -> io::Result<()> { + #[cfg(windows)] + { + windows::replace(source, destination) + } + #[cfg(not(windows))] + { + crate::atomic_file::replace(source, destination) + } +} diff --git a/nebula_app/src/platform/trust_file/windows.rs b/nebula_app/src/platform/trust_file/windows.rs new file mode 100644 index 000000000..b8701eb15 --- /dev/null +++ b/nebula_app/src/platform/trust_file/windows.rs @@ -0,0 +1,103 @@ +//! Trust-file replacement preserves the target DACL and never ignores ACL merge errors. +use std::io; +use std::os::windows::ffi::OsStrExt as _; +use std::path::Path; +use windows_sys::Win32::Storage::FileSystem::ReplaceFileW; + +pub(crate) fn replace(source: &Path, destination: &Path) -> io::Result<()> { + let backup = tempfile::Builder::new() + .prefix(".known-hosts-backup-") + .tempfile_in(destination.parent().ok_or_else(|| io::Error::other("No trust directory"))?)? + .into_temp_path(); + std::fs::remove_file(&backup)?; + let wide = |path: &Path| path.as_os_str().encode_wide().chain(Some(0)).collect::>(); + let original = wide(destination); + let replacement = wide(source); + let saved = wide(&backup); + // No IGNORE_ACL_ERRORS / IGNORE_MERGE_ERRORS: permission preservation is + // mandatory. A backup is required because error 1176 without one can remove + // the destination, and 1177 can move it to the backup before returning failure. + let success = unsafe { + ReplaceFileW( + original.as_ptr(), + replacement.as_ptr(), + saved.as_ptr(), + 0, + std::ptr::null(), + std::ptr::null(), + ) + }; + if success != 0 { + return Ok(()); + } + let error = io::Error::last_os_error(); + if backup.exists() { + // An exclusive hard-link creation restores the original file identity + // and DACL if the destination was moved away, without overwriting any + // concurrent writer. If recovery cannot complete, retain the backup. + if !destination.exists() && std::fs::hard_link(&backup, destination).is_ok() { + return Err(error); + } + let preserved = backup.keep().map_err(io::Error::other)?; + return Err(io::Error::new( + error.kind(), + format!("{error}; original trust file retained at {}", preserved.display()), + )); + } + Err(error) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn acl(path: &Path, setup: bool) -> String { + let script = if setup { + r#"$ErrorActionPreference='Stop'; $p=$env:PEBREL_KNOWN_HOSTS; $parent=Split-Path $p; $a=Get-Acl $parent; $rule=[System.Security.AccessControl.FileSystemAccessRule]::new([System.Security.Principal.SecurityIdentifier]::new('S-1-1-0'),'Modify','ContainerInherit,ObjectInherit','None','Allow'); $a.AddAccessRule($rule); Set-Acl $parent $a; $a=Get-Acl $p; $a.SetAccessRuleProtection($true,$false); foreach($rule in @($a.Access)) { $a.RemoveAccessRuleSpecific($rule) }; $user=[System.Security.Principal.WindowsIdentity]::GetCurrent().User; $a.AddAccessRule([System.Security.AccessControl.FileSystemAccessRule]::new($user,'FullControl','Allow')); Set-Acl $p $a; $a=Get-Acl $p; $parentAcl=Get-Acl $parent; if($a.GetSecurityDescriptorSddlForm('Access') -eq $parentAcl.GetSecurityDescriptorSddlForm('Access')) { throw 'Target must have a stricter DACL than its parent' }; $a.GetSecurityDescriptorSddlForm('Access')"# + } else { + r#"$ErrorActionPreference='Stop'; (Get-Acl $env:PEBREL_KNOWN_HOSTS).GetSecurityDescriptorSddlForm('Access')"# + }; + // Indirect launches from pwsh inherit incompatible PowerShell 7 modules. + // Let Windows PowerShell rebuild its own module paths. + let output = std::process::Command::new("powershell.exe") + .args(["-NoProfile", "-NonInteractive", "-Command", script]) + .env_remove("PSModulePath") + .env("PEBREL_KNOWN_HOSTS", path) + .output() + .unwrap(); + assert!(output.status.success(), "{}", String::from_utf8_lossy(&output.stderr)); + String::from_utf8(output.stdout).unwrap().trim().to_owned() + } + + #[test] + fn changed_trust_file_keeps_its_stricter_windows_dacl() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("known_hosts"); + let replacement = directory.path().join("replacement"); + std::fs::write(&path, "original trusted key\n").unwrap(); + std::fs::write(&replacement, "new trusted key\n").unwrap(); + let before = acl(&path, true); + replace(&replacement, &path).unwrap(); + assert_eq!( + acl(&path, false), + before, + "Parent's broader inherited DACL must not replace target DACL" + ); + assert_eq!(std::fs::read_to_string(&path).unwrap(), "new trusted key\n"); + } + + #[test] + fn locked_destination_failure_retains_original_trust_file() { + use std::os::windows::fs::OpenOptionsExt as _; + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("known_hosts"); + let replacement = directory.path().join("replacement"); + std::fs::write(&path, "original trust").unwrap(); + std::fs::write(&replacement, "changed trust").unwrap(); + let locked = std::fs::OpenOptions::new().read(true).share_mode(0).open(&path).unwrap(); + assert!(replace(&replacement, &path).is_err()); + drop(locked); + assert_eq!(std::fs::read_to_string(&path).unwrap(), "original trust"); + assert_eq!(std::fs::read_to_string(&replacement).unwrap(), "changed trust"); + } +} diff --git a/nebula_app/src/ssh_prompt.rs b/nebula_app/src/ssh_prompt.rs index 0ecd4f1dc..d815b3ba7 100644 --- a/nebula_app/src/ssh_prompt.rs +++ b/nebula_app/src/ssh_prompt.rs @@ -7,6 +7,7 @@ use zeroize::Zeroizing; pub enum PromptKind { HostKey { host: String, port: u16, fingerprint: String }, + ChangedHostKey { host: String, port: u16, previous: String, fingerprint: String }, Secret { label: String, allow_save: bool }, } @@ -34,7 +35,10 @@ impl Prompt { pub fn respond(&self, response: PromptResponse) -> bool { let response = match (&self.kind, response) { - (PromptKind::HostKey { .. }, PromptResponse::Trust) => PromptResponse::Trust, + ( + PromptKind::HostKey { .. } | PromptKind::ChangedHostKey { .. }, + PromptResponse::Trust, + ) => PromptResponse::Trust, (PromptKind::Secret { allow_save, .. }, PromptResponse::Secret { value, save }) => { PromptResponse::Secret { value, save: save && *allow_save } }, @@ -98,6 +102,18 @@ pub async fn confirm_host(host: &str, port: u16, fingerprint: String) -> io::Res Ok(matches!(response, PromptResponse::Trust)) } +pub async fn confirm_changed_host( + host: &str, + port: u16, + previous: String, + fingerprint: String, +) -> io::Result { + let response = + request(PromptKind::ChangedHostKey { host: host.to_owned(), port, previous, fingerprint }) + .await?; + Ok(matches!(response, PromptResponse::Trust)) +} + pub async fn secret( label: String, allow_save: bool, diff --git a/nebula_app/src/ssh_session.rs b/nebula_app/src/ssh_session.rs index eee9fe097..63c56378e 100644 --- a/nebula_app/src/ssh_session.rs +++ b/nebula_app/src/ssh_session.rs @@ -30,6 +30,7 @@ pub(crate) mod completion; mod config; mod exec; mod forward; +mod host_key; mod integration; mod lifecycle; mod pane_connection; @@ -531,6 +532,28 @@ struct ClientHandler { } impl ClientHandler { + fn host_key_path(&self) -> io::Result { + #[cfg(test)] + if let Some(path) = &self.known_hosts_path { + return Ok(path.clone()); + } + home::home_dir() + .map(|home| home.join(".ssh/known_hosts")) + .ok_or_else(|| io::Error::other("SSH home directory unavailable")) + } + + async fn confirm_changed_key( + &self, + change: host_key::Change, + confirmation: impl std::future::Future, + ) -> io::Result { + if !self.allow_prompt || !self.handshake.confirm(confirmation).await { + return Ok(false); + } + change.save(&self.host_key_path()?)?; + Ok(true) + } + fn verify_host_key(&self, key: &ssh_key::PublicKey) -> Result { #[cfg(test)] if let Some(path) = self.known_hosts_path.as_deref() { @@ -574,6 +597,30 @@ impl client::Handler for ClientHandler { &mut self, server_public_key: &ssh_key::PublicKey, ) -> Result { + // Inspect marked records even on a first visit: russh's literal checker + // otherwise treats @revoked / @cert-authority entries as unknown hosts. + let path = self.host_key_path()?; + match host_key::inspect(&path, &self.host, self.port, server_public_key)? { + host_key::Verification::Trusted => return Ok(true), + host_key::Verification::Unknown => {}, + host_key::Verification::Changed(change) => { + if !self.allow_prompt { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "SSH host identity changed; explicit confirmation required", + ) + .into()); + } + let confirmation = crate::ssh_prompt::confirm_changed_host( + &self.host, + self.port, + change.fingerprints.join("\n"), + server_public_key.fingerprint(ssh_key::HashAlg::Sha256).to_string(), + ); + let accepted = async { confirmation.await.unwrap_or(false) }; + return Ok(self.confirm_changed_key(change, accepted).await?); + }, + } match self.verify_host_key(server_public_key) { Ok(true) => Ok(true), Ok(false) => Ok(self diff --git a/nebula_app/src/ssh_session/host_key.rs b/nebula_app/src/ssh_session/host_key.rs new file mode 100644 index 000000000..bd5f63083 --- /dev/null +++ b/nebula_app/src/ssh_session/host_key.rs @@ -0,0 +1,304 @@ +//! Changed-key updates preserve unrelated known_hosts bytes and require a fresh snapshot. +use std::io::{self, Write as _}; +use std::path::Path; + +use hmac::{Hmac, KeyInit as _, Mac as _}; +use russh::keys::ssh_key::{ + self, + known_hosts::{Entry, HostPatterns}, +}; + +pub(super) enum Verification { + Trusted, + Unknown, + Changed(Change), +} + +pub(super) struct Change { + snapshot: Vec, + replacement: Vec, + pub(super) fingerprints: Vec, +} + +fn denied(message: &str) -> io::Error { + io::Error::new(io::ErrorKind::PermissionDenied, message) +} + +fn matches(patterns: &HostPatterns, endpoint: &str) -> bool { + match patterns { + HostPatterns::HashedName { salt, hash } => { + let Ok(mut mac) = Hmac::::new_from_slice(salt) else { return false }; + mac.update(endpoint.as_bytes()); + mac.verify_slice(hash).is_ok() + }, + HostPatterns::Patterns(patterns) => { + let matches = |pattern: &str| { + let expression = regex::escape(pattern).replace("\\*", ".*").replace("\\?", "."); + regex::Regex::new(&format!("^{expression}$")) + .is_ok_and(|regex| regex.is_match(endpoint)) + }; + !patterns.iter().any(|pattern| pattern.strip_prefix('!').is_some_and(matches)) + && patterns.iter().any(|pattern| !pattern.starts_with('!') && matches(pattern)) + }, + } +} + +/// Marked entries are never downgraded to an ordinary trust prompt. Pattern-based +/// replacement is deliberately refused: a literal update cannot revoke a wildcard. +pub(super) fn inspect( + path: &Path, + host: &str, + port: u16, + key: &ssh_key::PublicKey, +) -> io::Result { + let snapshot = match std::fs::read(path) { + Ok(bytes) => bytes, + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(Verification::Unknown), + Err(error) => return Err(error), + }; + let source = std::str::from_utf8(&snapshot).map_err(|_| denied("known_hosts is not UTF-8"))?; + let endpoint = if port == 22 { host.to_owned() } else { format!("[{host}]:{port}") }; + let mut replacement = String::new(); + let mut fingerprints = Vec::new(); + let mut trusted = false; + let mut unsupported_change = false; + for line in source.split_inclusive('\n') { + let content = line.trim_end_matches(['\r', '\n']); + let ending = &line[content.len()..]; + let normalized = content.split_whitespace().collect::>().join(" "); + if normalized.is_empty() || normalized.starts_with('#') { + replacement.push_str(line); + continue; + } + let mut fields = content.split_whitespace(); + let first = fields.next().expect("nonempty record"); + let hosts = if first.starts_with('@') { + let Some(hosts) = fields.next() else { + replacement.push_str(line); + continue; + }; + hosts + } else { + first + }; + let Ok(patterns) = hosts.parse::() else { + // An invalid pattern cannot establish trust for this endpoint. + // Preserve it without interpreting its unrelated key material. + replacement.push_str(line); + continue; + }; + if !matches(&patterns, &endpoint) { + replacement.push_str(line); + continue; + } + if first.starts_with('@') { + return Err(denied("SSH host has a marked trust record; update it outside Pebrel")); + } + if fields.next() != Some(key.algorithm().as_str()) { + replacement.push_str(line); + continue; + } + let entry: Entry = normalized + .parse() + .map_err(|_| denied("Cannot safely parse matching known_hosts record"))?; + if entry.public_key().key_data() == key.key_data() { + trusted = true; + replacement.push_str(line); + continue; + } + let fingerprint = entry.public_key().fingerprint(ssh_key::HashAlg::Sha256).to_string(); + if !fingerprints.contains(&fingerprint) { + fingerprints.push(fingerprint); + } + let hosts = content.split_whitespace().next().expect("parsed host field"); + let prefix_length = content.find(hosts).expect("original host field"); + let suffix = &content[prefix_length + hosts.len()..]; + let remaining = match entry.host_patterns() { + HostPatterns::HashedName { .. } => Vec::new(), + HostPatterns::Patterns(patterns) => { + if patterns + .iter() + .any(|pattern| pattern.contains(['*', '?']) || pattern.starts_with('!')) + { + unsupported_change = true; + replacement.push_str(line); + continue; + } + patterns.iter().filter(|pattern| *pattern != &endpoint).cloned().collect::>() + }, + }; + if !remaining.is_empty() { + replacement.push_str(&content[..prefix_length]); + replacement.push_str(&remaining.join(",")); + replacement.push_str(suffix); + replacement.push_str(if ending.is_empty() { "\n" } else { ending }); + } + let target = if matches!(entry.host_patterns(), HostPatterns::HashedName { .. }) { + hosts + } else { + &endpoint + }; + replacement.push_str(&content[..prefix_length]); + replacement.push_str(target); + replacement.push(' '); + let mut saved_key = key.clone(); + saved_key.set_comment(entry.public_key().comment().clone()); + replacement.push_str(&saved_key.to_openssh().map_err(io::Error::other)?); + replacement.push_str(ending); + } + if trusted { + return Ok(Verification::Trusted); + } + if unsupported_change { + return Err(denied("Pattern-based SSH host records cannot be safely replaced")); + } + if fingerprints.is_empty() { + return Ok(Verification::Unknown); + } + Ok(Verification::Changed(Change { + snapshot, + replacement: replacement.into_bytes(), + fingerprints, + })) +} + +impl Change { + pub(super) fn save(self, path: &Path) -> io::Result<()> { + let _lock = + crate::atomic_file::try_lock(path)?.ok_or_else(|| denied("known_hosts is busy"))?; + let metadata = std::fs::symlink_metadata(path)?; + if !metadata.is_file() || metadata.file_type().is_symlink() { + return Err(denied("Only a regular known_hosts file can be updated")); + } + if std::fs::read(path)? != self.snapshot { + return Err(denied( + "known_hosts changed during confirmation; reconnect to verify again", + )); + } + let mut temporary = tempfile::NamedTempFile::new_in( + path.parent().ok_or_else(|| denied("No known_hosts directory"))?, + )?; + temporary.as_file().set_permissions(metadata.permissions())?; + temporary.write_all(&self.replacement)?; + temporary.as_file().sync_all()?; + // Independent OpenSSH writers do not share Pebrel's lock. Compare again + // immediately before replace; this is not a cross-process compare-and-swap. + if std::fs::read(path)? != self.snapshot { + return Err(denied( + "known_hosts changed during confirmation; reconnect to verify again", + )); + } + let temporary = temporary.into_temp_path(); + crate::platform::trust_file::replace(&temporary, path) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use base64::Engine as _; + use ssh_key::{Algorithm, PrivateKey}; + + #[test] + fn update_preserves_other_hosts_ports_algorithms_hashes_comments_and_endings() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("known_hosts"); + let old = PrivateKey::random(&mut rand::rng(), Algorithm::Ed25519).unwrap(); + let new = PrivateKey::random(&mut rand::rng(), Algorithm::Ed25519).unwrap(); + let other = PrivateKey::random( + &mut rand::rng(), + Algorithm::Ecdsa { curve: ssh_key::EcdsaCurve::NistP256 }, + ) + .unwrap(); + let mut commented = old.public_key().clone(); + commented.set_comment("operator comment"); + let old_line = commented.to_openssh().unwrap(); + let endpoint = "[fixture.example]:2200"; + let salt = b"fixed regression salt"; + let mut mac = Hmac::::new_from_slice(salt).unwrap(); + mac.update(endpoint.as_bytes()); + let encoder = base64::engine::general_purpose::STANDARD; + let hash = + format!("|1|{}|{}", encoder.encode(salt), encoder.encode(mac.finalize().into_bytes())); + let unrelated = format!( + "# retained comment\r\nfixture.example {}\r\n{endpoint} {}\r\n", + old.public_key().to_openssh().unwrap(), + other.public_key().to_openssh().unwrap() + ); + let source = format!("{unrelated}{endpoint},other.example {old_line}\r\n{hash} {old_line}"); + std::fs::write(&path, &source).unwrap(); + let Verification::Changed(change) = + inspect(&path, "fixture.example", 2200, new.public_key()).unwrap() + else { + panic!("A different recorded key needs confirmation"); + }; + assert_eq!( + change.fingerprints, + [old.public_key().fingerprint(ssh_key::HashAlg::Sha256).to_string()] + ); + change.save(&path).unwrap(); + let actual = std::fs::read_to_string(&path).unwrap(); + assert!(actual.starts_with(&unrelated)); + assert!(actual.contains(&format!("other.example {old_line}\r\n"))); + assert!(actual.contains("operator comment")); + assert!(actual.lines().any(|line| line.starts_with(&hash))); + assert!(!actual.ends_with('\n')); + assert!( + russh::keys::known_hosts::check_known_hosts_path( + "fixture.example", + 2200, + new.public_key(), + &path + ) + .unwrap() + ); + assert!( + russh::keys::known_hosts::check_known_hosts_path( + "other.example", + 22, + old.public_key(), + &path + ) + .unwrap() + ); + assert!( + russh::keys::known_hosts::check_known_hosts_path( + "fixture.example", + 22, + old.public_key(), + &path + ) + .unwrap() + ); + } + + #[test] + fn stale_snapshot_marked_and_wildcard_records_cannot_be_updated() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("known_hosts"); + let old = PrivateKey::random(&mut rand::rng(), Algorithm::Ed25519).unwrap(); + let new = PrivateKey::random(&mut rand::rng(), Algorithm::Ed25519).unwrap(); + let source = format!("fixture.example {}\n", old.public_key().to_openssh().unwrap()); + std::fs::write(&path, &source).unwrap(); + let Verification::Changed(change) = + inspect(&path, "fixture.example", 22, new.public_key()).unwrap() + else { + panic!("A different recorded key needs confirmation"); + }; + let concurrent = format!("{source}# another writer\n"); + std::fs::write(&path, &concurrent).unwrap(); + assert!(change.save(&path).is_err()); + assert_eq!(std::fs::read_to_string(&path).unwrap(), concurrent); + for hosts in [ + "@revoked fixture.example", + "@cert-authority fixture.example", + "*.example", + "*.example,!excluded.example", + ] { + let source = format!("{hosts} {}\n", old.public_key().to_openssh().unwrap()); + std::fs::write(&path, &source).unwrap(); + assert!(inspect(&path, "fixture.example", 22, new.public_key()).is_err(), "{hosts}"); + assert_eq!(std::fs::read_to_string(&path).unwrap(), source); + } + } +} diff --git a/nebula_app/src/ssh_session/lifecycle/tests.rs b/nebula_app/src/ssh_session/lifecycle/tests.rs index dc4018ffb..1adb72d6f 100644 --- a/nebula_app/src/ssh_session/lifecycle/tests.rs +++ b/nebula_app/src/ssh_session/lifecycle/tests.rs @@ -1238,6 +1238,68 @@ fn completion_real_ssh_git_and_project_scripts_end_to_end() { }); } +#[test] +fn changed_loopback_host_refuses_before_confirmation_and_connects_only_after_saved_update() { + check(async { + let fixture = Fixture::new(Mode::Open).await; + let path = fixture.route.known_hosts_path.as_ref().unwrap(); + let actual = std::fs::read_to_string(path).unwrap(); + let entry: russh::keys::ssh_key::known_hosts::Entry = actual.trim().parse().unwrap(); + let old = PrivateKey::random(&mut rand::rng(), Algorithm::Ed25519).unwrap(); + let source = format!( + "{} {}\n", + entry.host_patterns().to_string(), + old.public_key().to_openssh().unwrap() + ); + std::fs::write(path, &source).unwrap(); + assert!( + authenticated_route(&fixture.route, None::<&NoopSshEventHost>, false, true) + .await + .is_err() + ); + assert_eq!(std::fs::read_to_string(path).unwrap(), source); + let destination = &fixture.route.destination; + let handler = super::super::ClientHandler { + host: "127.0.0.1".into(), + port: destination.port, + allow_prompt: true, + handshake: Handshake::default(), + known_hosts_path: Some(path.clone()), + }; + let change = || { + let super::super::host_key::Verification::Changed(change) = + super::super::host_key::inspect( + path, + "127.0.0.1", + destination.port, + entry.public_key(), + ) + .unwrap() + else { + panic!("Loopback fixture has a different saved identity"); + }; + change + }; + assert!(!handler.confirm_changed_key(change(), async { false }).await.unwrap()); + assert_eq!(std::fs::read_to_string(path).unwrap(), source); + let concurrent = format!("{source}# independent writer\n"); + assert!( + handler + .confirm_changed_key(change(), async { + std::fs::write(path, &concurrent).unwrap(); + true + }) + .await + .is_err() + ); + assert_eq!(std::fs::read_to_string(path).unwrap(), concurrent); + std::fs::write(path, &source).unwrap(); + assert!(handler.confirm_changed_key(change(), async { true }).await.unwrap()); + let acquired = fixture.connect().await; + fixture.forget(&acquired.session).await; + }); +} + #[test] fn ssh_echo_keeps_draining_while_a_large_input_waits_for_window_space() { check(async { diff --git a/nebula_app/src/ssh_session/tests.rs b/nebula_app/src/ssh_session/tests.rs index bd67ed76e..dfce15737 100644 --- a/nebula_app/src/ssh_session/tests.rs +++ b/nebula_app/src/ssh_session/tests.rs @@ -415,3 +415,53 @@ fn all_keys_failing_locally_is_not_reported_as_server_rejection() { assert!(partial.contains("服务器拒绝"), "实际文案: {partial}"); assert!(partial.contains("本地密钥问题"), "实际文案: {partial}"); } + +#[test] +fn trusted_host_is_not_blocked_by_unrelated_invalid_keys_or_unknown_algorithms() { + use russh::client::Handler as _; + use russh::keys::ssh_key::{Algorithm, PrivateKey}; + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("known_hosts"); + let key = PrivateKey::random(&mut rand::rng(), Algorithm::Ed25519).unwrap(); + let source = format!( + "fixture.example {}\nother.example ssh-ed25519 NOT_BASE64\nunknown.example ssh-future-format opaque\n|1|BAD|BAD ssh-ed25519 NOT_BASE64\n@revoked\n", + key.public_key().to_openssh().unwrap() + ); + std::fs::write(&path, &source).unwrap(); + let mut handler = super::ClientHandler { + host: "fixture.example".into(), + port: 22, + allow_prompt: true, + handshake: super::lifecycle::Handshake::default(), + known_hosts_path: Some(path.clone()), + }; + let runtime = tokio::runtime::Builder::new_current_thread().enable_all().build().unwrap(); + assert!(runtime.block_on(handler.check_server_key(key.public_key())).unwrap()); + assert_eq!(std::fs::read_to_string(&path).unwrap(), source); +} + +#[test] +fn a_recorded_server_key_is_trusted_without_prompt_despite_another_old_key() { + use russh::client::Handler as _; + use russh::keys::ssh_key::{Algorithm, PrivateKey}; + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("known_hosts"); + let current = PrivateKey::random(&mut rand::rng(), Algorithm::Ed25519).unwrap(); + let old = PrivateKey::random(&mut rand::rng(), Algorithm::Ed25519).unwrap(); + let source = format!( + "fixture.example {}\nfixture.example {}\nfixture.example ssh-future-format opaque\n", + current.public_key().to_openssh().unwrap(), + old.public_key().to_openssh().unwrap() + ); + std::fs::write(&path, &source).unwrap(); + let mut handler = super::ClientHandler { + host: "fixture.example".into(), + port: 22, + allow_prompt: false, + handshake: super::lifecycle::Handshake::default(), + known_hosts_path: Some(path.clone()), + }; + let runtime = tokio::runtime::Builder::new_current_thread().enable_all().build().unwrap(); + assert!(runtime.block_on(handler.check_server_key(current.public_key())).unwrap()); + assert_eq!(std::fs::read_to_string(path).unwrap(), source); +}