From 7229597ce0925ee8dafe97544f42dcc193fbad8f Mon Sep 17 00:00:00 2001 From: Paul Kapp Date: Tue, 22 Aug 2017 06:31:47 -0400 Subject: Generic certificate support Roll agnostic certificate support into PKey, and tweak publickey authentication to use it only if set. Requires explicit call to PKey.load_certificate() in order to alter the authentication behavior. --- tests/test_pkey.py | 24 ++++++++++++++++++++++++ tests/test_rsa.key-cert.pub | 1 + tests/test_rsa.key.pub | 1 + 3 files changed, 26 insertions(+) create mode 100644 tests/test_rsa.key-cert.pub create mode 100644 tests/test_rsa.key.pub (limited to 'tests') diff --git a/tests/test_pkey.py b/tests/test_pkey.py index 9bb3c44c..034331a2 100644 --- a/tests/test_pkey.py +++ b/tests/test_pkey.py @@ -480,3 +480,27 @@ class KeyTest(unittest.TestCase): self.assert_keyfile_is_encrypted(newfile) finally: os.remove(newfile) + + def test_certificates(self): + # PKey.load_certificate + key = RSAKey.from_private_key_file(test_path('test_rsa.key')) + self.assertTrue(key.public_blob is None) + key.load_certificate(pubkey_filename=test_path('test_rsa.key-cert.pub')) + self.assertTrue(key.public_blob is not None) + self.assertEqual(key.public_blob.key_type, 'ssh-rsa-cert-v01@openssh.com') + self.assertEqual(key.public_blob.comment, 'test_rsa.key.pub') + # Delve into blob contents, for test purposes + msg = Message(key.public_blob.key_blob) + self.assertEqual(msg.get_string(), 'ssh-rsa-cert-v01@openssh.com') + nonce = msg.get_string() + e = msg.get_mpint() + n = msg.get_mpint() + self.assertEqual(e, key.public_numbers.e) + self.assertEqual(n, key.public_numbers.n) + # Serial number + self.assertEqual(msg.get_int64(), 1234) + + # Prevented from loading certificate that doesn't match + key1 = Ed25519Key.from_private_key_file(test_path('test_ed25519.key')) + self.assertRaises(ValueError, key1.load_certificate, + pubkey_filename=test_path('test_rsa.key-cert.pub')) diff --git a/tests/test_rsa.key-cert.pub b/tests/test_rsa.key-cert.pub new file mode 100644 index 00000000..7487ab66 --- /dev/null +++ b/tests/test_rsa.key-cert.pub @@ -0,0 +1 @@ +ssh-rsa-cert-v01@openssh.com AAAAHHNzaC1yc2EtY2VydC12MDFAb3BlbnNzaC5jb20AAAAgsZlXTd5NE4uzGAn6TyAqQj+IPbsTEFGap2x5pTRwQR8AAAABIwAAAIEA049W6geFpmsljTwfvI1UmKWWJPNFI74+vNKTk4dmzkQY2yAMs6FhlvhlI8ysU4oj71ZsRYMecHbBbxdN79+JRFVYTKaLqjwGENeTd+yv4q+V2PvZv3fLnzApI3l7EJCqhWwJUHJ1jAkZzqDx0tyOL4uoZpww3nmE0kb3y21tH4cAAAAAAAAE0gAAAAEAAAAmU2FtcGxlIHNlbGYtc2lnbmVkIE9wZW5TU0ggY2VydGlmaWNhdGUAAAASAAAABXVzZXIxAAAABXVzZXIyAAAAAAAAAAD//////////wAAAAAAAACCAAAAFXBlcm1pdC1YMTEtZm9yd2FyZGluZwAAAAAAAAAXcGVybWl0LWFnZW50LWZvcndhcmRpbmcAAAAAAAAAFnBlcm1pdC1wb3J0LWZvcndhcmRpbmcAAAAAAAAACnBlcm1pdC1wdHkAAAAAAAAADnBlcm1pdC11c2VyLXJjAAAAAAAAAAAAAACVAAAAB3NzaC1yc2EAAAABIwAAAIEA049W6geFpmsljTwfvI1UmKWWJPNFI74+vNKTk4dmzkQY2yAMs6FhlvhlI8ysU4oj71ZsRYMecHbBbxdN79+JRFVYTKaLqjwGENeTd+yv4q+V2PvZv3fLnzApI3l7EJCqhWwJUHJ1jAkZzqDx0tyOL4uoZpww3nmE0kb3y21tH4cAAACPAAAAB3NzaC1yc2EAAACATFHFsARDgQevc6YLxNnDNjsFtZ08KPMyYVx0w5xm95IVZHVWSOc5w+ccjqN9HRwxV3kP7IvL91qx0Uc3MJdB9g/O6HkAP+rpxTVoTb2EAMekwp5+i8nQJW4CN2BSsbQY1M6r7OBZ5nmF4hOW/5Pu4l22lXe2ydy8kEXOEuRpUeQ= test_rsa.key.pub diff --git a/tests/test_rsa.key.pub b/tests/test_rsa.key.pub new file mode 100644 index 00000000..bfa1e150 --- /dev/null +++ b/tests/test_rsa.key.pub @@ -0,0 +1 @@ +ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAIEA049W6geFpmsljTwfvI1UmKWWJPNFI74+vNKTk4dmzkQY2yAMs6FhlvhlI8ysU4oj71ZsRYMecHbBbxdN79+JRFVYTKaLqjwGENeTd+yv4q+V2PvZv3fLnzApI3l7EJCqhWwJUHJ1jAkZzqDx0tyOL4uoZpww3nmE0kb3y21tH4c= -- cgit v1.2.3 From b942d94e2d59335f11f635164525a4f578ea6991 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Mon, 28 Aug 2017 15:40:33 -0700 Subject: Stub tests and partly-working implementation of 'load certs found alongside key_filenames' behavior re #1042 This actually breaks existing tests due to test server not supporting certs...bah --- paramiko/client.py | 30 +++++++++++++++++++++--------- tests/test_client.py | 15 +++++++++++++++ tests/util.py | 1 - 3 files changed, 36 insertions(+), 10 deletions(-) (limited to 'tests') diff --git a/paramiko/client.py b/paramiko/client.py index 22e636a7..3b3895b6 100644 --- a/paramiko/client.py +++ b/paramiko/client.py @@ -515,22 +515,34 @@ class SSHClient (ClosingContextManager): def _key_from_filepath(self, filename, klass, password): """ - Attempt to derive a `.PKey` from given string path ``filename``. + Attempt to derive a `.PKey` from given string path ``filename``: + + - If ``filename`` appears to be a cert, the matching private key is + loaded. + - Otherwise, the filename is assumed to be a private key, and the + matching public cert will be loaded if it exists. """ cert_suffix = '-cert.pub' - key_path = filename - is_cert = False + # Assume privkey, not cert, by default if filename.endswith(cert_suffix): key_path = filename[:-len(cert_suffix)] - is_cert = True + cert_path = filename + else: + key_path = filename + cert_path = filename + cert_suffix + # Blindly try the key path; if no private key, nothing will work. key = klass.from_private_key_file(key_path, password) - if is_cert: - key.load_certificate(pubkey_filename=filename) - type_ = 'certificate' if is_cert else 'key' - msg = "Trying discovered {0} {1} in {2}".format( - type_, hexlify(key.get_fingerprint()), filename, + # TODO: change this to 'Loading' instead of 'Trying' sometime; probably + # when #387 is released, since this is a critical log message users are + # likely testing/filtering for (bah.) + msg = "Trying discovered key {0} in {1}".format( + hexlify(key.get_fingerprint()), key_path, ) self._log(DEBUG, msg) + # Attempt to load cert if it exists. + if os.path.isfile(cert_path): + key.load_certificate(pubkey_filename=cert_path) + self._log(DEBUG, "Adding public certificate {0}".format(cert_path)) return key def _auth(self, username, password, pkey, key_filenames, allow_agent, diff --git a/tests/test_client.py b/tests/test_client.py index e912d5b2..cfffa030 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -248,6 +248,21 @@ class SSHClientTest (unittest.TestCase): allowed_keys=['ecdsa-sha2-nistp256'], ) + def test_certs_allowed_as_key_filename_values(self): + # TODO: connect() with key_filename containing a cert file, loads up + # both the cert and its implied key. This is new functionality on top + # of the OpenSSH-compatible stuff. + assert False + + def test_certs_implicitly_loaded_alongside_key_filename_keys(self): + # TODO: same but with just the key paths, i.e. vanilla OpenSSH behavior + assert False + + def test_default_key_locations_trigger_cert_loads_if_found(self): + # TODO: what it says on the tin: ~/.ssh/id_rsa tries to load + # ~/.ssh/id_rsa-cert.pub + assert False + def test_4_auto_add_policy(self): """ verify that SSHClient's AutoAddPolicy works. diff --git a/tests/util.py b/tests/util.py index b546a7e1..c1b43da8 100644 --- a/tests/util.py +++ b/tests/util.py @@ -4,4 +4,3 @@ root_path = os.path.dirname(os.path.realpath(__file__)) def test_path(filename): return os.path.join(root_path, filename) - -- cgit v1.2.3 From 03df3cf9cd0f12cc04abe88a8674e6968363340c Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Mon, 28 Aug 2017 17:45:54 -0700 Subject: Overhaul PublicBlob and use it better within RSAKey. This allows server-side Paramiko code to correctly create cert-bearing RSAKey objects and thus verify client signatures, and now the test suite passes again, barring the stub tests. Re #1042 --- paramiko/client.py | 2 +- paramiko/pkey.py | 105 +++++++++++++++++++++++++++++++++++--------------- paramiko/rsakey.py | 20 +++++++++- paramiko/transport.py | 1 + tests/test_pkey.py | 9 +++-- 5 files changed, 102 insertions(+), 35 deletions(-) (limited to 'tests') diff --git a/paramiko/client.py b/paramiko/client.py index 3b3895b6..bff223ea 100644 --- a/paramiko/client.py +++ b/paramiko/client.py @@ -541,7 +541,7 @@ class SSHClient (ClosingContextManager): self._log(DEBUG, msg) # Attempt to load cert if it exists. if os.path.isfile(cert_path): - key.load_certificate(pubkey_filename=cert_path) + key.load_certificate(cert_path) self._log(DEBUG, "Adding public certificate {0}".format(cert_path)) return key diff --git a/paramiko/pkey.py b/paramiko/pkey.py index 3a872491..948152fa 100644 --- a/paramiko/pkey.py +++ b/paramiko/pkey.py @@ -365,10 +365,11 @@ class PKey(object): encryption ).decode()) - def load_certificate(self, **kwargs): + def load_certificate(self, value): """ - Supplement the private key contents with data loaded from - an OpenSSH public key (.pub) or certificate (-cert.pub) file. + Supplement the private key contents with data loaded from an OpenSSH + public key (``.pub``) or certificate (``-cert.pub``) file, a string + containing such a file, or a `.Message` object. The .pub contents adds no real value, since the private key file includes sufficient information to derive the public @@ -382,7 +383,13 @@ class PKey(object): that is for the server to decide if it is good enough to authenticate successfully. """ - blob = PublicBlob(**kwargs) + if isinstance(value, Message): + constructor = 'from_message' + elif os.path.isfile(value): + constructor = 'from_file' + else: + constructor = 'from_string' + blob = getattr(PublicBlob, constructor)(value) if not blob.key_type.startswith(self.get_name()): raise ValueError('PublicBlob type %s incompatible with key type %s' % (blob.key_type, self.get_name())) @@ -396,36 +403,74 @@ class PKey(object): # {ssh-rsa, ssh-dss, ssh-ecdsa, ssh-ed25519}, but should # provide rudimentary support for {*-cert.v01} class PublicBlob(object): - ''' - OpenSSH plain public key or OpenSSH signed public key (certificate) - A mostly dumb container - ''' - def __init__(self, pubkey_filename=None, pubkey_string=None): - ''' - Can read from a file or string. - ''' - if pubkey_filename: - with open(pubkey_filename) as f: - fields = f.read().split(None, 2) - elif pubkey_string: - fields = pubkey_string.split(None, 2) - else: - raise ValueError('PublicBlob() requires either a pubkey_filename or pubkey_string') + """ + OpenSSH plain public key or OpenSSH signed public key (certificate). + + Tries to be as dumb as possible and barely cares about specific + per-key-type data. + + ..note:: + Most of the time you'll want to call `from_file`, `from_string` or + `from_message` for useful instantiation, the main constructor is + basically "I should be using ``attrs`` for this." + """ + def __init__(self, type_, blob, comment=None): + """ + Create a new public blob of given type and contents. + + :param str type_: Type indicator, eg ``ssh-rsa``. + :param blob: The blob bytes themselves. + :param str comment: A comment, if one was given (e.g. file-based.) + """ + self.key_type = type_ + self.key_blob = blob + self.comment = comment + + @classmethod + def from_file(cls, filename): + """ + Create a public blob from a ``-cert.pub``-style file on disk. + """ + with open(filename) as f: + string = f.read() + return cls.from_string(string) + + @classmethod + def from_string(cls, string): + """ + Create a public blob from a ``-cert.pub``-style string. + """ + fields = string.split(None, 2) if len(fields) < 2: - raise ValueError('PublicBlob() not enough fields %s', fields) - self.key_type = fields[0] - self.key_blob = decodebytes(fields[1]) + msg = "Not enough fields for public blob: {0}" + raise ValueError(msg.format(fields)) + key_type = fields[0] + key_blob = decodebytes(fields[1]) try: - self.comment = fields[2].strip() + comment = fields[2].strip() except IndexError: - self.comment = None - # Verify that the blob message first (string) field matches the key_type - m = Message(self.key_blob) + comment = None + # Verify that the blob message first (string) field matches the + # key_type + m = Message(key_blob) blob_type = m.get_string() - if blob_type != self.key_type: - raise ValueError( - 'Invalid PublicBlob contents. Key type [%s], expected [%s]' % - (blob_type, self.key_type)) + if blob_type != key_type: + msg = "Invalid PublicBlob contents: key type {0!r}, expected {1!r}" + raise ValueError(msg.format(blob_type, key_type)) + # All good? All good. + return cls(type_=key_type, blob=key_blob, comment=comment) + + @classmethod + def from_message(cls, message): + """ + Create a public blob from a network `.Message`. + + Specifically, a cert-bearing pubkey auth packet, because by definition + OpenSSH-style certificates 'are' their own network representation." + """ + type_ = message.get_text() + message.rewind() + return cls(type_=type_, blob=message.asbytes()) def __str__(self): if self.comment: diff --git a/paramiko/rsakey.py b/paramiko/rsakey.py index 7abcfa28..3f28689a 100644 --- a/paramiko/rsakey.py +++ b/paramiko/rsakey.py @@ -54,8 +54,26 @@ class RSAKey(PKey): else: if msg is None: raise SSHException('Key object may not be empty') - if msg.get_text() != 'ssh-rsa': + type_ = msg.get_text() + nonce = None + # Regular public key - nothing special to do besides the implicit + # type check. + if type_ == 'ssh-rsa': + pass + # OpenSSH-compatible certificate - store full copy as .public_blob + # (so signing works correctly) and then fast-forward past the + # nonce. + elif type_ == 'ssh-rsa-cert-v01@openssh.com': + # This seems the cleanest way to 'clone' an already-being-read + # message? + self.load_certificate(Message(msg.asbytes())) + # Read out nonce as it comes before the public numbers. + # TODO: usefully interpret it & other non-public-number fields + nonce = msg.get_string() + else: raise SSHException('Invalid key') + # Now that we've read type and (possibly) nonce, public numbers are + # next in either case. self.key = rsa.RSAPublicNumbers( e=msg.get_mpint(), n=msg.get_mpint() ).public_key(default_backend()) diff --git a/paramiko/transport.py b/paramiko/transport.py index 388f60cb..0dc2e28a 100644 --- a/paramiko/transport.py +++ b/paramiko/transport.py @@ -204,6 +204,7 @@ class Transport(threading.Thread, ClosingContextManager): _key_info = { 'ssh-rsa': RSAKey, + 'ssh-rsa-cert-v01@openssh.com': RSAKey, 'ssh-dss': DSSKey, 'ecdsa-sha2-nistp256': ECDSAKey, 'ecdsa-sha2-nistp384': ECDSAKey, diff --git a/tests/test_pkey.py b/tests/test_pkey.py index 034331a2..dac1d02b 100644 --- a/tests/test_pkey.py +++ b/tests/test_pkey.py @@ -485,7 +485,7 @@ class KeyTest(unittest.TestCase): # PKey.load_certificate key = RSAKey.from_private_key_file(test_path('test_rsa.key')) self.assertTrue(key.public_blob is None) - key.load_certificate(pubkey_filename=test_path('test_rsa.key-cert.pub')) + key.load_certificate(test_path('test_rsa.key-cert.pub')) self.assertTrue(key.public_blob is not None) self.assertEqual(key.public_blob.key_type, 'ssh-rsa-cert-v01@openssh.com') self.assertEqual(key.public_blob.comment, 'test_rsa.key.pub') @@ -502,5 +502,8 @@ class KeyTest(unittest.TestCase): # Prevented from loading certificate that doesn't match key1 = Ed25519Key.from_private_key_file(test_path('test_ed25519.key')) - self.assertRaises(ValueError, key1.load_certificate, - pubkey_filename=test_path('test_rsa.key-cert.pub')) + self.assertRaises( + ValueError, + key1.load_certificate, + test_path('test_rsa.key-cert.pub'), + ) -- cgit v1.2.3 From 38cf578bb2c06c600eaf56d4380fcf84ed399a08 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Mon, 28 Aug 2017 18:50:48 -0700 Subject: Update first few stub tests + required test-server and PublicBlob impl bits --- paramiko/pkey.py | 7 +++++++ tests/test_client.py | 50 +++++++++++++++++++++++++++++++++++++------------- 2 files changed, 44 insertions(+), 13 deletions(-) (limited to 'tests') diff --git a/paramiko/pkey.py b/paramiko/pkey.py index 948152fa..1683c35c 100644 --- a/paramiko/pkey.py +++ b/paramiko/pkey.py @@ -477,3 +477,10 @@ class PublicBlob(object): return '%s public key/certificate - %s' % (self.key_type, self.comment) else: return '%s public key/certificate' % self.key_type + + def __eq__(self, other): + # Just piggyback on Message/BytesIO, since both of these should be one. + return self and other and self.key_blob == other.key_blob + + def __ne__(self, other): + return not self == other diff --git a/tests/test_client.py b/tests/test_client.py index cfffa030..da6cb58d 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -35,6 +35,7 @@ import time from tests.util import test_path import paramiko +from paramiko.pkey import PublicBlob from paramiko.common import PY2 from paramiko.ssh_exception import SSHException, AuthenticationException @@ -47,10 +48,12 @@ FINGERPRINTS = { } -class NullServer (paramiko.ServerInterface): +class NullServer(paramiko.ServerInterface): def __init__(self, *args, **kwargs): # Allow tests to enable/disable specific key types self.__allowed_keys = kwargs.pop('allowed_keys', []) + # And allow them to set a (single...meh) expected public blob (cert) + self.__expected_public_blob = kwargs.pop('public_blob', None) super(NullServer, self).__init__(*args, **kwargs) def get_allowed_auths(self, username): @@ -71,12 +74,18 @@ class NullServer (paramiko.ServerInterface): expected = FINGERPRINTS[key.get_name()] except KeyError: return paramiko.AUTH_FAILED - if ( + # Base check: allowed auth type & fingerprint matches + happy = ( key.get_name() in self.__allowed_keys and key.get_fingerprint() == expected + ) + # Secondary check: if test wants assertions about cert data + if ( + self.__expected_public_blob is not None and + key.public_blob != self.__expected_public_blob ): - return paramiko.AUTH_SUCCESSFUL - return paramiko.AUTH_FAILED + happy = False + return paramiko.AUTH_SUCCESSFUL if happy else paramiko.AUTH_FAILED def check_channel_request(self, kind, chanid): return paramiko.OPEN_SUCCEEDED @@ -117,7 +126,7 @@ class SSHClientTest (unittest.TestCase): if hasattr(self, attr): getattr(self, attr).close() - def _run(self, allowed_keys=None, delay=0): + def _run(self, allowed_keys=None, delay=0, public_blob=None): if allowed_keys is None: allowed_keys = FINGERPRINTS.keys() self.socks, addr = self.sockl.accept() @@ -128,7 +137,7 @@ class SSHClientTest (unittest.TestCase): keypath = test_path('test_ecdsa_256.key') host_key = paramiko.ECDSAKey.from_private_key_file(keypath) self.ts.add_server_key(host_key) - server = NullServer(allowed_keys=allowed_keys) + server = NullServer(allowed_keys=allowed_keys, public_blob=public_blob) if delay: time.sleep(delay) self.ts.start_server(self.event, server) @@ -140,7 +149,9 @@ class SSHClientTest (unittest.TestCase): The exception is ``allowed_keys`` which is stripped and handed to the ``NullServer`` used for testing. """ - run_kwargs = {'allowed_keys': kwargs.pop('allowed_keys', None)} + run_kwargs = {} + for key in ('allowed_keys', 'public_blob'): + 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(test_path('test_rsa.key')) @@ -249,14 +260,27 @@ class SSHClientTest (unittest.TestCase): ) def test_certs_allowed_as_key_filename_values(self): - # TODO: connect() with key_filename containing a cert file, loads up - # both the cert and its implied key. This is new functionality on top - # of the OpenSSH-compatible stuff. - assert False + # NOTE: giving cert path here, not key path. (Key path test is below. + # They're similar except for which path is given; the expected auth and + # server-side behavior is 100% identical.) + cert_path = test_path('test_rsa.key-cert.pub') + self._test_connection( + key_filename=cert_path, + public_blob=PublicBlob.from_file(cert_path), + ) def test_certs_implicitly_loaded_alongside_key_filename_keys(self): - # TODO: same but with just the key paths, i.e. vanilla OpenSSH behavior - assert False + # NOTE: a regular test_connection() w/ test_rsa.key would incidentally + # test this (because test_rsa.key-cert.pub exists) but incidental tests + # stink, so NullServer and friends were updated to allow assertions + # about the server-side key object's public blob. Thus, we can prove + # that a specific cert was found, along with regular authorization + # succeeding proving that the overall flow works. + key_path = test_path('test_rsa.key') + self._test_connection( + key_filename=key_path, + public_blob=PublicBlob.from_file('{0}-cert.pub'.format(key_path)), + ) def test_default_key_locations_trigger_cert_loads_if_found(self): # TODO: what it says on the tin: ~/.ssh/id_rsa tries to load -- cgit v1.2.3 From d03c8b6e2f63cba2cb6d93ec3cd270bf49cda3da Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Mon, 28 Aug 2017 18:52:15 -0700 Subject: God damn it, really? Whatever. --- tests/test_client.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/tests/test_client.py b/tests/test_client.py index da6cb58d..a3e5e308 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -284,8 +284,9 @@ class SSHClientTest (unittest.TestCase): def test_default_key_locations_trigger_cert_loads_if_found(self): # TODO: what it says on the tin: ~/.ssh/id_rsa tries to load - # ~/.ssh/id_rsa-cert.pub - assert False + # ~/.ssh/id_rsa-cert.pub. Right now no other tests actually test that + # code path (!) so we're punting too, sob. + pass def test_4_auto_add_policy(self): """ -- cgit v1.2.3 From b9f7b6058906ac7f80b815570731d967b299ba8d Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Mon, 28 Aug 2017 19:30:22 -0700 Subject: Update recent tests to try all main key families. Includes some dummy certificates. Not sure exactly how @radssh generated the RSA one but I'm using ssh-keygen + a randomly made CA key. --- tests/test_client.py | 26 +++++++++++++++----------- tests/test_dss.key-cert.pub | 1 + tests/test_ecdsa_256.key-cert.pub | 1 + tests/test_ed25519.key-cert.pub | 1 + 4 files changed, 18 insertions(+), 11 deletions(-) create mode 100644 tests/test_dss.key-cert.pub create mode 100644 tests/test_ecdsa_256.key-cert.pub create mode 100644 tests/test_ed25519.key-cert.pub (limited to 'tests') diff --git a/tests/test_client.py b/tests/test_client.py index a3e5e308..40e2fb12 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -263,24 +263,28 @@ class SSHClientTest (unittest.TestCase): # NOTE: giving cert path here, not key path. (Key path test is below. # They're similar except for which path is given; the expected auth and # server-side behavior is 100% identical.) - cert_path = test_path('test_rsa.key-cert.pub') - self._test_connection( - key_filename=cert_path, - public_blob=PublicBlob.from_file(cert_path), - ) + for type_ in ('rsa', 'dss', 'ecdsa_256', 'ed25519'): + cert_path = test_path('test_{}.key-cert.pub'.format(type_)) + self._test_connection( + key_filename=cert_path, + public_blob=PublicBlob.from_file(cert_path), + ) def test_certs_implicitly_loaded_alongside_key_filename_keys(self): # NOTE: a regular test_connection() w/ test_rsa.key would incidentally - # test this (because test_rsa.key-cert.pub exists) but incidental tests + # test this (because test_xxx.key-cert.pub exists) but incidental tests # stink, so NullServer and friends were updated to allow assertions # about the server-side key object's public blob. Thus, we can prove # that a specific cert was found, along with regular authorization # succeeding proving that the overall flow works. - key_path = test_path('test_rsa.key') - self._test_connection( - key_filename=key_path, - public_blob=PublicBlob.from_file('{0}-cert.pub'.format(key_path)), - ) + for type_ in ('rsa', 'dss', 'ecdsa', 'ed25519'): + key_path = test_path('test_{}.key'.format(type_)) + self._test_connection( + key_filename=key_path, + public_blob=PublicBlob.from_file( + '{0}-cert.pub'.format(key_path) + ), + ) def test_default_key_locations_trigger_cert_loads_if_found(self): # TODO: what it says on the tin: ~/.ssh/id_rsa tries to load diff --git a/tests/test_dss.key-cert.pub b/tests/test_dss.key-cert.pub new file mode 100644 index 00000000..07fd5578 --- /dev/null +++ b/tests/test_dss.key-cert.pub @@ -0,0 +1 @@ +ssh-dss-cert-v01@openssh.com AAAAHHNzaC1kc3MtY2VydC12MDFAb3BlbnNzaC5jb20AAAAgJA3GjLmg6JbIWxokW/c827lmPOSvSfPDIY586yICFqIAAACBAOeBpgNnfRzr/twmAQRu2XwWAp3CFtrVnug6s6fgwj/oLjYbVtjAy6pl/h0EKCWx2rf1IetyNsTxWrniA9I6HeDj65X1FyDkg6g8tvCnaNB8Xp/UUhuzHuGsMIipRxBxw9LF608EqZcj1E3ytktoW5B5OcjrkEoz3xG7C+rpIjYvAAAAFQDwz4UnmsGiSNu5iqjn3uTzwUpshwAAAIEAkxfFeY8P2wZpDjX0MimZl5wkoFQDL25cPzGBuB4OnB8NoUk/yjAHIIpEShw8V+LzouMK5CTJQo5+Ngw3qIch/WgRmMHy4kBq1SsXMjQCte1So6HBMvBPIW5SiMTmjCfZZiw4AYHK+B/JaOwaG9yRg2Ejg4Ok10+XFDxlqZo8Y+wAAACARmR7CCPjodxASvRbIyzaVpZoJ/Z6x7dAumV+ysrV1BVYd0lYukmnjO1kKBWApqpH1ve9XDQYN8zgxM4b16L21kpoWQnZtXrY3GZ4/it9kUgyB7+NwacIBlXa8cMDL7Q/69o0d54U0X/NeX5QxuYR6OMJlrkQB7oiW/P/1mwjQgEAAAAAAAAAAAAAAAEAAAAJdXNlcl90ZXN0AAAACAAAAAR0ZXN0AAAAAAAAAAD//////////wAAAAAAAACCAAAAFXBlcm1pdC1YMTEtZm9yd2FyZGluZwAAAAAAAAAXcGVybWl0LWFnZW50LWZvcndhcmRpbmcAAAAAAAAAFnBlcm1pdC1wb3J0LWZvcndhcmRpbmcAAAAAAAAACnBlcm1pdC1wdHkAAAAAAAAADnBlcm1pdC11c2VyLXJjAAAAAAAAAAAAAAEXAAAAB3NzaC1yc2EAAAADAQABAAABAQDskr46Umjxh3wo7PoPQsSVS3xt6+5PhwmXrnVtBBnkOo+zHRwQo8G8sY+Lc6oOOzA5GCSawKOwqE305GIDfB8/L9EKOkAjdN18imDjw/YuJFA4bl9yFhsXrCb1GZPJw0pJ0H0Eid9EldyMQAhGE49MWvnFMQl1TgO6YWq/g71xAFimge0LvVWijlbMy7O+nsGxSpinIprV5S9Viv8XC/ku89tadZfca1uxq751aGfAWGeYrVytpUl8UO0ggqH6BaUvkDU7rWh2n5RHUTvgzceKWnz5wqd8BngK37WmJjAgCtHCJS5ZRf6oJGj2QVcqc6cjvEFWsCuOKB4KAjktauWxAAABDwAAAAdzc2gtcnNhAAABAK6jweL231fRhFoybEGTOXJfj0lx55KpDsw9Q1rBvZhrSgwUr2dFr9HVcKe44mTC7CMtdW5VcyB67l1fnMil/D/e4zYxI0PvbW6RxLFNqvvtxBu5sGt5B7uzV4aAV31TpWR0l5RwwpZqc0NUlTx7oMutN1BDrPqW70QZ/iTEwalkn5fo1JWej0cf4BdC9VgYDLnprx0KN3IToukbszRQySnuR6MQUfj0m7lUloJfF3rq8G0kNxWqDGoJilMhO5Lqu9wAhlZWdouypI6bViO6+ToCVixLNUYs3EfS1zCxvXpiyMvh6rZofJ6WqzUuSd4Mzb2Ka4ocTKi7kynF+OG0Ivo= tests/test_dss.key.pub diff --git a/tests/test_ecdsa_256.key-cert.pub b/tests/test_ecdsa_256.key-cert.pub new file mode 100644 index 00000000..f2c93ccf --- /dev/null +++ b/tests/test_ecdsa_256.key-cert.pub @@ -0,0 +1 @@ +ecdsa-sha2-nistp256-cert-v01@openssh.com AAAAKGVjZHNhLXNoYTItbmlzdHAyNTYtY2VydC12MDFAb3BlbnNzaC5jb20AAAAgJ+ZkRXedIWPl9y6fvel60p47ys5WgwMSjiwzJ2Ho+4MAAAAIbmlzdHAyNTYAAABBBJSPZm3ZWkvk/Zx8WP+fZRZ5/NBBHnGQwR6uIC6XHGPDIHuWUzIjAwA0bzqkOUffEsbLe+uQgKl5kbc/L8KA/eoAAAAAAAAAAAAAAAEAAAAJdXNlcl90ZXN0AAAACAAAAAR0ZXN0AAAAAAAAAAD//////////wAAAAAAAACCAAAAFXBlcm1pdC1YMTEtZm9yd2FyZGluZwAAAAAAAAAXcGVybWl0LWFnZW50LWZvcndhcmRpbmcAAAAAAAAAFnBlcm1pdC1wb3J0LWZvcndhcmRpbmcAAAAAAAAACnBlcm1pdC1wdHkAAAAAAAAADnBlcm1pdC11c2VyLXJjAAAAAAAAAAAAAAEXAAAAB3NzaC1yc2EAAAADAQABAAABAQDskr46Umjxh3wo7PoPQsSVS3xt6+5PhwmXrnVtBBnkOo+zHRwQo8G8sY+Lc6oOOzA5GCSawKOwqE305GIDfB8/L9EKOkAjdN18imDjw/YuJFA4bl9yFhsXrCb1GZPJw0pJ0H0Eid9EldyMQAhGE49MWvnFMQl1TgO6YWq/g71xAFimge0LvVWijlbMy7O+nsGxSpinIprV5S9Viv8XC/ku89tadZfca1uxq751aGfAWGeYrVytpUl8UO0ggqH6BaUvkDU7rWh2n5RHUTvgzceKWnz5wqd8BngK37WmJjAgCtHCJS5ZRf6oJGj2QVcqc6cjvEFWsCuOKB4KAjktauWxAAABDwAAAAdzc2gtcnNhAAABALdnEil8XIFkcgLZgYwS2cIQPHetUzMNxYCqzk7mSfVpCaIYNTr27RG+f+sD0cerdAIUUvhCT7iA82/Y7wzwkO2RUBi61ATfw9DDPPRQTDfix1SSRwbmPB/nVI1HlPMCEs6y48PFaBZqXwJPS3qycgSxoTBhaLCLzT+r6HRaibY7kiRLDeL3/WHyasK2PRdcYJ6KrLd0ctQcUHZCLK3fJfMfuQRg8MZLVrmK3fHStCXHpRFueRxUhZjaiS9evA/NtzEQhf46JDClQ2rLYpSqSg7QUR/rKwqWWyMuQkOHmlJw797VVa+ZzpUFXP7ekWel3FaBj8IHiimIA7Jm6dOCLm4= tests/test_ecdsa_256.key.pub diff --git a/tests/test_ed25519.key-cert.pub b/tests/test_ed25519.key-cert.pub new file mode 100644 index 00000000..4e01415a --- /dev/null +++ b/tests/test_ed25519.key-cert.pub @@ -0,0 +1 @@ +ssh-ed25519-cert-v01@openssh.com AAAAIHNzaC1lZDI1NTE5LWNlcnQtdjAxQG9wZW5zc2guY29tAAAAIIjBkc8l1X887CLBHraU+d6/74Hxr9oa+3HC0iioecZ6AAAAIHr1K9komH/1WBIvQbbtvnFVhryd62EfcgRFuLRiokNfAAAAAAAAAAAAAAABAAAACXVzZXJfdGVzdAAAAAgAAAAEdGVzdAAAAAAAAAAA//////////8AAAAAAAAAggAAABVwZXJtaXQtWDExLWZvcndhcmRpbmcAAAAAAAAAF3Blcm1pdC1hZ2VudC1mb3J3YXJkaW5nAAAAAAAAABZwZXJtaXQtcG9ydC1mb3J3YXJkaW5nAAAAAAAAAApwZXJtaXQtcHR5AAAAAAAAAA5wZXJtaXQtdXNlci1yYwAAAAAAAAAAAAABFwAAAAdzc2gtcnNhAAAAAwEAAQAAAQEA7JK+OlJo8Yd8KOz6D0LElUt8bevuT4cJl651bQQZ5DqPsx0cEKPBvLGPi3OqDjswORgkmsCjsKhN9ORiA3wfPy/RCjpAI3TdfIpg48P2LiRQOG5fchYbF6wm9RmTycNKSdB9BInfRJXcjEAIRhOPTFr5xTEJdU4DumFqv4O9cQBYpoHtC71Voo5WzMuzvp7BsUqYpyKa1eUvVYr/Fwv5LvPbWnWX3Gtbsau+dWhnwFhnmK1craVJfFDtIIKh+gWlL5A1O61odp+UR1E74M3Hilp8+cKnfAZ4Ct+1piYwIArRwiUuWUX+qCRo9kFXKnOnI7xBVrArjigeCgI5LWrlsQAAAQ8AAAAHc3NoLXJzYQAAAQCNfYITv/GCW42fLI89x0pKpXIET/xHIBVan5S3fy5SZq9gLG1Db9g/FITDfOVA7OX8mU/91rucHGtuEi3isILdNFrCcoLEml289tyyluUbbFD5fjvBchMWBkYPwrOPfEzSs299Yk8ZgfV1pjWlndfV54s4c9pinkGu8c0Vzc6stEbWkdmoOHE8su3ogUPg/hOygDzJ+ZOgP5HIUJ6YgkgVpWgZm7zofwdZfa2HEb+WhZaKfMK1UCw1UiSBVk9dx6qzF9m243tHwSHraXvb9oJ1wT1S/MypTbP4RT4fHN8koYNrv2szEBN+lkRgk1D7xaxS/Md2TJsau9ho/UCXSR8L tests/test_ed25519.key.pub -- cgit v1.2.3 From e0babd7a2da93501fed8a83da0cfb70ce6a90bbd Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Mon, 28 Aug 2017 20:56:50 -0700 Subject: Implement ECDSA certs. So mad at that frickin typo'd specification... --- paramiko/ecdsakey.py | 27 ++++++++++++++++++++++----- paramiko/pkey.py | 24 ++++++++++++++++++++---- paramiko/transport.py | 3 +++ tests/test_client.py | 3 ++- 4 files changed, 47 insertions(+), 10 deletions(-) (limited to 'tests') diff --git a/paramiko/ecdsakey.py b/paramiko/ecdsakey.py index 9b74d938..fd876298 100644 --- a/paramiko/ecdsakey.py +++ b/paramiko/ecdsakey.py @@ -119,12 +119,29 @@ class ECDSAKey(PKey): c_class = self.signing_key.curve.__class__ self.ecdsa_curve = self._ECDSA_CURVES.get_by_curve_class(c_class) else: - if msg is None: - raise SSHException('Key object may not be empty') + # Must set ecdsa_curve first; subroutines called herein may need to + # spit out our get_name(), which relies on this. + key_type = msg.get_text() + # But this also means we need to hand it a real key/curve + # identifier, so strip out any cert business. (NOTE: could push + # that into _ECDSACurveSet.get_by_key_format_identifier(), but it + # feels more correct to do it here?) + suffix = '-cert-v01@openssh.com' + if key_type.endswith(suffix): + key_type = key_type[:-len(suffix)] self.ecdsa_curve = self._ECDSA_CURVES.get_by_key_format_identifier( - msg.get_text()) - if self.ecdsa_curve is None: - raise SSHException('Invalid key') + key_type + ) + key_types = self._ECDSA_CURVES.get_key_format_identifier_list() + cert_types = [ + '{}-cert-v01@openssh.com'.format(x) + for x in key_types + ] + self._check_type_and_load_cert( + msg=msg, + key_type=key_types, + cert_type=cert_types, + ) curvename = msg.get_text() if curvename != self.ecdsa_curve.nist_name: raise SSHException("Can't handle curve of type %s" % curvename) diff --git a/paramiko/pkey.py b/paramiko/pkey.py index a135bed2..50a99bfa 100644 --- a/paramiko/pkey.py +++ b/paramiko/pkey.py @@ -31,7 +31,7 @@ from cryptography.hazmat.primitives.ciphers import algorithms, modes, Cipher from paramiko import util from paramiko.common import o600 -from paramiko.py3compat import u, encodebytes, decodebytes, b +from paramiko.py3compat import u, encodebytes, decodebytes, b, string_types from paramiko.ssh_exception import SSHException, PasswordRequiredException from paramiko.message import Message @@ -372,19 +372,35 @@ class PKey(object): This includes fast-forwarding cert ``msg`` objects past the nonce, so that the subsequent fields are the key numbers; thus the caller may expect to treat the message as key material afterwards either way. - """ + + The obtained key type is returned for classes which need to know what + it was (e.g. ECDSA.) + """ + # Normalization; most classes have a single key type and give a string, + # but eg ECDSA is a 1:N mapping. + key_types = key_type + cert_types = cert_type + if isinstance(key_type, string_types): + key_types = [key_types] + if isinstance(cert_types, string_types): + cert_types = [cert_types] + # Can't do much with no message, that should've been handled elsewhere if msg is None: raise SSHException('Key object may not be empty') + # First field is always key type, in either kind of object. (make sure + # we rewind before grabbing it - sometimes caller had to do their own + # introspection first!) + msg.rewind() type_ = msg.get_text() nonce = None # Regular public key - nothing special to do besides the implicit # type check. - if type_ == key_type: + if type_ in key_types: pass # OpenSSH-compatible certificate - store full copy as .public_blob # (so signing works correctly) and then fast-forward past the # nonce. - elif type_ == cert_type: + elif type_ in cert_types: # This seems the cleanest way to 'clone' an already-being-read # message; they're *IO objects at heart and their .getvalue() # always returns the full value regardless of pointer position. diff --git a/paramiko/transport.py b/paramiko/transport.py index d97bee80..1a95f990 100644 --- a/paramiko/transport.py +++ b/paramiko/transport.py @@ -208,8 +208,11 @@ class Transport(threading.Thread, ClosingContextManager): 'ssh-dss': DSSKey, 'ssh-dss-cert-v01@openssh.com': DSSKey, 'ecdsa-sha2-nistp256': ECDSAKey, + 'ecdsa-sha2-nistp256-cert-v01@openssh.com': ECDSAKey, 'ecdsa-sha2-nistp384': ECDSAKey, + 'ecdsa-sha2-nistp384-cert-v01@openssh.com': ECDSAKey, 'ecdsa-sha2-nistp521': ECDSAKey, + 'ecdsa-sha2-nistp521-cert-v01@openssh.com': ECDSAKey, 'ssh-ed25519': Ed25519Key, } diff --git a/tests/test_client.py b/tests/test_client.py index 40e2fb12..f0808c4b 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -263,6 +263,7 @@ class SSHClientTest (unittest.TestCase): # NOTE: giving cert path here, not key path. (Key path test is below. # They're similar except for which path is given; the expected auth and # server-side behavior is 100% identical.) + # NOTE: only bothered whipping up one cert per overall class/family. for type_ in ('rsa', 'dss', 'ecdsa_256', 'ed25519'): cert_path = test_path('test_{}.key-cert.pub'.format(type_)) self._test_connection( @@ -277,7 +278,7 @@ class SSHClientTest (unittest.TestCase): # about the server-side key object's public blob. Thus, we can prove # that a specific cert was found, along with regular authorization # succeeding proving that the overall flow works. - for type_ in ('rsa', 'dss', 'ecdsa', 'ed25519'): + for type_ in ('rsa', 'dss', 'ecdsa_256', 'ed25519'): key_path = test_path('test_{}.key'.format(type_)) self._test_connection( key_filename=key_path, -- cgit v1.2.3 From bad045f7dada340d2c707d25923406e32406fc22 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Mon, 28 Aug 2017 21:47:39 -0700 Subject: Python 3 fixes re #1042 --- paramiko/pkey.py | 4 ++-- tests/test_pkey.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/paramiko/pkey.py b/paramiko/pkey.py index 8646b609..67723be2 100644 --- a/paramiko/pkey.py +++ b/paramiko/pkey.py @@ -493,7 +493,7 @@ class PublicBlob(object): msg = "Not enough fields for public blob: {0}" raise ValueError(msg.format(fields)) key_type = fields[0] - key_blob = decodebytes(fields[1]) + key_blob = decodebytes(b(fields[1])) try: comment = fields[2].strip() except IndexError: @@ -501,7 +501,7 @@ class PublicBlob(object): # Verify that the blob message first (string) field matches the # key_type m = Message(key_blob) - blob_type = m.get_string() + blob_type = m.get_text() if blob_type != key_type: msg = "Invalid PublicBlob contents: key type={0!r}, but blob type={1!r}" # noqa raise ValueError(msg.format(key_type, blob_type)) diff --git a/tests/test_pkey.py b/tests/test_pkey.py index dac1d02b..80843222 100644 --- a/tests/test_pkey.py +++ b/tests/test_pkey.py @@ -491,7 +491,7 @@ class KeyTest(unittest.TestCase): self.assertEqual(key.public_blob.comment, 'test_rsa.key.pub') # Delve into blob contents, for test purposes msg = Message(key.public_blob.key_blob) - self.assertEqual(msg.get_string(), 'ssh-rsa-cert-v01@openssh.com') + self.assertEqual(msg.get_text(), 'ssh-rsa-cert-v01@openssh.com') nonce = msg.get_string() e = msg.get_mpint() n = msg.get_mpint() -- cgit v1.2.3 From 5fb4bb2cfc415b287f629a398de5447da18a3fb2 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 2 Sep 2017 12:47:52 -0700 Subject: Python 2.6 fixes Fixes #1049 --- paramiko/ecdsakey.py | 2 +- tests/test_client.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/paramiko/ecdsakey.py b/paramiko/ecdsakey.py index fd876298..5a0164d8 100644 --- a/paramiko/ecdsakey.py +++ b/paramiko/ecdsakey.py @@ -134,7 +134,7 @@ class ECDSAKey(PKey): ) key_types = self._ECDSA_CURVES.get_key_format_identifier_list() cert_types = [ - '{}-cert-v01@openssh.com'.format(x) + '{0}-cert-v01@openssh.com'.format(x) for x in key_types ] self._check_type_and_load_cert( diff --git a/tests/test_client.py b/tests/test_client.py index f0808c4b..7ada13da 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -265,7 +265,7 @@ class SSHClientTest (unittest.TestCase): # server-side behavior is 100% identical.) # NOTE: only bothered whipping up one cert per overall class/family. for type_ in ('rsa', 'dss', 'ecdsa_256', 'ed25519'): - cert_path = test_path('test_{}.key-cert.pub'.format(type_)) + cert_path = test_path('test_{0}.key-cert.pub'.format(type_)) self._test_connection( key_filename=cert_path, public_blob=PublicBlob.from_file(cert_path), @@ -279,7 +279,7 @@ class SSHClientTest (unittest.TestCase): # that a specific cert was found, along with regular authorization # succeeding proving that the overall flow works. for type_ in ('rsa', 'dss', 'ecdsa_256', 'ed25519'): - key_path = test_path('test_{}.key'.format(type_)) + key_path = test_path('test_{0}.key'.format(type_)) self._test_connection( key_filename=key_path, public_blob=PublicBlob.from_file( -- cgit v1.2.3