From 961c0e63b9bbc1154149a8d55fe416b2c39ec9c7 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Tue, 3 Dec 2019 19:06:42 -0500 Subject: [PATCH 001/117] Add Github oriented FUNDING.yml w/ Tidelift hook --- .github/FUNDING.yml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .github/FUNDING.yml diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml new file mode 100644 index 000000000..30ce38c88 --- /dev/null +++ b/.github/FUNDING.yml @@ -0,0 +1,3 @@ +# These are supported funding model platforms + +tidelift: "pypi/paramiko" From 3603512fa3871aceb0ed1e2df47d09ca5025bd81 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 10 Jan 2020 16:18:16 -0500 Subject: [PATCH 002/117] Last ever LICENSE copyright update Just so it goes out on a cool looking year :D --- README.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index 72861d181..5fb2f77fa 100644 --- a/README.rst +++ b/README.rst @@ -10,8 +10,8 @@ Paramiko :target: https://codecov.io/gh/paramiko/paramiko :Paramiko: Python SSH module -:Copyright: Copyright (c) 2003-2009 Robey Pointer -:Copyright: Copyright (c) 2013-2019 Jeff Forcier +:Copyright: Copyright (c) 2009 Robey Pointer +:Copyright: Copyright (c) 2020 Jeff Forcier :License: `LGPL `_ :Homepage: http://www.paramiko.org/ :API docs: http://docs.paramiko.org From f645639004ecf2b1de93c8ed237ef6bd19fd3dd9 Mon Sep 17 00:00:00 2001 From: Mark Watts Date: Sun, 17 May 2020 20:46:39 -0500 Subject: [PATCH 003/117] Fixing syntax for the "note" directive --- paramiko/pkey.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/paramiko/pkey.py b/paramiko/pkey.py index 3a07426f1..a54d502d5 100644 --- a/paramiko/pkey.py +++ b/paramiko/pkey.py @@ -652,7 +652,8 @@ class PublicBlob(object): Tries to be as dumb as possible and barely cares about specific per-key-type data. - ..note:: + .. 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." From 14c6c8e786fdaae5b143cb81ccd32d21ce4164fc Mon Sep 17 00:00:00 2001 From: Mark Watts Date: Sun, 17 May 2020 20:46:39 -0500 Subject: [PATCH 004/117] Fixing syntax for the "note" directive --- paramiko/pkey.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/paramiko/pkey.py b/paramiko/pkey.py index e37f7751b..e8e65589d 100644 --- a/paramiko/pkey.py +++ b/paramiko/pkey.py @@ -460,7 +460,8 @@ class PublicBlob(object): Tries to be as dumb as possible and barely cares about specific per-key-type data. - ..note:: + .. 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." From 22f44011dfce54b4c3a36621d36a6d26ee3a4f27 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 12 Feb 2021 12:04:44 -0500 Subject: [PATCH 005/117] Include Cryptography Rust requirement note/links in our install docs --- sites/www/installing.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sites/www/installing.rst b/sites/www/installing.rst index 3631eb0d1..058ea8e80 100644 --- a/sites/www/installing.rst +++ b/sites/www/installing.rst @@ -86,8 +86,14 @@ In general, you'll need one of the following setups: 1.4 and newer - which Paramiko will gladly install or upgrade, if you e.g. ``pip install -U`` - drop that support. +* Similarly, Cryptography 3.4 and above require Rust language tooling to + install from source; once again see Cryptography's documentation for details + here, such as `their Rust install section`_ and `this FAQ entry`_. + .. _installation instructions: .. _Cryptography's install docs: https://cryptography.io/en/latest/installation/ +.. _their Rust install section: https://cryptography.io/en/latest/installation.html#rust +.. _this FAQ entry: https://cryptography.io/en/latest/faq.html#installing-cryptography-fails-with-error-can-not-find-rust-compiler .. _gssapi: From c5dc17ba2b987c7f77ae832d5677c473b05cf4f8 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 12 Feb 2021 12:08:41 -0500 Subject: [PATCH 006/117] Turns out our existing Cryptography install link had rotted too, gah --- sites/www/installing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sites/www/installing.rst b/sites/www/installing.rst index 058ea8e80..1a77bbe40 100644 --- a/sites/www/installing.rst +++ b/sites/www/installing.rst @@ -91,7 +91,7 @@ In general, you'll need one of the following setups: here, such as `their Rust install section`_ and `this FAQ entry`_. .. _installation instructions: -.. _Cryptography's install docs: https://cryptography.io/en/latest/installation/ +.. _Cryptography's install docs: https://cryptography.io/en/latest/installation.html .. _their Rust install section: https://cryptography.io/en/latest/installation.html#rust .. _this FAQ entry: https://cryptography.io/en/latest/faq.html#installing-cryptography-fails-with-error-can-not-find-rust-compiler From 447b0e92555b8d6340c236bbaf96a28eeb665ce1 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 12 Feb 2021 12:43:47 -0500 Subject: [PATCH 007/117] Add a RTD yaml to trigger specific docker build env Re #1813 --- .readthedocs.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .readthedocs.yml diff --git a/.readthedocs.yml b/.readthedocs.yml new file mode 100644 index 000000000..0a214ea75 --- /dev/null +++ b/.readthedocs.yml @@ -0,0 +1,10 @@ +# NOTE: not migrating other settings into this file yet as I don't know how +# well they'd play with our 'dual doc trees' setup. But the below should apply +# to both equally. +version: 2 + +build: + # Specific version of RTD docker image that's new enough to have Rust around + # (so Cryptography can build from source if needed). + # See https://github.com/readthedocs/readthedocs-docker-images + image: "7.0" From 2aa48b6edcc3517aa71ed723eb75e22974f7eafd Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 12 Feb 2021 12:54:41 -0500 Subject: [PATCH 008/117] Temporary hacks to get www/install docs publishing Re #1813 --- .readthedocs.yml | 10 ---------- dev-requirements.txt | 3 +++ 2 files changed, 3 insertions(+), 10 deletions(-) delete mode 100644 .readthedocs.yml diff --git a/.readthedocs.yml b/.readthedocs.yml deleted file mode 100644 index 0a214ea75..000000000 --- a/.readthedocs.yml +++ /dev/null @@ -1,10 +0,0 @@ -# NOTE: not migrating other settings into this file yet as I don't know how -# well they'd play with our 'dual doc trees' setup. But the below should apply -# to both equally. -version: 2 - -build: - # Specific version of RTD docker image that's new enough to have Rust around - # (so Cryptography can build from source if needed). - # See https://github.com/readthedocs/readthedocs-docker-images - image: "7.0" diff --git a/dev-requirements.txt b/dev-requirements.txt index 142153bd0..a23ad1e9d 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -19,3 +19,6 @@ releases>=1.5,<2.0 semantic_version>=2.4,<2.5 wheel==0.24 twine==1.11.0 +# TEMPORARY pin on cryptography until I get RTD sorted +# See #1813 +cryptography<3.4 From 5ad689f70b188e11b88b63c239defcab3feac2a3 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 12 Feb 2021 14:48:36 -0500 Subject: [PATCH 009/117] Flesh out RTD yaml a bit so things build again Blessedly, the web's setting for which sphinx conf to use appears to work if not overridden by the file. --- .readthedocs.yml | 15 ++++++++++++--- dev-requirements.txt | 2 ++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/.readthedocs.yml b/.readthedocs.yml index 0a214ea75..1f6737f48 100644 --- a/.readthedocs.yml +++ b/.readthedocs.yml @@ -1,10 +1,19 @@ -# NOTE: not migrating other settings into this file yet as I don't know how -# well they'd play with our 'dual doc trees' setup. But the below should apply -# to both equally. version: 2 +formats: all + build: # Specific version of RTD docker image that's new enough to have Rust around # (so Cryptography can build from source if needed). # See https://github.com/readthedocs/readthedocs-docker-images image: "7.0" + +python: + version: 3.7 + install: + - requirements: dev-requirements.txt + system_packages: false + +# NOTE: leaving sphinx.* alone as there's currently no way to differ the conf +# file used based on project (so cannot differentiate between www and docs). +# The web UI setting for that SHOULD take precedence? diff --git a/dev-requirements.txt b/dev-requirements.txt index 9a7be339a..6e1e0e76e 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -20,3 +20,5 @@ releases>=1.5,<2.0 semantic_version>=2.4,<2.5 wheel==0.24 twine==1.11.0 +# Self +-e . From 33daa8e541cb91cdee01a085c368e1b388472e6a Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 12 Feb 2021 15:08:03 -0500 Subject: [PATCH 010/117] Git is still hard I guess --- .readthedocs.yml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 .readthedocs.yml diff --git a/.readthedocs.yml b/.readthedocs.yml new file mode 100644 index 000000000..1f6737f48 --- /dev/null +++ b/.readthedocs.yml @@ -0,0 +1,19 @@ +version: 2 + +formats: all + +build: + # Specific version of RTD docker image that's new enough to have Rust around + # (so Cryptography can build from source if needed). + # See https://github.com/readthedocs/readthedocs-docker-images + image: "7.0" + +python: + version: 3.7 + install: + - requirements: dev-requirements.txt + system_packages: false + +# NOTE: leaving sphinx.* alone as there's currently no way to differ the conf +# file used based on project (so cannot differentiate between www and docs). +# The web UI setting for that SHOULD take precedence? From d4f5d725f75683879aadd08b3cd335db629eea2b Mon Sep 17 00:00:00 2001 From: h3ll0r Date: Mon, 26 Apr 2021 16:02:38 +0200 Subject: [PATCH 011/117] Make prefetching optional in get() and getfo() In some cases prefetching can lead to problems when downloading larger files. The connection to the SFTP server is disconnected due to the query behavior. Proprietary implementations in particular tend to behave that way. Switching off prefetching is a slow but resilient solution to this problem. --- paramiko/sftp_client.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/paramiko/sftp_client.py b/paramiko/sftp_client.py index 93190d85e..88e6eee0e 100644 --- a/paramiko/sftp_client.py +++ b/paramiko/sftp_client.py @@ -758,7 +758,7 @@ def put(self, localpath, remotepath, callback=None, confirm=True): with open(localpath, "rb") as fl: return self.putfo(fl, remotepath, file_size, callback, confirm) - def getfo(self, remotepath, fl, callback=None): + def getfo(self, remotepath, fl, callback=None, prefetch=True): """ Copy a remote file (``remotepath``) from the SFTP server and write to an open file or file-like object, ``fl``. Any exception raised by @@ -771,18 +771,21 @@ def getfo(self, remotepath, fl, callback=None): :param callable callback: optional callback function (form: ``func(int, int)``) that accepts the bytes transferred so far and the total bytes to be transferred + :param bool prefetch: + option to deactivate prefetching :return: the `number ` of bytes written to the opened file object .. versionadded:: 1.10 """ file_size = self.stat(remotepath).st_size with self.open(remotepath, "rb") as fr: - fr.prefetch(file_size) + if prefetch: + fr.prefetch(file_size) return self._transfer_with_callback( reader=fr, writer=fl, file_size=file_size, callback=callback ) - def get(self, remotepath, localpath, callback=None): + def get(self, remotepath, localpath, callback=None, prefetch=True): """ Copy a remote file (``remotepath``) from the SFTP server to the local host as ``localpath``. Any exception raised by operations will be @@ -793,13 +796,15 @@ def get(self, remotepath, localpath, callback=None): :param callable callback: optional callback function (form: ``func(int, int)``) that accepts the bytes transferred so far and the total bytes to be transferred + :param bool prefetch: + option to deactivate prefetching .. versionadded:: 1.4 .. versionchanged:: 1.7.4 Added the ``callback`` param """ with open(localpath, "wb") as fl: - size = self.getfo(remotepath, fl, callback) + size = self.getfo(remotepath, fl, callback, prefetch) s = os.stat(localpath) if s.st_size != size: raise IOError( From 70696fe80e0f9131fbf5c1f529c60cbff9600151 Mon Sep 17 00:00:00 2001 From: Tim Stolarski Date: Wed, 28 Apr 2021 13:29:38 +0200 Subject: [PATCH 012/117] Add Test fpr get() without using prefetch --- tests/test_sftp.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_sftp.py b/tests/test_sftp.py index e4e18e5a5..31a0eb2de 100644 --- a/tests/test_sftp.py +++ b/tests/test_sftp.py @@ -554,6 +554,32 @@ def progress_callback(x, y): os.unlink(localname) sftp.unlink(sftp.FOLDER + "/bunny.txt") + def test_get_without_prefetch(self, sftp): + """ + Create a 4MB file. Verify that pull works without prefetching + using a lager file. + """ + + fd, localname = mkstemp() + os.close(fd) + + with open(localname, 'wb') as f: + num_chars = 1024 * 1024 * 4 + f.write(b'0' * num_chars) + + sftp.put(localname, sftp.FOLDER + "/dummy_file") + + os.unlink(localname) + fd, localname = mkstemp() + os.close(fd) + + sftp.get(sftp.FOLDER + "/dummy_file", localname, prefetch=False) + + assert os.stat(localname).st_size == 4194304 + + os.unlink(localname) + sftp.unlink(sftp.FOLDER + "/dummy_file") + def test_check(self, sftp): """ verify that file.check() works against our own server. From f914e1288eb0f62537919cef6362a876e9c648b4 Mon Sep 17 00:00:00 2001 From: Tim Stolarski Date: Fri, 30 Apr 2021 09:08:14 +0200 Subject: [PATCH 013/117] Fixup readability: Add Test for get() without using prefetch --- tests/test_sftp.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/test_sftp.py b/tests/test_sftp.py index 31a0eb2de..3a5224c93 100644 --- a/tests/test_sftp.py +++ b/tests/test_sftp.py @@ -560,25 +560,27 @@ def test_get_without_prefetch(self, sftp): using a lager file. """ + sftp_filename = sftp.FOLDER + "/dummy_file" + num_chars = 1024 * 1024 * 4 + fd, localname = mkstemp() os.close(fd) with open(localname, 'wb') as f: - num_chars = 1024 * 1024 * 4 f.write(b'0' * num_chars) - sftp.put(localname, sftp.FOLDER + "/dummy_file") + sftp.put(localname, sftp_filename) os.unlink(localname) fd, localname = mkstemp() os.close(fd) - sftp.get(sftp.FOLDER + "/dummy_file", localname, prefetch=False) + sftp.get(sftp_filename, localname, prefetch=False) - assert os.stat(localname).st_size == 4194304 + assert os.stat(localname).st_size == num_chars os.unlink(localname) - sftp.unlink(sftp.FOLDER + "/dummy_file") + sftp.unlink(sftp_filename) def test_check(self, sftp): """ From ce2f45962493a36f84349c488f6162304331aa8b Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Thu, 27 May 2021 18:30:17 -0400 Subject: [PATCH 014/117] Contact methods update Relevant: http://bitprophet.org/blog/2021/05/27/goodbye-freenode/ --- sites/www/contact.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sites/www/contact.rst b/sites/www/contact.rst index dafc1bd4c..202815f8d 100644 --- a/sites/www/contact.rst +++ b/sites/www/contact.rst @@ -5,6 +5,8 @@ Contact You can get in touch with the developer & user community in any of the following ways: -* IRC: ``#paramiko`` on Freenode -* This website - a blog section is forthcoming. * Submit contributions on Github - see the :doc:`contributing` page. +* Follow ``@bitprophet`` on Twitter, though it's not a dedicated account and + mostly just retweets funny pictures. +* Subscribe to the ``paramiko`` category on the developer's blog: + http://bitprophet.org/categories/paramiko/ From 773034515ce7e0b56bd18e32b4ac9ff25aa3e4c6 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 6 Jun 2021 14:53:47 -0400 Subject: [PATCH 015/117] Changelog and docstring tweaks re #1846 --- paramiko/sftp_client.py | 8 ++++++-- sites/www/changelog.rst | 3 +++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/paramiko/sftp_client.py b/paramiko/sftp_client.py index 88e6eee0e..6294fb482 100644 --- a/paramiko/sftp_client.py +++ b/paramiko/sftp_client.py @@ -772,10 +772,12 @@ def getfo(self, remotepath, fl, callback=None, prefetch=True): optional callback function (form: ``func(int, int)``) that accepts the bytes transferred so far and the total bytes to be transferred :param bool prefetch: - option to deactivate prefetching + controls whether prefetching is performed (default: True) :return: the `number ` of bytes written to the opened file object .. versionadded:: 1.10 + .. versionchanged:: 2.8 + Added the ``prefetch`` keyword argument. """ file_size = self.stat(remotepath).st_size with self.open(remotepath, "rb") as fr: @@ -797,11 +799,13 @@ def get(self, remotepath, localpath, callback=None, prefetch=True): optional callback function (form: ``func(int, int)``) that accepts the bytes transferred so far and the total bytes to be transferred :param bool prefetch: - option to deactivate prefetching + controls whether prefetching is performed (default: True) .. versionadded:: 1.4 .. versionchanged:: 1.7.4 Added the ``callback`` param + .. versionchanged:: 2.8 + Added the ``prefetch`` keyword argument. """ with open(localpath, "wb") as fl: size = self.getfo(remotepath, fl, callback, prefetch) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index f3ff0749f..7f70c2323 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,9 @@ Changelog ========= +- :feature:`1846` Add a ``prefetch`` keyword argument to `SFTPClient.get `/`SFTPClient.getfo ` + so users who need to skip SFTP prefetching are able to conditionally turn it + off. Thanks to Github user ``@h3ll0r`` for the PR. - :release:`2.7.2 <2020-08-30>` - :support:`- backported` Update our CI to catch issues with sdist generation, installation and testing. From d2d3d77395d1079a2e335080b7f9f79d3f70ad2a Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 6 Jun 2021 14:55:20 -0400 Subject: [PATCH 016/117] Blacken --- tests/test_sftp.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_sftp.py b/tests/test_sftp.py index 3a5224c93..a98a46c66 100644 --- a/tests/test_sftp.py +++ b/tests/test_sftp.py @@ -566,8 +566,8 @@ def test_get_without_prefetch(self, sftp): fd, localname = mkstemp() os.close(fd) - with open(localname, 'wb') as f: - f.write(b'0' * num_chars) + with open(localname, "wb") as f: + f.write(b"0" * num_chars) sftp.put(localname, sftp_filename) From 5df405f34db85c054b080ced8cdc9342ad321ff4 Mon Sep 17 00:00:00 2001 From: Benno Rice Date: Wed, 21 Jul 2021 10:19:48 +1000 Subject: [PATCH 017/117] Don't force all algorithms to use sha1 if they don't want to. Fixes #1882 --- paramiko/kex_group1.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/paramiko/kex_group1.py b/paramiko/kex_group1.py index 5131e895b..dce3fd91a 100644 --- a/paramiko/kex_group1.py +++ b/paramiko/kex_group1.py @@ -140,7 +140,7 @@ def _parse_kexdh_init(self, m): hm.add_mpint(self.e) hm.add_mpint(self.f) hm.add_mpint(K) - H = sha1(hm.asbytes()).digest() + H = self.hash_algo(hm.asbytes()).digest() self.transport._set_K_H(K, H) # sign it sig = self.transport.get_server_key().sign_ssh_data(H) From ad66ecad6b67d3869629b5ea87713fe4120ad14f Mon Sep 17 00:00:00 2001 From: Benno Rice Date: Wed, 21 Jul 2021 11:31:58 +1000 Subject: [PATCH 018/117] Add some tests for server-mode Diffie-Hellman key exchange. Tests existed for diffie-hellman-group1-sha1 in both client and server modes but tests for diffie-hellman-group14-sha256 and diffie-hellman-group16-sha512 were missing in server mode. --- tests/test_kex.py | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/test_kex.py b/tests/test_kex.py index 0244ae84d..246e00203 100644 --- a/tests/test_kex.py +++ b/tests/test_kex.py @@ -548,6 +548,27 @@ def test_kex_group14_sha256_client(self): self.assertEqual((b"fake-host-key", b"fake-sig"), transport._verify) self.assertTrue(transport._activated) + def test_kex_group14_sha256_server(self): + transport = FakeTransport() + transport.server_mode = True + kex = KexGroup14SHA256(transport) + kex.start_kex() + self.assertEqual( + (paramiko.kex_group1._MSG_KEXDH_INIT,), transport._expect + ) + + msg = Message() + msg.add_mpint(69) + msg.rewind() + kex.parse_next(paramiko.kex_group1._MSG_KEXDH_INIT, msg) + K = 21526936926159575624241589599003964979640840086252478029709904308461709651400109485351462666820496096345766733042945918306284902585618061272525323382142547359684512114160415969631877620660064043178086464811345023251493620331559440565662862858765724251890489795332144543057725932216208403143759943169004775947331771556537814494448612329251887435553890674764339328444948425882382475260315505741818518926349729970262019325118040559191290279100613049085709127598666890434114956464502529053036826173452792849566280474995114751780998069614898221773345705289637708545219204637224261997310181473787577166103031529148842107599 # noqa + H = b"15080A19894D489ACD0DA724480E1B08E71293E07EBC25FAD10F263C00B343DC" + x = b"1F0000000866616B652D6B657900000101009850B3A8DE3ECCD3F19644139137C93D9C11BC28ED8BE850908EE294E1D43B88B9295311EFAEF5B736A1B652EBE184CCF36CFB0681C1ED66430088FA448B83619F928E7B9592ED6160EC11D639D51C303603F930F743C646B1B67DA38A1D44598DCE6C3F3019422B898044141420E9A10C29B9C58668F7F20A40F154B2C4768FCF7A9AA7179FB6366A7167EE26DD58963E8B880A0572F641DE0A73DC74C930F7C3A0C9388553F3F8403E40CF8B95FEDB1D366596FCF3FDDEB21A0005ADA650EF1733628D807BE5ACB83925462765D9076570056E39994FB328E3108FE406275758D6BF5F32790EF15D8416BF5548164859E785DB45E7787BB0E727ADE08641ED0000000866616B652D736967" # noqa + self.assertEqual(K, transport._K) + self.assertEqual(H, hexlify(transport._H).upper()) + self.assertEqual(x, hexlify(transport._message.asbytes()).upper()) + self.assertTrue(transport._activated) + def test_kex_group16_sha512_client(self): transport = FakeTransport() transport.server_mode = False @@ -573,6 +594,27 @@ def test_kex_group16_sha512_client(self): self.assertEqual((b"fake-host-key", b"fake-sig"), transport._verify) self.assertTrue(transport._activated) + def test_kex_group16_sha512_server(self): + transport = FakeTransport() + transport.server_mode = True + kex = KexGroup16SHA512(transport) + kex.start_kex() + self.assertEqual( + (paramiko.kex_group1._MSG_KEXDH_INIT,), transport._expect + ) + + msg = Message() + msg.add_mpint(69) + msg.rewind() + kex.parse_next(paramiko.kex_group1._MSG_KEXDH_INIT, msg) + K = 933242830095376162107925500057692534838883186615567574891154103836907630698358649443101764908667358576734565553213003142941996368306996312915844839972197961603283544950658467545799914435739152351344917376359963584614213874232577733869049670230112638724993540996854599166318001059065780674008011575015459772051180901213815080343343801745386220342919837913506966863570473712948197760657442974564354432738520446202131551650771882909329069340612274196233658123593466135642819578182367229641847749149740891990379052266213711500434128970973602206842980669193719602075489724202241641553472106310932258574377789863734311328542715212248147206865762697424822447603031087553480483833829498375309975229907460562402877655519980113688369262871485777790149373908739910846630414678346163764464587129010141922982925829457954376352735653834300282864445132624993186496129911208133529828461690634463092007726349795944930302881758403402084584307180896465875803621285362317770276493727205689466142632599776710824902573926951951209239626732358074877997756011804454926541386215567756538832824717436605031489511654178384081883801272314328403020205577714999460724519735573055540814037716770051316113795603990199374791348798218428912977728347485489266146775472 # noqa + H = b"F97BB05A572A663688CA7EA1AA812D3C82EE6C8FA9D4B1D69435783D931157F199909EA38B003E4E4385C8861183CBFF0CF0EF1433A8B3C69AB4DD9420FCC85F" + x = b"1F0000000866616B652D6B65790000020100859FF55A23E0F66463561DD8BFC4764C69C05F85665B06EC9E29EF5003A53A8FA890B6A6EB624DEB55A4FB279DE7010A53580A126817E3D235B05A1081662B1500961D0625F0AAD287F1B597CBA9DB9550D9CC26355C4C59F92E613B5C21AC191F152C09A5DB46DCBA5EA58E3CA6A8B0EB7183E27FAC10106022E8521FA91240FB389060F1E1E4A355049D29DCC82921CE6588791743E4B1DEEE0166F7CC5180C3C75F3773342DF95C8C10AAA5D12975257027936B99B3DED6E6E98CF27EADEAEAE04E7F0A28071F578646B985FCE28A59CEB36287CB65759BE0544D4C4018CDF03C9078FE9CA79ECA611CB6966899E6FD29BE0781491C659FE2380E0D99D50D9CFAAB94E61BE311779719C4C43C6D223AD3799C3915A9E55076A21152DBBF911D6594296D6ECDC1B6FA71997CD29DF987B80FCA7F36BB7F19863C72BBBF839746AFBF9A5B407D468C976AA3E36FA118D3EAAD2E08BF6AE219F81F2CE2BE946337F06CC09BBFABE938A4087E413921CBEC1965ED905999B83396ECA226110CDF6EFB80F815F6489AF87561DA3857F13A7705921306D94176231FBB336B17C3724BC17A28BECB910093AB040873D5D760E8C182B88ECCE3E38DDA68CE35BD152DF7550BD908791FCCEDD1FFDF5ED2A57FFAE79599E487A7726D8A3D950B1729A08FBB60EE462A6BBE8BF0F5F0E1358129A37840FE5B3EEB8BF26E99FA222EAE830000000866616B652D736967" # noqa + self.assertEqual(K, transport._K) + self.assertEqual(H, hexlify(transport._H).upper()) + self.assertEqual(x, hexlify(transport._message.asbytes()).upper()) + self.assertTrue(transport._activated) + @pytest.mark.skipif("not KexCurve25519.is_available()") def test_kex_c25519_client(self): K = 71294722834835117201316639182051104803802881348227506835068888449366462300724 # noqa From 291f1e668bb0d749a95d0359cfbe00fd5b1e557c Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 23 Jul 2021 13:57:22 -0400 Subject: [PATCH 019/117] Changelog closes #1882, closes #1883 --- sites/www/changelog.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index b83a83dfb..d06d6ffbd 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,11 @@ Changelog ========= +- :bug:`1882` Newer server-side key exchange algorithms not intended to use + SHA1 (``diffie-hellman-group14-sha256``, ``diffie-hellman-group16-sha512``) + were incorrectly using SHA1 after all, due to a bug causing them to ignore + the ``hash_algo`` class attribute. This has been corrected. Big thanks to + Benno Rice for catch & patch. - :release:`2.5.1 <2019-06-23>` - :release:`2.4.3 <2019-06-23>` - :bug:`1306` (via :issue:`1400`) Fix Ed25519 key handling so certain key From e9c80e9a63c467f01be5a67f10b55f12f6792c7f Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 23 Jul 2021 14:24:26 -0400 Subject: [PATCH 020/117] noqa long line --- tests/test_kex.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_kex.py b/tests/test_kex.py index 246e00203..c251611ac 100644 --- a/tests/test_kex.py +++ b/tests/test_kex.py @@ -608,7 +608,7 @@ def test_kex_group16_sha512_server(self): msg.rewind() kex.parse_next(paramiko.kex_group1._MSG_KEXDH_INIT, msg) K = 933242830095376162107925500057692534838883186615567574891154103836907630698358649443101764908667358576734565553213003142941996368306996312915844839972197961603283544950658467545799914435739152351344917376359963584614213874232577733869049670230112638724993540996854599166318001059065780674008011575015459772051180901213815080343343801745386220342919837913506966863570473712948197760657442974564354432738520446202131551650771882909329069340612274196233658123593466135642819578182367229641847749149740891990379052266213711500434128970973602206842980669193719602075489724202241641553472106310932258574377789863734311328542715212248147206865762697424822447603031087553480483833829498375309975229907460562402877655519980113688369262871485777790149373908739910846630414678346163764464587129010141922982925829457954376352735653834300282864445132624993186496129911208133529828461690634463092007726349795944930302881758403402084584307180896465875803621285362317770276493727205689466142632599776710824902573926951951209239626732358074877997756011804454926541386215567756538832824717436605031489511654178384081883801272314328403020205577714999460724519735573055540814037716770051316113795603990199374791348798218428912977728347485489266146775472 # noqa - H = b"F97BB05A572A663688CA7EA1AA812D3C82EE6C8FA9D4B1D69435783D931157F199909EA38B003E4E4385C8861183CBFF0CF0EF1433A8B3C69AB4DD9420FCC85F" + H = b"F97BB05A572A663688CA7EA1AA812D3C82EE6C8FA9D4B1D69435783D931157F199909EA38B003E4E4385C8861183CBFF0CF0EF1433A8B3C69AB4DD9420FCC85F" # noqa x = b"1F0000000866616B652D6B65790000020100859FF55A23E0F66463561DD8BFC4764C69C05F85665B06EC9E29EF5003A53A8FA890B6A6EB624DEB55A4FB279DE7010A53580A126817E3D235B05A1081662B1500961D0625F0AAD287F1B597CBA9DB9550D9CC26355C4C59F92E613B5C21AC191F152C09A5DB46DCBA5EA58E3CA6A8B0EB7183E27FAC10106022E8521FA91240FB389060F1E1E4A355049D29DCC82921CE6588791743E4B1DEEE0166F7CC5180C3C75F3773342DF95C8C10AAA5D12975257027936B99B3DED6E6E98CF27EADEAEAE04E7F0A28071F578646B985FCE28A59CEB36287CB65759BE0544D4C4018CDF03C9078FE9CA79ECA611CB6966899E6FD29BE0781491C659FE2380E0D99D50D9CFAAB94E61BE311779719C4C43C6D223AD3799C3915A9E55076A21152DBBF911D6594296D6ECDC1B6FA71997CD29DF987B80FCA7F36BB7F19863C72BBBF839746AFBF9A5B407D468C976AA3E36FA118D3EAAD2E08BF6AE219F81F2CE2BE946337F06CC09BBFABE938A4087E413921CBEC1965ED905999B83396ECA226110CDF6EFB80F815F6489AF87561DA3857F13A7705921306D94176231FBB336B17C3724BC17A28BECB910093AB040873D5D760E8C182B88ECCE3E38DDA68CE35BD152DF7550BD908791FCCEDD1FFDF5ED2A57FFAE79599E487A7726D8A3D950B1729A08FBB60EE462A6BBE8BF0F5F0E1358129A37840FE5B3EEB8BF26E99FA222EAE830000000866616B652D736967" # noqa self.assertEqual(K, transport._K) self.assertEqual(H, hexlify(transport._H).upper()) From 45cfb6dd7f463b26d1c13828690c809ba6a189c2 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 24 Sep 2021 11:03:44 -0400 Subject: [PATCH 021/117] Tweak changelog re: correct credit for bugfix --- sites/www/changelog.rst | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index d06d6ffbd..dea7f2058 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,11 +2,12 @@ Changelog ========= -- :bug:`1882` Newer server-side key exchange algorithms not intended to use - SHA1 (``diffie-hellman-group14-sha256``, ``diffie-hellman-group16-sha512``) - were incorrectly using SHA1 after all, due to a bug causing them to ignore - the ``hash_algo`` class attribute. This has been corrected. Big thanks to - Benno Rice for catch & patch. +- :bug:`1462` (via :issue:`1882`) Newer server-side key exchange algorithms not + intended to use SHA1 (``diffie-hellman-group14-sha256``, + ``diffie-hellman-group16-sha512``) were incorrectly using SHA1 after all, due + to a bug causing them to ignore the ``hash_algo`` class attribute. This has + been corrected. Big thanks to ``@miverson`` for the report and to Benno Rice + for the patch. - :release:`2.5.1 <2019-06-23>` - :release:`2.4.3 <2019-06-23>` - :bug:`1306` (via :issue:`1400`) Fix Ed25519 key handling so certain key From 13f28d65894e7737b01999a7475dccc71078c734 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 24 Sep 2021 16:46:52 -0400 Subject: [PATCH 022/117] Link to new roadmap on BPO --- sites/www/index.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sites/www/index.rst b/sites/www/index.rst index 26961f245..656031aa1 100644 --- a/sites/www/index.rst +++ b/sites/www/index.rst @@ -10,7 +10,9 @@ SSH networking concepts. This website covers project information for Paramiko such as the changelog, contribution guidelines, development roadmap, news/blog, and so forth. Detailed usage and API documentation can be found at our code documentation site, -`docs.paramiko.org `_. +`docs.paramiko.org `_. Additionally, the project +maintainer keeps a `roadmap `_ on his +website. Please see the sidebar to the left to begin. From e9c2005750a0d95a7cc9a100e1edeb3bfc2a984a Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 1 Oct 2021 15:09:30 -0400 Subject: [PATCH 023/117] Modern invoke/invocations Including nuking pointless old pins of wheel/twine, new invocations sets those for release task stuff itself --- dev-requirements.txt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 56f53ad9c..f107c53cc 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,6 +1,6 @@ # Invocations for common project tasks -invoke>=1.0,<2.0 -invocations>=1.2.0,<2.0 +invoke==1.6.0 +invocations==2.3.0 pytest==4.4.2 pytest-relaxed==1.1.5 # pytest-xdist for test dir watching and the inv guard task @@ -17,7 +17,5 @@ alabaster==0.7.12 releases>=1.5,<2.0 # Release tools semantic_version>=2.4,<2.5 -wheel==0.24 -twine==1.11.0 # Self -e . From a5157531f3811f3ff48f15355bbc29b23a0b607b Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 1 Oct 2021 15:09:43 -0400 Subject: [PATCH 024/117] Use README for long description like other projects --- setup.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/setup.py b/setup.py index d439fd032..93c08d153 100644 --- a/setup.py +++ b/setup.py @@ -24,16 +24,15 @@ setup_helper.install_custom_make_tarball() -longdesc = """ -This is a library for making SSH2 connections (client or server). -Emphasis is on using SSH2 as an alternative to SSL for making secure -connections between python scripts. All major ciphers and hash methods -are supported. SFTP client and server mode are both supported too. - -To install the development version, ``pip install -e -git+https://github.com/paramiko/paramiko/#egg=paramiko``. -""" +text = open("README.rst").read() +long_description = """ +To find out what's new in this version of Paramiko, please see `the changelog +`_. +{} +""".format( + version, text +) # Version info -- read without importing _locals = {} @@ -61,7 +60,7 @@ name="paramiko", version=version, description="SSH2 protocol library", - long_description=longdesc, + long_description=long_description, author="Jeff Forcier", author_email="jeff@bitprophet.org", url="https://github.com/paramiko/paramiko/", From 6359fb16d4d31a0ae1cdca865dd428334023a363 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 1 Oct 2021 15:09:52 -0400 Subject: [PATCH 025/117] Changelog re: administrivia overhaul --- sites/www/changelog.rst | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 5d5c2953a..8d446990d 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,18 @@ Changelog ========= +- :support:`-` Administrivia overhaul, including but not limited to: + + - Migrate CI to CircleCI + - Primary dev branch is now ``main`` (renamed) + - Many README edits for clarity, modernization etc; including a bunch more + (and consistent) status badges + - PyPI page much more fleshed out (long_description is now filled in with the + README; sidebar links expanded; etc) + - flake8, pytest configs split out of setup.cfg into their own files + - Invoke/invocations (used by maintainers/contributors) upgraded to modern + versions + - :bug:`1462` (via :issue:`1882`) Newer server-side key exchange algorithms not intended to use SHA1 (``diffie-hellman-group14-sha256``, ``diffie-hellman-group16-sha512``) were incorrectly using SHA1 after all, due From 985c5d0c811987a9d4d9fff712169e5125acc95a Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 1 Oct 2021 15:12:46 -0400 Subject: [PATCH 026/117] README / www-index overhaul - Nuke old README in its entirety: - We don't want people using demos if possible, stop advertising them - Nix a bunch of bits that will now be replaced by standard PyPI sidebar links (docs, tests etc) - Ditto the 'headers', most of that stuff is either PyPI metadata or the about to be added shields.io badges - Some stuff deleted was also just old, eg IRC is no longer a support channel and I think librelist is dead too - Rip out www site index for use as README and inject back in via include - Rewrite a bunch of that too for clarity - Leverage it in long_description too --- README.rst | 174 +++++++--------------------------------- setup.py | 10 +-- sites/www/changelog.rst | 2 +- sites/www/index.rst | 25 +----- 4 files changed, 32 insertions(+), 179 deletions(-) diff --git a/README.rst b/README.rst index 5fb2f77fa..804d8fdb5 100644 --- a/README.rst +++ b/README.rst @@ -1,145 +1,29 @@ -======== -Paramiko -======== - -.. Continuous integration and code coverage badges - -.. image:: https://travis-ci.org/paramiko/paramiko.svg?branch=master - :target: https://travis-ci.org/paramiko/paramiko -.. image:: https://codecov.io/gh/paramiko/paramiko/branch/master/graph/badge.svg - :target: https://codecov.io/gh/paramiko/paramiko - -:Paramiko: Python SSH module -:Copyright: Copyright (c) 2009 Robey Pointer -:Copyright: Copyright (c) 2020 Jeff Forcier -:License: `LGPL `_ -:Homepage: http://www.paramiko.org/ -:API docs: http://docs.paramiko.org -:Development: https://github.com/paramiko/paramiko - - -What ----- - -"Paramiko" is a combination of the Esperanto words for "paranoid" and -"friend". It's a module for Python 2.7/3.4+ that implements the SSH2 protocol -for secure (encrypted and authenticated) connections to remote machines. Unlike -SSL (aka TLS), SSH2 protocol does not require hierarchical certificates signed -by a powerful central authority. You may know SSH2 as the protocol that -replaced Telnet and rsh for secure access to remote shells, but the protocol -also includes the ability to open arbitrary channels to remote services across -the encrypted tunnel (this is how SFTP works, for example). - -It is written entirely in Python (though it depends on third-party C wrappers -for low level crypto; these are often available precompiled) and is released -under the GNU Lesser General Public License (`LGPL -`_). - -The package and its API is fairly well documented in the ``docs`` folder that -should have come with this repository. - - -Installation ------------- - -For most users, the recommended method to install is via pip:: - - pip install paramiko - -For more detailed instructions, see the `Installing -`_ page on the main Paramiko website. - - -Portability Issues ------------------- - -Paramiko primarily supports POSIX platforms with standard OpenSSH -implementations, and is most frequently tested on Linux and OS X. Windows is -supported as well, though it may not be as straightforward. - -Bugs & Support --------------- - -:Bug Reports: `Github `_ -:Mailing List: ``paramiko@librelist.com`` (see the `LibreList website - `_ for usage details). -:IRC: ``#paramiko`` on Freenode - - -Kerberos Support ----------------- - -Paramiko ships with optional Kerberos/GSSAPI support; for info on the extra -dependencies for this, see the `GSS-API section -`_ -on the main Paramiko website. - - -Demo ----- - -Several demo scripts come with Paramiko to demonstrate how to use it. -Probably the simplest demo is this:: - - import base64 - import paramiko - key = paramiko.RSAKey(data=base64.b64decode(b'AAA...')) - client = paramiko.SSHClient() - client.get_host_keys().add('ssh.example.com', 'ssh-rsa', key) - client.connect('ssh.example.com', username='strongbad', password='thecheat') - stdin, stdout, stderr = client.exec_command('ls') - for line in stdout: - print('... ' + line.strip('\n')) - client.close() - -This prints out the results of executing ``ls`` on a remote server. The host -key ``b'AAA...'`` should of course be replaced by the actual base64 encoding of the -host key. If you skip host key verification, the connection is not secure! - -The following example scripts (in demos/) get progressively more detailed: - -:demo_simple.py: - Calls invoke_shell() and emulates a terminal/TTY through which you can - execute commands interactively on a remote server. Think of it as a - poor man's SSH command-line client. - -:demo.py: - Same as demo_simple.py, but allows you to authenticate using a private - key, attempts to use an SSH agent if present, and uses the long form of - some of the API calls. - -:forward.py: - Command-line script to set up port-forwarding across an SSH transport. - -:demo_sftp.py: - Opens an SFTP session and does a few simple file operations. - -:demo_server.py: - An SSH server that listens on port 2200 and accepts a login for - 'robey' (password 'foo'), and pretends to be a BBS. Meant to be a - very simple demo of writing an SSH server. - -:demo_keygen.py: - A key generator similar to OpenSSH ``ssh-keygen(1)`` program with - Paramiko keys generation and progress functions. - -Use ---- - -The demo scripts are probably the best example of how to use this package. -Also a lot of documentation is generated by Sphinx autodoc, in the -doc/ folder. - -There are also unit tests here:: - - $ pip install -r dev-requirements.txt - $ pytest - -Which will verify that most of the core components are working correctly. - -To test Kerberos/GSSAPI, you need a Kerberos environment. On UNIX you can -use the package k5test to setup a Kerberos environment on the fly:: - - $ pip install -r dev-requirements.txt - $ pip install k5test gssapi pyasn1 - $ pytest +Welcome to Paramiko! +==================== + +Paramiko is a pure-Python [#]_ (2.7, 3.4+) implementation of the SSHv2 protocol +[#]_, providing both client and server functionality. It provides the +foundation for the high-level SSH library `Fabric `_, +which is what we recommend you use for common client use-cases such as running +remote shell commands or transferring files. + +Direct use of Paramiko itself is only intended for users who need +advanced/low-level primitives or want to run an in-Python sshd. + +For installation information, changelogs, FAQs and similar, please visit `our +main project website `_; for API details, see `the +versioned docs `_. Additionally, the project +maintainer keeps a `roadmap `_ on his +personal site. + +.. [#] + Paramiko relies on `cryptography `_ for crypto + functionality, which makes use of C and Rust extensions but has many + precompiled options available. See `our installation page + `_ for details. + +.. [#] + SSH is defined in :rfc:`4251`, :rfc:`4252`, :rfc:`4253` and :rfc:`4254`. The + primary working implementation of the protocol is the `OpenSSH project + `_. Paramiko implements a large portion of the SSH + feature set, but there are occasional gaps. diff --git a/setup.py b/setup.py index 93c08d153..428f231b1 100644 --- a/setup.py +++ b/setup.py @@ -24,15 +24,7 @@ setup_helper.install_custom_make_tarball() -text = open("README.rst").read() -long_description = """ -To find out what's new in this version of Paramiko, please see `the changelog -`_. - -{} -""".format( - version, text -) +long_description = open("README.rst").read() # Version info -- read without importing _locals = {} diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 8d446990d..55a6a785d 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -7,7 +7,7 @@ Changelog - Migrate CI to CircleCI - Primary dev branch is now ``main`` (renamed) - Many README edits for clarity, modernization etc; including a bunch more - (and consistent) status badges + (and consistent) status badges & unification with main project site index - PyPI page much more fleshed out (long_description is now filled in with the README; sidebar links expanded; etc) - flake8, pytest configs split out of setup.cfg into their own files diff --git a/sites/www/index.rst b/sites/www/index.rst index 656031aa1..9c3bb3aae 100644 --- a/sites/www/index.rst +++ b/sites/www/index.rst @@ -1,20 +1,4 @@ -Welcome to Paramiko! -==================== - -Paramiko is a Python (2.7, 3.4+) implementation of the SSHv2 protocol [#]_, -providing both client and server functionality. While it leverages a Python C -extension for low level cryptography (`Cryptography -`_), Paramiko itself is a pure Python interface around -SSH networking concepts. - -This website covers project information for Paramiko such as the changelog, -contribution guidelines, development roadmap, news/blog, and so forth. Detailed -usage and API documentation can be found at our code documentation site, -`docs.paramiko.org `_. Additionally, the project -maintainer keeps a `roadmap `_ on his -website. - -Please see the sidebar to the left to begin. +.. include:: ../../README.rst .. toctree:: :hidden: @@ -27,10 +11,3 @@ Please see the sidebar to the left to begin. contact -.. rubric:: Footnotes - -.. [#] - SSH is defined in :rfc:`4251`, :rfc:`4252`, :rfc:`4253` and :rfc:`4254`. The - primary working implementation of the protocol is the `OpenSSH project - `_. Paramiko implements a large portion of the SSH - feature set, but there are occasional gaps. From bd5117bf02288e7537b8fe8aed9793fa9b63b806 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 1 Oct 2021 18:09:40 -0400 Subject: [PATCH 027/117] Badges? We do need some stinkin' badges! --- README.rst | 18 ++++++++++++++++++ sites/shared_conf.py | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 804d8fdb5..909c67a4d 100644 --- a/README.rst +++ b/README.rst @@ -1,3 +1,21 @@ +|version| |python| |license| |ci| |coverage| + +.. |version| image:: https://img.shields.io/pypi/v/paramiko + :target: https://pypi.org/project/paramiko/ + :alt: PyPI - Package Version +.. |python| image:: https://img.shields.io/pypi/pyversions/paramiko + :target: https://pypi.org/project/paramiko/ + :alt: PyPI - Python Version +.. |license| image:: https://img.shields.io/pypi/l/paramiko + :target: https://github.com/paramiko/paramiko/blob/main/LICENSE + :alt: PyPI - License +.. |ci| image:: https://img.shields.io/circleci/build/github/paramiko/paramiko/main + :target: https://app.circleci.com/pipelines/github/paramiko/paramiko + :alt: CircleCI +.. |coverage| image:: https://img.shields.io/codecov/c/gh/paramiko/paramiko + :target: https://app.codecov.io/gh/paramiko/paramiko + :alt: Codecov + Welcome to Paramiko! ==================== diff --git a/sites/shared_conf.py b/sites/shared_conf.py index 7bb503ce0..c406054c8 100644 --- a/sites/shared_conf.py +++ b/sites/shared_conf.py @@ -13,7 +13,7 @@ "github_user": "paramiko", "github_repo": "paramiko", "analytics_id": "UA-18486793-2", - "travis_button": True, + "travis_button": False, "tidelift_url": "https://tidelift.com/subscription/pkg/pypi-paramiko?utm_source=pypi-paramiko&utm_medium=referral&utm_campaign=docs", } html_sidebars = { From 42c7eff94b081e31fb2c2759d3ef9b3197c48c0c Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 1 Oct 2021 18:20:44 -0400 Subject: [PATCH 028/117] setup.py URL updates --- setup.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 428f231b1..1c68b0938 100644 --- a/setup.py +++ b/setup.py @@ -55,7 +55,14 @@ long_description=long_description, author="Jeff Forcier", author_email="jeff@bitprophet.org", - url="https://github.com/paramiko/paramiko/", + url="https://paramiko.org", + project_urls={ + "Docs": "https://docs.paramiko.org", + "Source": "https://github.com/paramiko/paramiko", + "Issues": "https://github.com/paramiko/paramiko/issues", + "Changelog": "https://www.paramiko.org/changelog.html", + "CI": "https://app.circleci.com/pipelines/github/paramiko/paramiko", + }, packages=["paramiko"], license="LGPL", platforms="Posix; MacOS X; Windows", From bb31651023ce9c54088f2963b4950f075b2fcd18 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 1 Oct 2021 18:27:01 -0400 Subject: [PATCH 029/117] Split out some setup.cfg sections --- .flake8 | 9 +++++++++ pytest.ini | 10 ++++++++++ setup.cfg | 20 -------------------- 3 files changed, 19 insertions(+), 20 deletions(-) create mode 100644 .flake8 create mode 100644 pytest.ini diff --git a/.flake8 b/.flake8 new file mode 100644 index 000000000..1317fd523 --- /dev/null +++ b/.flake8 @@ -0,0 +1,9 @@ +[flake8] +exclude = sites,.git,build,dist,demos +# NOTE: W503, E203 are concessions to black 18.0b5 and could be reinstated +# later if fixed on that end. +# NOTE: E722 seems to only have started popping up on move to flake8 3.6.0 from +# 2.4.0. Not sure why, bare excepts have been a (regrettable) thing forever... +ignore = E124,E125,E128,E261,E301,E302,E303,E402,E721,W503,E203,E722 +max-line-length = 79 + diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 000000000..be207cd86 --- /dev/null +++ b/pytest.ini @@ -0,0 +1,10 @@ +[pytest] +# We use pytest-relaxed just for its utils at the moment, so disable it at the +# plugin level until we adapt test organization to really use it. +addopts = -p no:relaxed +# Loop on failure +looponfailroots = tests paramiko +# Ignore some warnings we cannot easily handle. +filterwarnings = + ignore::DeprecationWarning:pkg_resources + ignore::cryptography.utils.CryptographyDeprecationWarning diff --git a/setup.cfg b/setup.cfg index 44d029c49..59b47d603 100644 --- a/setup.cfg +++ b/setup.cfg @@ -6,23 +6,3 @@ license_file = LICENSE [coverage:run] omit = paramiko/_winapi.py - -[flake8] -exclude = sites,.git,build,dist,demos -# NOTE: W503, E203 are concessions to black 18.0b5 and could be reinstated -# later if fixed on that end. -# NOTE: E722 seems to only have started popping up on move to flake8 3.6.0 from -# 2.4.0. Not sure why, bare excepts have been a (regrettable) thing forever... -ignore = E124,E125,E128,E261,E301,E302,E303,E402,E721,W503,E203,E722 -max-line-length = 79 - -[tool:pytest] -# We use pytest-relaxed just for its utils at the moment, so disable it at the -# plugin level until we adapt test organization to really use it. -addopts = -p no:relaxed -# Loop on failure -looponfailroots = tests paramiko -# Ignore some warnings we cannot easily handle. -filterwarnings = - ignore::DeprecationWarning:pkg_resources - ignore::cryptography.utils.CryptographyDeprecationWarning From 534700c80185e1365f2fcae0f018df08efb8c05f Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 1 Oct 2021 18:34:21 -0400 Subject: [PATCH 030/117] Add Py3.9 to classifiers --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index 1c68b0938..824786eb7 100644 --- a/setup.py +++ b/setup.py @@ -83,6 +83,7 @@ "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", + "Programming Language :: Python :: 3.9", ], # TODO 3.0: remove bcrypt, pynacl and update installation docs noting that # use of the extras_require(s) is now required for those From 2340dee7879bb49c0500a2c514501ff48f8547e3 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 1 Oct 2021 18:34:34 -0400 Subject: [PATCH 031/117] Copy/paste/modify invocations' circleci config --- .circleci/config.yml | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 .circleci/config.yml diff --git a/.circleci/config.yml b/.circleci/config.yml new file mode 100644 index 000000000..914f19b12 --- /dev/null +++ b/.circleci/config.yml @@ -0,0 +1,24 @@ +version: 2.1 + +orbs: + orb: invocations/orb@1.0.2 + +workflows: + main: + jobs: + - orb/lint: + name: Lint + - orb/format: + name: Style check + - orb/coverage: + name: Test 3.6 (w/ coverage) + - orb/test-release: + name: Release test + - orb/test: + name: Test << matrix.version >> + # It's not worth testing on other interpreters if the baseline one + # failed. Can't run >4 jobs at a time anyhow! + requires: ["Test 3.6 (w/ coverage)"] + matrix: + parameters: + version: ["3.7", "3.8", "3.9"] From 2160deed9135e3a45294ed3a5b4c7215a0c83567 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 1 Oct 2021 21:04:22 -0400 Subject: [PATCH 032/117] Can now rely on black for 3.6+ CI --- dev-requirements.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dev-requirements.txt b/dev-requirements.txt index f107c53cc..c3c32d499 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -8,6 +8,8 @@ pytest-xdist==1.28.0 mock==2.0.0 # Linting! flake8==3.8.3 +# Formatting! +black==18.6b4 # Coverage! coverage==4.5.4 codecov==1.6.3 From aac13fb192d3f4a5ca01026aa544d29a2c0dcfb1 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 1 Oct 2021 21:04:34 -0400 Subject: [PATCH 033/117] Codecov support --- .codecov.yml | 3 +++ .coveragerc | 5 +++++ dev-requirements.txt | 2 +- tasks.py | 8 ++++++-- 4 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 .codecov.yml create mode 100644 .coveragerc diff --git a/.codecov.yml b/.codecov.yml new file mode 100644 index 000000000..41ab9cc4a --- /dev/null +++ b/.codecov.yml @@ -0,0 +1,3 @@ +comment: false +coverage: + precision: 0 diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 000000000..47b6f4c3d --- /dev/null +++ b/.coveragerc @@ -0,0 +1,5 @@ +[run] +branch = True +include = + paramiko/* + tests/* diff --git a/dev-requirements.txt b/dev-requirements.txt index c3c32d499..e5f103d1e 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -12,7 +12,7 @@ flake8==3.8.3 black==18.6b4 # Coverage! coverage==4.5.4 -codecov==1.6.3 +codecov==2.1.11 # Documentation tools sphinx>=1.4,<1.7 alabaster==0.7.12 diff --git a/tasks.py b/tasks.py index 3d2fdf659..bfa43ae8e 100644 --- a/tasks.py +++ b/tasks.py @@ -75,11 +75,15 @@ def test( @task -def coverage(ctx, opts=""): +def coverage(ctx, opts="", codecov=False): """ Execute all tests (normal and slow) with coverage enabled. """ - return test(ctx, coverage=True, include_slow=True, opts=opts) + test(ctx, coverage=True, include_slow=True, opts=opts) + # Cribbed from invocations.pytest.coverage for now + if codecov: + ctx.run("coverage xml") + ctx.run("codecov") @task From e5d9dcced95948ef61df2b5d500d37216b8f781f Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 1 Oct 2021 21:40:19 -0400 Subject: [PATCH 034/117] Coverage tweaks - our customized test runner needed to stop using cli flag now that rcfile is in use, lol. NOW tests are shown. - add coverage files to gitignore --- .gitignore | 2 ++ tasks.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 89a9eee04..c6a6c4947 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,5 @@ _build .coverage .cache .idea +coverage.xml +htmlcov diff --git a/tasks.py b/tasks.py index bfa43ae8e..7da6eff1e 100644 --- a/tasks.py +++ b/tasks.py @@ -60,7 +60,7 @@ def test( # Leverage how pytest can be run as 'python -m pytest', and then how # coverage can be told to run things in that manner instead of # expecting a literal .py file. - runner = "coverage run --source=paramiko -m pytest" + runner = "coverage run -m pytest" # Strip SSH_AUTH_SOCK from parent env to avoid pollution by interactive # users. # TODO: once pytest coverage plugin works, see if there's a pytest-native From b3a7f365ba67d1f08bf295052778ffbf33c5a2e0 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 8 Oct 2021 13:44:29 -0400 Subject: [PATCH 035/117] Expand an old slightly confusing comment --- setup.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 824786eb7..569975321 100644 --- a/setup.py +++ b/setup.py @@ -86,7 +86,10 @@ "Programming Language :: Python :: 3.9", ], # TODO 3.0: remove bcrypt, pynacl and update installation docs noting that - # use of the extras_require(s) is now required for those + # use of the extras_require ("paramiko[ed2559]") is now required for those + # TODO 3.0: alternately, given how prevalent ed25519 is now, and how we use + # invoke for (increasing) subproc stuff, consider making the default flavor + # "full" and adding a "minimal" or similar that is just-crypto? install_requires=["bcrypt>=3.1.3", "cryptography>=2.5", "pynacl>=1.0.1"], extras_require=extras_require, ) From d3235f9b5e4bc028ae3475c7b34af2e230050676 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 8 Oct 2021 15:01:49 -0400 Subject: [PATCH 036/117] Add some older crypto versions to new 2nd py3.6 test step (no coverage for this one tho) --- .circleci/config.yml | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 914f19b12..4585c2f92 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,7 +1,7 @@ version: 2.1 orbs: - orb: invocations/orb@1.0.2 + orb: invocations/orb@1.0.3 workflows: main: @@ -10,15 +10,26 @@ workflows: name: Lint - orb/format: name: Style check + # Main test run, w/ coverage, and latest-supported cryptography - orb/coverage: - name: Test 3.6 (w/ coverage) + name: Test 3.6 (w/ coverage, latest crypto) + # Non-coverage runs w/ other crypto versions. + # (Phrased as 2-dimensional matrix but 3.6 only for now to save credits) + - orb/test: + name: Test << matrix.version >> w/ << matrix.pip-overrides >> + matrix: + parameters: + version: ["3.6"] + # TODO: I don't see a nicer way to do this that doesn't require + # making the orb know too much about its client code... + pip-overrides: ["cryptography==2.5", "cryptography==3.4"] - orb/test-release: name: Release test - orb/test: name: Test << matrix.version >> # It's not worth testing on other interpreters if the baseline one # failed. Can't run >4 jobs at a time anyhow! - requires: ["Test 3.6 (w/ coverage)"] + requires: ["Test 3.6 (w/ coverage, latest crypto)"] matrix: parameters: version: ["3.7", "3.8", "3.9"] From bf167341d8b90e5dee5b70b49dabe9a70a9982b8 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 8 Oct 2021 15:27:25 -0400 Subject: [PATCH 037/117] Moar orb debuggery --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 4585c2f92..d51b74cf4 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,7 +1,7 @@ version: 2.1 orbs: - orb: invocations/orb@1.0.3 + orb: invocations/orb@dev:debuggery workflows: main: From 521817ab3c2f077720b33539bdbf047466b098d4 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 8 Oct 2021 16:31:02 -0400 Subject: [PATCH 038/117] bump test/ci related sleep :( --- tests/test_client.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_client.py b/tests/test_client.py index 60ad310c2..f14aac230 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -421,7 +421,10 @@ def test_cleanup(self): # force a collection to see whether the SSHClient object is deallocated # 2 GCs are needed on PyPy, time is needed for Python 3 - time.sleep(0.3) + # TODO: this still fails randomly under CircleCI under Python 3.7, 3.8 + # at the very least. bumped sleep 0.3->1.0s but the underlying + # functionality should get reevaluated once we drop Python 2. + time.sleep(1) gc.collect() gc.collect() From 36c8a0e944530efd35f8896223293324cce986e0 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 8 Oct 2021 16:33:04 -0400 Subject: [PATCH 039/117] Shift 'full' extras install to our self-install in dev-reqs Makes sure the extras function ok, removes need to do it within CI. anyone using dev-reqs is implicitly dev'ing and wanting the full package --- dev-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index e5f103d1e..aa2f8d490 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -20,4 +20,4 @@ releases>=1.5,<2.0 # Release tools semantic_version>=2.4,<2.5 # Self --e . +-e ".[ed2559,invoke]" From 3128fff093f23e063e30330a90777357140b950a Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 8 Oct 2021 18:18:12 -0400 Subject: [PATCH 040/117] Try getting Kerberos working on Circle Sadly it fails. Unclear whether I'm missing something, or if this represents a diff between the Ubuntu versions Travis last used and what Circle's images are on. I did get the tests themselves truly running, and while I trimmed back the deps a bit vs Travis, it's only as some of them were redundant/required by other packages we install here. Leaving it defined but not actually used in the workflow. --- .circleci/config.yml | 29 +++++++++++++++++++++++++++++ tests/test_gssapi.py | 7 +++++++ 2 files changed, 36 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index d51b74cf4..a0f976219 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,8 +1,32 @@ version: 2.1 + orbs: orb: invocations/orb@dev:debuggery + +jobs: + kerberos: + executor: + name: orb/default + version: "3.6" + steps: + - orb/setup + # Required to actually see all of universe/multiverse :( + - run: sudo apt update + # System reqs to install/build gssapi c-ext & friends (who only + # appear to offer wheels for Windows) + - run: sudo apt install -y libkrb5-dev krb5-admin-server krb5-kdc + # Our gssapi-supporting flavor, eg gssapi, pyasn1 etc + - run: pip install -e '.[gssapi]' + # Test-only deps for Kerberos (if they are importable it triggers + # running the kerberos tests instead of skipping them) + - run: pip install k5test + # Do the thing, win the points! + - run: inv test + - orb/debug + + workflows: main: jobs: @@ -23,6 +47,11 @@ workflows: # TODO: I don't see a nicer way to do this that doesn't require # making the orb know too much about its client code... pip-overrides: ["cryptography==2.5", "cryptography==3.4"] + # Kerberos tests. Currently broken :( + #- kerberos: + # name: Test 3.6 w/ Kerberos support + # # No point testing k5 if base tests already fail + # requires: ["Test 3.6 (w/ coverage, latest crypto)"] - orb/test-release: name: Release test - orb/test: diff --git a/tests/test_gssapi.py b/tests/test_gssapi.py index 308caa93f..acdc7c821 100644 --- a/tests/test_gssapi.py +++ b/tests/test_gssapi.py @@ -26,6 +26,13 @@ from .util import needs_gssapi, KerberosTestCase, update_env +# +# NOTE: KerberosTestCase skips all tests if it was unable to import k5test +# third-party library. That's the primary trigger for whether this module +# effectively gets run or not. See tests/util.py for other triggers (a set of +# env vars a human might have defined). +# + @needs_gssapi class GSSAPITest(KerberosTestCase): From 5189a4f2029b186f022e84b71c42ba0e8d995cb0 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 8 Oct 2021 19:04:20 -0400 Subject: [PATCH 041/117] Port tests-folder-in-sdist safety check from Travis This isn't as much of a thing in my other projects so not in the orb for now --- .circleci/config.yml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index a0f976219..bb047bb63 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -6,6 +6,22 @@ orbs: jobs: + sdist-test-suite: + executor: + name: orb/default + version: "3.6" + steps: + - orb/setup + - run: inv release.build --no-wheel --directory . + - run: | + cd dist + tar xzvf *.tar.gz + rm -v *.tar.gz + cd paramiko-* + pip install -e . + inv -e test + - orb/debug + kerberos: executor: name: orb/default @@ -54,6 +70,12 @@ workflows: # requires: ["Test 3.6 (w/ coverage, latest crypto)"] - orb/test-release: name: Release test + # Ensure test suite is included in sdist & functions appropriately + - sdist-test-suite: + name: Test within sdist + requires: + - "Test 3.6 (w/ coverage, latest crypto)" + - "Release test" - orb/test: name: Test << matrix.version >> # It's not worth testing on other interpreters if the baseline one From 6e31c19e173dd842d5dad1566d01ceff488de6e1 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 8 Oct 2021 19:40:40 -0400 Subject: [PATCH 042/117] Use new orb docs job --- .circleci/config.yml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index bb047bb63..d478ba976 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -46,6 +46,7 @@ jobs: workflows: main: jobs: + # The basics - orb/lint: name: Lint - orb/format: @@ -76,11 +77,15 @@ workflows: requires: - "Test 3.6 (w/ coverage, latest crypto)" - "Release test" + # Test other interpreters if main passed - orb/test: name: Test << matrix.version >> - # It's not worth testing on other interpreters if the baseline one - # failed. Can't run >4 jobs at a time anyhow! requires: ["Test 3.6 (w/ coverage, latest crypto)"] matrix: parameters: version: ["3.7", "3.8", "3.9"] + # Test doc building if main test suite passed (no real reason to spend + # all those credits if the main tests would also fail...) + - orb/docs: + name: "Docs" + requires: ["Test 3.6 (w/ coverage, latest crypto)"] From 9c0d00b614b2ecf8269e93a5d8813ef90c0b1c80 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 8 Oct 2021 19:40:57 -0400 Subject: [PATCH 043/117] bye felicia^Wtravis --- .travis.yml | 88 ----------------------------------------------------- 1 file changed, 88 deletions(-) delete mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 58a3ac429..000000000 --- a/.travis.yml +++ /dev/null @@ -1,88 +0,0 @@ -dist: xenial -language: python -sudo: false -cache: - directories: - - $HOME/.cache/pip -python: - - "2.7" - - "3.4" - - "3.5" - - "3.6" - - "3.7" - - "3.8" - - "pypy" - - "pypy3" -matrix: - # Explicitly test against our oldest supported cryptography.io, in addition - # to whatever the latest default is. - include: - - python: 2.7 - env: "OLDEST_CRYPTO=2.5" - - python: 3.7 - env: "OLDEST_CRYPTO=2.5" - - python: 2.7 - env: "USE_K5TEST=yes" - - python: 3.7 - env: "USE_K5TEST=yes" -install: - # Ensure modern pip/etc to avoid some issues w/ older worker environs - - pip install pip==9.0.1 setuptools==36.6.0 - # Grab a specific version of Cryptography if desired. Doing this before other - # installations ensures we don't have to do any downgrading/overriding. - - | - if [[ -n "$OLDEST_CRYPTO" ]]; then - pip install "cryptography==${OLDEST_CRYPTO}" - fi - # Self-install for setup.py-driven deps (plus additional - # safe-enough-for-all-matrix-cells optional deps) - # TODO: additional matrices or test steps to test all the entrypoints - - pip install -e ".[ed25519,invoke]" - # Dev (doc/test running) requirements - # TODO: use poetry + whatever contexty-type stuff it has, should be more than - # just prod/dev split. Also apply to the above re: extras_require. - - pip install codecov # For codecov specifically - - pip install -r dev-requirements.txt - - | - if [[ -n "$USE_K5TEST" ]]; then - # we need a few commands and libraries - # Debian/Ubuntu package: commands used by package k5test - # libkrb5-dev: krb5-config - # krb5-kdc: kdb5_util, krb5kdc - # krb5-admin-server: kadmin.local, kprop, kadmind - # krb5-user: kinit, klist - # - # krb5-multidev: required to build gssapi - sudo apt-get -y install libkrb5-dev krb5-admin-server \ - krb5-kdc krb5-user krb5-multidev && \ - pip install k5test gssapi pyasn1 - fi - # In case of problems uncomment the following to get the krb environment - # - | - # if [[ -n "$USE_K5TEST" ]]; then - # python -c 'from tests.util import k5shell; k5shell()' env | sort - # fi -script: - # Fast syntax check failures for more rapid feedback to submitters - # (Travis-oriented metatask that version checks Python, installs, runs.) - - inv travis.blacken - # I have this in my git pre-push hook, but contributors probably don't - - flake8 - # All (including slow) tests, w/ coverage! - - inv coverage - # Ensure documentation builds, both sites, maxxed nitpicking - - inv sites - # Sanity check ability to build sdist - - python setup.py sdist - # And ability to run tests from within the result - - cd dist && tar xzvf *.tar.gz && rm *.tar.gz && cd paramiko-* && ls -l && pip install -e . && inv test -notifications: - irc: - channels: "irc.freenode.org#paramiko" - template: - - "%{repository}@%{branch}: %{message} (%{build_url})" - on_success: change - on_failure: change - email: false -after_success: - - codecov From d53296338716751ed4915b46a6f859a1dcbfc8b9 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 8 Oct 2021 20:10:50 -0400 Subject: [PATCH 044/117] Released orb version --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index d478ba976..6b6220572 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -2,7 +2,7 @@ version: 2.1 orbs: - orb: invocations/orb@dev:debuggery + orb: invocations/orb@1.0.4 jobs: From 143cb7c0d888db3594ff5d312674e8e3afb5d0a3 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 8 Oct 2021 21:22:28 -0400 Subject: [PATCH 045/117] TODO --- tasks.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tasks.py b/tasks.py index 7da6eff1e..bea7e27af 100644 --- a/tasks.py +++ b/tasks.py @@ -98,6 +98,8 @@ def guard(ctx, opts=""): # Until we stop bundling docs w/ releases. Need to discover use cases first. # TODO: would be nice to tie this into our own version of build() too, but # still have publish() use that build()...really need to try out classes! +# TODO 3.0: I'd like to just axe the 'built docs in sdist', none of my other +# projects do it. @task def release(ctx, sdist=True, wheel=True, sign=True, dry_run=False, index=None): """ From 28571d8d7f7b1d10feeb0d120cd837a16d194ed6 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 8 Oct 2021 21:23:15 -0400 Subject: [PATCH 046/117] Less confusing name of overriding task please --- tasks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tasks.py b/tasks.py index bea7e27af..06cba0173 100644 --- a/tasks.py +++ b/tasks.py @@ -101,7 +101,7 @@ def guard(ctx, opts=""): # TODO 3.0: I'd like to just axe the 'built docs in sdist', none of my other # projects do it. @task -def release(ctx, sdist=True, wheel=True, sign=True, dry_run=False, index=None): +def publish_(ctx, sdist=True, wheel=True, sign=True, dry_run=False, index=None): """ Wraps invocations.packaging.publish to add baked-in docs folder. """ @@ -126,7 +126,7 @@ def release(ctx, sdist=True, wheel=True, sign=True, dry_run=False, index=None): # TODO: "replace one task with another" needs a better public API, this is # using unpublished internals & skips all the stuff add_task() does re: # aliasing, defaults etc. -release_coll.tasks["publish"] = release +release_coll.tasks["publish"] = publish_ ns = Collection( test, From 6fe0db1f93e878d1b0bef29eef3666e0a51d5ec6 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 9 Oct 2021 13:35:19 -0400 Subject: [PATCH 047/117] Massage upstream 'release.all' task too re: custom publish --- tasks.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tasks.py b/tasks.py index 06cba0173..362384fa1 100644 --- a/tasks.py +++ b/tasks.py @@ -116,17 +116,22 @@ def publish_(ctx, sdist=True, wheel=True, sign=True, dry_run=False, index=None): publish( ctx, sdist=sdist, wheel=wheel, sign=sign, dry_run=dry_run, index=index ) - # Remind - print( - "\n\nDon't forget to update RTD's versions page for new minor " - "releases!" - ) + + +# Also have to hack up the newly enhanced all_() so it uses our publish +@task(name="all", default=True) +def all_(c, dry_run=False): + release_coll["prepare"](c, dry_run=dry_run) + publish_(c, dry_run=dry_run) + release_coll["push"](c, dry_run=dry_run) + release_coll["tidelift"](c, dry_run=dry_run) # TODO: "replace one task with another" needs a better public API, this is # using unpublished internals & skips all the stuff add_task() does re: # aliasing, defaults etc. release_coll.tasks["publish"] = publish_ +release_coll.tasks["all"] = all_ ns = Collection( test, From 842adf958b5012c159fdf0725d0c2892f6a045fa Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 9 Oct 2021 17:32:33 -0400 Subject: [PATCH 048/117] For some reason :rfc: blows up where :rfc-reference: does not Only when running as part of my fullstack release task, specifically when I load up sphinx+releases to parse the changelog. Makes no sense at all but no time to really dig more... --- README.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index 909c67a4d..f99090d33 100644 --- a/README.rst +++ b/README.rst @@ -41,7 +41,8 @@ personal site. `_ for details. .. [#] - SSH is defined in :rfc:`4251`, :rfc:`4252`, :rfc:`4253` and :rfc:`4254`. The - primary working implementation of the protocol is the `OpenSSH project + SSH is defined in :rfc-reference:`4251`, :rfc-reference:`4252`, + :rfc-reference:`4253` and :rfc-reference:`4254`. The primary working + implementation of the protocol is the `OpenSSH project `_. Paramiko implements a large portion of the SSH feature set, but there are occasional gaps. From eef08a9dfd4d17323ae6b25acef609b34467b691 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 9 Oct 2021 17:35:52 -0400 Subject: [PATCH 049/117] Classify bug as major in case I don't cut a bugfix for 2.7 --- sites/www/changelog.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 55a6a785d..24207a373 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -14,8 +14,8 @@ Changelog - Invoke/invocations (used by maintainers/contributors) upgraded to modern versions -- :bug:`1462` (via :issue:`1882`) Newer server-side key exchange algorithms not - intended to use SHA1 (``diffie-hellman-group14-sha256``, +- :bug:`1462 major` (via :issue:`1882`) Newer server-side key exchange + algorithms not intended to use SHA1 (``diffie-hellman-group14-sha256``, ``diffie-hellman-group16-sha512``) were incorrectly using SHA1 after all, due to a bug causing them to ignore the ``hash_algo`` class attribute. This has been corrected. Big thanks to ``@miverson`` for the report and to Benno Rice From 37054197a99848d36b302a00c906188e37df8615 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 9 Oct 2021 17:44:01 -0400 Subject: [PATCH 050/117] Nix travis from tasks --- tasks.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tasks.py b/tasks.py index 362384fa1..6512e4065 100644 --- a/tasks.py +++ b/tasks.py @@ -3,7 +3,6 @@ from shutil import rmtree, copytree from invoke import Collection, task -from invocations import travis from invocations.checks import blacken from invocations.docs import docs, www, sites from invocations.packaging.release import ns as release_coll, publish @@ -142,7 +141,6 @@ def all_(c, dry_run=False): www, sites, count_errors, - travis, blacken, ) ns.configure( @@ -157,6 +155,5 @@ def all_(c, dry_run=False): www.configuration()["sphinx"]["source"], "changelog.rst" ), }, - "travis": {"black": {"version": "18.6b4"}}, } ) From 7714caf79a09dc455a32c6071dd22ba37c399758 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 9 Oct 2021 17:36:16 -0400 Subject: [PATCH 051/117] Cut 2.8.0 --- paramiko/_version.py | 2 +- sites/www/changelog.rst | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/paramiko/_version.py b/paramiko/_version.py index 8b3ae0e7d..07aff82b9 100644 --- a/paramiko/_version.py +++ b/paramiko/_version.py @@ -1,2 +1,2 @@ -__version_info__ = (2, 7, 2) +__version_info__ = (2, 8, 0) __version__ = ".".join(map(str, __version_info__)) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 24207a373..49b41f416 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,7 @@ Changelog ========= +- :release:`2.8.0 <2021-10-09>` - :support:`-` Administrivia overhaul, including but not limited to: - Migrate CI to CircleCI From ff6082608026b618d93c67d68d15fa452eef52a4 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 9 Oct 2021 18:09:53 -0400 Subject: [PATCH 052/117] blacken --- tasks.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tasks.py b/tasks.py index 6512e4065..2e1052977 100644 --- a/tasks.py +++ b/tasks.py @@ -100,7 +100,9 @@ def guard(ctx, opts=""): # TODO 3.0: I'd like to just axe the 'built docs in sdist', none of my other # projects do it. @task -def publish_(ctx, sdist=True, wheel=True, sign=True, dry_run=False, index=None): +def publish_( + ctx, sdist=True, wheel=True, sign=True, dry_run=False, index=None +): """ Wraps invocations.packaging.publish to add baked-in docs folder. """ @@ -154,6 +156,6 @@ def all_(c, dry_run=False): "changelog_file": join( www.configuration()["sphinx"]["source"], "changelog.rst" ), - }, + } } ) From a73bf04c4c1105ec731cbd92b7dec350b3d1bdb6 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 15 Oct 2021 19:30:20 -0400 Subject: [PATCH 053/117] Enhance language around SSHClient.close() --- paramiko/client.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/paramiko/client.py b/paramiko/client.py index 6bf479d46..6d1636bef 100644 --- a/paramiko/client.py +++ b/paramiko/client.py @@ -441,11 +441,13 @@ def close(self): """ Close this SSHClient and its underlying `.Transport`. + This should be called anytime you are done using the client object. + .. warning:: - Failure to do this may, in some situations, cause your Python - interpreter to hang at shutdown (often due to race conditions). - It's good practice to `close` your client objects anytime you're - done using them, instead of relying on garbage collection. + Paramiko registers garbage collection hooks that will try to + automatically close connections for you, but this is not presently + reliable. Failure to explicitly close your client after use may + lead to end-of-process hangs! """ if self._transport is None: return From b79892d9e096851fc3f4d594afc509a745167863 Mon Sep 17 00:00:00 2001 From: Lans Zhang Date: Tue, 25 Jul 2017 14:42:46 +0800 Subject: [PATCH 054/117] hostkeys: fix raising KeyError in SubDict.__delitem__() If the specified key type is found out, the exception KeyError should not happen. Signed-off-by: Lans Zhang --- paramiko/hostkeys.py | 1 + 1 file changed, 1 insertion(+) diff --git a/paramiko/hostkeys.py b/paramiko/hostkeys.py index d0660cc82..94474e40a 100644 --- a/paramiko/hostkeys.py +++ b/paramiko/hostkeys.py @@ -156,6 +156,7 @@ def __delitem__(self, key): for e in list(self._entries): if e.key.get_name() == key: self._entries.remove(e) + break else: raise KeyError(key) From 15fa5b8d2a7750b2ead67580343bf96ff93f6840 Mon Sep 17 00:00:00 2001 From: Lans Zhang Date: Wed, 26 Jul 2017 09:54:41 +0800 Subject: [PATCH 055/117] test_hostkeys: test SubDict.__delitem__() SubDict.__delitem__() would trigger the KeyError exception if a key type to be deleted is not present. Signed-off-by: Lans Zhang --- tests/test_hostkeys.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_hostkeys.py b/tests/test_hostkeys.py index da47362c0..e375687cf 100644 --- a/tests/test_hostkeys.py +++ b/tests/test_hostkeys.py @@ -130,3 +130,20 @@ def test_delitem(self): pass # Good else: assert False, "Entry was not deleted from HostKeys on delitem!" + + def test_entry_delitem(self): + hostdict = paramiko.HostKeys('hostfile.temp') + target = 'happy.example.com' + entry = hostdict[target] + key_type_list = [ key_type for key_type in entry ] + for key_type in key_type_list: + del entry[key_type] + + # will KeyError if not present + for key_type in key_type_list: + try: + del entry[key_type] + except KeyError: + pass # Good + else: + assert False, "Key was not deleted from Entry on delitem!" From 37cda1dd85430910c7867430d04ec6ac846823b2 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 28 Nov 2021 20:22:50 -0500 Subject: [PATCH 056/117] Blacken for hostkeys fix --- tests/test_hostkeys.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_hostkeys.py b/tests/test_hostkeys.py index e375687cf..41a9244f1 100644 --- a/tests/test_hostkeys.py +++ b/tests/test_hostkeys.py @@ -132,10 +132,10 @@ def test_delitem(self): assert False, "Entry was not deleted from HostKeys on delitem!" def test_entry_delitem(self): - hostdict = paramiko.HostKeys('hostfile.temp') - target = 'happy.example.com' + hostdict = paramiko.HostKeys("hostfile.temp") + target = "happy.example.com" entry = hostdict[target] - key_type_list = [ key_type for key_type in entry ] + key_type_list = [key_type for key_type in entry] for key_type in key_type_list: del entry[key_type] @@ -144,6 +144,6 @@ def test_entry_delitem(self): try: del entry[key_type] except KeyError: - pass # Good + pass # Good else: assert False, "Key was not deleted from Entry on delitem!" From 656afd59f8ab2352ecead9433f841af128bf445c Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 27 Nov 2021 14:47:21 -0500 Subject: [PATCH 057/117] Changelog re security flaw email, re #908 --- sites/www/changelog.rst | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 49b41f416..4ee19f824 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,30 @@ Changelog ========= +- :bug:`-` (also :issue:`908`) Update `~paramiko.pkey.PKey` and subclasses to + compare (``__eq__``) via direct field/attribute comparison instead of hashing + (while retaining the existing behavior of ``__hash__`` via a slight + refactor). Big thanks to Josh Snyder and Jun Omae for the reports, and to + Josh Snyder for reproduction details & patch. + + .. warning:: + This fixes a security flaw! If you are running Paramiko on 32-bit systems + with low entropy (such as any 32-bit Python 2, or a 32-bit Python 3 which + is running with ``PYTHONHASHSEED=0``) it is possible for an attacker to + craft a new keypair from an exfiltrated public key, which Paramiko would + consider equal to the original key. + + This could enable attacks such as, but not limited to, the following: + + - Paramiko server processes would incorrectly authenticate the attacker + (using their generated private key) as if they were the victim. We see + this as the most plausible attack using this flaw. + - Paramiko client processes would incorrectly validate a connected server + (when host key verification is enabled) while subjected + to a man-in-the-middle attack. This impacts more users than the + server-side version, but also carries higher requirements for the + attacker, namely successful DNS poisoning or other MITM techniques. + - :release:`2.8.0 <2021-10-09>` - :support:`-` Administrivia overhaul, including but not limited to: From aab38bdccca4e1aae7967d5b2b560743ee36a0dc Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 27 Nov 2021 22:12:41 -0500 Subject: [PATCH 058/117] Fix silly but apparently nonfatal dev-req self-install typo --- dev-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index aa2f8d490..3edcc812f 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -20,4 +20,4 @@ releases>=1.5,<2.0 # Release tools semantic_version>=2.4,<2.5 # Self --e ".[ed2559,invoke]" +-e ".[ed25519,invoke]" From f4b5ce36e75a2aca2b11ef54b0ce04e1188f667c Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 27 Nov 2021 22:16:56 -0500 Subject: [PATCH 059/117] basic alpine i386 dockerfile --- Dockerfile.i386 | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 Dockerfile.i386 diff --git a/Dockerfile.i386 b/Dockerfile.i386 new file mode 100644 index 000000000..76418cede --- /dev/null +++ b/Dockerfile.i386 @@ -0,0 +1,10 @@ +# Convenience tool for testing a 32-bit related security flaw. May be +# generalized in future to be more useful; until then, it is NOT +# officially supported but purely a maintainer-facing artifact. + +FROM --platform=i386 i386/alpine:3.15 + +RUN apk add openssl-dev python3-dev libffi-dev make cargo + +RUN python3 -m venv env +RUN env/bin/pip install -U pip From aca7613171937334c47377faf0cf2e4a9126c0d4 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 27 Nov 2021 22:13:23 -0500 Subject: [PATCH 060/117] Pre-patch test proving security flaw re: 32bit key hash comparisons --- tests/badhash_key1.ed25519.key | 7 +++++++ tests/badhash_key2.ed25519.key | 7 +++++++ tests/test_pkey.py | 18 +++++++++++++++++- tests/util.py | 19 ++++++++++++++++++- 4 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 tests/badhash_key1.ed25519.key create mode 100644 tests/badhash_key2.ed25519.key diff --git a/tests/badhash_key1.ed25519.key b/tests/badhash_key1.ed25519.key new file mode 100644 index 000000000..3e33781b3 --- /dev/null +++ b/tests/badhash_key1.ed25519.key @@ -0,0 +1,7 @@ +-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZWQyNTUx +OQAAACCULQjdmVfwpbDAFYz4mhKo6aCiAfkbaC+dEdq5eP1R9QAAAIjXZhzv12Yc7wAAAAtzc2gt +ZWQyNTUxOQAAACCULQjdmVfwpbDAFYz4mhKo6aCiAfkbaC+dEdq5eP1R9QAAAEByeJbhZUBL2aJ6 +wP85amzQuqDJRrNyAGMtDBJ43SURxpQtCN2ZV/ClsMAVjPiaEqjpoKIB+RtoL50R2rl4/VH1AAAA +AAECAwQF +-----END OPENSSH PRIVATE KEY----- diff --git a/tests/badhash_key2.ed25519.key b/tests/badhash_key2.ed25519.key new file mode 100644 index 000000000..bf48edac0 --- /dev/null +++ b/tests/badhash_key2.ed25519.key @@ -0,0 +1,7 @@ +-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZWQyNTUx +OQAAACACJbNmFu2Bk34HArxhiRYajoIN03Vr0umfNvsc9atE0AAAAIi5+po1ufqaNQAAAAtzc2gt +ZWQyNTUxOQAAACACJbNmFu2Bk34HArxhiRYajoIN03Vr0umfNvsc9atE0AAAAECh/ZzZJDOZGnil +BxJMm+nOhBpc07IVBjU1ii+S8zqFaAIls2YW7YGTfgcCvGGJFhqOgg3TdWvS6Z82+xz1q0TQAAAA +AAECAwQF +-----END OPENSSH PRIVATE KEY----- diff --git a/tests/test_pkey.py b/tests/test_pkey.py index 18c27bbe5..caa6d7b3b 100644 --- a/tests/test_pkey.py +++ b/tests/test_pkey.py @@ -31,8 +31,9 @@ from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateNumbers from mock import patch +import pytest -from .util import _support +from .util import _support, is_low_entropy # from openssh's ssh-keygen @@ -551,6 +552,21 @@ def test_ed25519_compare(self): self.assertTrue(not pub.can_sign()) self.assertEqual(key, pub) + # No point testing on systems that never exhibited the bug originally + @pytest.mark.skipif( + not is_low_entropy(), reason="Not a low-entropy system" + ) + def test_ed25519_32bit_collision(self): + # Re: 2021.10.19 security report email: two different private keys + # which Paramiko compared as equal on low-entropy platforms. + original = Ed25519Key.from_private_key_file( + _support("badhash_key1.ed25519.key") + ) + generated = Ed25519Key.from_private_key_file( + _support("badhash_key2.ed25519.key") + ) + assert original != generated + def test_ed25519_nonbytes_password(self): # https://github.com/paramiko/paramiko/issues/1039 Ed25519Key.from_private_key_file( diff --git a/tests/util.py b/tests/util.py index 9057f516d..1355ce8a8 100644 --- a/tests/util.py +++ b/tests/util.py @@ -1,11 +1,12 @@ from os.path import dirname, realpath, join import os +import struct import sys import unittest import pytest -from paramiko.py3compat import builtins +from paramiko.py3compat import builtins, PY2 from paramiko.ssh_gss import GSS_AUTH_AVAILABLE @@ -127,3 +128,19 @@ def k5shell(args=None): if not args: args = [os.environ.get("SHELL", "bash")] sys.exit(subprocess.call(args)) + + +def is_low_entropy(): + """ + Attempts to detect whether running interpreter is low-entropy. + + Classified as being in 32-bit mode under Python 2, or 32-bit mode and with + the hash seed set to zero under Python 3. + """ + is_32bit = struct.calcsize("P") == 32 / 8 + if PY2: + return is_32bit + else: + # I don't see a way to tell internally if the hash seed was set this + # way, but env should be plenty sufficient, this is only for testing. + return is_32bit and os.environ.get("PYTHONHASHSEED", None) == "0" From 0344146de514ab1f2643bad5bfce0a92d4dcf1ee Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 27 Nov 2021 23:13:31 -0500 Subject: [PATCH 061/117] Enhance tests of existing/general key cmp/hash behavior Not sure why these were never explicitly tested --- tests/test_pkey.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test_pkey.py b/tests/test_pkey.py index caa6d7b3b..9b144d22a 100644 --- a/tests/test_pkey.py +++ b/tests/test_pkey.py @@ -567,6 +567,28 @@ def test_ed25519_32bit_collision(self): ) assert original != generated + def keys(self): + for key_class, filename in [ + (RSAKey, "test_rsa.key"), + (DSSKey, "test_dss.key"), + (ECDSAKey, "test_ecdsa_256.key"), + (Ed25519Key, "test_ed25519.key"), + ]: + key1 = key_class.from_private_key_file(_support(filename)) + key2 = key_class.from_private_key_file(_support(filename)) + yield key1, key2 + + def test_keys_are_comparable(self): + for key1, key2 in self.keys(): + assert key1 == key2 + + def test_keys_are_hashable(self): + # NOTE: this isn't a great test due to hashseed randomization under + # Python 3 preventing use of static values, but it does still prove + # that __hash__ is implemented/doesn't explode & works across instances + for key1, key2 in self.keys(): + assert hash(key1) == hash(key2) + def test_ed25519_nonbytes_password(self): # https://github.com/paramiko/paramiko/issues/1039 Ed25519Key.from_private_key_file( From 2c574ea29456726177eece3d8f740182ea34f0b4 Mon Sep 17 00:00:00 2001 From: Josh Snyder Date: Thu, 14 Oct 2021 18:29:37 -0700 Subject: [PATCH 062/117] Check all data on keys when comparing for equality This eliminates the possibility that a hash collision results in two PKey objects that == each other. --- paramiko/agent.py | 4 ++++ paramiko/dsskey.py | 5 +++-- paramiko/ecdsakey.py | 15 +++++++-------- paramiko/ed25519key.py | 5 +++-- paramiko/pkey.py | 14 ++++++++++++-- 5 files changed, 29 insertions(+), 14 deletions(-) diff --git a/paramiko/agent.py b/paramiko/agent.py index 622b95e4e..c7c8b7cb6 100644 --- a/paramiko/agent.py +++ b/paramiko/agent.py @@ -407,6 +407,10 @@ def __str__(self): def get_name(self): return self.name + @property + def _fields(self): + raise NotImplementedError + def sign_ssh_data(self, data): msg = Message() msg.add_byte(cSSH2_AGENTC_SIGN_REQUEST) diff --git a/paramiko/dsskey.py b/paramiko/dsskey.py index 7e74836c2..09d6f6483 100644 --- a/paramiko/dsskey.py +++ b/paramiko/dsskey.py @@ -92,8 +92,9 @@ def asbytes(self): def __str__(self): return self.asbytes() - def __hash__(self): - return hash((self.get_name(), self.p, self.q, self.g, self.y)) + @property + def _fields(self): + return (self.get_name(), self.p, self.q, self.g, self.y) def get_name(self): return "ssh-dss" diff --git a/paramiko/ecdsakey.py b/paramiko/ecdsakey.py index 3d3d09beb..a59017f5b 100644 --- a/paramiko/ecdsakey.py +++ b/paramiko/ecdsakey.py @@ -194,14 +194,13 @@ def asbytes(self): def __str__(self): return self.asbytes() - def __hash__(self): - return hash( - ( - self.get_name(), - self.verifying_key.public_numbers().x, - self.verifying_key.public_numbers().y, - ) - ) + @property + def _fields(self): + return ( + self.get_name(), + self.verifying_key.public_numbers().x, + self.verifying_key.public_numbers().y, + ) def get_name(self): return self.ecdsa_curve.key_format_identifier diff --git a/paramiko/ed25519key.py b/paramiko/ed25519key.py index b584f5218..7b19e3528 100644 --- a/paramiko/ed25519key.py +++ b/paramiko/ed25519key.py @@ -174,12 +174,13 @@ def asbytes(self): m.add_string(v.encode()) return m.asbytes() - def __hash__(self): + @property + def _fields(self): if self.can_sign(): v = self._signing_key.verify_key else: v = self._verifying_key - return hash((self.get_name(), v)) + return (self.get_name(), v) def get_name(self): return "ssh-ed25519" diff --git a/paramiko/pkey.py b/paramiko/pkey.py index a54d502d5..fed68da8b 100644 --- a/paramiko/pkey.py +++ b/paramiko/pkey.py @@ -20,6 +20,9 @@ Common API for all public keys. """ +from abc import ABC +from abc import abstractproperty +from abc import abstractmethod import base64 from binascii import unhexlify import os @@ -59,7 +62,7 @@ def _unpad_openssh(data): return data[:-padding_length] -class PKey(object): +class PKey(ABC): """ Base class for public keys. """ @@ -140,7 +143,14 @@ def __cmp__(self, other): return cmp(self.asbytes(), other.asbytes()) # noqa def __eq__(self, other): - return hash(self) == hash(other) + return self._fields == other._fields + + def __hash__(self): + return hash(self._fields) + + @abstractproperty + def _fields(self): + pass def get_name(self): """ From 6a6f56da9b00fd3f1fc7ca9ecbe4108853db501e Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 28 Nov 2021 14:01:05 -0500 Subject: [PATCH 063/117] Blacken patch. --- paramiko/ecdsakey.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/paramiko/ecdsakey.py b/paramiko/ecdsakey.py index a59017f5b..05bd10f96 100644 --- a/paramiko/ecdsakey.py +++ b/paramiko/ecdsakey.py @@ -197,10 +197,10 @@ def __str__(self): @property def _fields(self): return ( - self.get_name(), - self.verifying_key.public_numbers().x, - self.verifying_key.public_numbers().y, - ) + self.get_name(), + self.verifying_key.public_numbers().x, + self.verifying_key.public_numbers().y, + ) def get_name(self): return self.ecdsa_curve.key_format_identifier From 1b4fcd3f0abb209060080034eb66f226196dd8b0 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 28 Nov 2021 14:06:42 -0500 Subject: [PATCH 064/117] Roll back ABC using kit from contributed patch - blew up good for me for reasons I don't entirely grok - also feels like too big of a change to do in a patch release - is not a critical piece of the bugfix - NotImplementedError is fine for now --- paramiko/pkey.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/paramiko/pkey.py b/paramiko/pkey.py index fed68da8b..5bdfb1d43 100644 --- a/paramiko/pkey.py +++ b/paramiko/pkey.py @@ -20,9 +20,6 @@ Common API for all public keys. """ -from abc import ABC -from abc import abstractproperty -from abc import abstractmethod import base64 from binascii import unhexlify import os @@ -62,7 +59,7 @@ def _unpad_openssh(data): return data[:-padding_length] -class PKey(ABC): +class PKey(object): """ Base class for public keys. """ @@ -148,9 +145,9 @@ def __eq__(self, other): def __hash__(self): return hash(self._fields) - @abstractproperty + @property def _fields(self): - pass + raise NotImplementedError def get_name(self): """ From bdb5d9a766e30b04c0df0eea8e7b313813ce6e1d Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 28 Nov 2021 14:07:16 -0500 Subject: [PATCH 065/117] Patch missed RSAKey for some reason. Fixed. --- paramiko/rsakey.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/paramiko/rsakey.py b/paramiko/rsakey.py index 172f42d41..142dd278d 100644 --- a/paramiko/rsakey.py +++ b/paramiko/rsakey.py @@ -98,10 +98,9 @@ def __str__(self): else: return self.asbytes().decode("utf8", errors="ignore") - def __hash__(self): - return hash( - (self.get_name(), self.public_numbers.e, self.public_numbers.n) - ) + @property + def _fields(self): + return (self.get_name(), self.public_numbers.e, self.public_numbers.n) def get_name(self): return "ssh-rsa" From 9c49dc721e25992c824035088d5a054715bd0d87 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 28 Nov 2021 14:56:29 -0500 Subject: [PATCH 066/117] Tests proving #1257 --- tests/test_pkey.py | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/tests/test_pkey.py b/tests/test_pkey.py index 9b144d22a..94b2492bb 100644 --- a/tests/test_pkey.py +++ b/tests/test_pkey.py @@ -26,9 +26,18 @@ from binascii import hexlify from hashlib import md5 -from paramiko import RSAKey, DSSKey, ECDSAKey, Ed25519Key, Message, util +from paramiko import ( + RSAKey, + DSSKey, + ECDSAKey, + Ed25519Key, + Message, + util, + SSHException, +) from paramiko.py3compat import StringIO, byte_chr, b, bytes, PY2 +from cryptography.exceptions import UnsupportedAlgorithm from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateNumbers from mock import patch import pytest @@ -163,6 +172,16 @@ def test_load_rsa(self): key2 = RSAKey.from_private_key(s) self.assertEqual(key, key2) + def test_load_rsa_transmutes_crypto_exceptions(self): + # TODO: nix unittest for pytest + for exception in (TypeError("onoz"), UnsupportedAlgorithm("oops")): + with patch( + "paramiko.rsakey.serialization.load_der_private_key" + ) as loader: + loader.side_effect = exception + with pytest.raises(SSHException, match=str(exception)): + RSAKey.from_private_key_file(_support("test_rsa.key")) + def test_load_rsa_password(self): key = RSAKey.from_private_key_file( _support("test_rsa_password.key"), "television" @@ -369,6 +388,17 @@ def test_load_ecdsa_password_384(self): self.assertEqual(PUB_ECDSA_384.split()[1], key.get_base64()) self.assertEqual(384, key.get_bits()) + def test_load_ecdsa_transmutes_crypto_exceptions(self): + path = _support("test_ecdsa_256.key") + # TODO: nix unittest for pytest + for exception in (TypeError("onoz"), UnsupportedAlgorithm("oops")): + with patch( + "paramiko.ecdsakey.serialization.load_der_private_key" + ) as loader: + loader.side_effect = exception + with pytest.raises(SSHException, match=str(exception)): + ECDSAKey.from_private_key_file(path) + def test_compare_ecdsa_384(self): # verify that the private & public keys compare equal key = ECDSAKey.from_private_key_file(_support("test_ecdsa_384.key")) From e9e411e25267ccff7ef5dc265280a5a2d8945e8b Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 28 Nov 2021 15:21:38 -0500 Subject: [PATCH 067/117] Changelog closes #1257, re #1266 --- sites/www/changelog.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 4ee19f824..de2319ea0 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,13 @@ Changelog ========= +- :bug:`1257` (also :issue:`1266`) Update RSA and ECDSA key decoding + subroutines to correctly catch exception types thrown by modern + versions of Cryptography (specifically ``TypeError`` and + its internal ``UnsupportedAlgorithm``). These exception classes will now + become `~paramiko.exceptions.SSHException` instances instead of bubbling up. + Thanks to Ignat Semenov for the report and ``@tylergarcianet`` for an early + patch. - :bug:`-` (also :issue:`908`) Update `~paramiko.pkey.PKey` and subclasses to compare (``__eq__``) via direct field/attribute comparison instead of hashing (while retaining the existing behavior of ``__hash__`` via a slight From 6b1513e79a3244ccd5879fdd3399ea97f87f16f9 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 28 Nov 2021 15:25:33 -0500 Subject: [PATCH 068/117] Catch TypeError and UnsupportedAlgorithm when using Cryptography to load private key material Prior to this change, these exceptions bubble up as-is instead of becoming SSHException instances like most other key-loading errors --- paramiko/ecdsakey.py | 9 +++++++-- paramiko/rsakey.py | 4 ++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/paramiko/ecdsakey.py b/paramiko/ecdsakey.py index 05bd10f96..b609d1302 100644 --- a/paramiko/ecdsakey.py +++ b/paramiko/ecdsakey.py @@ -20,7 +20,7 @@ ECDSA keys """ -from cryptography.exceptions import InvalidSignature +from cryptography.exceptions import InvalidSignature, UnsupportedAlgorithm from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import hashes, serialization from cryptography.hazmat.primitives.asymmetric import ec @@ -288,7 +288,12 @@ def _decode_key(self, data): key = serialization.load_der_private_key( data, password=None, backend=default_backend() ) - except (ValueError, AssertionError) as e: + except ( + ValueError, + AssertionError, + TypeError, + UnsupportedAlgorithm, + ) as e: raise SSHException(str(e)) elif pkformat == self._PRIVATE_KEY_FORMAT_OPENSSH: try: diff --git a/paramiko/rsakey.py b/paramiko/rsakey.py index 142dd278d..292d0ccc5 100644 --- a/paramiko/rsakey.py +++ b/paramiko/rsakey.py @@ -20,7 +20,7 @@ RSA keys. """ -from cryptography.exceptions import InvalidSignature +from cryptography.exceptions import InvalidSignature, UnsupportedAlgorithm from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import hashes, serialization from cryptography.hazmat.primitives.asymmetric import rsa, padding @@ -185,7 +185,7 @@ def _decode_key(self, data): key = serialization.load_der_private_key( data, password=None, backend=default_backend() ) - except ValueError as e: + except (ValueError, TypeError, UnsupportedAlgorithm) as e: raise SSHException(str(e)) elif pkformat == self._PRIVATE_KEY_FORMAT_OPENSSH: n, e, d, iqmp, p, q = self._uint32_cstruct_unpack(data, "iiiiii") From c81ffd0c41e76ea7c65c2f31c89371ca866aae1f Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 28 Nov 2021 15:52:23 -0500 Subject: [PATCH 069/117] Changelog re #1024 --- sites/www/changelog.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index de2319ea0..38a29606d 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,9 @@ Changelog ========= +- :bug:`1024` Deleting items from `~paramiko.hostkeys.HostKeys` would + incorrectly raise `KeyError` even for valid keys, due to a logic bug. This + has been fixed. Report & patch credit: Jia Zhang. - :bug:`1257` (also :issue:`1266`) Update RSA and ECDSA key decoding subroutines to correctly catch exception types thrown by modern versions of Cryptography (specifically ``TypeError`` and From 3bed33ab150cb1a858899b17a116ca56cbcaacae Mon Sep 17 00:00:00 2001 From: Martin Packman Date: Tue, 6 Jun 2017 20:55:35 +0100 Subject: [PATCH 070/117] Expose Python 2.6 compatible test skip decorator --- tests/__init__.py | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/tests/__init__.py b/tests/__init__.py index be1d2daa3..75350a5ca 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1 +1,39 @@ -# This file's just here so test modules can use explicit-relative imports. +# Copyright (C) 2017 Martin Packman +# +# This file is part of paramiko. +# +# Paramiko is free software; you can redistribute it and/or modify it under the +# terms of the GNU Lesser General Public License as published by the Free +# Software Foundation; either version 2.1 of the License, or (at your option) +# any later version. +# +# Paramiko is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more +# details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with Paramiko; if not, write to the Free Software Foundation, Inc., +# 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA. + +"""Base classes and helpers for testing paramiko.""" + +import unittest + +from paramiko.py3compat import ( + builtins, + ) + + +skip = getattr(unittest, "skip", None) +if skip is None: + def skip(reason): + """Stub skip decorator for Python 2.6 compatibility.""" + return lambda func: None + + +def skipUnlessBuiltin(name): + """Skip decorated test if builtin name does not exist.""" + if getattr(builtins, name, None) is None: + return skip("No builtin " + repr(name)) + return lambda func: func From 54cbd9b27c30db5ac08c18ad8aadd2075a078a22 Mon Sep 17 00:00:00 2001 From: Martin Packman Date: Tue, 6 Jun 2017 20:58:13 +0100 Subject: [PATCH 071/117] Add new requireNonAsciiLocale test decorator --- tests/__init__.py | 48 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/__init__.py b/tests/__init__.py index 75350a5ca..e767c090f 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -18,6 +18,9 @@ """Base classes and helpers for testing paramiko.""" +import functools +import locale +import os import unittest from paramiko.py3compat import ( @@ -37,3 +40,48 @@ def skipUnlessBuiltin(name): if getattr(builtins, name, None) is None: return skip("No builtin " + repr(name)) return lambda func: func + + +# List of locales which have non-ascii characters in all categories. +# Omits most European languages which for instance may have only some months +# with names that include accented characters. +_non_ascii_locales = [ + # East Asian locales + "ja_JP", "ko_KR", "zh_CN", "zh_TW", + # European locales with non-latin alphabets + "el_GR", "ru_RU", "uk_UA", +] +# Also include UTF-8 versions of these locales +_non_ascii_locales.extend([name + ".utf8" for name in _non_ascii_locales]) + + +def requireNonAsciiLocale(category_name="LC_ALL"): + """Run decorated test under a non-ascii locale or skip if not possible.""" + if os.name != "posix": + return skip("Non-posix OSes don't really use C locales") + cat = getattr(locale, category_name) + return functools.partial(_decorate_with_locale, cat, _non_ascii_locales) + + +def _decorate_with_locale(category, try_locales, test_method): + """Decorate test_method to run after switching to a different locale.""" + + def _test_under_locale(testself): + original = locale.setlocale(category) + while try_locales: + try: + locale.setlocale(category, try_locales[0]) + except locale.Error: + # Mutating original list is ok, setlocale would keep failing + try_locales.pop(0) + else: + try: + return test_method(testself) + finally: + locale.setlocale(category, original) + skipTest = getattr(testself, "skipTest", None) + if skipTest is not None: + skipTest("No usable locales installed") + + functools.update_wrapper(_test_under_locale, test_method) + return _test_under_locale From ba8c57f54d359de89920c1dbb7355ec2cbd5ed3a Mon Sep 17 00:00:00 2001 From: Martin Packman Date: Tue, 6 Jun 2017 21:25:15 +0100 Subject: [PATCH 072/117] Fix failure in listdir when server uses a locale Fixes #985 SFTPAttributes uses the locale-aware %b directive for the abbreviated month name with time.strftime, but was not decoding the result on Python 2.7. Add a strftime alias in py3compat that will always return unicode, and resolve the mixing of bytes and text in SFTPAttributes methods. Add a test at the listdir level, and a more specific test for the SFTPAttributes asbytes method. --- paramiko/py3compat.py | 18 ++++++++++++++++-- paramiko/sftp_attr.py | 23 +++++++++++++---------- tests/test_sftp.py | 22 ++++++++++++++++++++++ 3 files changed, 51 insertions(+), 12 deletions(-) diff --git a/paramiko/py3compat.py b/paramiko/py3compat.py index 0f80e19fa..0330abac6 100644 --- a/paramiko/py3compat.py +++ b/paramiko/py3compat.py @@ -1,5 +1,6 @@ -import sys import base64 +import sys +import time __all__ = [ "BytesIO", @@ -29,6 +30,9 @@ PY2 = sys.version_info[0] < 3 if PY2: + import __builtin__ as builtins + import locale + string_types = basestring # NOQA text_type = unicode # NOQA bytes_types = str @@ -39,7 +43,10 @@ decodebytes = base64.decodestring encodebytes = base64.encodestring - import __builtin__ as builtins + def bytestring(s): # NOQA + if isinstance(s, unicode): # NOQA + return s.encode('utf-8') + return s byte_ord = ord # NOQA byte_chr = chr # NOQA @@ -100,6 +107,11 @@ def __len__(self): # 64-bit MAXSIZE = int((1 << 63) - 1) # NOQA del X + + def strftime(format, t): + """Same as time.strftime but returns unicode.""" + _, encoding = locale.getlocale(locale.LC_TIME) + return time.strftime(format, t).decode(encoding or 'ascii') else: import collections import struct @@ -167,3 +179,5 @@ def get_next(c): next = next MAXSIZE = sys.maxsize # NOQA + + strftime = time.strftime # NOQA diff --git a/paramiko/sftp_attr.py b/paramiko/sftp_attr.py index f16ac7463..1ad16349d 100644 --- a/paramiko/sftp_attr.py +++ b/paramiko/sftp_attr.py @@ -19,7 +19,7 @@ import stat import time from paramiko.common import x80000000, o700, o70, xffffffff -from paramiko.py3compat import long, b +from paramiko.py3compat import long, PY2, strftime class SFTPAttributes(object): @@ -169,7 +169,7 @@ def _rwx(n, suid, sticky=False): out += "-xSs"[suid + (n & 1)] return out - def __str__(self): + def _as_text(self): """create a unix-style long description of the file (like ls -l)""" if self.st_mode is not None: kind = stat.S_IFMT(self.st_mode) @@ -205,16 +205,13 @@ def __str__(self): # shouldn't really happen datestr = "(unknown date)" else: + time_tuple = time.localtime(self.st_mtime) if abs(time.time() - self.st_mtime) > 15552000: # (15552000 = 6 months) - datestr = time.strftime( - "%d %b %Y", time.localtime(self.st_mtime) - ) + datestr = strftime('%d %b %Y', time_tuple) else: - datestr = time.strftime( - "%d %b %H:%M", time.localtime(self.st_mtime) - ) - filename = getattr(self, "filename", "?") + datestr = strftime('%d %b %H:%M', time_tuple) + filename = getattr(self, 'filename', '?') # not all servers support uid/gid uid = self.st_uid @@ -240,4 +237,10 @@ def __str__(self): ) def asbytes(self): - return b(str(self)) + return self._as_text().encode('utf-8') + + if PY2: + __unicode__ = _as_text + __str__ = asbytes + else: + __str__ = _as_text diff --git a/tests/test_sftp.py b/tests/test_sftp.py index a98a46c66..84c5252b1 100644 --- a/tests/test_sftp.py +++ b/tests/test_sftp.py @@ -34,6 +34,11 @@ from paramiko.py3compat import PY2, b, u, StringIO from paramiko.common import o777, o600, o666, o644 +from tests import requireNonAsciiLocale, skipUnlessBuiltin +from tests.stub_sftp import StubServer, StubSFTPServer +from tests.loop import LoopSocket +from tests.util import test_path +import paramiko.util from paramiko.sftp_attr import SFTPAttributes from .util import needs_builtin @@ -270,6 +275,16 @@ def test_listdir_iter(self, sftp): sftp.remove(sftp.FOLDER + "/fish.txt") sftp.remove(sftp.FOLDER + "/tertiary.py") + @requireNonAsciiLocale() + def test_listdir_in_locale(self): + """Test listdir under a locale that uses non-ascii text.""" + sftp.open(FOLDER + '/canard.txt', 'w').close() + try: + folder_contents = sftp.listdir(FOLDER) + self.assertEqual(['canard.txt'], folder_contents) + finally: + sftp.remove(FOLDER + '/canard.txt') + def test_setstat(self, sftp): """ verify that the setstat functions (chown, chmod, utime, truncate) work. @@ -781,6 +796,13 @@ def test_non_utf8_data(self, sftp): finally: sftp.remove("%s/nonutf8data" % sftp.FOLDER) + @requireNonAsciiLocale('LC_TIME') + def test_sftp_attributes_locale_time(self): + """Test SFTPAttributes under a locale with non-ascii time strings.""" + some_stat = os.stat(sftp.FOLDER) + sftp_attributes = SFTPAttributes.from_stat(some_stat, u('a_directory')) + self.assertTrue(b'a_directory' in sftp_attributes.asbytes()) + def test_sftp_attributes_empty_str(self, sftp): sftp_attributes = SFTPAttributes() assert ( From 854583de6554d86a7333956a75dce562acbbc2ee Mon Sep 17 00:00:00 2001 From: Martin Packman Date: Sun, 11 Jun 2017 16:45:03 +0100 Subject: [PATCH 073/117] Add changelog entry for #992 --- sites/www/changelog.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 38a29606d..998147a0c 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,10 @@ Changelog ========= +* :bug:`985` (via :issue:`992`) Fix listdir failure when server uses a locale. + Now on Python 2.7 `SFTPAttributes ` will + decode abbreviated month names correctly rather than raise + ``UnicodeDecodeError```. - :bug:`1024` Deleting items from `~paramiko.hostkeys.HostKeys` would incorrectly raise `KeyError` even for valid keys, due to a logic bug. This has been fixed. Report & patch credit: Jia Zhang. From bbdf8040f5c24afd6e26089a3eb7da5f774f8e8b Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 28 Nov 2021 20:19:13 -0500 Subject: [PATCH 074/117] Cleanup re #992 - remove apparently vestigial bits - remove Python 2.6 related bits - use pytest.skip instead of custom skip --- tests/__init__.py | 37 +------------------------------------ tests/test_sftp.py | 16 ++++++---------- 2 files changed, 7 insertions(+), 46 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index e767c090f..7ded9f18a 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,45 +1,10 @@ -# Copyright (C) 2017 Martin Packman -# -# This file is part of paramiko. -# -# Paramiko is free software; you can redistribute it and/or modify it under the -# terms of the GNU Lesser General Public License as published by the Free -# Software Foundation; either version 2.1 of the License, or (at your option) -# any later version. -# -# Paramiko is distributed in the hope that it will be useful, but WITHOUT ANY -# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR -# A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more -# details. -# -# You should have received a copy of the GNU Lesser General Public License -# along with Paramiko; if not, write to the Free Software Foundation, Inc., -# 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA. - """Base classes and helpers for testing paramiko.""" import functools import locale import os -import unittest - -from paramiko.py3compat import ( - builtins, - ) - - -skip = getattr(unittest, "skip", None) -if skip is None: - def skip(reason): - """Stub skip decorator for Python 2.6 compatibility.""" - return lambda func: None - -def skipUnlessBuiltin(name): - """Skip decorated test if builtin name does not exist.""" - if getattr(builtins, name, None) is None: - return skip("No builtin " + repr(name)) - return lambda func: func +from pytest import skip # List of locales which have non-ascii characters in all categories. diff --git a/tests/test_sftp.py b/tests/test_sftp.py index 84c5252b1..1d6048bb2 100644 --- a/tests/test_sftp.py +++ b/tests/test_sftp.py @@ -34,11 +34,7 @@ from paramiko.py3compat import PY2, b, u, StringIO from paramiko.common import o777, o600, o666, o644 -from tests import requireNonAsciiLocale, skipUnlessBuiltin -from tests.stub_sftp import StubServer, StubSFTPServer -from tests.loop import LoopSocket -from tests.util import test_path -import paramiko.util +from tests import requireNonAsciiLocale from paramiko.sftp_attr import SFTPAttributes from .util import needs_builtin @@ -276,14 +272,14 @@ def test_listdir_iter(self, sftp): sftp.remove(sftp.FOLDER + "/tertiary.py") @requireNonAsciiLocale() - def test_listdir_in_locale(self): + def test_listdir_in_locale(self, sftp): """Test listdir under a locale that uses non-ascii text.""" - sftp.open(FOLDER + '/canard.txt', 'w').close() + sftp.open(sftp.FOLDER + '/canard.txt', 'w').close() try: - folder_contents = sftp.listdir(FOLDER) + folder_contents = sftp.listdir(sftp.FOLDER) self.assertEqual(['canard.txt'], folder_contents) finally: - sftp.remove(FOLDER + '/canard.txt') + sftp.remove(sftp.FOLDER + '/canard.txt') def test_setstat(self, sftp): """ @@ -797,7 +793,7 @@ def test_non_utf8_data(self, sftp): sftp.remove("%s/nonutf8data" % sftp.FOLDER) @requireNonAsciiLocale('LC_TIME') - def test_sftp_attributes_locale_time(self): + def test_sftp_attributes_locale_time(self, sftp): """Test SFTPAttributes under a locale with non-ascii time strings.""" some_stat = os.stat(sftp.FOLDER) sftp_attributes = SFTPAttributes.from_stat(some_stat, u('a_directory')) From 81f49ed4fcd46685002d4dd8b299a6c8fb3b2367 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 28 Nov 2021 20:22:40 -0500 Subject: [PATCH 075/117] Blacken code for #992 --- paramiko/py3compat.py | 8 +++++--- paramiko/sftp_attr.py | 8 ++++---- tests/__init__.py | 9 +++++++-- tests/test_sftp.py | 12 ++++++------ 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/paramiko/py3compat.py b/paramiko/py3compat.py index 0330abac6..d6d136fbd 100644 --- a/paramiko/py3compat.py +++ b/paramiko/py3compat.py @@ -45,7 +45,7 @@ def bytestring(s): # NOQA if isinstance(s, unicode): # NOQA - return s.encode('utf-8') + return s.encode("utf-8") return s byte_ord = ord # NOQA @@ -111,7 +111,9 @@ def __len__(self): def strftime(format, t): """Same as time.strftime but returns unicode.""" _, encoding = locale.getlocale(locale.LC_TIME) - return time.strftime(format, t).decode(encoding or 'ascii') + return time.strftime(format, t).decode(encoding or "ascii") + + else: import collections import struct @@ -180,4 +182,4 @@ def get_next(c): MAXSIZE = sys.maxsize # NOQA - strftime = time.strftime # NOQA + strftime = time.strftime # NOQA diff --git a/paramiko/sftp_attr.py b/paramiko/sftp_attr.py index 1ad16349d..8b1c17bd5 100644 --- a/paramiko/sftp_attr.py +++ b/paramiko/sftp_attr.py @@ -208,10 +208,10 @@ def _as_text(self): time_tuple = time.localtime(self.st_mtime) if abs(time.time() - self.st_mtime) > 15552000: # (15552000 = 6 months) - datestr = strftime('%d %b %Y', time_tuple) + datestr = strftime("%d %b %Y", time_tuple) else: - datestr = strftime('%d %b %H:%M', time_tuple) - filename = getattr(self, 'filename', '?') + datestr = strftime("%d %b %H:%M", time_tuple) + filename = getattr(self, "filename", "?") # not all servers support uid/gid uid = self.st_uid @@ -237,7 +237,7 @@ def _as_text(self): ) def asbytes(self): - return self._as_text().encode('utf-8') + return self._as_text().encode("utf-8") if PY2: __unicode__ = _as_text diff --git a/tests/__init__.py b/tests/__init__.py index 7ded9f18a..db5ade5a9 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -12,9 +12,14 @@ # with names that include accented characters. _non_ascii_locales = [ # East Asian locales - "ja_JP", "ko_KR", "zh_CN", "zh_TW", + "ja_JP", + "ko_KR", + "zh_CN", + "zh_TW", # European locales with non-latin alphabets - "el_GR", "ru_RU", "uk_UA", + "el_GR", + "ru_RU", + "uk_UA", ] # Also include UTF-8 versions of these locales _non_ascii_locales.extend([name + ".utf8" for name in _non_ascii_locales]) diff --git a/tests/test_sftp.py b/tests/test_sftp.py index 1d6048bb2..6134d070d 100644 --- a/tests/test_sftp.py +++ b/tests/test_sftp.py @@ -274,12 +274,12 @@ def test_listdir_iter(self, sftp): @requireNonAsciiLocale() def test_listdir_in_locale(self, sftp): """Test listdir under a locale that uses non-ascii text.""" - sftp.open(sftp.FOLDER + '/canard.txt', 'w').close() + sftp.open(sftp.FOLDER + "/canard.txt", "w").close() try: folder_contents = sftp.listdir(sftp.FOLDER) - self.assertEqual(['canard.txt'], folder_contents) + self.assertEqual(["canard.txt"], folder_contents) finally: - sftp.remove(sftp.FOLDER + '/canard.txt') + sftp.remove(sftp.FOLDER + "/canard.txt") def test_setstat(self, sftp): """ @@ -792,12 +792,12 @@ def test_non_utf8_data(self, sftp): finally: sftp.remove("%s/nonutf8data" % sftp.FOLDER) - @requireNonAsciiLocale('LC_TIME') + @requireNonAsciiLocale("LC_TIME") def test_sftp_attributes_locale_time(self, sftp): """Test SFTPAttributes under a locale with non-ascii time strings.""" some_stat = os.stat(sftp.FOLDER) - sftp_attributes = SFTPAttributes.from_stat(some_stat, u('a_directory')) - self.assertTrue(b'a_directory' in sftp_attributes.asbytes()) + sftp_attributes = SFTPAttributes.from_stat(some_stat, u("a_directory")) + self.assertTrue(b"a_directory" in sftp_attributes.asbytes()) def test_sftp_attributes_empty_str(self, sftp): sftp_attributes = SFTPAttributes() From 4081c783e62966e35ec7c4a18e8b7cf508d07e9f Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 28 Nov 2021 20:23:44 -0500 Subject: [PATCH 076/117] Credit re #992 --- sites/www/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 998147a0c..0bcc539d9 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -5,7 +5,7 @@ Changelog * :bug:`985` (via :issue:`992`) Fix listdir failure when server uses a locale. Now on Python 2.7 `SFTPAttributes ` will decode abbreviated month names correctly rather than raise - ``UnicodeDecodeError```. + ``UnicodeDecodeError```. Patch courtesy of Martin Packman. - :bug:`1024` Deleting items from `~paramiko.hostkeys.HostKeys` would incorrectly raise `KeyError` even for valid keys, due to a logic bug. This has been fixed. Report & patch credit: Jia Zhang. From d01691cb72c7b9479b844e1eb3ae00ef76c0ccea Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 28 Nov 2021 21:26:14 -0500 Subject: [PATCH 077/117] Changelog format fix re #992 --- sites/www/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 0bcc539d9..352d9bcc7 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,7 +2,7 @@ Changelog ========= -* :bug:`985` (via :issue:`992`) Fix listdir failure when server uses a locale. +- :bug:`985` (via :issue:`992`) Fix listdir failure when server uses a locale. Now on Python 2.7 `SFTPAttributes ` will decode abbreviated month names correctly rather than raise ``UnicodeDecodeError```. Patch courtesy of Martin Packman. From c648199836db920cf9828d66880100f9d67dd359 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 28 Nov 2021 21:30:48 -0500 Subject: [PATCH 078/117] Fix changelog reference --- sites/www/changelog.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 352d9bcc7..477fab985 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -13,9 +13,9 @@ Changelog subroutines to correctly catch exception types thrown by modern versions of Cryptography (specifically ``TypeError`` and its internal ``UnsupportedAlgorithm``). These exception classes will now - become `~paramiko.exceptions.SSHException` instances instead of bubbling up. - Thanks to Ignat Semenov for the report and ``@tylergarcianet`` for an early - patch. + become `~paramiko.ssh_exception.SSHException` instances instead of bubbling + up. Thanks to Ignat Semenov for the report and ``@tylergarcianet`` for an + early patch. - :bug:`-` (also :issue:`908`) Update `~paramiko.pkey.PKey` and subclasses to compare (``__eq__``) via direct field/attribute comparison instead of hashing (while retaining the existing behavior of ``__hash__`` via a slight From 6346662358b5b0ef16706ed6f23c2282507f7bb8 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 28 Nov 2021 21:35:28 -0500 Subject: [PATCH 079/117] Cut 2.8.1 --- paramiko/_version.py | 2 +- sites/www/changelog.rst | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/paramiko/_version.py b/paramiko/_version.py index 07aff82b9..0f0c65612 100644 --- a/paramiko/_version.py +++ b/paramiko/_version.py @@ -1,2 +1,2 @@ -__version_info__ = (2, 8, 0) +__version_info__ = (2, 8, 1) __version__ = ".".join(map(str, __version_info__)) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 477fab985..c423f5a5f 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,7 @@ Changelog ========= +- :release:`2.8.1 <2021-11-28>` - :bug:`985` (via :issue:`992`) Fix listdir failure when server uses a locale. Now on Python 2.7 `SFTPAttributes ` will decode abbreviated month names correctly rather than raise From 14fd03ec45fed0546aa01b1ef5ccf84508680e02 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sun, 28 Nov 2021 23:04:59 -0500 Subject: [PATCH 080/117] No idea why the lack of this blew up on circle but not locally Probably the locales differ? sigh --- tests/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/__init__.py b/tests/__init__.py index db5ade5a9..9850f3c1a 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -36,7 +36,7 @@ def requireNonAsciiLocale(category_name="LC_ALL"): def _decorate_with_locale(category, try_locales, test_method): """Decorate test_method to run after switching to a different locale.""" - def _test_under_locale(testself): + def _test_under_locale(testself, sftp): original = locale.setlocale(category) while try_locales: try: From ea373f9f4c0b4e13936e16f8ae642b05a4ce21c8 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Thu, 9 Dec 2021 17:27:08 -0500 Subject: [PATCH 081/117] Weird typos introduced 2 years ago, bah Test currently passes on my workstation tho --- tests/test_transport.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_transport.py b/tests/test_transport.py index e21748965..e1e37e47e 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -245,7 +245,7 @@ def test_simple(self): self.assertEqual(True, self.tc.is_authenticated()) self.assertEqual(True, self.ts.is_authenticated()) - def testa_long_banner(self): + def test_long_banner(self): """ verify that a long banner doesn't mess up the handshake. """ @@ -339,7 +339,7 @@ def test_exec_command(self): self.assertEqual("This is on stderr.\n", f.readline()) self.assertEqual("", f.readline()) - def testa_channel_can_be_used_as_context_manager(self): + def test_channel_can_be_used_as_context_manager(self): """ verify that exec_command() does something reasonable. """ From 5bf2d8ae5de981883dcce49f2275d03f5a7decd6 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Wed, 8 Dec 2021 21:19:13 -0500 Subject: [PATCH 082/117] Longterm TODOs --- paramiko/auth_handler.py | 3 +++ paramiko/transport.py | 17 +++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/paramiko/auth_handler.py b/paramiko/auth_handler.py index 5c7d6be68..011e57f32 100644 --- a/paramiko/auth_handler.py +++ b/paramiko/auth_handler.py @@ -272,6 +272,9 @@ def _parse_service_request(self, m): def _parse_service_accept(self, m): service = m.get_text() if service == "ssh-userauth": + # TODO 3.0: this message sucks ass. change it to something more + # obvious. it always appears to mean "we already authed" but no! it + # just means "we are allowed to TRY authing!" self._log(DEBUG, "userauth is OK") m = Message() m.add_byte(cMSG_USERAUTH_REQUEST) diff --git a/paramiko/transport.py b/paramiko/transport.py index 8919043f5..a09ed101e 100644 --- a/paramiko/transport.py +++ b/paramiko/transport.py @@ -2272,6 +2272,14 @@ def _send_kex_init(self): available_server_keys = list( filter( list(self.server_key_dict.keys()).__contains__, + # TODO: ensure tests will catch if somebody streamlines + # this by mistake - case is the admittedly silly one where + # the only calls to add_server_key() contain keys which + # were filtered out of the below via disabled_algorithms. + # If this is streamlined, we would then be allowing the + # disabled algorithm(s) for hostkey use + # TODO: honestly this prob just wants to get thrown out + # when we make kex configuration more straightforward self.preferred_keys, ) ) @@ -2291,6 +2299,9 @@ def _send_kex_init(self): m.add_list(self.preferred_compression) m.add_string(bytes()) m.add_string(bytes()) + # TODO: guess Robey never implemented the "guessing" part of the + # protocol. (Transport also never stores or acts on this flag's value + # in _parse_kex_init(), besides logging it to DEBUG.) m.add_boolean(False) m.add_int(0) # save a copy for later (needed to compute a hash) @@ -2351,6 +2362,9 @@ def _parse_kex_init(self, m): filter(kex_algo_list.__contains__, self.preferred_kex) ) if len(agreed_kex) == 0: + # TODO: do an auth-overhaul style aggregate exception here? + # TODO: would let us streamline log output & show all failures up + # front raise SSHException( "Incompatible ssh peer (no acceptable kex algorithm)" ) # noqa @@ -2877,6 +2891,9 @@ def _get_subsystem_handler(self, name): } +# TODO 3.0: drop this, we barely use it ourselves, it badly replicates the +# Transport-internal algorithm management, AND does so in a way which doesn't +# honor newer things like disabled_algorithms! class SecurityOptions(object): """ Simple object containing the security preferences of an ssh transport. From dfffaeaa0170c784307d1c89dad60528a59b6ff2 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Wed, 8 Dec 2021 21:20:04 -0500 Subject: [PATCH 083/117] Enhance kex DEBUG logging to be more readable The one-liner from 2005 is not cutting it, sorry --- paramiko/transport.py | 51 ++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/paramiko/transport.py b/paramiko/transport.py index a09ed101e..d4f0b149c 100644 --- a/paramiko/transport.py +++ b/paramiko/transport.py @@ -2176,7 +2176,7 @@ def _log_agreement(self, which, local, remote): # Log useful, non-duplicative line re: an agreed-upon algorithm. # Old code implied algorithms could be asymmetrical (different for # inbound vs outbound) so we preserve that possibility. - msg = "{} agreed: ".format(which) + msg = "{}: ".format(which) if local == remote: msg += local else: @@ -2323,31 +2323,27 @@ def _parse_kex_init(self, m): kex_follows = m.get_boolean() m.get_int() # unused - self._log( - DEBUG, - "kex algos:" - + str(kex_algo_list) - + " server key:" - + str(server_key_algo_list) - + " client encrypt:" - + str(client_encrypt_algo_list) - + " server encrypt:" - + str(server_encrypt_algo_list) - + " client mac:" - + str(client_mac_algo_list) - + " server mac:" - + str(server_mac_algo_list) - + " client compress:" - + str(client_compress_algo_list) - + " server compress:" - + str(server_compress_algo_list) - + " client lang:" - + str(client_lang_list) - + " server lang:" - + str(server_lang_list) - + " kex follows?" - + str(kex_follows), - ) + self._log(DEBUG, "=== Key exchange possibilities ===") + for prefix, value in ( + ("kex algos", kex_algo_list), + ("server key", server_key_algo_list), + # TODO: shouldn't these two lines say "cipher" to match usual + # terminology (including elsewhere in paramiko!)? + ("client encrypt", client_encrypt_algo_list), + ("server encrypt", server_encrypt_algo_list), + ("client mac", client_mac_algo_list), + ("server mac", server_mac_algo_list), + ("client compress", client_compress_algo_list), + ("server compress", server_compress_algo_list), + ("client lang", client_lang_list), + ("server lang", server_lang_list), + ): + if value == [""]: + value = [""] + value = ", ".join(value) + self._log(DEBUG, "{}: {}".format(prefix, value)) + self._log(DEBUG, "kex follows: {}".format(kex_follows)) + self._log(DEBUG, "=== Key exchange agreements ===") # as a server, we pick the first item in the client's list that we # support. @@ -2369,7 +2365,7 @@ def _parse_kex_init(self, m): "Incompatible ssh peer (no acceptable kex algorithm)" ) # noqa self.kex_engine = self._kex_info[agreed_kex[0]](self) - self._log(DEBUG, "Kex agreed: {}".format(agreed_kex[0])) + self._log(DEBUG, "Kex: {}".format(agreed_kex[0])) if self.server_mode: available_server_keys = list( @@ -2502,6 +2498,7 @@ def _parse_kex_init(self, m): local=self.local_compression, remote=self.remote_compression, ) + self._log(DEBUG, "=== End of kex handshake ===") # save for computing hash later... # now wait! openssh has a bug (and others might too) where there are From 363a28d94cada17f012c1604a3c99c71a2bda003 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Mon, 13 Dec 2021 15:55:36 -0500 Subject: [PATCH 084/117] Add support for RSA SHA2 host and public keys Includes a handful of refactors and new semiprivate attributes on Transport and AuthHandler for better test visibility. --- paramiko/__init__.py | 1 + paramiko/agent.py | 2 +- paramiko/auth_handler.py | 81 ++++++++--- paramiko/common.py | 5 +- paramiko/dsskey.py | 2 +- paramiko/ecdsakey.py | 2 +- paramiko/ed25519key.py | 2 +- paramiko/kex_curve25519.py | 4 +- paramiko/kex_ecdh_nist.py | 4 +- paramiko/kex_gex.py | 4 +- paramiko/kex_group1.py | 4 +- paramiko/pkey.py | 11 +- paramiko/rsakey.py | 28 +++- paramiko/ssh_exception.py | 15 +++ paramiko/transport.py | 179 +++++++++++++++++++----- sites/www/changelog.rst | 64 +++++++++ tests/test_kex.py | 3 +- tests/test_pkey.py | 26 ++-- tests/test_transport.py | 269 +++++++++++++++++++++++++++++++++++++ 19 files changed, 631 insertions(+), 75 deletions(-) diff --git a/paramiko/__init__.py b/paramiko/__init__.py index 8642f84af..5318cc9c5 100644 --- a/paramiko/__init__.py +++ b/paramiko/__init__.py @@ -42,6 +42,7 @@ ChannelException, ConfigParseError, CouldNotCanonicalize, + IncompatiblePeer, PasswordRequiredException, ProxyCommandFailure, SSHException, diff --git a/paramiko/agent.py b/paramiko/agent.py index c7c8b7cb6..3a02c06c6 100644 --- a/paramiko/agent.py +++ b/paramiko/agent.py @@ -411,7 +411,7 @@ def get_name(self): def _fields(self): raise NotImplementedError - def sign_ssh_data(self, data): + def sign_ssh_data(self, data, algorithm=None): msg = Message() msg.add_byte(cSSH2_AGENTC_SIGN_REQUEST) msg.add_string(self.blob) diff --git a/paramiko/auth_handler.py b/paramiko/auth_handler.py index 011e57f32..845b9143e 100644 --- a/paramiko/auth_handler.py +++ b/paramiko/auth_handler.py @@ -61,7 +61,7 @@ cMSG_USERAUTH_BANNER, ) from paramiko.message import Message -from paramiko.py3compat import b +from paramiko.py3compat import b, u from paramiko.ssh_exception import ( SSHException, AuthenticationException, @@ -206,6 +206,23 @@ def _disconnect_no_more_auth(self): self.transport._send_message(m) self.transport.close() + def _get_algorithm_and_bits(self, key): + """ + Given any key, return appropriate signing algorithm & bits-to-sign. + + Intended for input to or verification of, key signatures. + """ + key_type, bits = None, None + # Use certificate contents, if available, plain pubkey otherwise + if key.public_blob: + key_type = key.public_blob.key_type + bits = key.public_blob.key_blob + else: + key_type = key.get_name() + bits = key + algorithm = self._finalize_pubkey_algorithm(key_type) + return algorithm, bits + def _get_session_blob(self, key, service, username): m = Message() m.add_string(self.transport.session_id) @@ -214,13 +231,9 @@ def _get_session_blob(self, key, service, username): m.add_string(service) m.add_string("publickey") m.add_boolean(True) - # Use certificate contents, if available, plain pubkey otherwise - if key.public_blob: - m.add_string(key.public_blob.key_type) - m.add_string(key.public_blob.key_blob) - else: - m.add_string(key.get_name()) - m.add_string(key) + algorithm, bits = self._get_algorithm_and_bits(key) + m.add_string(algorithm) + m.add_string(bits) return m.asbytes() def wait_for_response(self, event): @@ -269,6 +282,39 @@ def _parse_service_request(self, m): # dunno this one self._disconnect_service_not_available() + def _finalize_pubkey_algorithm(self, key_type): + # Short-circuit for non-RSA keys + if "rsa" not in key_type: + return key_type + self._log( + DEBUG, + "Finalizing pubkey algorithm for key of type {!r}".format( + key_type + ), + ) + # 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)) + # Short-circuit negatively if user disabled all RSA algos (heh) + if not my_algos: + raise SSHException( + "An RSA key was specified, but no RSA pubkey algorithms are configured!" # noqa + ) + # Check for server-sig-algs if supported & sent + server_algos = u( + self.transport.server_extensions.get("server-sig-algs", b("")) + ).split(",") + self._log(DEBUG, "Server-side algorithm list: {}".format(server_algos)) + # Only use algos from our list that the server likes, in our own + # preference order. (NOTE: purposefully using same style as in + # Transport...expect to refactor later) + agreement = list(filter(server_algos.__contains__, my_algos)) + # Fallback: first one in our (possibly tweaked by caller) list + final = agreement[0] if agreement else my_algos[0] + self.transport._agreed_pubkey_algorithm = final + self._log(DEBUG, "Agreed upon {!r} pubkey algorithm".format(final)) + return final + def _parse_service_accept(self, m): service = m.get_text() if service == "ssh-userauth": @@ -287,18 +333,15 @@ def _parse_service_accept(self, m): m.add_string(password) elif self.auth_method == "publickey": m.add_boolean(True) - # Use certificate contents, if available, plain pubkey - # otherwise - if self.private_key.public_blob: - m.add_string(self.private_key.public_blob.key_type) - m.add_string(self.private_key.public_blob.key_blob) - else: - m.add_string(self.private_key.get_name()) - m.add_string(self.private_key) + algorithm, bits = self._get_algorithm_and_bits( + self.private_key + ) + m.add_string(algorithm) + m.add_string(bits) blob = self._get_session_blob( self.private_key, "ssh-connection", self.username ) - sig = self.private_key.sign_ssh_data(blob) + sig = self.private_key.sign_ssh_data(blob, algorithm) m.add_string(sig) elif self.auth_method == "keyboard-interactive": m.add_string("") @@ -529,13 +572,15 @@ def _parse_userauth_request(self, m): username, key ) if result != AUTH_FAILED: + sig_algo = self._finalize_pubkey_algorithm(keytype) # key is okay, verify it if not sig_attached: # client wants to know if this key is acceptable, before it # signs anything... send special "ok" message m = Message() m.add_byte(cMSG_USERAUTH_PK_OK) - m.add_string(keytype) + # TODO: suspect we're not testing this + m.add_string(sig_algo) m.add_string(keyblob) self.transport._send_message(m) return diff --git a/paramiko/common.py b/paramiko/common.py index 7bd0cb104..55dd4bdf2 100644 --- a/paramiko/common.py +++ b/paramiko/common.py @@ -29,7 +29,8 @@ MSG_DEBUG, MSG_SERVICE_REQUEST, MSG_SERVICE_ACCEPT, -) = range(1, 7) + MSG_EXT_INFO, +) = range(1, 8) (MSG_KEXINIT, MSG_NEWKEYS) = range(20, 22) ( MSG_USERAUTH_REQUEST, @@ -68,6 +69,7 @@ cMSG_DEBUG = byte_chr(MSG_DEBUG) cMSG_SERVICE_REQUEST = byte_chr(MSG_SERVICE_REQUEST) cMSG_SERVICE_ACCEPT = byte_chr(MSG_SERVICE_ACCEPT) +cMSG_EXT_INFO = byte_chr(MSG_EXT_INFO) cMSG_KEXINIT = byte_chr(MSG_KEXINIT) cMSG_NEWKEYS = byte_chr(MSG_NEWKEYS) cMSG_USERAUTH_REQUEST = byte_chr(MSG_USERAUTH_REQUEST) @@ -109,6 +111,7 @@ MSG_SERVICE_REQUEST: "service-request", MSG_SERVICE_ACCEPT: "service-accept", MSG_KEXINIT: "kexinit", + MSG_EXT_INFO: "ext-info", MSG_NEWKEYS: "newkeys", 30: "kex30", 31: "kex31", diff --git a/paramiko/dsskey.py b/paramiko/dsskey.py index 09d6f6483..1a0c47978 100644 --- a/paramiko/dsskey.py +++ b/paramiko/dsskey.py @@ -105,7 +105,7 @@ def get_bits(self): def can_sign(self): return self.x is not None - def sign_ssh_data(self, data): + def sign_ssh_data(self, data, algorithm=None): key = dsa.DSAPrivateNumbers( x=self.x, public_numbers=dsa.DSAPublicNumbers( diff --git a/paramiko/ecdsakey.py b/paramiko/ecdsakey.py index b609d1302..c4e2b1aff 100644 --- a/paramiko/ecdsakey.py +++ b/paramiko/ecdsakey.py @@ -211,7 +211,7 @@ def get_bits(self): def can_sign(self): return self.signing_key is not None - def sign_ssh_data(self, data): + def sign_ssh_data(self, data, algorithm=None): ecdsa = ec.ECDSA(self.ecdsa_curve.hash_object()) sig = self.signing_key.sign(data, ecdsa) r, s = decode_dss_signature(sig) diff --git a/paramiko/ed25519key.py b/paramiko/ed25519key.py index 7b19e3528..d322a0c13 100644 --- a/paramiko/ed25519key.py +++ b/paramiko/ed25519key.py @@ -191,7 +191,7 @@ def get_bits(self): def can_sign(self): return self._signing_key is not None - def sign_ssh_data(self, data): + def sign_ssh_data(self, data, algorithm=None): m = Message() m.add_string("ssh-ed25519") m.add_string(self._signing_key.sign(data).signature) diff --git a/paramiko/kex_curve25519.py b/paramiko/kex_curve25519.py index 59710c1aa..3420fb4f2 100644 --- a/paramiko/kex_curve25519.py +++ b/paramiko/kex_curve25519.py @@ -89,7 +89,9 @@ def _parse_kexecdh_init(self, m): hm.add_mpint(K) H = self.hash_algo(hm.asbytes()).digest() self.transport._set_K_H(K, H) - sig = self.transport.get_server_key().sign_ssh_data(H) + sig = self.transport.get_server_key().sign_ssh_data( + H, self.transport.host_key_type + ) # construct reply m = Message() m.add_byte(c_MSG_KEXECDH_REPLY) diff --git a/paramiko/kex_ecdh_nist.py b/paramiko/kex_ecdh_nist.py index ad5c9c79d..19de24313 100644 --- a/paramiko/kex_ecdh_nist.py +++ b/paramiko/kex_ecdh_nist.py @@ -90,7 +90,9 @@ def _parse_kexecdh_init(self, m): hm.add_mpint(long(K)) H = self.hash_algo(hm.asbytes()).digest() self.transport._set_K_H(K, H) - sig = self.transport.get_server_key().sign_ssh_data(H) + sig = self.transport.get_server_key().sign_ssh_data( + H, self.transport.host_key_type + ) # construct reply m = Message() m.add_byte(c_MSG_KEXECDH_REPLY) diff --git a/paramiko/kex_gex.py b/paramiko/kex_gex.py index fb8f01fdc..ab462e6d7 100644 --- a/paramiko/kex_gex.py +++ b/paramiko/kex_gex.py @@ -240,7 +240,9 @@ def _parse_kexdh_gex_init(self, m): H = self.hash_algo(hm.asbytes()).digest() self.transport._set_K_H(K, H) # sign it - sig = self.transport.get_server_key().sign_ssh_data(H) + sig = self.transport.get_server_key().sign_ssh_data( + H, self.transport.host_key_type + ) # send reply m = Message() m.add_byte(c_MSG_KEXDH_GEX_REPLY) diff --git a/paramiko/kex_group1.py b/paramiko/kex_group1.py index dce3fd91a..6d548b013 100644 --- a/paramiko/kex_group1.py +++ b/paramiko/kex_group1.py @@ -143,7 +143,9 @@ def _parse_kexdh_init(self, m): H = self.hash_algo(hm.asbytes()).digest() self.transport._set_K_H(K, H) # sign it - sig = self.transport.get_server_key().sign_ssh_data(H) + sig = self.transport.get_server_key().sign_ssh_data( + H, self.transport.host_key_type + ) # send reply m = Message() m.add_byte(c_MSG_KEXDH_REPLY) diff --git a/paramiko/pkey.py b/paramiko/pkey.py index 5bdfb1d43..7865a6eae 100644 --- a/paramiko/pkey.py +++ b/paramiko/pkey.py @@ -196,13 +196,20 @@ def get_base64(self): """ return u(encodebytes(self.asbytes())).replace("\n", "") - def sign_ssh_data(self, data): + def sign_ssh_data(self, data, algorithm=None): """ Sign a blob of data with this private key, and return a `.Message` representing an SSH signature message. - :param str data: the data to sign. + :param str data: + the data to sign. + :param str algorithm: + the signature algorithm to use, if different from the key's + internal name. Default: ``None``. :return: an SSH signature `message <.Message>`. + + .. versionchanged:: 2.9 + Added the ``algorithm`` kwarg. """ return bytes() diff --git a/paramiko/rsakey.py b/paramiko/rsakey.py index 292d0ccc5..26c5313cc 100644 --- a/paramiko/rsakey.py +++ b/paramiko/rsakey.py @@ -37,6 +37,15 @@ class RSAKey(PKey): data. """ + HASHES = { + "ssh-rsa": hashes.SHA1, + "ssh-rsa-cert-v01@openssh.com": hashes.SHA1, + "rsa-sha2-256": hashes.SHA256, + "rsa-sha2-256-cert-v01@openssh.com": hashes.SHA256, + "rsa-sha2-512": hashes.SHA512, + "rsa-sha2-512-cert-v01@openssh.com": hashes.SHA512, + } + def __init__( self, msg=None, @@ -61,6 +70,8 @@ def __init__( else: self._check_type_and_load_cert( 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", cert_type="ssh-rsa-cert-v01@openssh.com", ) @@ -111,18 +122,20 @@ def get_bits(self): def can_sign(self): return isinstance(self.key, rsa.RSAPrivateKey) - def sign_ssh_data(self, data): + def sign_ssh_data(self, data, algorithm="ssh-rsa"): sig = self.key.sign( - data, padding=padding.PKCS1v15(), algorithm=hashes.SHA1() + data, + padding=padding.PKCS1v15(), + algorithm=self.HASHES[algorithm](), ) - m = Message() - m.add_string("ssh-rsa") + m.add_string(algorithm) m.add_string(sig) return m def verify_ssh_sig(self, data, msg): - if msg.get_text() != "ssh-rsa": + sig_algorithm = msg.get_text() + if sig_algorithm not in self.HASHES: return False key = self.key if isinstance(key, rsa.RSAPrivateKey): @@ -130,7 +143,10 @@ def verify_ssh_sig(self, data, msg): try: key.verify( - msg.get_binary(), data, padding.PKCS1v15(), hashes.SHA1() + msg.get_binary(), + data, + padding.PKCS1v15(), + self.HASHES[sig_algorithm](), ) except InvalidSignature: return False diff --git a/paramiko/ssh_exception.py b/paramiko/ssh_exception.py index 2789be997..39fcb10dd 100644 --- a/paramiko/ssh_exception.py +++ b/paramiko/ssh_exception.py @@ -135,6 +135,21 @@ def __str__(self): ) +class IncompatiblePeer(SSHException): + """ + A disagreement arose regarding an algorithm required for key exchange. + + .. versionadded:: 2.9 + """ + + # TODO 3.0: consider making this annotate w/ 1..N 'missing' algorithms, + # either just the first one that would halt kex, or even updating the + # Transport logic so we record /all/ that /could/ halt kex. + # TODO: update docstrings where this may end up raised so they are more + # specific. + pass + + class ProxyCommandFailure(SSHException): """ The "ProxyCommand" found in the .ssh/config file returned an error. diff --git a/paramiko/transport.py b/paramiko/transport.py index d4f0b149c..b99b3278d 100644 --- a/paramiko/transport.py +++ b/paramiko/transport.py @@ -84,6 +84,8 @@ HIGHEST_USERAUTH_MESSAGE_ID, MSG_UNIMPLEMENTED, MSG_NAMES, + MSG_EXT_INFO, + cMSG_EXT_INFO, ) from paramiko.compress import ZlibCompressor, ZlibDecompressor from paramiko.dsskey import DSSKey @@ -107,6 +109,7 @@ SSHException, BadAuthenticationType, ChannelException, + IncompatiblePeer, ProxyCommandFailure, ) from paramiko.util import retry_on_signal, ClosingContextManager, clamp_value @@ -168,11 +171,25 @@ class Transport(threading.Thread, ClosingContextManager): "hmac-sha1-96", "hmac-md5-96", ) + # ~= HostKeyAlgorithms in OpenSSH land _preferred_keys = ( "ssh-ed25519", "ecdsa-sha2-nistp256", "ecdsa-sha2-nistp384", "ecdsa-sha2-nistp521", + "rsa-sha2-512", + "rsa-sha2-256", + "ssh-rsa", + "ssh-dss", + ) + # ~= PubKeyAcceptedAlgorithms + _preferred_pubkeys = ( + "ssh-ed25519", + "ecdsa-sha2-nistp256", + "ecdsa-sha2-nistp384", + "ecdsa-sha2-nistp521", + "rsa-sha2-512", + "rsa-sha2-256", "ssh-rsa", "ssh-dss", ) @@ -259,8 +276,16 @@ class Transport(threading.Thread, ClosingContextManager): } _key_info = { + # TODO: at some point we will want to drop this as it's no longer + # considered secure due to using SHA-1 for signatures. OpenSSH 8.8 no + # longer supports it. Question becomes at what point do we want to + # prevent users with older setups from using this? "ssh-rsa": RSAKey, "ssh-rsa-cert-v01@openssh.com": RSAKey, + "rsa-sha2-256": RSAKey, + "rsa-sha2-256-cert-v01@openssh.com": RSAKey, + "rsa-sha2-512": RSAKey, + "rsa-sha2-512-cert-v01@openssh.com": RSAKey, "ssh-dss": DSSKey, "ssh-dss-cert-v01@openssh.com": DSSKey, "ecdsa-sha2-nistp256": ECDSAKey, @@ -310,6 +335,7 @@ def __init__( gss_kex=False, gss_deleg_creds=True, disabled_algorithms=None, + server_sig_algs=True, ): """ Create a new SSH session over an existing socket, or socket-like @@ -372,6 +398,10 @@ def __init__( your code talks to a server which implements it differently from Paramiko), specify ``disabled_algorithms={"kex": ["diffie-hellman-group16-sha512"]}``. + :param bool server_sig_algs: + Whether to send an extra message to compatible clients, in server + mode, with a list of supported pubkey algorithms. Default: + ``True``. .. versionchanged:: 1.15 Added the ``default_window_size`` and ``default_max_packet_size`` @@ -380,9 +410,12 @@ def __init__( Added the ``gss_kex`` and ``gss_deleg_creds`` kwargs. .. versionchanged:: 2.6 Added the ``disabled_algorithms`` kwarg. + .. versionchanged:: 2.9 + Added the ``server_sig_algs`` kwarg. """ self.active = False self.hostname = None + self.server_extensions = {} if isinstance(sock, string_types): # convert "host:port" into (host, port) @@ -488,6 +521,7 @@ def __init__( # how long (seconds) to wait for the auth response. self.auth_timeout = 30 self.disabled_algorithms = disabled_algorithms or {} + self.server_sig_algs = server_sig_algs # server mode: self.server_mode = False @@ -517,6 +551,10 @@ def preferred_macs(self): def preferred_keys(self): return self._filter_algorithm("keys") + @property + def preferred_pubkeys(self): + return self._filter_algorithm("pubkeys") + @property def preferred_kex(self): return self._filter_algorithm("kex") @@ -743,6 +781,12 @@ def add_server_key(self, key): the host key to add, usually an `.RSAKey` or `.DSSKey`. """ self.server_key_dict[key.get_name()] = key + # Handle SHA-2 extensions for RSA by ensuring that lookups into + # self.server_key_dict will yield this key for any of the algorithm + # names. + if isinstance(key, RSAKey): + self.server_key_dict["rsa-sha2-256"] = key + self.server_key_dict["rsa-sha2-512"] = key def get_server_key(self): """ @@ -1280,7 +1324,17 @@ def connect( Added the ``gss_trust_dns`` argument. """ if hostkey is not None: - self._preferred_keys = [hostkey.get_name()] + # TODO: a more robust implementation would be to ask each key class + # for its nameS plural, and just use that. + # TODO: that could be used in a bunch of other spots too + if isinstance(hostkey, RSAKey): + self._preferred_keys = [ + "rsa-sha2-512", + "rsa-sha2-256", + "ssh-rsa", + ] + else: + self._preferred_keys = [hostkey.get_name()] self.set_gss_host( gss_host=gss_host, @@ -2126,7 +2180,12 @@ def run(self): self._send_message(msg) self.packetizer.complete_handshake() except SSHException as e: - self._log(ERROR, "Exception: " + str(e)) + self._log( + ERROR, + "Exception ({}): {}".format( + "server" if self.server_mode else "client", e + ), + ) self._log(ERROR, util.tb_strings()) self.saved_exception = e except EOFError as e: @@ -2237,7 +2296,7 @@ def _check_banner(self): client = segs[2] if version != "1.99" and version != "2.0": msg = "Incompatible version ({} instead of 2.0)" - raise SSHException(msg.format(version)) + raise IncompatiblePeer(msg.format(version)) msg = "Connected (version {}, client {})".format(version, client) self._log(INFO, msg) @@ -2253,13 +2312,10 @@ def _send_kex_init(self): self.clear_to_send_lock.release() self.gss_kex_used = False self.in_kex = True + kex_algos = list(self.preferred_kex) if self.server_mode: mp_required_prefix = "diffie-hellman-group-exchange-sha" - kex_mp = [ - k - for k in self.preferred_kex - if k.startswith(mp_required_prefix) - ] + kex_mp = [k for k in kex_algos if k.startswith(mp_required_prefix)] if (self._modulus_pack is None) and (len(kex_mp) > 0): # can't do group-exchange if we don't have a pack of potential # primes @@ -2285,11 +2341,16 @@ def _send_kex_init(self): ) else: available_server_keys = self.preferred_keys + # Signal support for MSG_EXT_INFO. + # NOTE: doing this here handily means we don't even consider this + # value when agreeing on real kex algo to use (which is a common + # pitfall when adding this apparently). + kex_algos.append("ext-info-c") m = Message() m.add_byte(cMSG_KEXINIT) m.add_bytes(os.urandom(16)) - m.add_list(self.preferred_kex) + m.add_list(kex_algos) m.add_list(available_server_keys) m.add_list(self.preferred_ciphers) m.add_list(self.preferred_ciphers) @@ -2299,29 +2360,49 @@ def _send_kex_init(self): m.add_list(self.preferred_compression) m.add_string(bytes()) m.add_string(bytes()) - # TODO: guess Robey never implemented the "guessing" part of the - # protocol. (Transport also never stores or acts on this flag's value - # in _parse_kex_init(), besides logging it to DEBUG.) m.add_boolean(False) m.add_int(0) # save a copy for later (needed to compute a hash) - self.local_kex_init = m.asbytes() + self.local_kex_init = self._latest_kex_init = m.asbytes() self._send_message(m) - def _parse_kex_init(self, m): + def _really_parse_kex_init(self, m, ignore_first_byte=False): + parsed = {} + if ignore_first_byte: + m.get_byte() m.get_bytes(16) # cookie, discarded - kex_algo_list = m.get_list() - server_key_algo_list = m.get_list() - client_encrypt_algo_list = m.get_list() - server_encrypt_algo_list = m.get_list() - client_mac_algo_list = m.get_list() - server_mac_algo_list = m.get_list() - client_compress_algo_list = m.get_list() - server_compress_algo_list = m.get_list() - client_lang_list = m.get_list() - server_lang_list = m.get_list() - kex_follows = m.get_boolean() + parsed["kex_algo_list"] = m.get_list() + parsed["server_key_algo_list"] = m.get_list() + parsed["client_encrypt_algo_list"] = m.get_list() + parsed["server_encrypt_algo_list"] = m.get_list() + parsed["client_mac_algo_list"] = m.get_list() + parsed["server_mac_algo_list"] = m.get_list() + parsed["client_compress_algo_list"] = m.get_list() + parsed["server_compress_algo_list"] = m.get_list() + parsed["client_lang_list"] = m.get_list() + parsed["server_lang_list"] = m.get_list() + parsed["kex_follows"] = m.get_boolean() m.get_int() # unused + return parsed + + def _get_latest_kex_init(self): + return self._really_parse_kex_init( + Message(self._latest_kex_init), ignore_first_byte=True + ) + + def _parse_kex_init(self, m): + parsed = self._really_parse_kex_init(m) + kex_algo_list = parsed["kex_algo_list"] + server_key_algo_list = parsed["server_key_algo_list"] + client_encrypt_algo_list = parsed["client_encrypt_algo_list"] + server_encrypt_algo_list = parsed["server_encrypt_algo_list"] + client_mac_algo_list = parsed["client_mac_algo_list"] + server_mac_algo_list = parsed["server_mac_algo_list"] + client_compress_algo_list = parsed["client_compress_algo_list"] + server_compress_algo_list = parsed["server_compress_algo_list"] + client_lang_list = parsed["client_lang_list"] + server_lang_list = parsed["server_lang_list"] + kex_follows = parsed["kex_follows"] self._log(DEBUG, "=== Key exchange possibilities ===") for prefix, value in ( @@ -2345,6 +2426,11 @@ def _parse_kex_init(self, m): self._log(DEBUG, "kex follows: {}".format(kex_follows)) self._log(DEBUG, "=== Key exchange agreements ===") + # Strip out ext-info "kex algo" + self._remote_ext_info = None + if kex_algo_list[-1].startswith("ext-info-"): + self._remote_ext_info = kex_algo_list.pop() + # as a server, we pick the first item in the client's list that we # support. # as a client, we pick the first item in our list that the server @@ -2361,7 +2447,7 @@ def _parse_kex_init(self, m): # TODO: do an auth-overhaul style aggregate exception here? # TODO: would let us streamline log output & show all failures up # front - raise SSHException( + raise IncompatiblePeer( "Incompatible ssh peer (no acceptable kex algorithm)" ) # noqa self.kex_engine = self._kex_info[agreed_kex[0]](self) @@ -2384,12 +2470,12 @@ def _parse_kex_init(self, m): filter(server_key_algo_list.__contains__, self.preferred_keys) ) if len(agreed_keys) == 0: - raise SSHException( + raise IncompatiblePeer( "Incompatible ssh peer (no acceptable host key)" ) # noqa self.host_key_type = agreed_keys[0] if self.server_mode and (self.get_server_key() is None): - raise SSHException( + raise IncompatiblePeer( "Incompatible ssh peer (can't match requested host key type)" ) # noqa self._log_agreement("HostKey", agreed_keys[0], agreed_keys[0]) @@ -2421,7 +2507,7 @@ def _parse_kex_init(self, m): ) ) if len(agreed_local_ciphers) == 0 or len(agreed_remote_ciphers) == 0: - raise SSHException( + raise IncompatiblePeer( "Incompatible ssh server (no acceptable ciphers)" ) # noqa self.local_cipher = agreed_local_ciphers[0] @@ -2445,7 +2531,9 @@ def _parse_kex_init(self, m): filter(server_mac_algo_list.__contains__, self.preferred_macs) ) if (len(agreed_local_macs) == 0) or (len(agreed_remote_macs) == 0): - raise SSHException("Incompatible ssh server (no acceptable macs)") + raise IncompatiblePeer( + "Incompatible ssh server (no acceptable macs)" + ) self.local_mac = agreed_local_macs[0] self.remote_mac = agreed_remote_macs[0] self._log_agreement( @@ -2484,7 +2572,7 @@ def _parse_kex_init(self, m): ): msg = "Incompatible ssh server (no acceptable compression)" msg += " {!r} {!r} {!r}" - raise SSHException( + raise IncompatiblePeer( msg.format( agreed_local_compression, agreed_remote_compression, @@ -2584,6 +2672,20 @@ def _activate_outbound(self): self.packetizer.set_outbound_compressor(compress_out()) if not self.packetizer.need_rekey(): self.in_kex = False + # If client indicated extension support, send that packet immediately + if ( + self.server_mode + and self.server_sig_algs + and self._remote_ext_info == "ext-info-c" + ): + extensions = {"server-sig-algs": ",".join(self.preferred_pubkeys)} + m = Message() + m.add_byte(cMSG_EXT_INFO) + m.add_int(len(extensions)) + for name, value in sorted(extensions.items()): + m.add_string(name) + m.add_string(value) + self._send_message(m) # we always expect to receive NEWKEYS now self._expect_packet(MSG_NEWKEYS) @@ -2599,6 +2701,20 @@ def _auth_trigger(self): self._log(DEBUG, "Switching on inbound compression ...") self.packetizer.set_inbound_compressor(compress_in()) + def _parse_ext_info(self, msg): + # Packet is a count followed by that many key-string to possibly-bytes + # pairs. + extensions = {} + for _ in range(msg.get_int()): + name = msg.get_text() + value = msg.get_string() + extensions[name] = value + self._log(DEBUG, "Got EXT_INFO: {}".format(extensions)) + # NOTE: this should work ok in cases where a server sends /two/ such + # messages; the RFC explicitly states a 2nd one should overwrite the + # 1st. + self.server_extensions = extensions + def _parse_newkeys(self, m): self._log(DEBUG, "Switch to new keys ...") self._activate_inbound() @@ -2866,6 +2982,7 @@ def _get_subsystem_handler(self, name): self.lock.release() _handler_table = { + MSG_EXT_INFO: _parse_ext_info, MSG_NEWKEYS: _parse_newkeys, MSG_GLOBAL_REQUEST: _parse_global_request, MSG_REQUEST_SUCCESS: _parse_request_success, diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index c423f5a5f..016a5ac9b 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,70 @@ Changelog ========= +- :feature:`1643` Add support for SHA-2 variants of RSA key verification + algorithms (as described in :rfc:`8332`) as well as limited SSH extension + negotiation (:rfc:`8308`). How SSH servers/clients decide when and how to use + this functionality can be complicated; Paramiko's support is as follows: + + - Client verification of server host key during key exchange will now prefer + ``rsa-sha2-512``, ``rsa-sha2-256``, and legacy ``ssh-rsa`` algorithms, in + that order, instead of just ``ssh-rsa``. + + - Note that the preference order of other algorithm families such as + ``ed25519`` and ``ecdsa`` has not changed; for example, those two + groups are still preferred over RSA. + + - Server mode will now offer all 3 RSA algorithms for host key verification + during key exchange, similar to client mode, if it has been configured with + an RSA host key. + - Client mode key exchange now sends the ``ext-info-c`` flag signaling + support for ``MSG_EXT_INFO``, and support for parsing the latter + (specifically, its ``server-sig-algs`` flag) has been added. + - Client mode, when performing public key authentication with an RSA key or + cert, will act as follows: + + - In all cases, the list of algorithms to consider is based on the new + ``preferred_pubkeys`` list (see below) and ``disabled_algorithms``; this + list, like with host keys, prefers SHA2-512, SHA2-256 and SHA1, in that + order. + - When the server does not send ``server-sig-algs``, Paramiko will attempt + the first algorithm in the above list. Clients connecting to legacy + servers should thus use ``disabled_algorithms`` to turn off SHA2. + - When the server does send ``server-sig-algs``, the first algorithm + supported by both ends is used, or if there is none, it falls back to the + previous behavior. + + - Server mode is now capable of pubkey auth involving SHA-2 signatures from + clients, provided one's server implementation actually provides for doing + so. + + - This includes basic support for sending ``MSG_EXT_INFO`` (containing + ``server-sig-algs`` only) to clients advertising ``ext-info-c`` in their + key exchange list. + + In order to implement the above, the following API additions were made: + + - `PKey.sign_ssh_data `: Grew an extra, optional + ``algorithm`` keyword argument (defaulting to ``None`` for most subclasses, + and to ``"ssh-rsa"`` for `~paramiko.rsakey.RSAKey`). + - A new `~paramiko.ssh_exception.SSHException` subclass was added, + `~paramiko.ssh_exception.IncompatiblePeer`, and is raised in all spots + where key exchange aborts due to algorithmic incompatibility. + + - Like all other exceptions in that module, it inherits from + ``SSHException``, and as we did not change anything else about the raising + (i.e. the attributes and message text are the same) this change is + backwards compatible. + + - `~paramiko.transport.Transport` grew a ``_preferred_pubkeys`` attribute and + matching ``preferred_pubkeys`` property to match the other, kex-focused, + such members. This allows client pubkey authentication to honor the + ``disabled_algorithms`` feature. + + Thanks to Krisztián Kovács for the report and an early stab at a patch, as + well as the numerous users who submitted feedback on the issue, including but + not limited to: Christopher Rabotin, Sam Bull, and Manfred Kaiser. + - :release:`2.8.1 <2021-11-28>` - :bug:`985` (via :issue:`992`) Fix listdir failure when server uses a locale. Now on Python 2.7 `SFTPAttributes ` will diff --git a/tests/test_kex.py b/tests/test_kex.py index c251611ac..b73989c20 100644 --- a/tests/test_kex.py +++ b/tests/test_kex.py @@ -76,7 +76,7 @@ def __str__(self): def asbytes(self): return b"fake-key" - def sign_ssh_data(self, H): + def sign_ssh_data(self, H, algorithm): return b"fake-sig" @@ -93,6 +93,7 @@ class FakeTransport(object): remote_version = "SSH-2.0-lame" local_kex_init = "local-kex-init" remote_kex_init = "remote-kex-init" + host_key_type = "fake-key" def _send_message(self, m): self._message = m diff --git a/tests/test_pkey.py b/tests/test_pkey.py index 94b2492bb..0cc201330 100644 --- a/tests/test_pkey.py +++ b/tests/test_pkey.py @@ -63,6 +63,8 @@ FINGER_ECDSA_384 = "384 c1:8d:a0:59:09:47:41:8e:a8:a6:07:01:29:23:b4:65" FINGER_ECDSA_521 = "521 44:58:22:52:12:33:16:0e:ce:0e:be:2c:7c:7e:cc:1e" SIGNED_RSA = "20:d7:8a:31:21:cb:f7:92:12:f2:a4:89:37:f5:78:af:e6:16:b6:25:b9:97:3d:a2:cd:5f:ca:20:21:73:4c:ad:34:73:8f:20:77:28:e2:94:15:08:d8:91:40:7a:85:83:bf:18:37:95:dc:54:1a:9b:88:29:6c:73:ca:38:b4:04:f1:56:b9:f2:42:9d:52:1b:29:29:b4:4f:fd:c9:2d:af:47:d2:40:76:30:f3:63:45:0c:d9:1d:43:86:0f:1c:70:e2:93:12:34:f3:ac:c5:0a:2f:14:50:66:59:f1:88:ee:c1:4a:e9:d1:9c:4e:46:f0:0e:47:6f:38:74:f1:44:a8" # noqa +SIGNED_RSA_256 = "cc:6:60:e0:0:2c:ac:9e:26:bc:d5:68:64:3f:9f:a7:e5:aa:41:eb:88:4a:25:5:9c:93:84:66:ef:ef:60:f4:34:fb:f4:c8:3d:55:33:6a:77:bd:b2:ee:83:f:71:27:41:7e:f5:7:5:0:a9:4c:7:80:6f:be:76:67:cb:58:35:b9:2b:f3:c2:d3:3c:ee:e1:3f:59:e0:fa:e4:5c:92:ed:ae:74:de:d:d6:27:16:8f:84:a3:86:68:c:94:90:7d:6e:cc:81:12:d8:b6:ad:aa:31:a8:13:3d:63:81:3e:bb:5:b6:38:4d:2:d:1b:5b:70:de:83:cc:3a:cb:31" # noqa +SIGNED_RSA_512 = "87:46:8b:75:92:33:78:a0:22:35:32:39:23:c6:ab:e1:6:92:ad:bc:7f:6e:ab:19:32:e4:78:b2:2c:8f:1d:c:65:da:fc:a5:7:ca:b6:55:55:31:83:b1:a0:af:d1:95:c5:2e:af:56:ba:f5:41:64:f:39:9d:af:82:43:22:8f:90:52:9d:89:e7:45:97:df:f3:f2:bc:7b:3a:db:89:e:34:fd:18:62:25:1b:ef:77:aa:c6:6c:99:36:3a:84:d6:9c:2a:34:8c:7f:f4:bb:c9:a5:9a:6c:11:f2:cf:da:51:5e:1e:7f:90:27:34:de:b2:f3:15:4f:db:47:32:6b:a7" # noqa FINGER_RSA_2K_OPENSSH = "2048 68:d1:72:01:bf:c0:0c:66:97:78:df:ce:75:74:46:d6" FINGER_DSS_1K_OPENSSH = "1024 cf:1d:eb:d7:61:d3:12:94:c6:c0:c6:54:35:35:b0:82" FINGER_EC_384_OPENSSH = "384 72:14:df:c1:9a:c3:e6:0e:11:29:d6:32:18:7b:ea:9b" @@ -238,21 +240,29 @@ def test_compare_dss(self): self.assertTrue(not pub.can_sign()) self.assertEqual(key, pub) - def test_sign_rsa(self): - # verify that the rsa private key can sign and verify + def _sign_and_verify_rsa(self, algorithm, saved_sig): key = RSAKey.from_private_key_file(_support("test_rsa.key")) - msg = key.sign_ssh_data(b"ice weasels") - self.assertTrue(type(msg) is Message) + msg = key.sign_ssh_data(b"ice weasels", algorithm) + assert isinstance(msg, Message) msg.rewind() - self.assertEqual("ssh-rsa", msg.get_text()) - sig = bytes().join( - [byte_chr(int(x, 16)) for x in SIGNED_RSA.split(":")] + assert msg.get_text() == algorithm + expected = bytes().join( + [byte_chr(int(x, 16)) for x in saved_sig.split(":")] ) - self.assertEqual(sig, msg.get_binary()) + assert msg.get_binary() == expected msg.rewind() pub = RSAKey(data=key.asbytes()) self.assertTrue(pub.verify_ssh_sig(b"ice weasels", msg)) + def test_sign_and_verify_ssh_rsa(self): + self._sign_and_verify_rsa("ssh-rsa", SIGNED_RSA) + + def test_sign_and_verify_rsa_sha2_512(self): + self._sign_and_verify_rsa("rsa-sha2-512", SIGNED_RSA_512) + + def test_sign_and_verify_rsa_sha2_256(self): + self._sign_and_verify_rsa("rsa-sha2-256", SIGNED_RSA_256) + def test_sign_dss(self): # verify that the dss private key can sign and verify key = DSSKey.from_private_key_file(_support("test_dss.key")) diff --git a/tests/test_transport.py b/tests/test_transport.py index e1e37e47e..6145e5cb6 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -23,6 +23,7 @@ from __future__ import with_statement from binascii import hexlify +from contextlib import contextmanager import select import socket import time @@ -38,6 +39,8 @@ Packetizer, RSAKey, SSHException, + AuthenticationException, + IncompatiblePeer, SecurityOptions, ServerInterface, Transport, @@ -80,6 +83,9 @@ class NullServer(ServerInterface): paranoid_did_public_key = False paranoid_key = DSSKey.from_private_key_file(_support("test_dss.key")) + def __init__(self, allowed_keys=None): + self.allowed_keys = allowed_keys if allowed_keys is not None else [] + def get_allowed_auths(self, username): if username == "slowdive": return "publickey,password" @@ -90,6 +96,11 @@ def check_auth_password(self, username, password): return AUTH_SUCCESSFUL return AUTH_FAILED + def check_auth_publickey(self, username, key): + if key in self.allowed_keys: + return AUTH_SUCCESSFUL + return AUTH_FAILED + def check_channel_request(self, kind, chanid): if kind == "bogus": return OPEN_FAILED_ADMINISTRATIVELY_PROHIBITED @@ -154,6 +165,7 @@ def tearDown(self): self.socks.close() self.sockc.close() + # TODO: unify with newer contextmanager def setup_test_server( self, client_options=None, server_options=None, connect_kwargs=None ): @@ -1169,3 +1181,260 @@ def test_implementation_refers_to_public_algo_lists(self): assert "ssh-dss" not in server_keys assert "diffie-hellman-group14-sha256" not in kexen assert "zlib" not in compressions + + +@contextmanager +def server( + hostkey=None, + init=None, + server_init=None, + client_init=None, + connect=None, + pubkeys=None, + catch_error=False, +): + """ + SSH server contextmanager for testing. + + :param hostkey: + Host key to use for the server; if None, loads + ``test_rsa.key``. + :param init: + Default `Transport` constructor kwargs to use for both sides. + :param server_init: + Extends and/or overrides ``init`` for server transport only. + :param client_init: + Extends and/or overrides ``init`` for client transport only. + :param connect: + Kwargs to use for ``connect()`` on the client. + :param pubkeys: + List of public keys for auth. + :param catch_error: + Whether to capture connection errors & yield from contextmanager. + Necessary for connection_time exception testing. + """ + if init is None: + init = {} + if server_init is None: + server_init = {} + if client_init is None: + client_init = {} + if connect is None: + connect = dict(username="slowdive", password="pygmalion") + socks = LoopSocket() + sockc = LoopSocket() + sockc.link(socks) + tc = Transport(sockc, **dict(init, **client_init)) + ts = Transport(socks, **dict(init, **server_init)) + + if hostkey is None: + hostkey = RSAKey.from_private_key_file(_support("test_rsa.key")) + ts.add_server_key(hostkey) + event = threading.Event() + server = NullServer(allowed_keys=pubkeys) + assert not event.is_set() + assert not ts.is_active() + assert tc.get_username() is None + assert ts.get_username() is None + assert not tc.is_authenticated() + assert not ts.is_authenticated() + + err = None + # Trap errors and yield instead of raising right away; otherwise callers + # cannot usefully deal with problems at connect time which stem from errors + # in the server side. + try: + ts.start_server(event, server) + tc.connect(**connect) + + event.wait(1.0) + assert event.is_set() + assert ts.is_active() + assert tc.is_active() + + except Exception as e: + if not catch_error: + raise + err = e + + yield (tc, ts, err) if catch_error else (tc, ts) + + tc.close() + ts.close() + socks.close() + sockc.close() + + +_disable_sha2 = dict( + disabled_algorithms=dict(keys=["rsa-sha2-256", "rsa-sha2-512"]) +) +_disable_sha1 = dict(disabled_algorithms=dict(keys=["ssh-rsa"])) +_disable_sha2_pubkey = dict( + disabled_algorithms=dict(pubkeys=["rsa-sha2-256", "rsa-sha2-512"]) +) +_disable_sha1_pubkey = dict(disabled_algorithms=dict(pubkeys=["ssh-rsa"])) + + +class TestSHA2SignatureKeyExchange(unittest.TestCase): + # NOTE: these all rely on the default server() hostkey being RSA + # NOTE: these rely on both sides being properly implemented re: agreed-upon + # hostkey during kex being what's actually used. Truly proving that eg + # SHA512 was used, is quite difficult w/o super gross hacks. However, there + # are new tests in test_pkey.py which use known signature blobs to prove + # the SHA2 family was in fact used! + + def test_base_case_ssh_rsa_still_used_as_fallback(self): + # Prove that ssh-rsa is used if either, or both, participants have SHA2 + # algorithms disabled + for which in ("init", "client_init", "server_init"): + with server(**{which: _disable_sha2}) as (tc, _): + assert tc.host_key_type == "ssh-rsa" + + def test_kex_with_sha2_512(self): + # It's the default! + with server() as (tc, _): + assert tc.host_key_type == "rsa-sha2-512" + + def test_kex_with_sha2_256(self): + # No 512 -> you get 256 + with server( + init=dict(disabled_algorithms=dict(keys=["rsa-sha2-512"])) + ) as (tc, _): + assert tc.host_key_type == "rsa-sha2-256" + + def _incompatible_peers(self, client_init, server_init): + with server( + client_init=client_init, server_init=server_init, catch_error=True + ) as (tc, ts, err): + # If neither side blew up then that's bad! + assert err is not None + # If client side blew up first, it'll be straightforward + if isinstance(err, IncompatiblePeer): + pass + # If server side blew up first, client sees EOF & we need to check + # the server transport for its saved error (otherwise it can only + # appear in log output) + elif isinstance(err, EOFError): + assert ts.saved_exception is not None + assert isinstance(ts.saved_exception, IncompatiblePeer) + # If it was something else, welp + else: + raise err + + def test_client_sha2_disabled_server_sha1_disabled_no_match(self): + self._incompatible_peers( + client_init=_disable_sha2, server_init=_disable_sha1 + ) + + def test_client_sha1_disabled_server_sha2_disabled_no_match(self): + self._incompatible_peers( + client_init=_disable_sha1, server_init=_disable_sha2 + ) + + def test_explicit_client_hostkey_not_limited(self): + # Be very explicit about the hostkey on BOTH ends, + # and ensure it still ends up choosing sha2-512. + # (This is a regression test vs previous implementation which overwrote + # the entire preferred-hostkeys structure when given an explicit key as + # a client.) + hostkey = RSAKey.from_private_key_file(_support("test_rsa.key")) + with server(hostkey=hostkey, connect=dict(hostkey=hostkey)) as (tc, _): + assert tc.host_key_type == "rsa-sha2-512" + + +class TestExtInfo(unittest.TestCase): + def test_ext_info_handshake(self): + with server() as (tc, _): + kex = tc._get_latest_kex_init() + assert kex["kex_algo_list"][-1] == "ext-info-c" + assert tc.server_extensions == { + "server-sig-algs": b"ssh-ed25519,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,rsa-sha2-512,rsa-sha2-256,ssh-rsa,ssh-dss" # noqa + } + + def test_client_uses_server_sig_algs_for_pubkey_auth(self): + privkey = RSAKey.from_private_key_file(_support("test_rsa.key")) + with server( + pubkeys=[privkey], + connect=dict(pkey=privkey), + server_init=dict( + disabled_algorithms=dict(pubkeys=["rsa-sha2-512"]) + ), + ) as (tc, _): + assert tc.is_authenticated() + # Client settled on 256 despite itself not having 512 disabled + assert tc._agreed_pubkey_algorithm == "rsa-sha2-256" + + +# TODO: these could move into test_auth.py but that badly needs refactoring +# with this module anyways... +class TestSHA2SignaturePubkeys(unittest.TestCase): + def test_pubkey_auth_honors_disabled_algorithms(self): + privkey = RSAKey.from_private_key_file(_support("test_rsa.key")) + with server( + pubkeys=[privkey], + connect=dict(pkey=privkey), + init=dict( + disabled_algorithms=dict( + pubkeys=["ssh-rsa", "rsa-sha2-256", "rsa-sha2-512"] + ) + ), + catch_error=True, + ) as (_, _, err): + assert isinstance(err, SSHException) + assert "no RSA pubkey algorithms" in str(err) + + def test_client_sha2_disabled_server_sha1_disabled_no_match(self): + privkey = RSAKey.from_private_key_file(_support("test_rsa.key")) + with server( + pubkeys=[privkey], + connect=dict(pkey=privkey), + client_init=_disable_sha2_pubkey, + server_init=_disable_sha1_pubkey, + catch_error=True, + ) as (tc, ts, err): + assert isinstance(err, AuthenticationException) + + def test_client_sha1_disabled_server_sha2_disabled_no_match(self): + privkey = RSAKey.from_private_key_file(_support("test_rsa.key")) + with server( + pubkeys=[privkey], + connect=dict(pkey=privkey), + client_init=_disable_sha1_pubkey, + server_init=_disable_sha2_pubkey, + catch_error=True, + ) as (tc, ts, err): + assert isinstance(err, AuthenticationException) + + def test_ssh_rsa_still_used_when_sha2_disabled(self): + privkey = RSAKey.from_private_key_file(_support("test_rsa.key")) + # NOTE: this works because key obj comparison uses public bytes + # TODO: would be nice for PKey to grow a legit "give me another obj of + # same class but just the public bits" using asbytes() + with server( + pubkeys=[privkey], connect=dict(pkey=privkey), init=_disable_sha2 + ) as (tc, _): + assert tc.is_authenticated() + + def test_sha2_512(self): + privkey = RSAKey.from_private_key_file(_support("test_rsa.key")) + with server( + pubkeys=[privkey], + connect=dict(pkey=privkey), + init=dict( + disabled_algorithms=dict(pubkeys=["ssh-rsa", "rsa-sha2-256"]) + ), + ) as (tc, ts): + assert tc.is_authenticated() + assert tc._agreed_pubkey_algorithm == "rsa-sha2-512" + + def test_sha2_256(self): + privkey = RSAKey.from_private_key_file(_support("test_rsa.key")) + with server( + pubkeys=[privkey], + connect=dict(pkey=privkey), + init=dict( + disabled_algorithms=dict(pubkeys=["ssh-rsa", "rsa-sha2-512"]) + ), + ) as (tc, ts): + assert tc.is_authenticated() + assert tc._agreed_pubkey_algorithm == "rsa-sha2-256" From 2b66625659e66858cb5f557325c5fdd9c35fd073 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Thu, 23 Dec 2021 15:13:54 -0500 Subject: [PATCH 085/117] Add agent RSA-SHA2 support, also tweak changelog w/ more tickets --- paramiko/agent.py | 14 +++++++++++- sites/www/changelog.rst | 12 ++++++---- tests/test_agent.py | 50 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 5 deletions(-) create mode 100644 tests/test_agent.py diff --git a/paramiko/agent.py b/paramiko/agent.py index 3a02c06c6..f28bf1283 100644 --- a/paramiko/agent.py +++ b/paramiko/agent.py @@ -42,6 +42,18 @@ cSSH2_AGENTC_SIGN_REQUEST = byte_chr(13) SSH2_AGENT_SIGN_RESPONSE = 14 +SSH_AGENT_RSA_SHA2_256 = 2 +SSH_AGENT_RSA_SHA2_512 = 4 +# NOTE: RFC mildly confusing; while these flags are OR'd together, OpenSSH at +# least really treats them like "AND"s, in the sense that if it finds the +# SHA256 flag set it won't continue looking at the SHA512 one; it +# short-circuits right away. +# Thus, we never want to eg submit 6 to say "either's good". +ALGORITHM_FLAG_MAP = { + "rsa-sha2-256": SSH_AGENT_RSA_SHA2_256, + "rsa-sha2-512": SSH_AGENT_RSA_SHA2_512, +} + class AgentSSH(object): def __init__(self): @@ -416,7 +428,7 @@ def sign_ssh_data(self, data, algorithm=None): msg.add_byte(cSSH2_AGENTC_SIGN_REQUEST) msg.add_string(self.blob) msg.add_string(data) - msg.add_int(0) + msg.add_int(ALGORITHM_FLAG_MAP.get(algorithm, 0)) ptype, result = self.agent._send_message(msg) if ptype != SSH2_AGENT_SIGN_RESPONSE: raise SSHException("key cannot be used for signing") diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 016a5ac9b..a519d3336 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,10 +2,11 @@ Changelog ========= -- :feature:`1643` Add support for SHA-2 variants of RSA key verification - algorithms (as described in :rfc:`8332`) as well as limited SSH extension - negotiation (:rfc:`8308`). How SSH servers/clients decide when and how to use - this functionality can be complicated; Paramiko's support is as follows: +- :feature:`1643` (also :issue:`1925`, :issue:`1644`, :issue:`1326`) Add + support for SHA-2 variants of RSA key verification algorithms (as described + in :rfc:`8332`) as well as limited SSH extension negotiation (:rfc:`8308`). + How SSH servers/clients decide when and how to use this functionality can be + complicated; Paramiko's support is as follows: - Client verification of server host key during key exchange will now prefer ``rsa-sha2-512``, ``rsa-sha2-256``, and legacy ``ssh-rsa`` algorithms, in @@ -35,6 +36,9 @@ Changelog supported by both ends is used, or if there is none, it falls back to the previous behavior. + - SSH agent support grew the ability to specify algorithm flags when + requesting private key signatures; this is now used to forward SHA2 + algorithms when appropriate. - Server mode is now capable of pubkey auth involving SHA-2 signatures from clients, provided one's server implementation actually provides for doing so. diff --git a/tests/test_agent.py b/tests/test_agent.py new file mode 100644 index 000000000..c3973bbb6 --- /dev/null +++ b/tests/test_agent.py @@ -0,0 +1,50 @@ +import unittest + +from paramiko.message import Message +from paramiko.agent import ( + SSH2_AGENT_SIGN_RESPONSE, + cSSH2_AGENTC_SIGN_REQUEST, + SSH_AGENT_RSA_SHA2_256, + SSH_AGENT_RSA_SHA2_512, + AgentKey, +) +from paramiko.py3compat import b + + +class ChaosAgent: + def _send_message(self, msg): + self._sent_message = msg + sig = Message() + sig.add_string(b("lol")) + sig.rewind() + return SSH2_AGENT_SIGN_RESPONSE, sig + + +class AgentTests(unittest.TestCase): + def _sign_with_agent(self, kwargs, expectation): + agent = ChaosAgent() + key = AgentKey(agent, b("secret!!!")) + result = key.sign_ssh_data(b("token"), **kwargs) + assert result == b("lol") + msg = agent._sent_message + msg.rewind() + assert msg.get_byte() == cSSH2_AGENTC_SIGN_REQUEST + assert msg.get_string() == b("secret!!!") + assert msg.get_string() == b("token") + assert msg.get_int() == expectation + + def test_agent_signing_defaults_to_0_for_flags_field(self): + # No algorithm kwarg at all + self._sign_with_agent(kwargs=dict(), expectation=0) + + def test_agent_signing_is_2_for_SHA256(self): + self._sign_with_agent( + kwargs=dict(algorithm="rsa-sha2-256"), + expectation=SSH_AGENT_RSA_SHA2_256, + ) + + def test_agent_signing_is_2_for_SHA512(self): + self._sign_with_agent( + kwargs=dict(algorithm="rsa-sha2-512"), + expectation=SSH_AGENT_RSA_SHA2_512, + ) From a88ea9b5e58ee0301be9cc48d3d7239a3e281c64 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Thu, 23 Dec 2021 16:24:40 -0500 Subject: [PATCH 086/117] Changelog format tweak --- sites/www/changelog.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index a519d3336..c10a35d80 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -57,9 +57,9 @@ Changelog where key exchange aborts due to algorithmic incompatibility. - Like all other exceptions in that module, it inherits from - ``SSHException``, and as we did not change anything else about the raising - (i.e. the attributes and message text are the same) this change is - backwards compatible. + ``SSHException``, and as we did not change anything else about the + raising (i.e. the attributes and message text are the same) this change + is backwards compatible. - `~paramiko.transport.Transport` grew a ``_preferred_pubkeys`` attribute and matching ``preferred_pubkeys`` property to match the other, kex-focused, From c42311a4b1c905c7a3ee129258490448e6e22203 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Thu, 23 Dec 2021 16:26:28 -0500 Subject: [PATCH 087/117] Cut 2.9.0 --- paramiko/_version.py | 2 +- sites/www/changelog.rst | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/paramiko/_version.py b/paramiko/_version.py index 0f0c65612..4e24c2f75 100644 --- a/paramiko/_version.py +++ b/paramiko/_version.py @@ -1,2 +1,2 @@ -__version_info__ = (2, 8, 1) +__version_info__ = (2, 9, 0) __version__ = ".".join(map(str, __version_info__)) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index c10a35d80..eeae86efe 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,7 @@ Changelog ========= +- :release:`2.9.0 <2021-12-23>` - :feature:`1643` (also :issue:`1925`, :issue:`1644`, :issue:`1326`) Add support for SHA-2 variants of RSA key verification algorithms (as described in :rfc:`8332`) as well as limited SSH extension negotiation (:rfc:`8308`). From 69fb31fcc14fef16b612d18b78016e74732b2de3 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 24 Dec 2021 12:58:25 -0500 Subject: [PATCH 088/117] Changelog and test re #1955 --- sites/www/changelog.rst | 4 ++++ tests/test_transport.py | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index eeae86efe..091fe1183 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,10 @@ Changelog ========= +- :bug:`1955` Server-side support for ``rsa-sha2-256`` and ``ssh-rsa`` wasn't + fully operable after 2.9.0's release (signatures for RSA pubkeys were always + run through ``rsa-sha2-512`` instead). Report and early stab at a fix + courtesy of Jun Omae. - :release:`2.9.0 <2021-12-23>` - :feature:`1643` (also :issue:`1925`, :issue:`1644`, :issue:`1326`) Add support for SHA-2 variants of RSA key verification algorithms (as described diff --git a/tests/test_transport.py b/tests/test_transport.py index 6145e5cb6..77ffd6c1f 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -1438,3 +1438,16 @@ def test_sha2_256(self): ) as (tc, ts): assert tc.is_authenticated() assert tc._agreed_pubkey_algorithm == "rsa-sha2-256" + + def test_sha2_256_when_client_only_enables_256(self): + privkey = RSAKey.from_private_key_file(_support("test_rsa.key")) + with server( + pubkeys=[privkey], + connect=dict(pkey=privkey), + # Client-side only; server still accepts all 3. + client_init=dict( + disabled_algorithms=dict(pubkeys=["ssh-rsa", "rsa-sha2-512"]) + ), + ) as (tc, ts): + assert tc.is_authenticated() + assert tc._agreed_pubkey_algorithm == "rsa-sha2-256" From d2865ed7c3364406b2f1f4f984dcdd315196a353 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 24 Dec 2021 13:30:06 -0500 Subject: [PATCH 089/117] Fix #1955 --- paramiko/auth_handler.py | 53 ++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/paramiko/auth_handler.py b/paramiko/auth_handler.py index 845b9143e..da109d7c7 100644 --- a/paramiko/auth_handler.py +++ b/paramiko/auth_handler.py @@ -206,24 +206,19 @@ def _disconnect_no_more_auth(self): self.transport._send_message(m) self.transport.close() - def _get_algorithm_and_bits(self, key): + def _get_key_type_and_bits(self, key): """ - Given any key, return appropriate signing algorithm & bits-to-sign. + Given any key, return its type/algorithm & bits-to-sign. Intended for input to or verification of, key signatures. """ - key_type, bits = None, None # Use certificate contents, if available, plain pubkey otherwise if key.public_blob: - key_type = key.public_blob.key_type - bits = key.public_blob.key_blob + return key.public_blob.key_type, key.public_blob.key_blob else: - key_type = key.get_name() - bits = key - algorithm = self._finalize_pubkey_algorithm(key_type) - return algorithm, bits + return key.get_name(), key - def _get_session_blob(self, key, service, username): + def _get_session_blob(self, key, service, username, algorithm): m = Message() m.add_string(self.transport.session_id) m.add_byte(cMSG_USERAUTH_REQUEST) @@ -231,7 +226,7 @@ def _get_session_blob(self, key, service, username): m.add_string(service) m.add_string("publickey") m.add_boolean(True) - algorithm, bits = self._get_algorithm_and_bits(key) + _, bits = self._get_key_type_and_bits(key) m.add_string(algorithm) m.add_string(bits) return m.asbytes() @@ -282,6 +277,17 @@ def _parse_service_request(self, m): # dunno this one self._disconnect_service_not_available() + def _generate_key_from_request(self, algorithm, keyblob): + # For use in server mode. + options = self.transport.preferred_pubkeys + if algorithm.replace("-cert-v01@openssh.com", "") not in options: + err = ( + "Auth rejected: pubkey algorithm '{}' unsupported or disabled" + ) + self._log(INFO, err.format(algorithm)) + return None + return self.transport._key_info[algorithm](Message(keyblob)) + def _finalize_pubkey_algorithm(self, key_type): # Short-circuit for non-RSA keys if "rsa" not in key_type: @@ -333,13 +339,15 @@ def _parse_service_accept(self, m): m.add_string(password) elif self.auth_method == "publickey": m.add_boolean(True) - algorithm, bits = self._get_algorithm_and_bits( - self.private_key - ) + key_type, bits = self._get_key_type_and_bits(self.private_key) + algorithm = self._finalize_pubkey_algorithm(key_type) m.add_string(algorithm) m.add_string(bits) blob = self._get_session_blob( - self.private_key, "ssh-connection", self.username + self.private_key, + "ssh-connection", + self.username, + algorithm, ) sig = self.private_key.sign_ssh_data(blob, algorithm) m.add_string(sig) @@ -551,10 +559,13 @@ def _parse_userauth_request(self, m): ) elif method == "publickey": sig_attached = m.get_boolean() - keytype = m.get_text() + # NOTE: server never wants to guess a client's algo, they're + # telling us directly. No need for _finalize_pubkey_algorithm + # anywhere in this flow. + algorithm = m.get_text() keyblob = m.get_binary() try: - key = self.transport._key_info[keytype](Message(keyblob)) + key = self._generate_key_from_request(algorithm, keyblob) except SSHException as e: self._log(INFO, "Auth rejected: public key: {}".format(str(e))) key = None @@ -572,20 +583,20 @@ def _parse_userauth_request(self, m): username, key ) if result != AUTH_FAILED: - sig_algo = self._finalize_pubkey_algorithm(keytype) # key is okay, verify it if not sig_attached: # client wants to know if this key is acceptable, before it # signs anything... send special "ok" message m = Message() m.add_byte(cMSG_USERAUTH_PK_OK) - # TODO: suspect we're not testing this - m.add_string(sig_algo) + m.add_string(algorithm) m.add_string(keyblob) self.transport._send_message(m) return sig = Message(m.get_binary()) - blob = self._get_session_blob(key, service, username) + blob = self._get_session_blob( + key, service, username, algorithm + ) if not key.verify_ssh_sig(blob, sig): self._log(INFO, "Auth rejected: invalid signature") result = AUTH_FAILED From bbefff00961125a35a5fb6a769679aa297224b45 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 24 Dec 2021 14:53:27 -0500 Subject: [PATCH 090/117] Cut 2.9.1 --- paramiko/_version.py | 2 +- sites/www/changelog.rst | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/paramiko/_version.py b/paramiko/_version.py index 4e24c2f75..a8a1c31ed 100644 --- a/paramiko/_version.py +++ b/paramiko/_version.py @@ -1,2 +1,2 @@ -__version_info__ = (2, 9, 0) +__version_info__ = (2, 9, 1) __version__ = ".".join(map(str, __version_info__)) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 091fe1183..5027ed422 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,7 @@ Changelog ========= +- :release:`2.9.1 <2021-12-24>` - :bug:`1955` Server-side support for ``rsa-sha2-256`` and ``ssh-rsa`` wasn't fully operable after 2.9.0's release (signatures for RSA pubkeys were always run through ``rsa-sha2-512`` instead). Report and early stab at a fix From 5f222495b5a62f3a1c465292bcace15888f40515 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 8 Jan 2022 12:16:48 -0500 Subject: [PATCH 091/117] Add more visible backwards compat warning re 2.9 RSA2 changes --- sites/www/changelog.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 5027ed422..ef7ed3672 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -11,6 +11,17 @@ Changelog - :feature:`1643` (also :issue:`1925`, :issue:`1644`, :issue:`1326`) Add support for SHA-2 variants of RSA key verification algorithms (as described in :rfc:`8332`) as well as limited SSH extension negotiation (:rfc:`8308`). + + .. warning:: + This change is slightly backwards incompatible, insofar as action is + required if your target systems do not support either RSA2 or the + ``server-sig-algs`` protocol extension. + + Specifically, you need to specify ``disabled_algorithms={'keys': + ['rsa-sha2-256', 'rsa-sha2-512']}`` in either `SSHClient + ` or `Transport + `. See below for details on why. + How SSH servers/clients decide when and how to use this functionality can be complicated; Paramiko's support is as follows: From 6699d35ad2d13fb74280a193e2e284a4a45f6f68 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 8 Jan 2022 13:43:50 -0500 Subject: [PATCH 092/117] Fix up logging and exception handling re: pubkey auth and presence/lack of server-sig-algs Re #1961 --- paramiko/auth_handler.py | 47 ++++++++++++++++++++++++++++++---------- sites/www/changelog.rst | 7 ++++++ 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/paramiko/auth_handler.py b/paramiko/auth_handler.py index da109d7c7..41ec44870 100644 --- a/paramiko/auth_handler.py +++ b/paramiko/auth_handler.py @@ -307,19 +307,42 @@ def _finalize_pubkey_algorithm(self, key_type): "An RSA key was specified, but no RSA pubkey algorithms are configured!" # noqa ) # Check for server-sig-algs if supported & sent - server_algos = u( + server_algo_str = u( self.transport.server_extensions.get("server-sig-algs", b("")) - ).split(",") - self._log(DEBUG, "Server-side algorithm list: {}".format(server_algos)) - # Only use algos from our list that the server likes, in our own - # preference order. (NOTE: purposefully using same style as in - # Transport...expect to refactor later) - agreement = list(filter(server_algos.__contains__, my_algos)) - # Fallback: first one in our (possibly tweaked by caller) list - final = agreement[0] if agreement else my_algos[0] - self.transport._agreed_pubkey_algorithm = final - self._log(DEBUG, "Agreed upon {!r} pubkey algorithm".format(final)) - return final + ) + pubkey_algo = None + if server_algo_str: + server_algos = server_algo_str.split(",") + self._log( + DEBUG, "Server-side algorithm list: {}".format(server_algos) + ) + # Only use algos from our list that the server likes, in our own + # preference order. (NOTE: purposefully using same style as in + # Transport...expect to refactor later) + agreement = list(filter(server_algos.__contains__, my_algos)) + if agreement: + pubkey_algo = agreement[0] + self._log( + DEBUG, + "Agreed upon {!r} pubkey algorithm".format(pubkey_algo), + ) + else: + self._log(DEBUG, "No common pubkey algorithms exist! Dying.") + # TODO: MAY want to use IncompatiblePeer again here but that's + # technically for initial key exchange, not pubkey auth. + err = "Unable to agree on a pubkey algorithm for signing a {!r} key!" # noqa + raise AuthenticationException(err.format(key_type)) + else: + # Fallback: first one in our (possibly tweaked by caller) list + pubkey_algo = my_algos[0] + msg = "Server did not send a server-sig-algs list; defaulting to our first preferred algo ({!r})" # noqa + self._log(DEBUG, msg.format(pubkey_algo)) + self._log( + DEBUG, + "NOTE: you may use the 'disabled_algorithms' SSHClient/Transport init kwarg to disable that or other algorithms if your server does not support them!", # noqa + ) + self.transport._agreed_pubkey_algorithm = pubkey_algo + return pubkey_algo def _parse_service_accept(self, m): service = m.get_text() diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index ef7ed3672..61a92acbb 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,13 @@ Changelog ========= +- :bug:`-` Connecting to servers which support ``server-sig-algs`` but which + have no overlap between that list and what a Paramiko client supports, now + raise an exception instead of defaulting to ``rsa-sha2-512`` (since the use + of ``server-sig-algs`` allows us to know what the server supports). +- :bug:`-` Enhanced log output when connecting to servers that do not support + ``server-sig-algs`` extensions, making the new-as-of-2.9 defaulting to SHA2 + pubkey algorithms more obvious when it kicks in. - :release:`2.9.1 <2021-12-24>` - :bug:`1955` Server-side support for ``rsa-sha2-256`` and ``ssh-rsa`` wasn't fully operable after 2.9.0's release (signatures for RSA pubkeys were always From 88f35a537428e430f7f26eee8026715e357b55d6 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Sat, 8 Jan 2022 14:29:57 -0500 Subject: [PATCH 093/117] Cut 2.9.2 --- paramiko/_version.py | 2 +- sites/www/changelog.rst | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/paramiko/_version.py b/paramiko/_version.py index a8a1c31ed..3ec897c59 100644 --- a/paramiko/_version.py +++ b/paramiko/_version.py @@ -1,2 +1,2 @@ -__version_info__ = (2, 9, 1) +__version_info__ = (2, 9, 2) __version__ = ".".join(map(str, __version_info__)) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 61a92acbb..460089fe1 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,7 @@ Changelog ========= +- :release:`2.9.2 <2022-01-08>` - :bug:`-` Connecting to servers which support ``server-sig-algs`` but which have no overlap between that list and what a Paramiko client supports, now raise an exception instead of defaulting to ``rsa-sha2-512`` (since the use From c9a3409b6d4078b77167467feafe5f102ec53671 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 11 Feb 2022 13:04:33 -0500 Subject: [PATCH 094/117] Clarify disabled algorithms keys vs pubkeys in changelog --- sites/www/changelog.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 460089fe1..0b1bd9fe0 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -51,9 +51,9 @@ Changelog cert, will act as follows: - In all cases, the list of algorithms to consider is based on the new - ``preferred_pubkeys`` list (see below) and ``disabled_algorithms``; this - list, like with host keys, prefers SHA2-512, SHA2-256 and SHA1, in that - order. + ``preferred_pubkeys`` list (see below) and ``disabled_algorithms`` + (specifically, its ``pubkeys`` key); this list, like with host keys, + prefers SHA2-512, SHA2-256 and SHA1, in that order. - When the server does not send ``server-sig-algs``, Paramiko will attempt the first algorithm in the above list. Clients connecting to legacy servers should thus use ``disabled_algorithms`` to turn off SHA2. From 11232cca7be7bc5fc51eef1e8869db4450bc79e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sondre=20Lilleb=C3=B8=20Gundersen?= Date: Thu, 10 Feb 2022 10:58:41 +0100 Subject: [PATCH 095/117] Add six to `install_requires` --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 569975321..c62a1bb46 100644 --- a/setup.py +++ b/setup.py @@ -90,6 +90,6 @@ # TODO 3.0: alternately, given how prevalent ed25519 is now, and how we use # invoke for (increasing) subproc stuff, consider making the default flavor # "full" and adding a "minimal" or similar that is just-crypto? - install_requires=["bcrypt>=3.1.3", "cryptography>=2.5", "pynacl>=1.0.1"], + install_requires=["bcrypt>=3.1.3", "cryptography>=2.5", "pynacl>=1.0.1", "six"], extras_require=extras_require, ) From e0d5b1c63e25509d8014a5abfa7edcb83da696ec Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 25 Feb 2022 14:27:16 -0500 Subject: [PATCH 096/117] Comment and changelog re #1985 --- setup.py | 1 + sites/www/changelog.rst | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/setup.py b/setup.py index c62a1bb46..5a0acbac0 100644 --- a/setup.py +++ b/setup.py @@ -90,6 +90,7 @@ # TODO 3.0: alternately, given how prevalent ed25519 is now, and how we use # invoke for (increasing) subproc stuff, consider making the default flavor # "full" and adding a "minimal" or similar that is just-crypto? + # TODO 3.0: remove six, obviously install_requires=["bcrypt>=3.1.3", "cryptography>=2.5", "pynacl>=1.0.1", "six"], extras_require=extras_require, ) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index c423f5a5f..af1cef055 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,12 @@ Changelog ========= +- :support:`1985` Add ``six`` explicitly to install-requires; it snuck into + active use at some point but has only been indicated by transitive dependency + on ``bcrypt`` until they somewhat-recently dropped it. This will be + short-lived until we `drop Python 2 + support `_. Thanks to Sondre + Lillebø Gundersen for catch & patch. - :release:`2.8.1 <2021-11-28>` - :bug:`985` (via :issue:`992`) Fix listdir failure when server uses a locale. Now on Python 2.7 `SFTPAttributes ` will From a318255e18accb7448ef76338b36483b06f40b4e Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 25 Feb 2022 14:28:31 -0500 Subject: [PATCH 097/117] blacken --- setup.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 5a0acbac0..c9de01540 100644 --- a/setup.py +++ b/setup.py @@ -91,6 +91,11 @@ # invoke for (increasing) subproc stuff, consider making the default flavor # "full" and adding a "minimal" or similar that is just-crypto? # TODO 3.0: remove six, obviously - install_requires=["bcrypt>=3.1.3", "cryptography>=2.5", "pynacl>=1.0.1", "six"], + install_requires=[ + "bcrypt>=3.1.3", + "cryptography>=2.5", + "pynacl>=1.0.1", + "six", + ], extras_require=extras_require, ) From b4bd81dce17d23b3e402b7fd492bb2ebd30b284c Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Mon, 7 Mar 2022 16:21:28 -0500 Subject: [PATCH 098/117] Massively speed up low-level SFTP read/write This doesn't impact most users who perform reads/writes using SFTPClient.get(fo)/put(fo) as those naturally perform chunking. However, users accessing the raw SFTPFile objects via SFTPClient.open() and then reading/writing large (more than a few MB) files, may experience severe slowdown due to inefficient slicing of the file being read/written. This change replaces the naive "slice a list of bytes" code with bytearray and memoryview, which are significantly more performant in these use cases, while remaining backwards compatible. Patch courtesy of Sevastian Tchernov. --- paramiko/file.py | 9 +++++---- sites/www/changelog.rst | 10 ++++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/paramiko/file.py b/paramiko/file.py index 9e9f6eb89..9dd9e9e79 100644 --- a/paramiko/file.py +++ b/paramiko/file.py @@ -192,7 +192,7 @@ def read(self, size=None): raise IOError("File is not open for reading") if (size is None) or (size < 0): # go for broke - result = self._rbuffer + result = bytearray(self._rbuffer) self._rbuffer = bytes() self._pos += len(result) while True: @@ -202,10 +202,10 @@ def read(self, size=None): new_data = None if (new_data is None) or (len(new_data) == 0): break - result += new_data + result.extend(new_data) self._realpos += len(new_data) self._pos += len(new_data) - return result + return bytes(result) if size <= len(self._rbuffer): result = self._rbuffer[:size] self._rbuffer = self._rbuffer[size:] @@ -515,9 +515,10 @@ def _set_mode(self, mode="r", bufsize=-1): # self.newlines = None - def _write_all(self, data): + def _write_all(self, raw_data): # the underlying stream may be something that does partial writes (like # a socket). + data = memoryview(raw_data) while len(data) > 0: count = self._write(data) data = data[count:] diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 48119f00c..9387aabf4 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,16 @@ Changelog ========= +- :bug:`892 major` Significantly speed up low-level read/write actions on + `~paramiko.sftp_file.SFTPFile` objects by using `bytearray`/`memoryview`. + This is unlikely to change anything for users of the higher level methods + like ``SFTPClient.get`` or ``SFTPClient.getfo``, but users of + `SFTPClient.open ` will likely see + orders of magnitude improvements for files larger than a few megabytes in + size. + + Thanks to ``@jkji`` for the original report and to Sevastian Tchernov for the + patch. - :support:`1985` Add ``six`` explicitly to install-requires; it snuck into active use at some point but has only been indicated by transitive dependency on ``bcrypt`` until they somewhat-recently dropped it. This will be From 58b56875e77be6c2b6b65bbec7300a16c6fd5573 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 11 Mar 2022 11:36:52 -0500 Subject: [PATCH 099/117] Clarify SFTPClient.open() docs re: actual behavior of BufferedFile Closes #2000 --- paramiko/sftp_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/paramiko/sftp_client.py b/paramiko/sftp_client.py index 6294fb482..d6c3a70d9 100644 --- a/paramiko/sftp_client.py +++ b/paramiko/sftp_client.py @@ -344,13 +344,13 @@ def open(self, filename, mode="r", bufsize=-1): ``O_EXCL`` flag in posix. The file will be buffered in standard Python style by default, but - can be altered with the ``bufsize`` parameter. ``0`` turns off + can be altered with the ``bufsize`` parameter. ``<=0`` turns off buffering, ``1`` uses line buffering, and any number greater than 1 (``>1``) uses that specific buffer size. :param str filename: name of the file to open :param str mode: mode (Python-style) to open in - :param int bufsize: desired buffering (-1 = default buffer size) + :param int bufsize: desired buffering (default: ``-1``) :return: an `.SFTPFile` object representing the open file :raises: ``IOError`` -- if the file could not be opened. From 3bb7877c13c22ec184ec77e4c4396539ae9f90b4 Mon Sep 17 00:00:00 2001 From: Lew Gordon Date: Mon, 29 Mar 2021 13:20:32 -0400 Subject: [PATCH 100/117] support Windows OpenSSH agent besides Putty's pageant (addresses #1509) Since quite a while there exists a native openssh port for windows. If the Putty pageant is not present, try to use the native port's agent instead. --- paramiko/agent.py | 18 ++++++++++++++++-- paramiko/win_openssh.py | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 paramiko/win_openssh.py diff --git a/paramiko/agent.py b/paramiko/agent.py index f28bf1283..fc3b66fb0 100644 --- a/paramiko/agent.py +++ b/paramiko/agent.py @@ -246,7 +246,12 @@ def connect(self): if win_pageant.can_talk_to_agent(): conn = win_pageant.PageantConnection() else: - return + import paramiko.win_openssh as win_openssh + + if win_openssh.can_talk_to_agent(): + conn = win_openssh.OpenSSHAgentConnection() + else: + return else: # no agent support return @@ -366,6 +371,10 @@ class Agent(AgentSSH): :raises: `.SSHException` -- if an SSH agent is found, but speaks an incompatible protocol + + .. versionchanged:: 2.10 + Added support for native openssh agent on windows (extending previous + putty pageant support) """ def __init__(self): @@ -384,7 +393,12 @@ def __init__(self): if win_pageant.can_talk_to_agent(): conn = win_pageant.PageantConnection() else: - return + import paramiko.win_openssh as win_openssh + + if win_openssh.can_talk_to_agent(): + conn = win_openssh.OpenSSHAgentConnection() + else: + return else: # no agent support return diff --git a/paramiko/win_openssh.py b/paramiko/win_openssh.py new file mode 100644 index 000000000..593cdbe45 --- /dev/null +++ b/paramiko/win_openssh.py @@ -0,0 +1,39 @@ +# Copyright (C) 2021 Lew Gordon +# Copyright (C) 2022 Patrick Spendrin +# +# This file is part of paramiko. +# +# Paramiko is free software; you can redistribute it and/or modify it under the +# terms of the GNU Lesser General Public License as published by the Free +# Software Foundation; either version 2.1 of the License, or (at your option) +# any later version. +# +# Paramiko is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more +# details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with Paramiko; if not, write to the Free Software Foundation, Inc., +# 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA. +import os.path + +PIPE_NAME = r"\\.\pipe\openssh-ssh-agent" + + +def can_talk_to_agent(): + return os.path.exists(PIPE_NAME) + + +class OpenSSHAgentConnection: + def __init__(self): + self._pipe = open(PIPE_NAME, "rb+", buffering=0) + + def send(self, data): + return self._pipe.write(data) + + def recv(self, n): + return self._pipe.read(n) + + def close(self): + return self._pipe.close() From a8e9a0fd717d1fe77b0b69419d49c9147711d4a5 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Thu, 10 Mar 2022 16:50:19 -0500 Subject: [PATCH 101/117] Changelog re #1509, re #1837, closes #1868 --- sites/www/changelog.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 9387aabf4..a12899680 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,10 @@ Changelog ========= +- :feature:`1509` (via :issue:`1868`, :issue:`1837`) Add support for OpenSSH's + Windows agent as a fallback when Putty/WinPageant isn't available or + functional. Reported by ``@benj56``, early patchset by ``@lewgordon``, and + later PR courtesy of Patrick Spendrin. - :bug:`892 major` Significantly speed up low-level read/write actions on `~paramiko.sftp_file.SFTPFile` objects by using `bytearray`/`memoryview`. This is unlikely to change anything for users of the higher level methods From 845b4fa9c14f7836e144a8838c10a5cc64a6c204 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Thu, 10 Mar 2022 16:58:36 -0500 Subject: [PATCH 102/117] Refactor SSH agent socket connection stuff Feels like this entire module wants more rigorous rewriting, but at least for now any future tweaks to agent bits won't hit this awful copypasta. Minor functionality update: now both methods of dealing with unix sockets will use retry_on_signal to try and skip over spurious EINTRs --- paramiko/agent.py | 80 ++++++++++++++++++----------------------- paramiko/win_openssh.py | 1 + 2 files changed, 36 insertions(+), 45 deletions(-) diff --git a/paramiko/agent.py b/paramiko/agent.py index fc3b66fb0..13dc79750 100644 --- a/paramiko/agent.py +++ b/paramiko/agent.py @@ -205,6 +205,36 @@ def get_connection(self): return self.__chan, None +def get_agent_connection(): + """ + Returns some SSH agent object, or None if none were found/supported. + + .. versionadded:: 2.10 + """ + if ("SSH_AUTH_SOCK" in os.environ) and (sys.platform != "win32"): + conn = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + try: + retry_on_signal( + lambda: conn.connect(os.environ["SSH_AUTH_SOCK"]) + ) + return conn + except: + # probably a dangling env var: the ssh agent is gone + return + elif sys.platform == "win32": + from . import win_pageant, win_openssh + + conn = None + if win_pageant.can_talk_to_agent(): + conn = win_pageant.PageantConnection() + elif win_openssh.can_talk_to_agent(): + conn = win_openssh.OpenSSHAgentConnection() + return conn + else: + # no agent support + return + + class AgentClientProxy(object): """ Class proxying request as a client: @@ -231,29 +261,8 @@ def connect(self): """ Method automatically called by ``AgentProxyThread.run``. """ - if ("SSH_AUTH_SOCK" in os.environ) and (sys.platform != "win32"): - conn = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) - try: - retry_on_signal( - lambda: conn.connect(os.environ["SSH_AUTH_SOCK"]) - ) - except: - # probably a dangling env var: the ssh agent is gone - return - elif sys.platform == "win32": - import paramiko.win_pageant as win_pageant - - if win_pageant.can_talk_to_agent(): - conn = win_pageant.PageantConnection() - else: - import paramiko.win_openssh as win_openssh - - if win_openssh.can_talk_to_agent(): - conn = win_openssh.OpenSSHAgentConnection() - else: - return - else: - # no agent support + conn = get_agent_connection() + if not conn: return self._conn = conn @@ -380,29 +389,10 @@ class Agent(AgentSSH): def __init__(self): AgentSSH.__init__(self) - if ("SSH_AUTH_SOCK" in os.environ) and (sys.platform != "win32"): - conn = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) - try: - conn.connect(os.environ["SSH_AUTH_SOCK"]) - except: - # probably a dangling env var: the ssh agent is gone - return - elif sys.platform == "win32": - from . import win_pageant - - if win_pageant.can_talk_to_agent(): - conn = win_pageant.PageantConnection() - else: - import paramiko.win_openssh as win_openssh - - if win_openssh.can_talk_to_agent(): - conn = win_openssh.OpenSSHAgentConnection() - else: - return - else: - # no agent support + conn = get_agent_connection() + if not conn: return - self._connect(conn) + self._connect() def close(self): """ diff --git a/paramiko/win_openssh.py b/paramiko/win_openssh.py index 593cdbe45..ece7c8fd7 100644 --- a/paramiko/win_openssh.py +++ b/paramiko/win_openssh.py @@ -16,6 +16,7 @@ # You should have received a copy of the GNU Lesser General Public License # along with Paramiko; if not, write to the Free Software Foundation, Inc., # 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA. + import os.path PIPE_NAME = r"\\.\pipe\openssh-ssh-agent" From 4313c026a8cb926ca984cc2039be3b8b06c36859 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 11 Mar 2022 17:37:58 -0500 Subject: [PATCH 103/117] Tweak changelog language a bit --- sites/www/changelog.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index a12899680..6ffd9cf1c 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -4,8 +4,8 @@ Changelog - :feature:`1509` (via :issue:`1868`, :issue:`1837`) Add support for OpenSSH's Windows agent as a fallback when Putty/WinPageant isn't available or - functional. Reported by ``@benj56``, early patchset by ``@lewgordon``, and - later PR courtesy of Patrick Spendrin. + functional. Reported by ``@benj56`` with patches/PRs from ``@lewgordon`` and + Patrick Spendrin. - :bug:`892 major` Significantly speed up low-level read/write actions on `~paramiko.sftp_file.SFTPFile` objects by using `bytearray`/`memoryview`. This is unlikely to change anything for users of the higher level methods From f96319f35b4e2965b14d91e57418325639380548 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 11 Mar 2022 18:33:42 -0500 Subject: [PATCH 104/117] blacken --- paramiko/agent.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/paramiko/agent.py b/paramiko/agent.py index 13dc79750..2ff7b449b 100644 --- a/paramiko/agent.py +++ b/paramiko/agent.py @@ -214,9 +214,7 @@ def get_agent_connection(): if ("SSH_AUTH_SOCK" in os.environ) and (sys.platform != "win32"): conn = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) try: - retry_on_signal( - lambda: conn.connect(os.environ["SSH_AUTH_SOCK"]) - ) + retry_on_signal(lambda: conn.connect(os.environ["SSH_AUTH_SOCK"])) return conn except: # probably a dangling env var: the ssh agent is gone From b9fc4f7c2cbfd319512b89d6af6421bb7cc1d2f0 Mon Sep 17 00:00:00 2001 From: Jason Brand Date: Tue, 15 Feb 2022 23:15:23 +0000 Subject: [PATCH 105/117] %C support in config file --- paramiko/config.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/paramiko/config.py b/paramiko/config.py index e6877d010..0e3a5eaa2 100644 --- a/paramiko/config.py +++ b/paramiko/config.py @@ -27,6 +27,7 @@ import re import shlex import socket +import hashlib from functools import partial from .py3compat import StringIO @@ -435,7 +436,10 @@ def _tokenize(self, config, target_hostname, key, value): # The actual tokens! replacements = { # TODO: %%??? - # TODO: %C? + "%C": hashlib.sha1((local_hostname + + target_hostname + + str(port) + + remoteuser).encode("utf-8")).hexdigest(), "%d": homedir, "%h": configured_hostname, # TODO: %i? From cd65798c2777e5ab5f15e96a672ccc238237ddcd Mon Sep 17 00:00:00 2001 From: Jason Brand Date: Tue, 15 Feb 2022 23:16:41 +0000 Subject: [PATCH 106/117] Add %C to doc --- sites/docs/api/config.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/sites/docs/api/config.rst b/sites/docs/api/config.rst index ea4939b28..d42de8ac0 100644 --- a/sites/docs/api/config.rst +++ b/sites/docs/api/config.rst @@ -99,6 +99,7 @@ properties of the local system). Specifically, we are known to support the below, where applicable (e.g. as in OpenSSH, ``%L`` works in ``ControlPath`` but not elsewhere): +- ``%C`` - ``%d`` - ``%h`` - ``%l`` From 0f90e5bdd908bbace418045ddd35224283f16afe Mon Sep 17 00:00:00 2001 From: Jason Brand Date: Tue, 15 Feb 2022 23:24:59 +0000 Subject: [PATCH 107/117] Import only sha1 --- paramiko/config.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/paramiko/config.py b/paramiko/config.py index 0e3a5eaa2..27fdca4f0 100644 --- a/paramiko/config.py +++ b/paramiko/config.py @@ -27,7 +27,7 @@ import re import shlex import socket -import hashlib +from hashlib import sha1 from functools import partial from .py3compat import StringIO @@ -436,10 +436,10 @@ def _tokenize(self, config, target_hostname, key, value): # The actual tokens! replacements = { # TODO: %%??? - "%C": hashlib.sha1((local_hostname + - target_hostname + - str(port) + - remoteuser).encode("utf-8")).hexdigest(), + "%C": sha1((local_hostname + + target_hostname + + str(port) + + remoteuser).encode("utf-8")).hexdigest(), "%d": homedir, "%h": configured_hostname, # TODO: %i? From 3f3451fd46353fa173f6c083b1c38438d04a68ea Mon Sep 17 00:00:00 2001 From: Jason Brand Date: Tue, 15 Feb 2022 23:34:22 +0000 Subject: [PATCH 108/117] Add to changelog --- sites/www/changelog.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 6ffd9cf1c..5dc444189 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,7 @@ Changelog ========= +- :feature:`1976` Support for ``%C`` token in configuration file. - :feature:`1509` (via :issue:`1868`, :issue:`1837`) Add support for OpenSSH's Windows agent as a fallback when Putty/WinPageant isn't available or functional. Reported by ``@benj56`` with patches/PRs from ``@lewgordon`` and From f6342fc5f00b48e679e7c2c3579b1b27d94ffc1f Mon Sep 17 00:00:00 2001 From: Jason Brand Date: Wed, 16 Feb 2022 00:22:37 +0000 Subject: [PATCH 109/117] Prettify, add %C as acceptable controlpath token, mock gethostname --- paramiko/config.py | 8 +++----- tests/test_config.py | 1 + 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/paramiko/config.py b/paramiko/config.py index 27fdca4f0..c2a58e4e1 100644 --- a/paramiko/config.py +++ b/paramiko/config.py @@ -60,7 +60,7 @@ class SSHConfig(object): # TODO: do a full scan of ssh.c & friends to make sure we're fully # compatible across the board, e.g. OpenSSH 8.1 added %n to ProxyCommand. TOKENS_BY_CONFIG_KEY = { - "controlpath": ["%h", "%l", "%L", "%n", "%p", "%r", "%u"], + "controlpath": ["%C", "%h", "%l", "%L", "%n", "%p", "%r", "%u"], "hostname": ["%h"], "identityfile": ["~", "%d", "%h", "%l", "%u", "%r"], "proxycommand": ["~", "%h", "%p", "%r"], @@ -433,13 +433,11 @@ def _tokenize(self, config, target_hostname, key, value): local_hostname = socket.gethostname().split(".")[0] local_fqdn = LazyFqdn(config, local_hostname) homedir = os.path.expanduser("~") + tohash = local_hostname + target_hostname + repr(port) + remoteuser # The actual tokens! replacements = { # TODO: %%??? - "%C": sha1((local_hostname + - target_hostname + - str(port) + - remoteuser).encode("utf-8")).hexdigest(), + "%C": sha1(tohash.encode()).hexdigest(), "%d": homedir, "%h": configured_hostname, # TODO: %i? diff --git a/tests/test_config.py b/tests/test_config.py index 5e9aa0592..892b4c926 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -42,6 +42,7 @@ def socket(): # Patch out getfqdn to return some real string for when it gets called; # some code (eg tokenization) gets mad w/ MagicMocks mocket.getfqdn.return_value = "some.fake.fqdn" + mocket.gethostname.return_value = "local.fake.fqdn" yield mocket From 1bf3dce7255ff2055dcdbc4d29454fb0184dfaf7 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 11 Mar 2022 18:05:33 -0500 Subject: [PATCH 110/117] Changelog enhancement --- sites/www/changelog.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 5dc444189..389c70fb8 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,7 +2,8 @@ Changelog ========= -- :feature:`1976` Support for ``%C`` token in configuration file. +- :feature:`1976` Add support for the ``%C`` token when parsing SSH config + files. Foundational PR submitted by ``@jbrand42``. - :feature:`1509` (via :issue:`1868`, :issue:`1837`) Add support for OpenSSH's Windows agent as a fallback when Putty/WinPageant isn't available or functional. Reported by ``@benj56`` with patches/PRs from ``@lewgordon`` and From 5fcb8da16d4b33fa52880c1c3e848654a698d34d Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 11 Mar 2022 18:08:39 -0500 Subject: [PATCH 111/117] OpenSSH docs state %C should also work in IdentityFile and Match exec (at least, of what we presently ourselves support - it's also allowed in others) --- paramiko/config.py | 4 ++-- tests/configs/match-exec | 2 +- tests/test_config.py | 23 +++++++++++++++++++---- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/paramiko/config.py b/paramiko/config.py index c2a58e4e1..9c21e4e54 100644 --- a/paramiko/config.py +++ b/paramiko/config.py @@ -62,11 +62,11 @@ class SSHConfig(object): TOKENS_BY_CONFIG_KEY = { "controlpath": ["%C", "%h", "%l", "%L", "%n", "%p", "%r", "%u"], "hostname": ["%h"], - "identityfile": ["~", "%d", "%h", "%l", "%u", "%r"], + "identityfile": ["%C", "~", "%d", "%h", "%l", "%u", "%r"], "proxycommand": ["~", "%h", "%p", "%r"], # Doesn't seem worth making this 'special' for now, it will fit well # enough (no actual match-exec config key to be confused with). - "match-exec": ["%d", "%h", "%L", "%l", "%n", "%p", "%r", "%u"], + "match-exec": ["%C", "%d", "%h", "%L", "%l", "%n", "%p", "%r", "%u"], } def __init__(self): diff --git a/tests/configs/match-exec b/tests/configs/match-exec index 763346eae..62a147aa2 100644 --- a/tests/configs/match-exec +++ b/tests/configs/match-exec @@ -12,5 +12,5 @@ Host target User intermediate HostName configured -Match exec "%d %h %L %l %n %p %r %u" +Match exec "%C %d %h %L %l %n %p %r %u" Port 1337 diff --git a/tests/test_config.py b/tests/test_config.py index 892b4c926..08096cff9 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -218,6 +218,9 @@ def test_controlpath_token_expansion(self, getpass): Host explicit_host HostName ohai ControlPath remoteuser %r host %h orighost %n + +Host hashbrowns + ControlPath %C """ ) result = config.lookup("explicit_user")["controlpath"] @@ -226,6 +229,9 @@ def test_controlpath_token_expansion(self, getpass): result = config.lookup("explicit_host")["controlpath"] # Remote user falls back to local user; host and orighost may differ assert result == "remoteuser gandalf host ohai orighost explicit_host" + # Supports %C + result = config.lookup("hashbrowns")["controlpath"] + assert result == "fc995d9f41ca1bcec7bc1d7f1ca87b9ff568a6d4" def test_negation(self): config = SSHConfig.from_text( @@ -280,7 +286,6 @@ def test_proxycommand(self): def test_identityfile(self): config = SSHConfig.from_text( """ - IdentityFile id_dsa0 Host * @@ -291,6 +296,9 @@ def test_identityfile(self): Host dsa2* IdentityFile id_dsa22 + +Host hashbrowns +IdentityFile %C """ ) for host, values in { @@ -303,8 +311,15 @@ def test_identityfile(self): "hostname": "dsa22", "identityfile": ["id_dsa0", "id_dsa1", "id_dsa22"], }, + "hashbrowns": { + "hostname": "hashbrowns", + "identityfile": [ + "id_dsa0", + "id_dsa1", + "d5c0115d09912e39ff27844ea9d6052fc6048f99", + ], + }, }.items(): - assert config.lookup(host) == values def test_config_addressfamily_and_lazy_fqdn(self): @@ -740,10 +755,10 @@ def test_does_not_match_nonzero_exit_codes(self, run): @patch("paramiko.config.getpass") @patch("paramiko.config.invoke.run") def test_tokenizes_argument(self, run, getpass, socket): - socket.gethostname.return_value = "local.fqdn" getpass.getuser.return_value = "gandalf" - # Actual exec value is "%d %h %L %l %n %p %r %u" + # Actual exec value is "%C %d %h %L %l %n %p %r %u" parts = ( + "bf5ba06778434a9384ee4217e462f64888bd0cd2", expanduser("~"), "configured", "local", From 29d7bf43f4a8beabcfe14cdc969d6a370e57ecf8 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 11 Mar 2022 21:09:21 -0500 Subject: [PATCH 112/117] Clearly our agent stuff is not fully tested yet... --- paramiko/agent.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/paramiko/agent.py b/paramiko/agent.py index 2ff7b449b..1dc99b18a 100644 --- a/paramiko/agent.py +++ b/paramiko/agent.py @@ -390,7 +390,7 @@ def __init__(self): conn = get_agent_connection() if not conn: return - self._connect() + self._connect(conn) def close(self): """ From 02ad67eaec68bacc18838158b902ccaade8f5dc8 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 11 Mar 2022 21:21:26 -0500 Subject: [PATCH 113/117] Helps to actually leverage your mocked system calls --- tests/test_config.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index 08096cff9..b46dc7b42 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -207,7 +207,7 @@ def test_proxycommand_tilde_expansion(self): assert got == expected @patch("paramiko.config.getpass") - def test_controlpath_token_expansion(self, getpass): + def test_controlpath_token_expansion(self, getpass, socket): getpass.getuser.return_value = "gandalf" config = SSHConfig.from_text( """ @@ -231,7 +231,7 @@ def test_controlpath_token_expansion(self, getpass): assert result == "remoteuser gandalf host ohai orighost explicit_host" # Supports %C result = config.lookup("hashbrowns")["controlpath"] - assert result == "fc995d9f41ca1bcec7bc1d7f1ca87b9ff568a6d4" + assert result == "a438e7dbf5308b923aba9db8fe2ca63447ac8688" def test_negation(self): config = SSHConfig.from_text( @@ -283,7 +283,9 @@ def test_proxycommand(self): assert config.lookup(host) == values - def test_identityfile(self): + @patch("paramiko.config.getpass") + def test_identityfile(self, getpass, socket): + getpass.getuser.return_value = "gandalf" config = SSHConfig.from_text( """ IdentityFile id_dsa0 @@ -316,7 +318,7 @@ def test_identityfile(self): "identityfile": [ "id_dsa0", "id_dsa1", - "d5c0115d09912e39ff27844ea9d6052fc6048f99", + "a438e7dbf5308b923aba9db8fe2ca63447ac8688", ], }, }.items(): From e50e19f7d26665fd60f143320cce2e9c03f27c80 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 11 Mar 2022 22:33:28 -0500 Subject: [PATCH 114/117] Fix up changelog entry with real links --- sites/www/changelog.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 389c70fb8..40b039ce5 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -11,7 +11,8 @@ Changelog - :bug:`892 major` Significantly speed up low-level read/write actions on `~paramiko.sftp_file.SFTPFile` objects by using `bytearray`/`memoryview`. This is unlikely to change anything for users of the higher level methods - like ``SFTPClient.get`` or ``SFTPClient.getfo``, but users of + like `SFTPClient.get ` or + `SFTPClient.getfo `, but users of `SFTPClient.open ` will likely see orders of magnitude improvements for files larger than a few megabytes in size. From aa3cc6fa3e9f1df72d4ffd2d5fc02ae734a6cba4 Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 11 Mar 2022 22:34:03 -0500 Subject: [PATCH 115/117] Cut 2.10.0 --- paramiko/_version.py | 2 +- sites/www/changelog.rst | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/paramiko/_version.py b/paramiko/_version.py index 3ec897c59..f71dc7b20 100644 --- a/paramiko/_version.py +++ b/paramiko/_version.py @@ -1,2 +1,2 @@ -__version_info__ = (2, 9, 2) +__version_info__ = (2, 10, 0) __version__ = ".".join(map(str, __version_info__)) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 40b039ce5..af648ddc9 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,7 @@ Changelog ========= +- :release:`2.10.0 <2022-03-11>` - :feature:`1976` Add support for the ``%C`` token when parsing SSH config files. Foundational PR submitted by ``@jbrand42``. - :feature:`1509` (via :issue:`1868`, :issue:`1837`) Add support for OpenSSH's From 4c491e299c9b800358b16fa4886d8d94f45abe2e Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 25 Feb 2022 14:50:42 -0500 Subject: [PATCH 116/117] Fix CVE re: PKey.write_private_key chmod race CVE-2022-24302 (see changelog for link) --- paramiko/pkey.py | 12 ++++++++- sites/www/changelog.rst | 14 ++++++++++ tests/test_pkey.py | 58 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/paramiko/pkey.py b/paramiko/pkey.py index 7865a6eae..67945261d 100644 --- a/paramiko/pkey.py +++ b/paramiko/pkey.py @@ -558,7 +558,17 @@ def _write_private_key_file(self, filename, key, format, password=None): :raises: ``IOError`` -- if there was an error writing the file. """ - with open(filename, "w") as f: + # Ensure that we create new key files directly with a user-only mode, + # instead of opening, writing, then chmodding, which leaves us open to + # CVE-2022-24302. + # NOTE: O_TRUNC is a noop on new files, and O_CREAT is a noop on + # existing files, so using all 3 in both cases is fine. Ditto the use + # of the 'mode' argument; it should be safe to give even for existing + # files (though it will not act like a chmod in that case). + kwargs = dict(flags=os.O_WRONLY | os.O_TRUNC | os.O_CREAT, mode=o600) + # NOTE: yea, you still gotta inform the FLO that it is in "write" mode + with os.fdopen(os.open(filename, **kwargs), mode="w") as f: + # TODO 3.0: remove the now redundant chmod os.chmod(filename, o600) self._write_private_key(f, key, format, password=password) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index af648ddc9..37d149f25 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,20 @@ Changelog ========= +- :bug:`-` (`CVE-2022-24302 + `_) Creation + of new private key files using `~paramiko.pkey.PKey` subclasses was subject + to a race condition between file creation & mode modification, which could be + exploited by an attacker with knowledge of where the Paramiko-using code + would write out such files. + + This has been patched by using `os.open` and `os.fdopen` to ensure new files + are opened with the correct mode immediately. We've left the subsequent + explicit ``chmod`` in place to minimize any possible disruption, though it + may get removed in future backwards-incompatible updates. + + Thanks to Jan Schejbal for the report & feedback on the solution, and to + Jeremy Katz at Tidelift for coordinating the disclosure. - :release:`2.10.0 <2022-03-11>` - :feature:`1976` Add support for the ``%C`` token when parsing SSH config files. Foundational PR submitted by ``@jbrand42``. diff --git a/tests/test_pkey.py b/tests/test_pkey.py index 0cc201330..d44a96ac4 100644 --- a/tests/test_pkey.py +++ b/tests/test_pkey.py @@ -23,6 +23,7 @@ import unittest import os +import stat from binascii import hexlify from hashlib import md5 @@ -36,10 +37,11 @@ SSHException, ) from paramiko.py3compat import StringIO, byte_chr, b, bytes, PY2 +from paramiko.common import o600 from cryptography.exceptions import UnsupportedAlgorithm from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateNumbers -from mock import patch +from mock import patch, Mock import pytest from .util import _support, is_low_entropy @@ -696,3 +698,57 @@ def test_certificates(self): key1.load_certificate, _support("test_rsa.key-cert.pub"), ) + + @patch("paramiko.pkey.os") + def _test_keyfile_race(self, os_, exists): + # Re: CVE-2022-24302 + password = "television" + newpassword = "radio" + source = _support("test_ecdsa_384.key") + new = source + ".new" + # Mock setup + os_.path.exists.return_value = exists + # Attach os flag values to mock + for attr, value in vars(os).items(): + if attr.startswith("O_"): + setattr(os_, attr, value) + # Load fixture key + key = ECDSAKey(filename=source, password=password) + key._write_private_key = Mock() + # Write out in new location + key.write_private_key_file(new, password=newpassword) + # Expected open via os module + os_.open.assert_called_once_with(new, flags=os.O_WRONLY | os.O_CREAT | os.O_TRUNC, mode=o600) + os_.fdopen.assert_called_once_with(os_.open.return_value, mode="w") + # Old chmod still around for backwards compat + os_.chmod.assert_called_once_with(new, o600) + assert ( + key._write_private_key.call_args[0][0] + == os_.fdopen.return_value.__enter__.return_value + ) + + def test_new_keyfiles_avoid_file_descriptor_race_on_chmod(self): + self._test_keyfile_race(exists=False) + + def test_existing_keyfiles_still_work_ok(self): + self._test_keyfile_race(exists=True) + + def test_new_keyfiles_avoid_descriptor_race_integration(self): + # Integration-style version of above + password = "television" + newpassword = "radio" + source = _support("test_ecdsa_384.key") + new = source + ".new" + # Load fixture key + key = ECDSAKey(filename=source, password=password) + try: + # Write out in new location + key.write_private_key_file(new, password=newpassword) + # Test mode + assert stat.S_IMODE(os.stat(new).st_mode) == o600 + # Prove can open with new password + reloaded = ECDSAKey(filename=new, password=newpassword) + assert reloaded == key + finally: + if os.path.exists(new): + os.unlink(new) From 286bd9f0374922341d48923b0c3ef09aab57919f Mon Sep 17 00:00:00 2001 From: Jeff Forcier Date: Fri, 11 Mar 2022 23:19:39 -0500 Subject: [PATCH 117/117] Cut 2.10.1 --- paramiko/_version.py | 2 +- sites/www/changelog.rst | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/paramiko/_version.py b/paramiko/_version.py index f71dc7b20..e955fe734 100644 --- a/paramiko/_version.py +++ b/paramiko/_version.py @@ -1,2 +1,2 @@ -__version_info__ = (2, 10, 0) +__version_info__ = (2, 10, 1) __version__ = ".".join(map(str, __version_info__)) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 37d149f25..45bacd225 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,7 @@ Changelog ========= +- :release:`2.10.1 <2022-03-11>` - :bug:`-` (`CVE-2022-24302 `_) Creation of new private key files using `~paramiko.pkey.PKey` subclasses was subject