Skip to content

Security: SSH host key verification disabled (MITM risk)#11309

Open
tomaioo wants to merge 1 commit intolablup:mainfrom
tomaioo:fix/security/ssh-host-key-verification-disabled-mitm-
Open

Security: SSH host key verification disabled (MITM risk)#11309
tomaioo wants to merge 1 commit intolablup:mainfrom
tomaioo:fix/security/ssh-host-key-verification-disabled-mitm-

Conversation

@tomaioo
Copy link
Copy Markdown

@tomaioo tomaioo commented Apr 26, 2026

Summary

Security: SSH host key verification disabled (MITM risk)

Problem

Severity: High | File: scripts/agent/deploy-static/deploy_static_files.py:L13

The deployment script sets AutoAddPolicy, which trusts unknown SSH host keys automatically. This allows man-in-the-middle attacks during deployment, potentially exposing credentials and enabling command interception/modification.

Solution

Use strict host key verification (RejectPolicy) and pre-provision known host keys (e.g., via load_system_host_keys() or a managed known_hosts file). Fail deployment on host key mismatch.

Changes

  • scripts/agent/deploy-static/deploy_static_files.py (modified)

The deployment script sets `AutoAddPolicy`, which trusts unknown SSH host keys automatically. This allows man-in-the-middle attacks during deployment, potentially exposing credentials and enabling command interception/modification.

Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
@cla-assistant
Copy link
Copy Markdown

cla-assistant Bot commented Apr 26, 2026

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

1 similar comment
@cla-assistant
Copy link
Copy Markdown

cla-assistant Bot commented Apr 26, 2026

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@rapsealk
Copy link
Copy Markdown
Member

rapsealk commented Apr 27, 2026

Thanks for the security fix — the direction is correct (AutoAddPolicy is a textbook MITM hazard, especially in this script where the trusted SSH session executes sudo rm -rf /opt/backend.ai and wget | tar chains). A few items to address before this can land:

Changelog required

This repo uses towncrier and every PR needs a news fragment. Please add one under changes/:

  • File: changes/11309.fix.md (use the PR number, .fix for the bug-fix type)

  • Suggested content (single imperative sentence):

    Reject unknown SSH host keys in deploy_static_files.py to prevent MITM during static-file deployment.

While here, the PR title Security: SSH host key verification disabled (MITM risk) describes the problem, not the fix, and lacks the conventional-commit prefix this repo enforces (the title becomes the squash-merge commit message). Please rename to:

fix: Reject unknown SSH host keys in deploy_static_files

Supporting references

This isn't just a stylistic preference — AutoAddPolicy is flagged as a vulnerability by Paramiko's own docs and three independent security authorities:

  • Paramiko upstream (API reference) — RejectPolicy is the documented default: "Policy for automatically rejecting the unknown hostname & key." AutoAddPolicy is described as "Policy for automatically adding the hostname and new host key to the local HostKeys object, and saving it" — i.e., trust-on-first-use with no verification. This PR is restoring upstream's intended secure default.
  • GitHub CodeQLpy/paramiko-missing-host-key-validation — CWE-295 (Improper Certificate Validation): "Accepting unknown host keys exposes connections to man-in-the-middle attacks." Recommended fix is RejectPolicy.
  • AWS CodeGuru / Amazon Qdo-not-auto-add-or-warning-missing-hostkey-policyHigh severity, CWE-322. Compliant example uses RejectPolicy, identical to this PR.
  • Code Pathfinder SAST rulePYTHON-LANG-SEC-071 "Paramiko Implicit Host Key Trust (AutoAddPolicy)."

Operational concern with load_system_host_keys()

Per Paramiko's docs, load_system_host_keys() "attempts to read from the user's local known hosts file without raising errors if unavailable" — meaning in CI runners, fresh jumphosts, or root-owned deploy contexts (where ~/.ssh/known_hosts is empty or absent), it silently loads nothing. The very first run will then abort with SSHException: Server ... not found in known_hosts and leave the operator guessing. Consider one of:

  1. Add ssh.load_host_keys(<managed_path>) pointing at a known_hosts file checked into the deploy environment, or
  2. Add a short comment/README note documenting the pre-provisioning step (ssh-keyscan <ip> >> ~/.ssh/known_hosts) so the next on-call doesn't get paged.

Per-host error handling

With RejectPolicy, the first unknown host in sys.argv[1:] now raises and aborts the entire batch loop with an uncaught exception — one bad fingerprint skips every remaining IP, and the operator only sees a Python traceback rather than which IP failed. Wrapping the loop body in try/except paramiko.SSHException and logging the offending IP would make this safe to use against a list of nodes.

Unrelated but adjacent (not blocking)

These pre-date your PR; flagging for awareness, not as a request:

  • deploy_static_files.py:33 shell-interpolates pwd/whoami from remote stdout into a sudo-bearing command string.
  • One SSHClient is reused across all IPs without close() between iterations.
  • import os, import subprocess, and the STATIC_FILE constant are unused.

Process

The license/cla check is still pending — please sign the CLA via the link in the bot comment so this is unblockable.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves deployment security by enforcing strict SSH host key verification to reduce man-in-the-middle (MITM) risk during static file deployment.

Changes:

  • Load system known host keys before connecting via SSH.
  • Switch Paramiko’s missing-host-key policy from auto-trust (AutoAddPolicy) to strict reject (RejectPolicy).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ssh.set_missing_host_key_policy(paramiko.RejectPolicy())

for ip in sys.argv[1:]:
ssh.connect(ip)
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With RejectPolicy enabled, ssh.connect(ip) will raise a paramiko SSHException for unknown/mismatched host keys and this script will exit with a traceback. Consider catching the host-key-related exception(s) and printing a clear actionable message (e.g., how to provision/update known_hosts for the target IP) so failures are easier to diagnose during deployments.

Suggested change
ssh.connect(ip)
try:
ssh.connect(ip)
except paramiko.BadHostKeyException as exc:
print(
f'Failed to connect to {ip}: the host key does not match the entry in known_hosts. '
f'Remove or update the stale key for this host and retry. Details: {exc}',
file=sys.stderr,
)
print(
f'Example: ssh-keygen -R {ip} && ssh-keyscan -H {ip} >> ~/.ssh/known_hosts',
file=sys.stderr,
)
sys.exit(1)
except paramiko.SSHException as exc:
print(
f'Failed to connect to {ip}: SSH host key verification failed or the host key is not present in known_hosts. '
f'Add the correct host key for this host and retry. Details: {exc}',
file=sys.stderr,
)
print(
f'Example: ssh-keyscan -H {ip} >> ~/.ssh/known_hosts',
file=sys.stderr,
)
sys.exit(1)

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants