Преглед на файлове

fix(ssh)!: harden SSH layer and refuse unpinned host keys

A missing `host_key` previously logged a warning and skipped server-key
verification entirely, exposing every SSH call to a man-in-the-middle.
Refuse instead and point the operator at `sm121 init-host-keys` so the
key is pinned before any session opens. `fetch_host_key` and the
init-host-keys flow use a separate capturing handler and stay exempt,
since their whole purpose is to discover the key in the first place.

Also harden the rest of the SSH layer while here, since the host-key fix
touches the same session path: relocate `shq` (the single-quote escaper)
into ssh so it is the canonical shell-quoting helper for the crate,
shell-quote `remote_dir` in `upload_dir` (a path with spaces or
metachars would otherwise break `mkdir -p` / `cd`), and give
`fetch_host_key` the same 5s connect timeout `ssh_session_with_timeout`
already had so `health --fingerprint` fails fast on unreachable nodes
instead of hanging.

BREAKING CHANGE: nodes without a `host_key` in config.toml now fail to
connect instead of connecting with verification disabled. Run
`sm121 init-host-keys --cluster <name>` (or set `host_key` manually)
before upgrading.
Alexander Korolev преди 2 месеца
родител
ревизия
ac922d02af
променени са 1 файла, в които са добавени 43 реда и са изтрити 8 реда
  1. 43 8
      src/ssh.rs

+ 43 - 8
src/ssh.rs

@@ -39,14 +39,22 @@ async fn ssh_session_with_timeout(
         inactivity_timeout,
         ..Default::default()
     };
-    let expected_key = Arc::new(std::sync::Mutex::new(node.host_key.clone()));
 
-    if expected_key.lock().unwrap().is_none() {
-        warn!(
-            "No host_key set for {} — skipping verification (insecure)",
-            node.name
+    // Refuse to connect without a pinned host key — a missing `host_key`
+    // would otherwise skip verification entirely (MITM vector). Run
+    // `sm121 init-host-keys` to discover and pin keys, or set `host_key`
+    // manually in config.toml. (`fetch_host_key` / init-host-keys uses a
+    // separate capturing handler and is intentionally exempt.)
+    if node.host_key.is_none() {
+        anyhow::bail!(
+            "No host_key set for node '{}' ({}). Refusing to connect without \
+             host-key verification. Run `sm121 init-host-keys --cluster <name>` \
+             to discover and pin host keys, or set `host_key` manually in config.toml.",
+            node.name,
+            node.ssh_ip
         );
     }
+    let expected_key = Arc::new(std::sync::Mutex::new(node.host_key.clone()));
 
     let handler = VerifyingHandler { expected_key };
     let mut session = tokio::time::timeout(
@@ -239,8 +247,19 @@ pub async fn fetch_host_key(config: &Config, node: &Node) -> Result<String> {
     let handler = CaptureHandler {
         captured: captured_clone,
     };
-    let mut session =
-        russh::client::connect(Arc::new(ssh_config), (node.ssh_ip.as_str(), 22), handler).await?;
+    // Apply the same connect timeout as `ssh_session_with_timeout` so an
+    // unreachable node fails fast instead of hanging `health --fingerprint`.
+    let mut session = tokio::time::timeout(
+        CONNECT_TIMEOUT,
+        russh::client::connect(Arc::new(ssh_config), (node.ssh_ip.as_str(), 22), handler),
+    )
+    .await
+    .context(format!(
+        "SSH connect to {}:{} timed out after {}s",
+        node.ssh_ip,
+        22,
+        CONNECT_TIMEOUT.as_secs()
+    ))??;
 
     session
         .authenticate_publickey(
@@ -259,6 +278,20 @@ pub async fn fetch_host_key(config: &Config, node: &Node) -> Result<String> {
     Ok(fp)
 }
 
+pub(crate) fn shq(s: &str) -> String {
+    let mut out = String::with_capacity(s.len() + 2);
+    out.push('\'');
+    for c in s.chars() {
+        if c == '\'' {
+            out.push_str(r"'\''");
+        } else {
+            out.push(c);
+        }
+    }
+    out.push('\'');
+    out
+}
+
 /// Upload a local directory to a remote node by tarring, base64-encoding,
 /// and piping the result through SSH for decoding and extraction.
 ///
@@ -297,9 +330,11 @@ pub async fn upload_dir(
 
     // Base64 is safe inside single quotes (charset: A-Za-z0-9+/=).
     // `printf '%s'` is portable and doesn't interpret escape sequences.
+    // `remote_dir` is shell-quoted — a path with spaces or metachars would
+    // otherwise break `mkdir -p` / `cd`.
     let cmd = format!(
         "mkdir -p {dir} && cd {dir} && printf '%s' '{b64}' | base64 -d | tar xzf -",
-        dir = remote_dir,
+        dir = shq(remote_dir),
         b64 = b64
     );