[Python-checkins] gh-94315: Check for DAC override capability (GH-94316)
tiran
webhook-mailer at python.org
Mon Jun 27 14:27:24 EDT 2022
https://github.com/python/cpython/commit/7e0d98ecb3f049841de9854e7a3eca3e2638e4b2
commit: 7e0d98ecb3f049841de9854e7a3eca3e2638e4b2
branch: main
author: Christian Heimes <christian at python.org>
committer: tiran <christian at python.org>
date: 2022-06-27T20:27:19+02:00
summary:
gh-94315: Check for DAC override capability (GH-94316)
``os.geteuid() == 0`` is not a reliable check whether the current user
has the capability to bypass permission checks. Tests now probe for DAC
override.
files:
A Misc/NEWS.d/next/Tests/2022-06-27-08-53-40.gh-issue-94315.MoZT9t.rst
M Lib/test/support/os_helper.py
M Lib/test/test_argparse.py
M Lib/test/test_import/__init__.py
M Lib/test/test_py_compile.py
M Lib/test/test_shutil.py
diff --git a/Lib/test/support/os_helper.py b/Lib/test/support/os_helper.py
index 589ef19dd34cc..4edb1abec202a 100644
--- a/Lib/test/support/os_helper.py
+++ b/Lib/test/support/os_helper.py
@@ -263,7 +263,7 @@ def can_chmod():
else:
can = stat.S_IMODE(mode1) != stat.S_IMODE(mode2)
finally:
- os.unlink(TESTFN)
+ unlink(TESTFN)
_can_chmod = can
return can
@@ -278,6 +278,48 @@ def skip_unless_working_chmod(test):
return test if ok else unittest.skip(msg)(test)
+# Check whether the current effective user has the capability to override
+# DAC (discretionary access control). Typically user root is able to
+# bypass file read, write, and execute permission checks. The capability
+# is independent of the effective user. See capabilities(7).
+_can_dac_override = None
+
+def can_dac_override():
+ global _can_dac_override
+
+ if not can_chmod():
+ _can_dac_override = False
+ if _can_dac_override is not None:
+ return _can_dac_override
+
+ try:
+ with open(TESTFN, "wb") as f:
+ os.chmod(TESTFN, 0o400)
+ try:
+ with open(TESTFN, "wb"):
+ pass
+ except OSError:
+ _can_dac_override = False
+ else:
+ _can_dac_override = True
+ finally:
+ unlink(TESTFN)
+
+ return _can_dac_override
+
+
+def skip_if_dac_override(test):
+ ok = not can_dac_override()
+ msg = "incompatible with CAP_DAC_OVERRIDE"
+ return test if ok else unittest.skip(msg)(test)
+
+
+def skip_unless_dac_override(test):
+ ok = can_dac_override()
+ msg = "requires CAP_DAC_OVERRIDE"
+ return test if ok else unittest.skip(msg)(test)
+
+
def unlink(filename):
try:
_unlink(filename)
diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py
index 673ef89b5e1cf..425b6bb3e0b4e 100644
--- a/Lib/test/test_argparse.py
+++ b/Lib/test/test_argparse.py
@@ -1734,8 +1734,7 @@ def __eq__(self, other):
return self.name == other.name
- at unittest.skipIf(hasattr(os, 'geteuid') and os.geteuid() == 0,
- "non-root user required")
+ at os_helper.skip_if_dac_override
class TestFileTypeW(TempDirMixin, ParserTestCase):
"""Test the FileType option/argument type for writing files"""
@@ -1757,8 +1756,8 @@ def setUp(self):
('-x - -', NS(x=eq_stdout, spam=eq_stdout)),
]
- at unittest.skipIf(hasattr(os, 'geteuid') and os.geteuid() == 0,
- "non-root user required")
+
+ at os_helper.skip_if_dac_override
class TestFileTypeX(TempDirMixin, ParserTestCase):
"""Test the FileType option/argument type for writing new files only"""
@@ -1778,8 +1777,7 @@ def setUp(self):
]
- at unittest.skipIf(hasattr(os, 'geteuid') and os.geteuid() == 0,
- "non-root user required")
+ at os_helper.skip_if_dac_override
class TestFileTypeWB(TempDirMixin, ParserTestCase):
"""Test the FileType option/argument type for writing binary files"""
@@ -1796,8 +1794,7 @@ class TestFileTypeWB(TempDirMixin, ParserTestCase):
]
- at unittest.skipIf(hasattr(os, 'geteuid') and os.geteuid() == 0,
- "non-root user required")
+ at os_helper.skip_if_dac_override
class TestFileTypeXB(TestFileTypeX):
"Test the FileType option/argument type for writing new binary files only"
diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py
index ab7f887c374ce..6c5b80bcee6c2 100644
--- a/Lib/test/test_import/__init__.py
+++ b/Lib/test/test_import/__init__.py
@@ -885,10 +885,9 @@ def test_import_pyc_path(self):
@unittest.skipUnless(os.name == 'posix',
"test meaningful only on posix systems")
- @unittest.skipIf(hasattr(os, 'geteuid') and os.geteuid() == 0,
- "due to varying filesystem permission semantics (issue #11956)")
@skip_if_dont_write_bytecode
@os_helper.skip_unless_working_chmod
+ @os_helper.skip_if_dac_override
@unittest.skipIf(is_emscripten, "umask is a stub")
def test_unwritable_directory(self):
# When the umask causes the new __pycache__ directory to be
diff --git a/Lib/test/test_py_compile.py b/Lib/test/test_py_compile.py
index f494aed42feae..a4a52b180dbb5 100644
--- a/Lib/test/test_py_compile.py
+++ b/Lib/test/test_py_compile.py
@@ -115,8 +115,7 @@ def test_relative_path(self):
self.assertTrue(os.path.exists(self.pyc_path))
self.assertFalse(os.path.exists(self.cache_path))
- @unittest.skipIf(hasattr(os, 'geteuid') and os.geteuid() == 0,
- 'non-root user required')
+ @os_helper.skip_if_dac_override
@unittest.skipIf(os.name == 'nt',
'cannot control directory permissions on Windows')
@os_helper.skip_unless_working_chmod
diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py
index 6fa07399007a3..a2c4ab508195b 100644
--- a/Lib/test/test_shutil.py
+++ b/Lib/test/test_shutil.py
@@ -312,8 +312,7 @@ def onerror(*args):
@unittest.skipIf(sys.platform[:6] == 'cygwin',
"This test can't be run on Cygwin (issue #1071513).")
- @unittest.skipIf(hasattr(os, 'geteuid') and os.geteuid() == 0,
- "This test can't be run reliably as root (issue #1076467).")
+ @os_helper.skip_if_dac_override
@os_helper.skip_unless_working_chmod
def test_on_error(self):
self.errorState = 0
@@ -1033,8 +1032,7 @@ def _raise_on_src(fname, *, follow_symlinks=True):
@os_helper.skip_unless_symlink
@os_helper.skip_unless_xattr
- @unittest.skipUnless(hasattr(os, 'geteuid') and os.geteuid() == 0,
- 'root privileges required')
+ @os_helper.skip_unless_dac_override
def test_copyxattr_symlinks(self):
# On Linux, it's only possible to access non-user xattr for symlinks;
# which in turn require root privileges. This test should be expanded
@@ -1830,8 +1828,7 @@ def test_cwd(self):
# Other platforms: shouldn't match in the current directory.
self.assertIsNone(rv)
- @unittest.skipIf(hasattr(os, 'geteuid') and os.geteuid() == 0,
- 'non-root user required')
+ @os_helper.skip_if_dac_override
def test_non_matching_mode(self):
# Set the file read-only and ask for writeable files.
os.chmod(self.temp_file.name, stat.S_IREAD)
@@ -2182,11 +2179,11 @@ def test_move_dir_caseinsensitive(self):
os.rmdir(dst_dir)
- @unittest.skipUnless(hasattr(os, 'geteuid') and os.geteuid() == 0
- and hasattr(os, 'lchflags')
+ @os_helper.skip_unless_dac_override
+ @unittest.skipUnless(hasattr(os, 'lchflags')
and hasattr(stat, 'SF_IMMUTABLE')
and hasattr(stat, 'UF_OPAQUE'),
- 'root privileges required')
+ 'requires lchflags')
def test_move_dir_permission_denied(self):
# bpo-42782: shutil.move should not create destination directories
# if the source directory cannot be removed.
diff --git a/Misc/NEWS.d/next/Tests/2022-06-27-08-53-40.gh-issue-94315.MoZT9t.rst b/Misc/NEWS.d/next/Tests/2022-06-27-08-53-40.gh-issue-94315.MoZT9t.rst
new file mode 100644
index 0000000000000..09d5d7e56ae83
--- /dev/null
+++ b/Misc/NEWS.d/next/Tests/2022-06-27-08-53-40.gh-issue-94315.MoZT9t.rst
@@ -0,0 +1,2 @@
+Tests now check for DAC override capability instead of relying on
+:func:`os.geteuid`.
More information about the Python-checkins
mailing list