File Fix-CVE-re-PKey.write_private_key-chmod-race.patch of Package saltbundlepy-paramiko
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