diff options
| -rw-r--r-- | paramiko/_version.py | 2 | ||||
| -rw-r--r-- | paramiko/agent.py | 70 | ||||
| -rw-r--r-- | paramiko/config.py | 10 | ||||
| -rw-r--r-- | paramiko/file.py | 9 | ||||
| -rw-r--r-- | paramiko/pkey.py | 13 | ||||
| -rw-r--r-- | paramiko/transport.py | 10 | ||||
| -rw-r--r-- | paramiko/win_openssh.py | 40 | ||||
| -rw-r--r-- | sites/docs/api/config.rst | 1 | ||||
| -rw-r--r-- | sites/www/changelog.rst | 47 | ||||
| -rw-r--r-- | tests/configs/match-exec | 2 | ||||
| -rw-r--r-- | tests/test_config.py | 30 | ||||
| -rw-r--r-- | tests/test_pkey.py | 60 | ||||
| -rw-r--r-- | tests/test_transport.py | 8 |
13 files changed, 248 insertions, 54 deletions
diff --git a/paramiko/_version.py b/paramiko/_version.py index c5f8c829..82bc1161 100644 --- a/paramiko/_version.py +++ b/paramiko/_version.py @@ -1,2 +1,2 @@ -__version_info__ = (2, 9, 3) +__version_info__ = (2, 10, 3) __version__ = ".".join(map(str, __version_info__)) diff --git a/paramiko/agent.py b/paramiko/agent.py index f28bf128..1dc99b18 100644 --- a/paramiko/agent.py +++ b/paramiko/agent.py @@ -205,6 +205,34 @@ class AgentRemoteProxy(AgentProxyThread): 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,24 +259,8 @@ class AgentClientProxy(object): """ 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: - return - else: - # no agent support + conn = get_agent_connection() + if not conn: return self._conn = conn @@ -366,27 +378,17 @@ 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): 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: - return - else: - # no agent support + conn = get_agent_connection() + if not conn: return self._connect(conn) diff --git a/paramiko/config.py b/paramiko/config.py index e6877d01..9c21e4e5 100644 --- a/paramiko/config.py +++ b/paramiko/config.py @@ -27,6 +27,7 @@ import os import re import shlex import socket +from hashlib import sha1 from functools import partial from .py3compat import StringIO @@ -59,13 +60,13 @@ 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"], + "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): @@ -432,10 +433,11 @@ class SSHConfig(object): 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: %%??? - # TODO: %C? + "%C": sha1(tohash.encode()).hexdigest(), "%d": homedir, "%h": configured_hostname, # TODO: %i? diff --git a/paramiko/file.py b/paramiko/file.py index 9e9f6eb8..9dd9e9e7 100644 --- a/paramiko/file.py +++ b/paramiko/file.py @@ -192,7 +192,7 @@ class BufferedFile(ClosingContextManager): 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 @@ class BufferedFile(ClosingContextManager): 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 @@ class BufferedFile(ClosingContextManager): # <http://www.python.org/doc/current/lib/built-in-funcs.html> 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/paramiko/pkey.py b/paramiko/pkey.py index f494c80e..f1919660 100644 --- a/paramiko/pkey.py +++ b/paramiko/pkey.py @@ -558,7 +558,18 @@ class PKey(object): :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). + # TODO 3.0: turn into kwargs again + args = [os.O_WRONLY | os.O_TRUNC | os.O_CREAT, o600] + # NOTE: yea, you still gotta inform the FLO that it is in "write" mode + with os.fdopen(os.open(filename, *args), "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/paramiko/transport.py b/paramiko/transport.py index c28c3828..77d32a85 100644 --- a/paramiko/transport.py +++ b/paramiko/transport.py @@ -549,7 +549,15 @@ class Transport(threading.Thread, ClosingContextManager): @property def preferred_keys(self): - return self._filter_algorithm("keys") + # Interleave cert variants here; resistant to various background + # overwriting of _preferred_keys, and necessary as hostkeys can't use + # the logic pubkey auth does re: injecting/checking for certs at + # runtime + filtered = self._filter_algorithm("keys") + return tuple( + filtered + + tuple("{}-cert-v01@openssh.com".format(x) for x in filtered) + ) @property def preferred_pubkeys(self): diff --git a/paramiko/win_openssh.py b/paramiko/win_openssh.py new file mode 100644 index 00000000..ece7c8fd --- /dev/null +++ b/paramiko/win_openssh.py @@ -0,0 +1,40 @@ +# Copyright (C) 2021 Lew Gordon <lew.gordon@genesys.com> +# Copyright (C) 2022 Patrick Spendrin <ps_ml@gmx.de> +# +# 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() diff --git a/sites/docs/api/config.rst b/sites/docs/api/config.rst index ea4939b2..d42de8ac 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`` diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index ea9fbc1c..7d5e81f6 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -13,6 +13,11 @@ Changelog causing occasional spurious ``BadHostKeyException`` (among other things). This has been fixed. Thanks to Shengdun Hua for the original report/patch and to Christopher Papke for the final version of the fix. +- :bug:`2035` Servers offering certificate variants of hostkey algorithms (eg + ``ssh-rsa-cert-v01@openssh.com``) could not have their host keys verified by + Paramiko clients, as it only ever considered non-cert key types for that part + of connection handshaking. This has been fixed. +- :release:`2.10.3 <2022-03-18>` - :release:`2.9.3 <2022-03-18>` - :bug:`1963` (via :issue:`1977`) Certificate-based pubkey auth was inadvertently broken when adding SHA2 support; this has been fixed. Reported @@ -21,6 +26,48 @@ Changelog storage when recording thread IDs for a logging helper; this should avoid one flavor of memory leak for long-running processes. Catch & patch via Richard Kojedzinszky. +- :release:`2.10.2 <2022-03-14>` +- :bug:`2001` Fix Python 2 compatibility breakage introduced in 2.10.1. Spotted + by Christian Hammond. + + .. warning:: + This is almost certainly the last time we will fix Python 2 related + errors! Please see `the roadmap + <https://bitprophet.org/projects/#roadmap>`_. + +- :release:`2.10.1 <2022-03-11>` +- :bug:`-` (`CVE-2022-24302 + <https://cve.mitre.org/cgi-bin/cvename.cgi?name=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``. +- :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 + 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 + like `SFTPClient.get <paramiko.sftp_client.SFTPClient.get>` or + `SFTPClient.getfo <paramiko.sftp_client.SFTPClient.getfo>`, but users of + `SFTPClient.open <paramiko.sftp_client.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 diff --git a/tests/configs/match-exec b/tests/configs/match-exec index 763346ea..62a147aa 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 5e9aa059..b46dc7b4 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 @@ -206,7 +207,7 @@ Host test 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( """ @@ -217,6 +218,9 @@ Host explicit_user Host explicit_host HostName ohai ControlPath remoteuser %r host %h orighost %n + +Host hashbrowns + ControlPath %C """ ) result = config.lookup("explicit_user")["controlpath"] @@ -225,6 +229,9 @@ Host explicit_host 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 == "a438e7dbf5308b923aba9db8fe2ca63447ac8688" def test_negation(self): config = SSHConfig.from_text( @@ -276,10 +283,11 @@ ProxyCommand foo=bar:%h-%p 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 Host * @@ -290,6 +298,9 @@ IdentityFile id_dsa2 Host dsa2* IdentityFile id_dsa22 + +Host hashbrowns +IdentityFile %C """ ) for host, values in { @@ -302,8 +313,15 @@ IdentityFile id_dsa22 "hostname": "dsa22", "identityfile": ["id_dsa0", "id_dsa1", "id_dsa22"], }, + "hashbrowns": { + "hostname": "hashbrowns", + "identityfile": [ + "id_dsa0", + "id_dsa1", + "a438e7dbf5308b923aba9db8fe2ca63447ac8688", + ], + }, }.items(): - assert config.lookup(host) == values def test_config_addressfamily_and_lazy_fqdn(self): @@ -739,10 +757,10 @@ class TestMatchExec(object): @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", diff --git a/tests/test_pkey.py b/tests/test_pkey.py index 687e776b..3bc0459a 100644 --- a/tests/test_pkey.py +++ b/tests/test_pkey.py @@ -23,6 +23,7 @@ Some unit tests for public/private key objects. import unittest import os +import stat from binascii import hexlify from hashlib import md5 @@ -36,10 +37,11 @@ from paramiko import ( 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 @@ -702,6 +704,62 @@ class KeyTest(unittest.TestCase): _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, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, o600 + ) + os_.fdopen.assert_called_once_with(os_.open.return_value, "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) + def test_sign_rsa_with_certificate(self): data = b"ice weasels" key_path = _support(os.path.join("cert_support", "test_rsa.key")) diff --git a/tests/test_transport.py b/tests/test_transport.py index 737ef705..fa7a3c1a 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -1121,7 +1121,12 @@ class AlgorithmDisablingTests(unittest.TestCase): t = Transport(sock=Mock()) assert t.preferred_ciphers == t._preferred_ciphers assert t.preferred_macs == t._preferred_macs - assert t.preferred_keys == t._preferred_keys + assert t.preferred_keys == tuple( + t._preferred_keys + + tuple( + "{}-cert-v01@openssh.com".format(x) for x in t._preferred_keys + ) + ) assert t.preferred_kex == t._preferred_kex def test_preferred_lists_filter_disabled_algorithms(self): @@ -1140,6 +1145,7 @@ class AlgorithmDisablingTests(unittest.TestCase): assert "hmac-md5" not in t.preferred_macs assert "ssh-dss" in t._preferred_keys assert "ssh-dss" not in t.preferred_keys + assert "ssh-dss-cert-v01@openssh.com" not in t.preferred_keys assert "diffie-hellman-group14-sha256" in t._preferred_kex assert "diffie-hellman-group14-sha256" not in t.preferred_kex |