Enhance PKey.from_path and test it better

8 files changed, 58 insertions, 33 deletions

diff --git a/paramiko/__init__.py b/paramiko/__init__.py
index 9a987e1a..b51ddfb7 100644
--- a/paramiko/__init__.py
+++ b/paramiko/__init__.py
@@ -67,7 +67,7 @@ from paramiko.message import Message
 from paramiko.packet import Packetizer
 from paramiko.file import BufferedFile
 from paramiko.agent import Agent, AgentKey
-from paramiko.pkey import PKey, PublicBlob
+from paramiko.pkey import PKey, PublicBlob, UnknownKeyType
 from paramiko.hostkeys import HostKeys
 from paramiko.config import SSHConfig, SSHConfigDict
 from paramiko.proxy import ProxyCommand
diff --git a/paramiko/pkey.py b/paramiko/pkey.py
index 98bd82cd..4eabac5d 100644
--- a/paramiko/pkey.py
+++ b/paramiko/pkey.py
@@ -24,6 +24,7 @@ import base64
 from base64 import encodebytes, decodebytes
 from binascii import unhexlify
 import os
+from pathlib import Path
 from hashlib import md5, sha256
 import re
 import struct
@@ -114,13 +115,21 @@ class PKey:
         """
         Attempt to instantiate appropriate key subclass from given file path.
 
-        :param Path path: The path to load.
+        :param Path path: The path to load (may be a `str`).
+
+        :returns:
+            A `PKey` subclass instance.
+
+        :raises:
+            `UnknownKeyType`, if our crypto backend doesn't know this key type.
 
         .. versionadded:: 3.2
         """
         # TODO: make sure sphinx is reading Path right in param list...
         from paramiko import DSSKey, RSAKey, Ed25519Key, ECDSAKey
 
+        if not isinstance(path, Path):
+            path = Path(path)
         data = path.read_bytes()
         # Like OpenSSH, try modern/OpenSSH-specific key load first
         try:
@@ -136,7 +145,7 @@ class PKey:
         # because the results from the loader are literal backend, eg openssl,
         # private classes, so isinstance tests work but exact 'x class is y'
         # tests will not work)
-        # TODO: leverage already-parsed/mathed obj to avoid duplicate cpu
+        # TODO: leverage already-parsed/math'd obj to avoid duplicate cpu
         # cycles? seemingly requires most of our key subclasses to be rewritten
         # to be cryptography-object-forward. this is still likely faster than
         # the old SSHClient code that just tried instantiating every class!
@@ -150,9 +159,7 @@ class PKey:
         elif isinstance(loaded, asymmetric.ec.EllipticCurvePrivateKey):
             key_class = ECDSAKey
         else:
-            raise UnknownKeyType(
-                key_bytes=data, key_type=loaded.__class__.__name__
-            )
+            raise UnknownKeyType(key_bytes=data, key_type=loaded.__class__)
         with path.open() as fd:
             return key_class.from_private_key(fd, password=passphrase)
 
diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst
index b1de893e..43f81115 100644
--- a/sites/www/changelog.rst
+++ b/sites/www/changelog.rst
@@ -77,15 +77,16 @@ Changelog
   `PKey.from_type_string <paramiko.pkey.PKey.from_type_string>` instead of
   iterating key classes or using if/else cascades.
 
-  As part of this change, `~paramiko.pkey.PKey` and friends grew an
+  `PKey.from_path <paramiko.pkey.PKey.from_path>` is also new, and offers a way
+  to load a file path without knowing *a priori* what type of key it is (thanks
+  to some handy methods within our cryptography dependency).
+
+  As part of these changes, `~paramiko.pkey.PKey` and friends grew an
   `~paramiko.pkey.PKey.identifiers` classmethod; this is inspired by the
   `~paramiko.ecdsakey.ECDSAKey.supported_key_format_identifiers` classmethod
   (which now refers to the new method.) This also includes adding a ``.name``
   attribute to most key classes (which will eventually replace ``.get_name()``.
 
-  In addition, there is a new convenience top-level API member,
-  ``paramiko.key_classes``, containing a list of all key classes.
-
 - :feature:`-` `~paramiko.pkey.PKey` grew a new ``.algorithm_name`` property
   which displays the key algorithm; this is typically derived from the value of
   `~paramiko.pkey.PKey.get_name`. For example, ED25519 keys have a ``get_name``
diff --git a/tests/_support/ed448.key b/tests/_support/ed448.key
new file mode 100644
index 00000000..887b51c6
--- /dev/null
+++ b/tests/_support/ed448.key
@@ -0,0 +1,4 @@
+-----BEGIN PRIVATE KEY-----
+MEcCAQAwBQYDK2VxBDsEOcvcl9IoD0ktR5RWtW84NM7O2e4LmD2cWfRg7Wht/OA9
+POkmRW12VNvlP6BsXKir5yygumIjD91SQQ==
+-----END PRIVATE KEY-----
diff --git a/tests/conftest.py b/tests/conftest.py
index beef87c2..f3dc2d61 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -136,7 +136,7 @@ for datum in key_data:
 
 
 @pytest.fixture(scope="session", params=key_data, ids=lambda x: x[0])
-def key(request):
+def keys(request):
     """
     Yield an object for each known type of key, with attributes:
 
diff --git a/tests/pkey.py b/tests/pkey.py
index b1cba825..9c8fe8fc 100644
--- a/tests/pkey.py
+++ b/tests/pkey.py
@@ -1,13 +1,38 @@
-from paramiko import PKey
+from pytest import raises
+
+from cryptography.hazmat.primitives.asymmetric.ed448 import Ed448PrivateKey
+from paramiko import PKey, UnknownKeyType, RSAKey
+
+from ._util import _support
 
 
 class PKey_:
     class from_type_string:
-        def loads_from_type_and_bytes(self, key):
-            obj = PKey.from_type_string(key.full_type, key.pkey.asbytes())
-            assert obj == key.pkey
+        def loads_from_type_and_bytes(self, keys):
+            obj = PKey.from_type_string(keys.full_type, keys.pkey.asbytes())
+            assert obj == keys.pkey
 
     class from_path:
-        def loads_from_file_path(self, key):
-            obj = PKey.from_path(key.path)
-            assert obj == key.pkey
+        def loads_from_Path(self, keys):
+            obj = PKey.from_path(keys.path)
+            assert obj == keys.pkey
+
+        def loads_from_str(self):
+            key = PKey.from_path(str(_support("rsa.key")))
+            assert isinstance(key, RSAKey)
+
+        def raises_UnknownKeyType_for_unknown_types(self):
+            # I.e. a real, becomes a useful object via cryptography.io, key
+            # class that we do NOT support. Chose Ed448 randomly as OpenSSH
+            # doesn't seem to support it either, going by ssh-keygen...
+            keypath = _support("ed448.key")
+            with raises(UnknownKeyType) as exc:
+                PKey.from_path(keypath)
+            assert issubclass(exc.value.key_type, Ed448PrivateKey)
+            with open(keypath, "rb") as fd:
+                assert exc.value.key_bytes == fd.read()
+
+        def leaves_cryptography_exceptions_untouched(self):
+            # a Python file is not a private key!
+            with raises(ValueError):
+                PKey.from_path(__file__)
diff --git a/tests/test_client.py b/tests/test_client.py
index 564cda00..ea7396d9 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -194,9 +194,7 @@ class ClientTest(unittest.TestCase):
             run_kwargs[key] = kwargs.pop(key, None)
         # Server setup
         threading.Thread(target=self._run, kwargs=run_kwargs).start()
-        host_key = paramiko.RSAKey.from_private_key_file(
-            _support("rsa.key")
-        )
+        host_key = paramiko.RSAKey.from_private_key_file(_support("rsa.key"))
         public_host_key = paramiko.RSAKey(data=host_key.asbytes())
 
         # Client setup
@@ -415,9 +413,7 @@ class SSHClientTest(ClientTest):
         """
         warnings.filterwarnings("ignore", "tempnam.*")
 
-        host_key = paramiko.RSAKey.from_private_key_file(
-            _support("rsa.key")
-        )
+        host_key = paramiko.RSAKey.from_private_key_file(_support("rsa.key"))
         public_host_key = paramiko.RSAKey(data=host_key.asbytes())
         fd, localname = mkstemp()
         os.close(fd)
@@ -517,9 +513,7 @@ class SSHClientTest(ClientTest):
         """
         # Start the thread with a 1 second wait.
         threading.Thread(target=self._run, kwargs={"delay": 1}).start()
-        host_key = paramiko.RSAKey.from_private_key_file(
-            _support("rsa.key")
-        )
+        host_key = paramiko.RSAKey.from_private_key_file(_support("rsa.key"))
         public_host_key = paramiko.RSAKey(data=host_key.asbytes())
 
         self.tc = SSHClient()
diff --git a/tests/test_pkey.py b/tests/test_pkey.py
index c5b20f91..47e19945 100644
--- a/tests/test_pkey.py
+++ b/tests/test_pkey.py
@@ -138,12 +138,6 @@ TEST_KEY_BYTESTR = "\x00\x00\x00\x07ssh-rsa\x00\x00\x00\x01#\x00\x00\x00\x00ӏV\
 
 
 class KeyTest(unittest.TestCase):
-    def setUp(self):
-        pass
-
-    def tearDown(self):
-        pass
-
     def assert_keyfile_is_encrypted(self, keyfile):
         """
         A quick check that filename looks like an encrypted key.