diff options
| -rw-r--r-- | paramiko/pkey.py | 10 | ||||
| -rw-r--r-- | paramiko/sftp_client.py | 33 | ||||
| -rw-r--r-- | paramiko/sftp_server.py | 6 | ||||
| -rw-r--r-- | paramiko/sftp_si.py | 12 | ||||
| -rw-r--r-- | sites/www/changelog.rst | 25 | ||||
| -rw-r--r-- | tests/stub_sftp.py | 14 | ||||
| -rw-r--r-- | tests/test_pkey.py | 35 | ||||
| -rwxr-xr-x | tests/test_sftp.py | 33 |
8 files changed, 159 insertions, 9 deletions
diff --git a/paramiko/pkey.py b/paramiko/pkey.py index f5b0cd18..35a26fc7 100644 --- a/paramiko/pkey.py +++ b/paramiko/pkey.py @@ -48,6 +48,12 @@ class PKey(object): 'blocksize': 16, 'mode': modes.CBC }, + 'AES-256-CBC': { + 'cipher': algorithms.AES, + 'keysize': 32, + 'blocksize': 16, + 'mode': modes.CBC + }, 'DES-EDE3-CBC': { 'cipher': algorithms.TripleDES, 'keysize': 24, @@ -344,13 +350,13 @@ class PKey(object): """ with open(filename, 'w') as f: os.chmod(filename, o600) - self._write_private_key(f, key, format) + self._write_private_key(f, key, format, password=password) def _write_private_key(self, f, key, format, password=None): if password is None: encryption = serialization.NoEncryption() else: - encryption = serialization.BestEncryption(password) + encryption = serialization.BestAvailableEncryption(b(password)) f.write(key.private_bytes( serialization.Encoding.PEM, diff --git a/paramiko/sftp_client.py b/paramiko/sftp_client.py index 12fccb2f..f2e6869f 100644 --- a/paramiko/sftp_client.py +++ b/paramiko/sftp_client.py @@ -36,8 +36,8 @@ from paramiko.sftp import ( CMD_CLOSE, SFTP_FLAG_READ, SFTP_FLAG_WRITE, SFTP_FLAG_CREATE, SFTP_FLAG_TRUNC, SFTP_FLAG_APPEND, SFTP_FLAG_EXCL, CMD_OPEN, CMD_REMOVE, CMD_RENAME, CMD_MKDIR, CMD_RMDIR, CMD_STAT, CMD_ATTRS, CMD_LSTAT, - CMD_SYMLINK, CMD_SETSTAT, CMD_READLINK, CMD_REALPATH, CMD_STATUS, SFTP_OK, - SFTP_EOF, SFTP_NO_SUCH_FILE, SFTP_PERMISSION_DENIED, + CMD_SYMLINK, CMD_SETSTAT, CMD_READLINK, CMD_REALPATH, CMD_STATUS, + CMD_EXTENDED, SFTP_OK, SFTP_EOF, SFTP_NO_SUCH_FILE, SFTP_PERMISSION_DENIED, ) from paramiko.sftp_attr import SFTPAttributes @@ -368,8 +368,10 @@ class SFTPClient(BaseSFTP, ClosingContextManager): """ Rename a file or folder from ``oldpath`` to ``newpath``. - :param str oldpath: existing name of the file or folder - :param str newpath: new name for the file or folder + :param str oldpath: + existing name of the file or folder + :param str newpath: + new name for the file or folder, must not exist already :raises: ``IOError`` -- if ``newpath`` is a folder, or something else goes @@ -380,6 +382,26 @@ class SFTPClient(BaseSFTP, ClosingContextManager): self._log(DEBUG, 'rename(%r, %r)' % (oldpath, newpath)) self._request(CMD_RENAME, oldpath, newpath) + def posix_rename(self, oldpath, newpath): + """ + Rename a file or folder from ``oldpath`` to ``newpath``, following + posix conventions. + + :param str oldpath: existing name of the file or folder + :param str newpath: new name for the file or folder, will be + overwritten if it already exists + + :raises: + ``IOError`` -- if ``newpath`` is a folder, posix-rename is not + supported by the server or something else goes wrong + """ + oldpath = self._adjust_cwd(oldpath) + newpath = self._adjust_cwd(newpath) + self._log(DEBUG, 'posix_rename(%r, %r)' % (oldpath, newpath)) + self._request( + CMD_EXTENDED, "posix-rename@openssh.com", oldpath, newpath + ) + def mkdir(self, path, mode=o777): """ Create a folder (directory) named ``path`` with numeric mode ``mode``. @@ -451,8 +473,7 @@ class SFTPClient(BaseSFTP, ClosingContextManager): def symlink(self, source, dest): """ - Create a symbolic link (shortcut) of the ``source`` path at - ``destination``. + Create a symbolic link to the ``source`` path at ``destination``. :param str source: path of the original file :param str dest: path of the newly created symlink diff --git a/paramiko/sftp_server.py b/paramiko/sftp_server.py index 1cfe286b..f7d1c657 100644 --- a/paramiko/sftp_server.py +++ b/paramiko/sftp_server.py @@ -469,6 +469,12 @@ class SFTPServer (BaseSFTP, SubsystemHandler): tag = msg.get_text() if tag == 'check-file': self._check_file(request_number, msg) + elif tag == 'posix-rename@openssh.com': + oldpath = msg.get_text() + newpath = msg.get_text() + self._send_status( + request_number, self.server.posix_rename(oldpath, newpath) + ) else: self._send_status(request_number, SFTP_OP_UNSUPPORTED) else: diff --git a/paramiko/sftp_si.py b/paramiko/sftp_si.py index 09e7025c..40969309 100644 --- a/paramiko/sftp_si.py +++ b/paramiko/sftp_si.py @@ -201,6 +201,18 @@ class SFTPServerInterface (object): """ return SFTP_OP_UNSUPPORTED + def posix_rename(self, oldpath, newpath): + """ + Rename (or move) a file, following posix conventions. If newpath + already exists, it will be overwritten. + + :param str oldpath: + the requested path (relative or absolute) of the existing file. + :param str newpath: the requested new path of the file. + :return: an SFTP error code `int` like ``SFTP_OK``. + """ + return SFTP_OP_UNSUPPORTED + def mkdir(self, path, attr): """ Create a new directory with the given attributes. The ``attr`` diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 57bc306d..64b089ac 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,10 +2,35 @@ Changelog ========= +* :bug:`741` (also :issue:`809`, :issue:`772`; all via :issue:`912`) Writing + encrypted/password-protected private key files was silently broken since 2.0 + due to an incorrect API call; this has been fixed. + + Includes a directly related fix, namely adding the ability to read + ``AES-256-CBC`` ciphered private keys (which is now what we tend to write out + as it is Cryptography's default private key cipher.) + + Thanks to ``@virlos`` for the original report, Chris Harris and ``@ibuler`` + for initial draft PRs, and ``@jhgorrell`` for the final patch. +* :feature:`65` (via :issue:`471`) Add support for OpenSSH's SFTP + ``posix-rename`` protocol extension (section 3.3 of `OpenSSH's protocol + extension document + <http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL?rev=1.31>`_), + via a new ``posix_rename`` method in `SFTPClient + <paramiko.sftp_client.SFTPClient.posix_rename>` and `SFTPServerInterface + <paramiko.sftp_si.SFTPServerInterface.posix_rename>`. Thanks to Wren Turkal + for the initial patch & Mika Pflüger for the enhanced, merged PR. * :feature:`869` Add an ``auth_timeout`` kwarg to `SSHClient.connect <paramiko.client.SSHClient.connect>` (default: 30s) to avoid hangs when the remote end becomes unresponsive during the authentication step. Credit to ``@timsavage``. + + .. note:: + This technically changes behavior, insofar as very slow auth steps >30s + will now cause timeout exceptions instead of completing. We doubt most + users will notice; those affected can simply give a higher value to + ``auth_timeout``. + * :support:`921` Tighten up the ``__hash__`` implementation for various key classes; less code is good code. Thanks to Francisco Couzo for the patch. * :bug:`983` Move ``sha1`` above the now-arguably-broken ``md5`` in the list of diff --git a/tests/stub_sftp.py b/tests/stub_sftp.py index 334af561..0d673091 100644 --- a/tests/stub_sftp.py +++ b/tests/stub_sftp.py @@ -24,7 +24,7 @@ import os import sys from paramiko import ( ServerInterface, SFTPServerInterface, SFTPServer, SFTPAttributes, - SFTPHandle, SFTP_OK, AUTH_SUCCESSFUL, OPEN_SUCCEEDED, + SFTPHandle, SFTP_OK, SFTP_FAILURE, AUTH_SUCCESSFUL, OPEN_SUCCEEDED, ) from paramiko.common import o666 @@ -141,12 +141,24 @@ class StubSFTPServer (SFTPServerInterface): def rename(self, oldpath, newpath): oldpath = self._realpath(oldpath) newpath = self._realpath(newpath) + if os.path.exists(newpath): + return SFTP_FAILURE try: os.rename(oldpath, newpath) except OSError as e: return SFTPServer.convert_errno(e.errno) return SFTP_OK + def posix_rename(self, oldpath, newpath): + oldpath = self._realpath(oldpath) + newpath = self._realpath(newpath) + try: + os.rename(oldpath, newpath) + except OSError as e: + return SFTPServer.convert_errno(e.errno) + return SFTP_OK + + def mkdir(self, path, attr): path = self._realpath(path) try: diff --git a/tests/test_pkey.py b/tests/test_pkey.py index a26ff170..6e589915 100644 --- a/tests/test_pkey.py +++ b/tests/test_pkey.py @@ -113,6 +113,25 @@ TEST_KEY_BYTESTR_3 = '\x00\x00\x00\x07ssh-rsa\x00\x00\x00\x01#\x00\x00\x00\x00ӏ class KeyTest(unittest.TestCase): + + def setUp(self): + pass + + def tearDown(self): + pass + + def assert_keyfile_is_encrypted(self, keyfile): + """ + A quick check that filename looks like an encrypted key. + """ + with open(keyfile, "r") as fh: + self.assertEqual( + fh.readline()[:-1], + "-----BEGIN RSA PRIVATE KEY-----" + ) + self.assertEqual(fh.readline()[:-1], "Proc-Type: 4,ENCRYPTED") + self.assertEqual(fh.readline()[0:10], "DEK-Info: ") + def test_1_generate_key_bytes(self): key = util.generate_key_bytes(md5, x1234, 'happy birthday', 30) exp = b'\x61\xE1\xF2\x72\xF4\xC1\xC4\x56\x15\x86\xBD\x32\x24\x98\xC0\xE9\x24\x67\x27\x80\xF4\x7B\xB3\x7D\xDA\x7D\x54\x01\x9E\x64' @@ -419,6 +438,7 @@ class KeyTest(unittest.TestCase): # When the bug under test exists, this will ValueError. try: key.write_private_key_file(newfile, password=newpassword) + self.assert_keyfile_is_encrypted(newfile) # Verify the inner key data still matches (when no ValueError) key2 = RSAKey(filename=newfile, password=newpassword) self.assertEqual(key, key2) @@ -437,3 +457,18 @@ class KeyTest(unittest.TestCase): ) self.assertNotEqual(key1.asbytes(), key2.asbytes()) + + def test_keyfile_is_actually_encrypted(self): + # Read an existing encrypted private key + file_ = test_path('test_rsa_password.key') + password = 'television' + newfile = file_ + '.new' + newpassword = 'radio' + key = RSAKey(filename=file_, password=password) + # Write out a newly re-encrypted copy with a new password. + # When the bug under test exists, this will ValueError. + try: + key.write_private_key_file(newfile, password=newpassword) + self.assert_keyfile_is_encrypted(newfile) + finally: + os.remove(newfile) diff --git a/tests/test_sftp.py b/tests/test_sftp.py index d3064fff..8f1b7d2e 100755 --- a/tests/test_sftp.py +++ b/tests/test_sftp.py @@ -276,6 +276,39 @@ class SFTPTest (unittest.TestCase): except: pass + + def test_5a_posix_rename(self): + """Test posix-rename@openssh.com protocol extension.""" + try: + # first check that the normal rename works as specified + with sftp.open(FOLDER + '/a', 'w') as f: + f.write('one') + sftp.rename(FOLDER + '/a', FOLDER + '/b') + with sftp.open(FOLDER + '/a', 'w') as f: + f.write('two') + try: + sftp.rename(FOLDER + '/a', FOLDER + '/b') + self.assertTrue(False, 'no exception when rename-ing onto existing file') + except (OSError, IOError): + pass + + # now check with the posix_rename + sftp.posix_rename(FOLDER + '/a', FOLDER + '/b') + with sftp.open(FOLDER + '/b', 'r') as f: + data = u(f.read()) + self.assertEqual('two', data, "Contents of renamed file not the same as original file") + + finally: + try: + sftp.remove(FOLDER + '/a') + except: + pass + try: + sftp.remove(FOLDER + '/b') + except: + pass + + def test_6_folder(self): """ create a temporary folder, verify that we can create a file in it, then |