Skip to content

Commit

Permalink
default host key policy to RejectPolicy
Browse files Browse the repository at this point in the history
It's a really bad idea to silently accept new keys. It's not how SSH
works, and it's not how Fabric *should* work. If we'd work like SSH,
we'd *prompt* the user, but the WarningPolicy doesn't do that, it just
warns, and doesn't prompt.

At the very least those policies should noisily warn the user when the
host keys get changed, but that's something Paramiko must do.

An alternative to this would be WarningPolicy but I think it's too
light and possibly too late: when you warn, Fabric probably has time
to run the bad stuff before the operator notices and interrupts.
  • Loading branch information
anarcat committed Sep 6, 2023
1 parent 5d2704d commit 083552c
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 5 deletions.
4 changes: 2 additions & 2 deletions fabric/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from invoke import Context
from invoke.exceptions import ThreadException
from paramiko.agent import AgentRequestHandler
from paramiko.client import SSHClient, AutoAddPolicy
from paramiko.client import SSHClient, RejectPolicy
from paramiko.config import SSHConfig
from paramiko.proxy import ProxyCommand

Expand Down Expand Up @@ -455,7 +455,7 @@ def __init__(

#: The `paramiko.client.SSHClient` instance this connection wraps.
client = SSHClient()
client.set_missing_host_key_policy(AutoAddPolicy())
client.set_missing_host_key_policy(RejectPolicy())
self.client = client

#: A convenience handle onto the return value of
Expand Down
6 changes: 3 additions & 3 deletions tests/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import time

from unittest.mock import patch, Mock, call, ANY
from paramiko.client import SSHClient, AutoAddPolicy
from paramiko.client import SSHClient, RejectPolicy
from paramiko import SSHConfig
import pytest # for mark, internal raises
from pytest import skip, param
Expand Down Expand Up @@ -65,7 +65,7 @@ class known_hosts_behavior:
def defaults_to_auto_add(self):
# TODO: change Paramiko API so this isn't a private access
# TODO: maybe just merge with the __init__ test that is similar
assert isinstance(Connection("host").client._policy, AutoAddPolicy)
assert isinstance(Connection("host").client._policy, RejectPolicy)

class init:
"__init__"
Expand Down Expand Up @@ -245,7 +245,7 @@ def instantiates_empty_SSHClient(self, Client):
Connection("host")
Client.assert_called_once_with()

@patch("fabric.connection.AutoAddPolicy")
@patch("fabric.connection.RejectPolicy")
def sets_missing_host_key_policy(self, Policy, client):
# TODO: should make the policy configurable early on
sentinel = Mock()
Expand Down

0 comments on commit 083552c

Please sign in to comment.