File Fix-CVE-re-PKey.write_private_key-chmod-race.patch of Package saltbundlepy-paramiko

Overview Repositories Revisions Requests Users Attributes Meta

File Fix-CVE-re-PKey.write_private_key-chmod-race.patch of Package saltbundlepy-paramiko (Revision 7164b662da2fd5375debf5aa75af954c)

Currently displaying revision 7164b662da2fd5375debf5aa75af954c , Show latest

From 1d1ec50fe83dd2f537fe15ee9aae64f89c7d54d3 Mon Sep 17 00:00:00 2001
From: Jeff Forcier <jeff@bitprophet.org>
Date: Fri, 25 Feb 2022 14:50:42 -0500
Subject: [PATCH] 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, 83 insertions(+), 1 deletion(-)

diff --git a/paramiko/pkey.py b/paramiko/pkey.py
index fa014800..5b547636 100644
--- a/paramiko/pkey.py
+++ b/paramiko/pkey.py
@@ -352,7 +352,17 @@ 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).
+        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/tests/test_pkey.py b/tests/test_pkey.py
index 08d38e3b..c8873edd 100644
--- a/tests/test_pkey.py
+++ b/tests/test_pkey.py
@@ -23,11 +23,15 @@ Some unit tests for public/private key objects.
 
 import unittest
 import os
+import stat
 from binascii import hexlify
 from hashlib import md5
 
 from paramiko import RSAKey, DSSKey, ECDSAKey, Ed25519Key, Message, util
 from paramiko.py3compat import StringIO, byte_chr, b, bytes, PY2
+from paramiko.common import o600
+
+from mock import patch, Mock
 
 from .util import _support
 
@@ -544,3 +548,57 @@ class KeyTest(unittest.TestCase):
             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)
-- 
2.26.2

Places

File Fix-CVE-re-PKey.write_private_key-chmod-race.patch of Package saltbundlepy-paramiko (Revision 7164b662da2fd5375debf5aa75af954c)

Places