customizable host key policy #2072

anarcat · 2020-03-18T16:51:14Z

The current Fabric code does not check or save SSH host keys at all. This PR fixes the problem by ensuring we properly call the Paramiko SSH keys loader when we create our connection object.

We also load the right UserKnownHosts file based on the ssh_config, which paramiko does not do out of the box.

We might want to push the latter into paramiko, but first it seems to me the default Fabric policy of "everything goes" should b fixed.

Note that this also allows callers to override the AutoAddPolicy that is currently in use.

This should fix #2071

Thanks to `_KaszpiR_` on IRC for the report!

One getattr and dozens of test/doc lines later...

Closes fabric#1776 🇺🇸

This makes it easier to monkeypatch to earlier versions. With this patch, I can copy-paste the setup_ssh_client() function into my code and monkeypatch older fabric versions with: # hack to fix Fabric key policy: # fabric#2071 def safe_open(self): SaferConnection.setup_ssh_client(self) Connection.open_orig(self) class SaferConnection(Connection): # this function is a copy-paste from fabric#2072 def setup_ssh_client(self): # [...] Connection.open_orig = Connection.open Connection.open = safe_open This is otherwise a noop.

anarcat · 2020-03-18T20:40:09Z

i got as far as this for the unit tests, which is "not very":

modified   tests/connection.py
@@ -12,6 +12,7 @@
 from mock import patch, Mock, call, ANY
 from paramiko.client import SSHClient, AutoAddPolicy
 from paramiko import SSHConfig
+from paramiko.ssh_exception import BadHostKeyException
 import pytest  # for mark
 from pytest import skip, param
 from pytest_relaxed import raises
@@ -1154,3 +1155,10 @@ def tunnel_errors_bubble_up(self):
 
         def listener_errors_bubble_up(self):
             skip()
+
+    class known_hosts:
+        @raises(BadHostKeyException)
+        def changed_host_key_fails(self):
+            c = Connection('localhost')
+            c.open()
+

anarcat · 2020-03-18T20:53:32Z

the test suite fails, but that's because sphinx cannot reach pyinvoke.org, i doubt it's related to this patch:

loading intersphinx inventory from http://pyinvoke.org/objects.inv...
[...]
sphinx.errors.SphinxWarning: failed to reach any of the inventories with the following issues:

https://travis-ci.org/github/fabric/fabric/jobs/664111778

... not sure what's up with that, but it's true i'm having trouble reaching pyinvoke.org myself

anarcat · 2020-03-18T20:54:02Z

#2073 should fix that particular build issue.

This makes it easier to monkeypatch to earlier versions. With this patch, I can copy-paste the setup_ssh_client() function into my code and monkeypatch older fabric versions with: # hack to fix Fabric key policy: # fabric#2071 def safe_open(self): SaferConnection.setup_ssh_client(self) Connection.open_orig(self) class SaferConnection(Connection): # this function is a copy-paste from fabric#2072 def setup_ssh_client(self): # [...] Connection.open_orig = Connection.open Connection.open = safe_open This is otherwise a noop.

anarcat · 2020-03-18T21:00:17Z

rebased on top of #2073 because that actually fixes the test suite

This makes it easier to monkeypatch to earlier versions. With this patch, I can copy-paste the setup_ssh_client() function into my code and monkeypatch older fabric versions with: # hack to fix Fabric key policy: # fabric#2071 def safe_open(self): SaferConnection.setup_ssh_client(self) Connection.open_orig(self) class SaferConnection(Connection): # this function is a copy-paste from fabric#2072 def setup_ssh_client(self): # [...] Connection.open_orig = Connection.open Connection.open = safe_open This is otherwise a noop.

It looks like http://pyinvoke.org/ does not respond. Assume the correct URL is www.pyinvoke.org instead, which does. And while we're here, switch all of those to HTTPS because we're in the future now and HTTPS just works.

This is essential in case we want Fabric library consumers and users to override the Paramiko host key policy. One, for example, might want to override the default to prompt users when a new key needs to be added (which is silently done by default right now).

This removes the Fabric-specific host key policy which is to automatically accept new keys. I don't see why Fabric would do anything different than the default here at this stage, especially since we don't seem to be *loading* any keys, let alone saving them. This is a stopgap measure to fix fabric#2071

This has the following consequences: 1. the AutoAddPolicy starts to work correctly, as the `load_host_keys` call signals that it should save new fingerprints to the known_hosts file 2. it will also correctly *fail* if the fingerprint on a host changes, which was *not* the case before, as the known_hosts file was never loaded 3. it will correctly load the right `UserKnownHostsFile` according to the rules defined in the ssh configuration Regarding the last point, that is essential in my setup because I have a config block like this: Host *.example.com UserKnownHostsFile ~/.ssh/known_hosts.example.com ... which allows me to "group" known_hosts file and synchronize them from remote servers.

This covers the following use case: Host *.example.org UserKnownHostsFile ~/.ssh/known_hosts ~/.ssh/known_hosts.example.org ... which is also valid.

This makes it easier to monkeypatch to earlier versions. With this patch, I can copy-paste the setup_ssh_client() function into my code and monkeypatch older fabric versions with: # hack to fix Fabric key policy: # fabric#2071 def safe_open(self): SaferConnection.setup_ssh_client(self) Connection.open_orig(self) class SaferConnection(Connection): # this function is a copy-paste from fabric#2072 def setup_ssh_client(self): # [...] Connection.open_orig = Connection.open Connection.open = safe_open This is otherwise a noop.

anarcat · 2020-03-19T15:15:31Z

this needs to be republished against the master branch, sorry for the noise. :(

This makes it easier to monkeypatch to earlier versions. With this patch, I can copy-paste the setup_ssh_client() function into my code and monkeypatch older fabric versions with: # hack to fix Fabric key policy: # fabric#2071 def safe_open(self): SaferConnection.setup_ssh_client(self) Connection.open_orig(self) class SaferConnection(Connection): # this function is a copy-paste from fabric#2072 def setup_ssh_client(self): # [...] Connection.open_orig = Connection.open Connection.open = safe_open This is otherwise a noop.

bitprophet added 30 commits July 13, 2018 13:43

Merge branch '2.1'

ed0e279

Cut 2.2.0

2723d23

What even in the fuck happened there? I blame vim

abd3361

Fix a couple bad doc refs

4fa1851

Fix 1.x install docs now that 2 is PyPI/git default

7122ca6

Thanks to `_KaszpiR_` on IRC for the report!

PyPM is shutting down in Nov 2018, may as well remove our refs now

c3b191d

Merge branch '2.1' into 2.2

64e37b5

Merge branch '2.2'

ed146b3

Merge branch '2.0' into 2.1

5aabf2e

Initial high level test proving fabric#1824

5a91693

Nuke old TODOs

c490cfe

Tweak

dabf365

Failing unit tests re fabric#1824

d2ede06

Optimistic changelog entry re fabric#1824

6643eb5

Fixes fabric#1824

2026b81

One getattr and dozens of test/doc lines later...

Merge branch '2.2'

94e9f4a

Cut 2.2.1

0b01e3b

Merge branch '2.2'

cd2541b

Add links to Invocations and Patchwork in the README.

e8e9900

Closes fabric#1776 🇺🇸

Merge branch '2.0' into 2.1

dd6fbdb

Merge branch '2.1' into 2.2

c49ef3d

Merge branch '2.2'

fe4f5d0

Upgrading entry re fabric#1653

a91f751

Merge branch '2.0' into 2.1

52ed270

Merge branch '2.1' into 2.2

8a36a91

Merge branch '2.2'

939ef0f

DDD, TDD re fabric#1831

7b8a5a2

Implement fabric#1831

f2d3d59

Merge branch '2.0' into 2.1

5b820dd

Merge branch '2.1' into 2.2

2d6b5d1

anarcat force-pushed the host-key-policy branch from fb0fc6b to d6fc866 Compare March 18, 2020 20:30

anarcat changed the title ~~WIP: customizable host key policy~~ customizable host key policy Mar 18, 2020

anarcat force-pushed the host-key-policy branch from d6fc866 to 3c0ed28 Compare March 18, 2020 21:00

anarcat force-pushed the host-key-policy branch from 3c0ed28 to 33dc279 Compare March 18, 2020 21:09

anarcat force-pushed the host-key-policy branch from 33dc279 to f876b7f Compare March 18, 2020 21:11

anarcat added 8 commits March 19, 2020 11:02

fix intersphinx URLs

986f24d

It looks like http://pyinvoke.org/ does not respond. Assume the correct URL is www.pyinvoke.org instead, which does. And while we're here, switch all of those to HTTPS because we're in the future now and HTTPS just works.

allow multiple known_hosts keys to be provided

346aa64

This covers the following use case: Host *.example.org UserKnownHostsFile ~/.ssh/known_hosts ~/.ssh/known_hosts.example.org ... which is also valid.

do not crash on missing known hosts keys file

6b799d9

fix unit tests, now that the key policy is changeable

e6d8c24

anarcat force-pushed the host-key-policy branch from f876b7f to 8242052 Compare March 19, 2020 15:04

anarcat closed this Mar 19, 2020

anarcat deleted the host-key-policy branch March 19, 2020 15:15

anarcat mentioned this pull request Mar 19, 2020

customizable host key policy #2076

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

customizable host key policy #2072

customizable host key policy #2072

anarcat commented Mar 18, 2020 •

edited

anarcat commented Mar 18, 2020

anarcat commented Mar 18, 2020

anarcat commented Mar 18, 2020

anarcat commented Mar 18, 2020

anarcat commented Mar 19, 2020

customizable host key policy #2072

customizable host key policy #2072

Conversation

anarcat commented Mar 18, 2020 • edited

anarcat commented Mar 18, 2020

anarcat commented Mar 18, 2020

anarcat commented Mar 18, 2020

anarcat commented Mar 18, 2020

anarcat commented Mar 19, 2020

anarcat commented Mar 18, 2020 •

edited