From dd1a32c428f1f5c9ea15fe97fde9358e695f4afc Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Mon, 16 May 2022 20:26:48 -0400 Subject: Fix OpenSSH<7.8 + RSA-CERT use re: SHA2 Closes #2017 --- paramiko/auth_handler.py | 18 ++++++++++++++++++ sites/www/changelog.rst | 7 +++++++ tests/test_client.py | 42 +++++++++++++++++++++++++++++++++++++----- 3 files changed, 62 insertions(+), 5 deletions(-) diff --git a/paramiko/auth_handler.py b/paramiko/auth_handler.py index e9023673..42a21a78 100644 --- a/paramiko/auth_handler.py +++ b/paramiko/auth_handler.py @@ -22,6 +22,7 @@ import weakref import time +import re from paramiko.common import ( cMSG_SERVICE_REQUEST, @@ -298,6 +299,23 @@ class AuthHandler(object): key_type ), ) + # NOTE re #2017: When the key is an RSA cert and the remote server is + # OpenSSH 7.7 or earlier, always use ssh-rsa-cert-v01@openssh.com. + # Those versions of the server won't support rsa-sha2 family sig algos + # for certs specifically, and in tandem with various server bugs + # regarding server-sig-algs, it's impossible to fit this into the rest + # of the logic here. + if key_type.endswith("-cert-v01@openssh.com") and re.search( + r"-OpenSSH_(?:[1-6]|7\.[0-7])", self.transport.remote_version + ): + pubkey_algo = "ssh-rsa-cert-v01@openssh.com" + self.transport._agreed_pubkey_algorithm = pubkey_algo + self._log(DEBUG, "OpenSSH<7.8 + RSA cert = forcing ssh-rsa!") + self._log( + DEBUG, "Agreed upon {!r} pubkey algorithm".format(pubkey_algo) + ) + return pubkey_algo + # Normal attempts to handshake follow from here. # Only consider RSA algos from our list, lest we agree on another! my_algos = [x for x in self.transport.preferred_pubkeys if "rsa" in x] self._log(DEBUG, "Our pubkey algorithm list: {}".format(my_algos)) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 4d1b71ce..2d4c1fcb 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,13 @@ Changelog ========= +- :bug:`2017` OpenSSH 7.7 and older has a bug preventing it from understanding + how to perform SHA2 signature verification for RSA certificates (specifically + certs - not keys), so when we added SHA2 support it broke all clients using + RSA certificates with these servers. This has been fixed in a manner similar + to what OpenSSH's own client does: a version check is performed and the + algorithm used is downgraded if needed. Reported by Adarsh Chauhan, with fix + suggested by Jun Omae. - :release:`2.9.4 <2022-04-25>` - :support:`1838 backported` (via :issue:`1870`/:issue:`2028`) Update ``camelCase`` method calls against the ``threading`` module to be diff --git a/tests/test_client.py b/tests/test_client.py index f14aac23..21694e28 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -152,7 +152,12 @@ class ClientTest(unittest.TestCase): self.sockl.close() def _run( - self, allowed_keys=None, delay=0, public_blob=None, kill_event=None + self, + allowed_keys=None, + delay=0, + public_blob=None, + kill_event=None, + server_name=None, ): if allowed_keys is None: allowed_keys = FINGERPRINTS.keys() @@ -164,6 +169,8 @@ class ClientTest(unittest.TestCase): self.socks.close() return self.ts = paramiko.Transport(self.socks) + if server_name is not None: + self.ts.local_version = server_name keypath = _support("test_rsa.key") host_key = paramiko.RSAKey.from_private_key_file(keypath) self.ts.add_server_key(host_key) @@ -179,11 +186,11 @@ class ClientTest(unittest.TestCase): """ (Most) kwargs get passed directly into SSHClient.connect(). - The exception is ``allowed_keys`` which is stripped and handed to the - ``NullServer`` used for testing. + The exceptions are ``allowed_keys``/``public_blob``/``server_name`` + which are stripped and handed to the ``NullServer`` used for testing. """ run_kwargs = {"kill_event": self.kill_event} - for key in ("allowed_keys", "public_blob"): + for key in ("allowed_keys", "public_blob", "server_name"): run_kwargs[key] = kwargs.pop(key, None) # Server setup threading.Thread(target=self._run, kwargs=run_kwargs).start() @@ -205,7 +212,9 @@ class ClientTest(unittest.TestCase): self.event.wait(1.0) self.assertTrue(self.event.is_set()) self.assertTrue(self.ts.is_active()) - self.assertEqual("slowdive", self.ts.get_username()) + self.assertEqual( + self.connect_kwargs["username"], self.ts.get_username() + ) self.assertEqual(True, self.ts.is_authenticated()) self.assertEqual(False, self.tc.get_transport().gss_kex_used) @@ -336,6 +345,29 @@ class SSHClientTest(ClientTest): ), ) + def _cert_algo_test(self, ver, alg): + # Issue #2017; see auth_handler.py + self.connect_kwargs["username"] = "somecertuser" # neuter pw auth + self._test_connection( + # NOTE: SSHClient is able to take either the key or the cert & will + # set up its internals as needed + key_filename=_support( + os.path.join("cert_support", "test_rsa.key-cert.pub") + ), + server_name="SSH-2.0-OpenSSH_{}".format(ver), + ) + assert ( + self.tc._transport._agreed_pubkey_algorithm + == "{}-cert-v01@openssh.com".format(alg) + ) + + def test_old_openssh_needs_ssh_rsa_for_certs_not_rsa_sha2(self): + self._cert_algo_test(ver="7.7", alg="ssh-rsa") + + def test_newer_openssh_uses_rsa_sha2_for_certs_not_ssh_rsa(self): + # NOTE: 512 happens to be first in our list and is thus chosen + self._cert_algo_test(ver="7.8", alg="rsa-sha2-512") + 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. Right now no other tests actually test that -- cgit v1.2.1 From d3f0d01c3eeaef74c0111bd5ade1c66fdfaaa8ec Mon Sep 17 00:00:00 2001 From: Jun Omae Date: Thu, 25 Nov 2021 15:21:08 +0900 Subject: Pad received signature with leading zeros when RSA key is used --- paramiko/rsakey.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/paramiko/rsakey.py b/paramiko/rsakey.py index d2dc99e4..1e2d5bfe 100644 --- a/paramiko/rsakey.py +++ b/paramiko/rsakey.py @@ -141,9 +141,16 @@ class RSAKey(PKey): if isinstance(key, rsa.RSAPrivateKey): key = key.public_key() + # NOTE: pad received signature with leading zeros, key.verify() + # expects a signature of key size (e.g. PuTTY doesn't pad) + sign = msg.get_binary() + diff = key.key_size - len(sign) * 8 + if diff > 0: + sign = b"\x00" * ((diff + 7) // 8) + sign + try: key.verify( - msg.get_binary(), + sign, data, padding.PKCS1v15(), self.HASHES[sig_algorithm](), -- cgit v1.2.1 From 77daf90dec14b265beca82df49bf973ffb082e63 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Mon, 16 May 2022 20:51:30 -0400 Subject: Changelog re #1933, closes #1933 --- sites/www/changelog.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 2d4c1fcb..2a85f503 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,11 @@ Changelog ========= +- :bug:`1933` Align signature verification algorithm with OpenSSH re: + zero-padding signatures which don't match their nominal size/length. This + shouldn't affect most users, but will help Paramiko-implemented SSH servers + handle poorly behaved clients such as PuTTY. Thanks to Jun Omae for catch & + patch. - :bug:`2017` OpenSSH 7.7 and older has a bug preventing it from understanding how to perform SHA2 signature verification for RSA certificates (specifically certs - not keys), so when we added SHA2 support it broke all clients using -- cgit v1.2.1