Hand-picked backport of #912, fixes #741

4 files changed, 47 insertions, 2 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/transport.py b/paramiko/transport.py
index 802b496f..136d7fb2 100644
--- a/paramiko/transport.py
+++ b/paramiko/transport.py
@@ -78,6 +78,7 @@ def _join_lingering_threads():
     for thr in _active_threads:
         thr.stop_thread()
 
+
 import atexit
 atexit.register(_join_lingering_threads)
 
diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst
index 9aead611..e4aa5261 100644
--- a/sites/www/changelog.rst
+++ b/sites/www/changelog.rst
@@ -2,6 +2,16 @@
 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.
 * :bug:`983` Move ``sha1`` above the now-arguably-broken ``md5`` in the list of
   preferred MAC algorithms, as an incremental security improvement for users
   whose target systems offer both. Credit: Pierce Lopez.
diff --git a/tests/test_pkey.py b/tests/test_pkey.py
index 24d78c3e..394a2cf4 100644
--- a/tests/test_pkey.py
+++ b/tests/test_pkey.py
@@ -120,6 +120,18 @@ class KeyTest (unittest.TestCase):
     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'
@@ -426,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)
@@ -436,3 +449,18 @@ class KeyTest (unittest.TestCase):
         key = RSAKey.from_private_key_file(test_path('test_rsa.key'))
         comparable = TEST_KEY_BYTESTR_2 if PY2 else TEST_KEY_BYTESTR_3
         self.assertEqual(str(key), comparable)
+
+    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)