Implement ssh_config hostname canonicalization (WIP)

- Refactor DNS lookup related junk previously only relevant to %h
- Refactor guts of lookup() so it can be done >1 time
- Changelog/tests/implementation for canonicalization itself

Closes #897

18 files changed, 329 insertions, 42 deletions

diff --git a/tests/configs/basic.config b/tests/configs/basic.config
new file mode 100644
index 00000000..1ae37cc6
--- /dev/null
+++ b/tests/configs/basic.config
@@ -0,0 +1,6 @@
+CanonicalDomains paramiko.org
+
+Host www.paramiko.org
+    User rando
+
+# vim: set ft=sshconfig :
diff --git a/tests/configs/canon-always.config b/tests/configs/canon-always.config
new file mode 100644
index 00000000..85058a14
--- /dev/null
+++ b/tests/configs/canon-always.config
@@ -0,0 +1,8 @@
+CanonicalDomains paramiko.org
+CanonicalizeHostname always
+
+Host www.paramiko.org
+    User rando
+
+
+# vim: set ft=sshconfig :
diff --git a/tests/configs/canon-ipv4.config b/tests/configs/canon-ipv4.config
new file mode 100644
index 00000000..9f48273e
--- /dev/null
+++ b/tests/configs/canon-ipv4.config
@@ -0,0 +1,9 @@
+CanonicalDomains paramiko.org
+CanonicalizeHostname yes
+AddressFamily inet
+
+Host www.paramiko.org
+    User rando
+
+
+# vim: set ft=sshconfig :
diff --git a/tests/configs/canon-local-always.config b/tests/configs/canon-local-always.config
new file mode 100644
index 00000000..c821d113
--- /dev/null
+++ b/tests/configs/canon-local-always.config
@@ -0,0 +1,9 @@
+Host www.paramiko.org
+    User rando
+
+Host www
+    CanonicalDomains paramiko.org
+    CanonicalizeHostname always
+
+
+# vim: set ft=sshconfig :
diff --git a/tests/configs/canon-local.config b/tests/configs/canon-local.config
new file mode 100644
index 00000000..418f7723
--- /dev/null
+++ b/tests/configs/canon-local.config
@@ -0,0 +1,9 @@
+Host www.paramiko.org
+    User rando
+
+Host www
+    CanonicalDomains paramiko.org
+    CanonicalizeHostname yes
+
+
+# vim: set ft=sshconfig :
diff --git a/tests/configs/canon.config b/tests/configs/canon.config
new file mode 100644
index 00000000..7a7ce6c6
--- /dev/null
+++ b/tests/configs/canon.config
@@ -0,0 +1,11 @@
+CanonicalizeHostname yes
+CanonicalDomains paramiko.org
+
+IdentityFile base.key
+
+Host www.paramiko.org
+    User rando
+    IdentityFile canonicalized.key
+
+
+# vim: set ft=sshconfig :
diff --git a/tests/configs/deep-canon-maxdots.config b/tests/configs/deep-canon-maxdots.config
new file mode 100644
index 00000000..37a82e72
--- /dev/null
+++ b/tests/configs/deep-canon-maxdots.config
@@ -0,0 +1,14 @@
+CanonicalizeHostname yes
+CanonicalDomains paramiko.org
+CanonicalizeMaxDots 2
+
+Host www.paramiko.org
+    User rando
+
+Host sub.www.paramiko.org
+    User deep
+
+Host subber.sub.www.paramiko.org
+    User deeper
+
+# vim: set ft=sshconfig :
diff --git a/tests/configs/deep-canon.config b/tests/configs/deep-canon.config
new file mode 100644
index 00000000..3c111f48
--- /dev/null
+++ b/tests/configs/deep-canon.config
@@ -0,0 +1,13 @@
+CanonicalizeHostname yes
+CanonicalDomains paramiko.org
+
+Host www.paramiko.org
+    User rando
+
+Host sub.www.paramiko.org
+    User deep
+
+Host subber.sub.www.paramiko.org
+    User deeper
+
+# vim: set ft=sshconfig :
diff --git a/tests/configs/empty-canon.config b/tests/configs/empty-canon.config
new file mode 100644
index 00000000..f268a2ca
--- /dev/null
+++ b/tests/configs/empty-canon.config
@@ -0,0 +1,9 @@
+CanonicalizeHostname yes
+CanonicalDomains
+AddressFamily inet
+
+Host www.paramiko.org
+    User rando
+
+
+# vim: set ft=sshconfig :
diff --git a/tests/configs/fallback-no.config b/tests/configs/fallback-no.config
new file mode 100644
index 00000000..86b6a484
--- /dev/null
+++ b/tests/configs/fallback-no.config
@@ -0,0 +1,9 @@
+CanonicalizeHostname yes
+CanonicalDomains paramiko.org
+CanonicalizeFallbackLocal no
+
+Host www.paramiko.org
+    User rando
+
+
+# vim: set ft=sshconfig :
diff --git a/tests/configs/fallback-yes.config b/tests/configs/fallback-yes.config
new file mode 100644
index 00000000..a07064a0
--- /dev/null
+++ b/tests/configs/fallback-yes.config
@@ -0,0 +1,8 @@
+CanonicalizeHostname yes
+CanonicalDomains paramiko.org
+CanonicalizeFallbackLocal yes
+
+Host www.paramiko.org
+    User rando
+
+# vim: set ft=sshconfig :
diff --git a/tests/configs/multi-canon-domains.config b/tests/configs/multi-canon-domains.config
new file mode 100644
index 00000000..f0cf521d
--- /dev/null
+++ b/tests/configs/multi-canon-domains.config
@@ -0,0 +1,8 @@
+CanonicalizeHostname yes
+CanonicalDomains not-a-real-tld paramiko.org
+
+Host www.paramiko.org
+    User rando
+
+
+# vim: set ft=sshconfig :
diff --git a/tests/configs/no-canon.config b/tests/configs/no-canon.config
new file mode 100644
index 00000000..bd48b790
--- /dev/null
+++ b/tests/configs/no-canon.config
@@ -0,0 +1,8 @@
+CanonicalizeHostname no
+CanonicalDomains paramiko.org
+
+Host www.paramiko.org
+    User rando
+
+
+# vim: set ft=sshconfig :
diff --git a/tests/robey.config b/tests/configs/robey.config
index 2175182f..2175182f 100644
--- a/tests/robey.config
+++ b/tests/configs/robey.config
diff --git a/tests/configs/zero-maxdots.config b/tests/configs/zero-maxdots.config
new file mode 100644
index 00000000..c7a095ab
--- /dev/null
+++ b/tests/configs/zero-maxdots.config
@@ -0,0 +1,11 @@
+CanonicalizeHostname yes
+CanonicalDomains paramiko.org
+CanonicalizeMaxDots 0
+
+Host www.paramiko.org
+    User rando
+
+Host sub.www.paramiko.org
+    User deep
+
+# vim: set ft=sshconfig :
diff --git a/tests/test_config.py b/tests/test_config.py
index 57dda537..f2c89bb5 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -2,18 +2,24 @@
 # repository
 
 from os.path import expanduser
+from socket import gaierror
 
-from pytest import raises, mark
+from mock import patch
+from pytest import raises, mark, fixture
 
-from paramiko import SSHConfig, SSHConfigDict
+from paramiko import SSHConfig, SSHConfigDict, CouldNotCanonicalize
 from paramiko.util import lookup_ssh_host_config
 
-from .util import _support
+from .util import _config
+
+
+def load_config(name):
+    return SSHConfig.from_path(_config(name))
 
 
 class TestSSHConfig(object):
     def setup(self):
-        self.config = SSHConfig.from_path(_support("robey.config"))
+        self.config = load_config("robey")
 
     def test_init(self):
         # No args!
@@ -27,12 +33,13 @@ class TestSSHConfig(object):
         assert config.lookup("foo.example.com")["user"] == "foo"
 
     def test_from_file(self):
-        with open(_support("robey.config")) as flo:
+        with open(_config("robey")) as flo:
             config = SSHConfig.from_file(flo)
         assert config.lookup("whatever")["user"] == "robey"
 
     def test_from_path(self):
-        config = SSHConfig.from_path(_support("robey.config"))
+        # NOTE: DO NOT replace with use of load_config() :D
+        config = SSHConfig.from_path(_config("robey"))
         assert config.lookup("meh.example.com")["port"] == "3333"
 
     def test_parse_config(self):
@@ -451,3 +458,141 @@ Host *
 """
         )
         assert config.lookup("anything-else").as_int("port") == 3333
+
+
+@fixture
+def socket():
+    with patch("paramiko.config.socket") as mocket:
+        # Reinstate gaierror as an actual exception and not a sub-mock.
+        # (Presumably this would work with any exception, but why not use the
+        # real one?)
+        mocket.gaierror = gaierror
+        # Patch out getaddrinfo, used to detect family-specific IP lookup -
+        # only useful for a few specific tests.
+        mocket.getaddrinfo.side_effect = mocket.gaierror
+        yield mocket
+
+
+class TestHostnameCanonicalization(object):
+    # NOTE: this class uses on-disk configs, and ones with real (at time of
+    # writing) DNS names, so that one can easily test OpenSSH's behavior using
+    # "ssh -F path/to/file.config -G <target>".
+
+    def test_off_by_default(self, socket):
+        result = load_config("basic").lookup("www")
+        assert result["hostname"] == "www"
+        assert "user" not in result
+        assert not socket.gethostbyname.called
+
+    def test_explicit_no_same_as_default(self, socket):
+        result = load_config("no-canon").lookup("www")
+        assert result["hostname"] == "www"
+        assert "user" not in result
+        assert not socket.gethostbyname.called
+
+    @mark.parametrize(
+        "config_name",
+        ("canon", "canon-always", "canon-local", "canon-local-always"),
+    )
+    def test_canonicalization_base_cases(self, socket, config_name):
+        result = load_config(config_name).lookup("www")
+        assert result["hostname"] == "www.paramiko.org"
+        assert result["user"] == "rando"
+        socket.gethostbyname.assert_called_once_with("www.paramiko.org")
+
+    def test_uses_getaddrinfo_when_AddressFamily_given(self, socket):
+        # Undo default 'always fails' mock
+        socket.getaddrinfo.side_effect = None
+        socket.getaddrinfo.return_value = [True]  # just need 1st value truthy
+        result = load_config("canon-ipv4").lookup("www")
+        assert result["hostname"] == "www.paramiko.org"
+        assert result["user"] == "rando"
+        assert not socket.gethostbyname.called
+        gai_args = socket.getaddrinfo.call_args[0]
+        assert gai_args[0] == "www.paramiko.org"
+        assert gai_args[2] is socket.AF_INET  # Mocked, but, still useful
+
+    @mark.skip
+    def test_empty_CanonicalDomains_canonicalizes_despite_noop(self, socket):
+        # Confirmed this is how OpenSSH behaves as well. Bit silly, but.
+        # TODO: this requires modifying SETTINGS_REGEX, which is a mite scary
+        # (honestly I'd prefer to move to a real parser lib anyhow) and since
+        # this is a very dumb corner case, it's marked skip for now.
+        result = load_config("empty-canon").lookup("www")
+        assert result["hostname"] == "www"  # no paramiko.org
+        assert "user" not in result  # did not discover canonicalized block
+
+    def test_CanonicalDomains_may_be_set_to_space_separated_list(self, socket):
+        # Test config has a bogus domain, followed by paramiko.org
+        socket.gethostbyname.side_effect = [socket.gaierror, True]
+        result = load_config("multi-canon-domains").lookup("www")
+        assert result["hostname"] == "www.paramiko.org"
+        assert result["user"] == "rando"
+        assert [x[0][0] for x in socket.gethostbyname.call_args_list] == [
+            "www.not-a-real-tld",
+            "www.paramiko.org",
+        ]
+
+    def test_canonicalization_applies_to_single_dot_by_default(self, socket):
+        result = load_config("deep-canon").lookup("sub.www")
+        assert result["hostname"] == "sub.www.paramiko.org"
+        assert result["user"] == "deep"
+
+    def test_canonicalization_not_applied_to_two_dots_by_default(self, socket):
+        result = load_config("deep-canon").lookup("subber.sub.www")
+        assert result["hostname"] == "subber.sub.www"
+        assert "user" not in result
+
+    def test_hostname_depth_controllable_with_max_dots_directive(self, socket):
+        # This config sets MaxDots of 2, so now canonicalization occurs
+        result = load_config("deep-canon-maxdots").lookup("subber.sub.www")
+        assert result["hostname"] == "subber.sub.www.paramiko.org"
+        assert result["user"] == "deeper"
+
+    def test_max_dots_may_be_zero(self, socket):
+        result = load_config("zero-maxdots").lookup("sub.www")
+        assert result["hostname"] == "sub.www"
+        assert "user" not in result
+
+    def test_fallback_yes_does_not_canonicalize_or_error(self, socket):
+        socket.gethostbyname.side_effect = socket.gaierror
+        result = load_config("fallback-yes").lookup("www")
+        assert result["hostname"] == "www"
+        assert "user" not in result
+
+    def test_fallback_no_causes_errors_for_unresolvable_names(self, socket):
+        socket.gethostbyname.side_effect = socket.gaierror
+        with raises(CouldNotCanonicalize) as info:
+            load_config("fallback-no").lookup("doesnotexist")
+        assert str(info.value) == "doesnotexist"
+
+    def test_identityfile_continues_being_appended_to(self, socket):
+        result = load_config("canon").lookup("www")
+        assert result["identityfile"] == ["base.key", "canonicalized.key"]
+
+
+@mark.skip
+class TestCanonicalizationOfCNAMEs(object):
+    def test_permitted_cnames_may_be_one_to_one_mapping(self):
+        # CanonicalizePermittedCNAMEs *.foo.com:*.bar.com
+        pass
+
+    def test_permitted_cnames_may_be_one_to_many_mapping(self):
+        # CanonicalizePermittedCNAMEs *.foo.com:*.bar.com,*.biz.com
+        pass
+
+    def test_permitted_cnames_may_be_many_to_one_mapping(self):
+        # CanonicalizePermittedCNAMEs *.foo.com,*.bar.com:*.biz.com
+        pass
+
+    def test_permitted_cnames_may_be_many_to_many_mapping(self):
+        # CanonicalizePermittedCNAMEs *.foo.com,*.bar.com:*.biz.com,*.baz.com
+        pass
+
+    def test_permitted_cnames_may_be_multiple_mappings(self):
+        # CanonicalizePermittedCNAMEs *.foo.com,*.bar.com *.biz.com:*.baz.com
+        pass
+
+    def test_permitted_cnames_may_be_multiple_complex_mappings(self):
+        # Same as prev but with multiple patterns on both ends in both args
+        pass
diff --git a/tests/test_util.py b/tests/test_util.py
index 7849fec3..84a48bd3 100644
--- a/tests/test_util.py
+++ b/tests/test_util.py
@@ -43,44 +43,47 @@ BGQ3GQ/Fc7SX6gkpXkwcZryoi4kNFhHu5LvHcZPdxXV1D+uTMfGS1eyd2Yz/DoNWXNAl8TI0cAsW\
 
 
 class UtilTest(unittest.TestCase):
-    def test_import(self):
+    def test_imports(self):
         """
         verify that all the classes can be imported from paramiko.
         """
-        symbols = paramiko.__all__
-        self.assertTrue("Agent" in symbols)
-        self.assertTrue("AgentKey" in symbols)
-        self.assertTrue("AuthenticationException" in symbols)
-        self.assertTrue("AutoAddPolicy" in symbols)
-        self.assertTrue("BadAuthenticationType" in symbols)
-        self.assertTrue("BufferedFile" in symbols)
-        self.assertTrue("Channel" in symbols)
-        self.assertTrue("ChannelException" in symbols)
-        self.assertTrue("DSSKey" in symbols)
-        self.assertTrue("HostKeys" in symbols)
-        self.assertTrue("Message" in symbols)
-        self.assertTrue("MissingHostKeyPolicy" in symbols)
-        self.assertTrue("PasswordRequiredException" in symbols)
-        self.assertTrue("RSAKey" in symbols)
-        self.assertTrue("RejectPolicy" in symbols)
-        self.assertTrue("SFTP" in symbols)
-        self.assertTrue("SFTPAttributes" in symbols)
-        self.assertTrue("SFTPClient" in symbols)
-        self.assertTrue("SFTPError" in symbols)
-        self.assertTrue("SFTPFile" in symbols)
-        self.assertTrue("SFTPHandle" in symbols)
-        self.assertTrue("SFTPServer" in symbols)
-        self.assertTrue("SFTPServerInterface" in symbols)
-        self.assertTrue("SSHClient" in symbols)
-        self.assertTrue("SSHConfig" in symbols)
-        self.assertTrue("SSHConfigDict" in symbols)
-        self.assertTrue("SSHException" in symbols)
-        self.assertTrue("SecurityOptions" in symbols)
-        self.assertTrue("ServerInterface" in symbols)
-        self.assertTrue("SubsystemHandler" in symbols)
-        self.assertTrue("Transport" in symbols)
-        self.assertTrue("WarningPolicy" in symbols)
-        self.assertTrue("util" in symbols)
+        for name in (
+            "Agent",
+            "AgentKey",
+            "AuthenticationException",
+            "AutoAddPolicy",
+            "BadAuthenticationType",
+            "BufferedFile",
+            "Channel",
+            "ChannelException",
+            "CouldNotCanonicalize",
+            "DSSKey",
+            "HostKeys",
+            "Message",
+            "MissingHostKeyPolicy",
+            "PasswordRequiredException",
+            "RSAKey",
+            "RejectPolicy",
+            "SFTP",
+            "SFTPAttributes",
+            "SFTPClient",
+            "SFTPError",
+            "SFTPFile",
+            "SFTPHandle",
+            "SFTPServer",
+            "SFTPServerInterface",
+            "SSHClient",
+            "SSHConfig",
+            "SSHConfigDict",
+            "SSHException",
+            "SecurityOptions",
+            "ServerInterface",
+            "SubsystemHandler",
+            "Transport",
+            "WarningPolicy",
+            "util",
+        ):
+            assert name in paramiko.__all__
 
     def test_generate_key_bytes(self):
         x = paramiko.util.generate_key_bytes(
diff --git a/tests/util.py b/tests/util.py
index cdc835c9..339677aa 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -9,8 +9,15 @@ from paramiko.py3compat import builtins
 from paramiko.ssh_gss import GSS_AUTH_AVAILABLE
 
 
+tests_dir = dirname(realpath(__file__))
+
+
 def _support(filename):
-    return join(dirname(realpath(__file__)), filename)
+    return join(tests_dir, filename)
+
+
+def _config(name):
+    return join(tests_dir, "configs", "{}.config".format(name))
 
 
 needs_gssapi = pytest.mark.skipif(