From 0146cf72012e46505851abe17d8eb3555a481967 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 16 Apr 2023 17:31:56 -0400 Subject: 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 --- paramiko/__init__.py | 4 +++ paramiko/dsskey.py | 15 ++++++----- paramiko/ecdsakey.py | 8 +++++- paramiko/ed25519key.py | 17 +++++++----- paramiko/hostkeys.py | 47 +++++++++++++++------------------ paramiko/pkey.py | 69 ++++++++++++++++++++++++++++++++++++++++++++++--- paramiko/rsakey.py | 15 ++++++++--- sites/www/changelog.rst | 16 ++++++++++++ tests/test_hostkeys.py | 14 +++++++--- 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 " __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", )`` + 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 ` 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"""\ -- cgit v1.2.1