Enhance PKey a bunch

- add .name to eventually replace .get_name
- use that in a bunch of spots to avoid some duplication
- add .identifiers classmethod to extend existing idea from ECDSAKey
- add from_type_string alt constructor which uses .identifiers
- use that in HostKeys (includes hopefully-minor refactoring)
- no longer giving outdated init kwarg to ECDSA host key loading

9 files changed, 153 insertions, 52 deletions

diff --git a/paramiko/__init__.py b/paramiko/__init__.py
index cbc240a6..3537f1d0 100644
--- a/paramiko/__init__.py
+++ b/paramiko/__init__.py
@@ -94,6 +94,10 @@ from paramiko.sftp import (
 from paramiko.common import io_sleep
 
 
+# TODO: I guess a real plugin system might be nice for future expansion...
+key_classes = [DSSKey, RSAKey, Ed25519Key, ECDSAKey]
+
+
 __author__ = "Jeff Forcier <jeff@bitprophet.org>"
 __license__ = "GNU Lesser General Public License (LGPL)"
 
diff --git a/paramiko/dsskey.py b/paramiko/dsskey.py
index 5a0f85eb..5215d282 100644
--- a/paramiko/dsskey.py
+++ b/paramiko/dsskey.py
@@ -43,6 +43,8 @@ class DSSKey(PKey):
     data.
     """
 
+    name = "ssh-dss"
+
     def __init__(
         self,
         msg=None,
@@ -71,8 +73,8 @@ class DSSKey(PKey):
         else:
             self._check_type_and_load_cert(
                 msg=msg,
-                key_type="ssh-dss",
-                cert_type="ssh-dss-cert-v01@openssh.com",
+                key_type=self.name,
+                cert_type=f"{self.name}-cert-v01@openssh.com",
             )
             self.p = msg.get_mpint()
             self.q = msg.get_mpint()
@@ -82,7 +84,7 @@ class DSSKey(PKey):
 
     def asbytes(self):
         m = Message()
-        m.add_string("ssh-dss")
+        m.add_string(self.name)
         m.add_mpint(self.p)
         m.add_mpint(self.q)
         m.add_mpint(self.g)
@@ -96,8 +98,9 @@ class DSSKey(PKey):
     def _fields(self):
         return (self.get_name(), self.p, self.q, self.g, self.y)
 
+    # TODO 4.0: remove
     def get_name(self):
-        return "ssh-dss"
+        return self.name
 
     def get_bits(self):
         return self.size
@@ -119,7 +122,7 @@ class DSSKey(PKey):
         r, s = decode_dss_signature(sig)
 
         m = Message()
-        m.add_string("ssh-dss")
+        m.add_string(self.name)
         # apparently, in rare cases, r or s may be shorter than 20 bytes!
         rstr = util.deflate_long(r, 0)
         sstr = util.deflate_long(s, 0)
@@ -136,7 +139,7 @@ class DSSKey(PKey):
             sig = msg.asbytes()
         else:
             kind = msg.get_text()
-            if kind != "ssh-dss":
+            if kind != self.name:
                 return 0
             sig = msg.get_binary()
 
diff --git a/paramiko/ecdsakey.py b/paramiko/ecdsakey.py
index e2279754..6fd95fab 100644
--- a/paramiko/ecdsakey.py
+++ b/paramiko/ecdsakey.py
@@ -114,6 +114,7 @@ class ECDSAKey(PKey):
         password=None,
         vals=None,
         file_obj=None,
+        # TODO 4.0: remove; it does nothing since porting to cryptography.io
         validate_point=True,
     ):
         self.verifying_key = None
@@ -168,9 +169,14 @@ class ECDSAKey(PKey):
                 raise SSHException("Invalid public key")
 
     @classmethod
-    def supported_key_format_identifiers(cls):
+    def identifiers(cls):
         return cls._ECDSA_CURVES.get_key_format_identifier_list()
 
+    # TODO 4.0: deprecate/remove
+    @classmethod
+    def supported_key_format_identifiers(cls):
+        return cls.identifiers()
+
     def asbytes(self):
         key = self.verifying_key
         m = Message()
diff --git a/paramiko/ed25519key.py b/paramiko/ed25519key.py
index c8c4da6c..e5e81ac5 100644
--- a/paramiko/ed25519key.py
+++ b/paramiko/ed25519key.py
@@ -39,6 +39,8 @@ class Ed25519Key(PKey):
         Added a ``file_obj`` parameter to match other key classes.
     """
 
+    name = "ssh-ed25519"
+
     def __init__(
         self, msg=None, data=None, filename=None, password=None, file_obj=None
     ):
@@ -49,7 +51,7 @@ class Ed25519Key(PKey):
         if msg is not None:
             self._check_type_and_load_cert(
                 msg=msg,
-                key_type="ssh-ed25519",
+                key_type=self.name,
                 cert_type="ssh-ed25519-cert-v01@openssh.com",
             )
             verifying_key = nacl.signing.VerifyKey(msg.get_binary())
@@ -108,7 +110,7 @@ class Ed25519Key(PKey):
         public_keys = []
         for _ in range(num_keys):
             pubkey = Message(message.get_binary())
-            if pubkey.get_text() != "ssh-ed25519":
+            if pubkey.get_text() != self.name:
                 raise SSHException("Invalid key")
             public_keys.append(pubkey.get_binary())
 
@@ -141,7 +143,7 @@ class Ed25519Key(PKey):
 
         signing_keys = []
         for i in range(num_keys):
-            if message.get_text() != "ssh-ed25519":
+            if message.get_text() != self.name:
                 raise SSHException("Invalid key")
             # A copy of the public key, again, ignore.
             public = message.get_binary()
@@ -170,7 +172,7 @@ class Ed25519Key(PKey):
         else:
             v = self._verifying_key
         m = Message()
-        m.add_string("ssh-ed25519")
+        m.add_string(self.name)
         m.add_string(v.encode())
         return m.asbytes()
 
@@ -182,8 +184,9 @@ class Ed25519Key(PKey):
             v = self._verifying_key
         return (self.get_name(), v)
 
+    # TODO 4.0: remove
     def get_name(self):
-        return "ssh-ed25519"
+        return self.name
 
     def get_bits(self):
         return 256
@@ -193,12 +196,12 @@ class Ed25519Key(PKey):
 
     def sign_ssh_data(self, data, algorithm=None):
         m = Message()
-        m.add_string("ssh-ed25519")
+        m.add_string(self.name)
         m.add_string(self._signing_key.sign(data).signature)
         return m
 
     def verify_ssh_sig(self, data, msg):
-        if msg.get_text() != "ssh-ed25519":
+        if msg.get_text() != self.name:
             return False
 
         try:
diff --git a/paramiko/hostkeys.py b/paramiko/hostkeys.py
index bbfa5755..4d47e950 100644
--- a/paramiko/hostkeys.py
+++ b/paramiko/hostkeys.py
@@ -27,11 +27,8 @@ from hashlib import sha1
 from hmac import HMAC
 
 
-from paramiko.dsskey import DSSKey
-from paramiko.rsakey import RSAKey
+from paramiko.pkey import PKey, UnknownKeyType
 from paramiko.util import get_logger, constant_time_bytes_eq, b, u
-from paramiko.ecdsakey import ECDSAKey
-from paramiko.ed25519key import Ed25519Key
 from paramiko.ssh_exception import SSHException
 
 
@@ -95,16 +92,16 @@ class HostKeys(MutableMapping):
                 if (len(line) == 0) or (line[0] == "#"):
                     continue
                 try:
-                    e = HostKeyEntry.from_line(line, lineno)
+                    entry = HostKeyEntry.from_line(line, lineno)
                 except SSHException:
                     continue
-                if e is not None:
-                    _hostnames = e.hostnames
+                if entry is not None:
+                    _hostnames = entry.hostnames
                     for h in _hostnames:
-                        if self.check(h, e.key):
-                            e.hostnames.remove(h)
-                    if len(e.hostnames):
-                        self._entries.append(e)
+                        if self.check(h, entry.key):
+                            entry.hostnames.remove(h)
+                    if len(entry.hostnames):
+                        self._entries.append(entry)
 
     def save(self, filename):
         """
@@ -347,29 +344,27 @@ class HostKeyEntry:
             return None
         fields = fields[:3]
 
-        names, keytype, key = fields
+        names, key_type, key = fields
         names = names.split(",")
 
         # Decide what kind of key we're looking at and create an object
         # to hold it accordingly.
         try:
-            key = b(key)
-            if keytype == "ssh-rsa":
-                key = RSAKey(data=decodebytes(key))
-            elif keytype == "ssh-dss":
-                key = DSSKey(data=decodebytes(key))
-            elif keytype in ECDSAKey.supported_key_format_identifiers():
-                key = ECDSAKey(data=decodebytes(key), validate_point=False)
-            elif keytype == "ssh-ed25519":
-                key = Ed25519Key(data=decodebytes(key))
-            else:
-                log.info("Unable to handle key of type {}".format(keytype))
-                return None
-
+            # TODO: this grew organically and doesn't seem /wrong/ per se (file
+            # read -> unicode str -> bytes for base64 decode -> decoded bytes);
+            # but in Python 3 forever land, can we simply use
+            # `base64.b64decode(str-from-file)` here?
+            key_bytes = decodebytes(b(key))
         except binascii.Error as e:
             raise InvalidHostKey(line, e)
 
-        return cls(names, key)
+        try:
+            return cls(names, PKey.from_type_string(key_type, key_bytes))
+        except UnknownKeyType:
+            # TODO 4.0: consider changing HostKeys API so this just raises
+            # naturally and the exception is muted higher up in the stack?
+            log.info("Unable to handle key of type {}".format(key_type))
+            return None
 
     def to_line(self):
         """
diff --git a/paramiko/pkey.py b/paramiko/pkey.py
index c0844407..02ae9ed5 100644
--- a/paramiko/pkey.py
+++ b/paramiko/pkey.py
@@ -59,9 +59,22 @@ def _unpad_openssh(data):
     return data[:-padding_length]
 
 
+class UnknownKeyType(Exception):
+    """
+    An unknown public/private key algorithm was attempted to be read.
+    """
+
+    def __init__(self, key_type, key_bytes):
+        self.key_type = key_type
+        self.key_bytes = key_bytes
+
+
 class PKey:
     """
     Base class for public keys.
+
+    Also includes some "meta" level convenience constructors such as
+    `.from_type_string`.
     """
 
     # known encryption types for private key files:
@@ -92,6 +105,46 @@ class PKey:
     )
     END_TAG = re.compile(r"^-{5}END (RSA|DSA|EC|OPENSSH) PRIVATE KEY-{5}\s*$")
 
+    @staticmethod
+    def from_type_string(key_type, key_bytes):
+        """
+        Given type `str` & raw `bytes`, return a `PKey` subclass instance.
+
+        For example, ``PKey.from_type_string("ssh-ed25519", <public bytes>)``
+        will (if successful) return a new `.Ed25519Key`.
+
+        :param str key_type:
+            The key type, eg ``"ssh-ed25519"``.
+        :param bytes key_bytes:
+            The raw byte data forming the key material, as expected by
+            subclasses' ``data`` parameter.
+
+        :returns:
+            A `PKey` subclass instance.
+
+        :raises:
+            `UnknownKeyType`, if no registered classes knew about this type.
+
+        .. versionadded:: 3.2
+        """
+        from paramiko import key_classes
+
+        for key_class in key_classes:
+            if key_type in key_class.identifiers():
+                return key_class(data=key_bytes)
+        raise UnknownKeyType(key_type=key_type, key_bytes=key_bytes)
+
+    @classmethod
+    def identifiers(cls):
+        """
+        returns an iterable of key format/name strings this class can handle.
+
+        Most classes only have a single identifier, and thus this default
+        implementation suffices; see `.ECDSAKey` for one example of an
+        override.
+        """
+        return [cls.name]
+
     def __init__(self, msg=None, data=None):
         """
         Create a new instance of this public key type.  If ``msg`` is given,
@@ -150,11 +203,17 @@ class PKey:
         Similar to `get_name`, but aimed at pure algorithm name instead of SSH
         protocol field value.
         """
+        # Nuke the leading 'ssh-'
         # TODO in Python 3.9: use .removeprefix()
-        no_prefix = self.get_name().replace("ssh-", "")
-        # Mostly for ECDSA's sake; OpenSSH does basically this too.
-        no_suffix = no_prefix.split("-")[0]
-        return no_suffix.upper()
+        name = self.get_name().replace("ssh-", "")
+        # Trim any cert suffix (but leave the -cert, as OpenSSH does)
+        cert_tail = "-cert-v01@openssh.com"
+        if cert_tail in name:
+            name = name.replace(cert_tail, "-cert")
+        # Nuke any eg ECDSA suffix, OpenSSH does basically this too.
+        else:
+            name = name.split("-")[0]
+        return name.upper()
 
     def get_bits(self):
         """
@@ -163,6 +222,8 @@ class PKey:
 
         :return: bits in the key (as an `int`)
         """
+        # TODO 4.0: raise NotImplementedError, 0 is unlikely to ever be
+        # _correct_ and nothing in the critical path seems to use this.
         return 0
 
     def can_sign(self):
diff --git a/paramiko/rsakey.py b/paramiko/rsakey.py
index 9ea00e95..b25768b6 100644
--- a/paramiko/rsakey.py
+++ b/paramiko/rsakey.py
@@ -36,6 +36,7 @@ class RSAKey(PKey):
     data.
     """
 
+    name = "ssh-rsa"
     HASHES = {
         "ssh-rsa": hashes.SHA1,
         "ssh-rsa-cert-v01@openssh.com": hashes.SHA1,
@@ -71,13 +72,17 @@ class RSAKey(PKey):
                 msg=msg,
                 # NOTE: this does NOT change when using rsa2 signatures; it's
                 # purely about key loading, not exchange or verification
-                key_type="ssh-rsa",
+                key_type=self.name,
                 cert_type="ssh-rsa-cert-v01@openssh.com",
             )
             self.key = rsa.RSAPublicNumbers(
                 e=msg.get_mpint(), n=msg.get_mpint()
             ).public_key(default_backend())
 
+    @classmethod
+    def identifiers(cls):
+        return cls.HASHES.keys()
+
     @property
     def size(self):
         return self.key.key_size
@@ -91,7 +96,7 @@ class RSAKey(PKey):
 
     def asbytes(self):
         m = Message()
-        m.add_string("ssh-rsa")
+        m.add_string(self.name)
         m.add_mpint(self.public_numbers.e)
         m.add_mpint(self.public_numbers.n)
         return m.asbytes()
@@ -106,7 +111,7 @@ class RSAKey(PKey):
         return (self.get_name(), self.public_numbers.e, self.public_numbers.n)
 
     def get_name(self):
-        return "ssh-rsa"
+        return self.name
 
     def get_bits(self):
         return self.size
@@ -114,7 +119,9 @@ class RSAKey(PKey):
     def can_sign(self):
         return isinstance(self.key, rsa.RSAPrivateKey)
 
-    def sign_ssh_data(self, data, algorithm="ssh-rsa"):
+    def sign_ssh_data(self, data, algorithm=None):
+        if algorithm is None:
+            algorithm = self.name
         sig = self.key.sign(
             data,
             padding=padding.PKCS1v15(),
diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst
index 0f652650..5a0907ff 100644
--- a/sites/www/changelog.rst
+++ b/sites/www/changelog.rst
@@ -2,6 +2,22 @@
 Changelog
 =========
 
+- :feature:`-` `~paramiko.pkey.PKey` now offers convenience
+  "meta-constructors", static methods that simplify the process of
+  instantiating the correct subclass for a given key type identifier or other
+  properties. For example, certain internals have been refactored to use
+  `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
+  `~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/test_hostkeys.py b/tests/test_hostkeys.py
index b0c94251..a028411d 100644
--- a/tests/test_hostkeys.py
+++ b/tests/test_hostkeys.py
@@ -36,8 +36,11 @@ broken.example.com ssh-rsa AAAA
 happy.example.com ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAIEA8bP1ZA7DCZDB9J0s50l31M\
 BGQ3GQ/Fc7SX6gkpXkwcZryoi4kNFhHu5LvHcZPdxXV1D+uTMfGS1eyd2Yz/DoNWXNAl8TI0cAsW\
 5ymME3bQ4J/k1IKxCtz/bAlAqFgKoc+EolMziDYqWIATtW0rYTJvzGAzTmMj80/QpsFH+Pc2M=
-modern.example.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIKHEChAIxsh2hr8Q+Ea1AAHZyfEB2elEc2YgduVzBtp+
-curvy.example.com ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBAa+pY7djSpbg5viAcZhPt56AO3U3Sd7h7dnlUp0EjfDgyYHYQxl2QZ4JGgfwR5iv9T9iRZjQzvJd5s+kBAZtpk=
+modern.example.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIKHEChAIxsh2hr8Q\
++Ea1AAHZyfEB2elEc2YgduVzBtp+
+curvy.example.com ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlz\
+dHAyNTYAAABBBAa+pY7djSpbg5viAcZhPt56AO3U3Sd7h7dnlUp0EjfDgyYHYQxl2QZ4JGgfwR5iv9\
+T9iRZjQzvJd5s+kBAZtpk=
 """
 
 test_hosts_file_tabs = """\
@@ -47,8 +50,11 @@ D+jrpI9cycZHqilK0HmxDeCuxbwyMuaCygU9gS2qoRvNLWZk70OpIKSSpBo0Wl3/XUmz9uhc=
 happy.example.com\tssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAIEA8bP1ZA7DCZDB9J0s50l31M\
 BGQ3GQ/Fc7SX6gkpXkwcZryoi4kNFhHu5LvHcZPdxXV1D+uTMfGS1eyd2Yz/DoNWXNAl8TI0cAsW\
 5ymME3bQ4J/k1IKxCtz/bAlAqFgKoc+EolMziDYqWIATtW0rYTJvzGAzTmMj80/QpsFH+Pc2M=
-modern.example.com\tssh-ed25519\tAAAAC3NzaC1lZDI1NTE5AAAAIKHEChAIxsh2hr8Q+Ea1AAHZyfEB2elEc2YgduVzBtp+
-curvy.example.com\tecdsa-sha2-nistp256\tAAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBAa+pY7djSpbg5viAcZhPt56AO3U3Sd7h7dnlUp0EjfDgyYHYQxl2QZ4JGgfwR5iv9T9iRZjQzvJd5s+kBAZtpk=
+modern.example.com\tssh-ed25519\tAAAAC3NzaC1lZDI1NTE5AAAAIKHEChAIxsh2hr8Q\
++Ea1AAHZyfEB2elEc2YgduVzBtp+
+curvy.example.com\tecdsa-sha2-nistp256\tAAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbml\
+zdHAyNTYAAABBBAa+pY7djSpbg5viAcZhPt56AO3U3Sd7h7dnlUp0EjfDgyYHYQxl2QZ4JGgfwR5iv\
+9T9iRZjQzvJd5s+kBAZtpk=
 """
 
 keyblob = b"""\