diff --git a/certbot/certbot/compat/filesystem.py b/certbot/certbot/compat/filesystem.py index b9b6e5cc6..e44e1b32a 100644 --- a/certbot/certbot/compat/filesystem.py +++ b/certbot/certbot/compat/filesystem.py @@ -78,6 +78,35 @@ def copy_ownership_and_apply_mode(src, dst, mode, copy_user, copy_group): chmod(dst, mode) +# Quite similar to copy_ownership_and_apply_mode, but this time the DACL is copied from +# the source file on Windows. The DACL stays consistent with the dynamic rights of the +# equivalent POSIX mode, because ownership and mode are copied altogether on the destination +# file, so no recomputing of the DACL against the new owner is needed, as it would be +# for a copy_ownership alone method. +def copy_ownership_and_mode(src, dst, copy_user=True, copy_group=True): + # type: (str, str, bool, bool) -> None + """ + Copy ownership (user and optionally group on Linux) and mode/DACL + from the source to the destination. + :param str src: Path of the source file + :param str dst: Path of the destination file + :param bool copy_user: Copy user if `True` + :param bool copy_group: Copy group if `True` on Linux (has no effect on Windows) + """ + if POSIX_MODE: + # On Linux, we just delegate to chown and chmod. + stats = os.stat(src) + user_id = stats.st_uid if copy_user else -1 + group_id = stats.st_gid if copy_group else -1 + os.chown(dst, user_id, group_id) + chmod(dst, stats.st_mode) + else: + if copy_user: + # There is no group handling in Windows + _copy_win_ownership(src, dst) + _copy_win_mode(src, dst) + + def check_mode(file_path, mode): # type: (str, int) -> bool """ @@ -515,6 +544,9 @@ def _analyze_mode(mode): def _copy_win_ownership(src, dst): + # Resolve symbolic links + src = realpath(src) + security_src = win32security.GetFileSecurity(src, win32security.OWNER_SECURITY_INFORMATION) user_src = security_src.GetSecurityDescriptorOwner() @@ -526,6 +558,19 @@ def _copy_win_ownership(src, dst): win32security.SetFileSecurity(dst, win32security.OWNER_SECURITY_INFORMATION, security_dst) +def _copy_win_mode(src, dst): + # Resolve symbolic links + src = realpath(src) + + # Copy the DACL from src to dst. + security_src = win32security.GetFileSecurity(src, win32security.DACL_SECURITY_INFORMATION) + dacl = security_src.GetSecurityDescriptorDacl() + + security_dst = win32security.GetFileSecurity(dst, win32security.DACL_SECURITY_INFORMATION) + security_dst.SetSecurityDescriptorDacl(1, dacl, 0) + win32security.SetFileSecurity(dst, win32security.DACL_SECURITY_INFORMATION, security_dst) + + def _generate_windows_flags(rights_desc): # Some notes about how each POSIX right is interpreted. # diff --git a/certbot/tests/compat/filesystem_test.py b/certbot/tests/compat/filesystem_test.py index 1c2d2df0d..3f6a5ec1d 100644 --- a/certbot/tests/compat/filesystem_test.py +++ b/certbot/tests/compat/filesystem_test.py @@ -280,14 +280,14 @@ class WindowsMkdirTests(test_util.TempDirTestCase): self.assertEqual(original_mkdir, std_os.mkdir) -class OwnershipTest(test_util.TempDirTestCase): - """Tests about copy_ownership_and_apply_mode and has_same_ownership""" +class CopyOwnershipAndModeTest(test_util.TempDirTestCase): + """Tests about copy_ownership_and_apply_mode, copy_ownership_and_mode and has_same_ownership""" def setUp(self): - super(OwnershipTest, self).setUp() + super(CopyOwnershipAndModeTest, self).setUp() self.probe_path = _create_probe(self.tempdir) @unittest.skipIf(POSIX_MODE, reason='Test specific to Windows security') - def test_copy_ownership_windows(self): + def test_copy_ownership_and_apply_mode_windows(self): system = win32security.ConvertStringSidToSid(SYSTEM_SID) security = win32security.SECURITY_ATTRIBUTES().SECURITY_DESCRIPTOR security.SetSecurityDescriptorOwner(system, False) @@ -313,7 +313,7 @@ class OwnershipTest(test_util.TempDirTestCase): if dacl.GetAce(index)[2] == everybody]) @unittest.skipUnless(POSIX_MODE, reason='Test specific to Linux security') - def test_copy_ownership_linux(self): + def test_copy_ownership_and_apply_mode_linux(self): with mock.patch('os.chown') as mock_chown: with mock.patch('os.chmod') as mock_chmod: with mock.patch('os.stat') as mock_stat: @@ -334,6 +334,24 @@ class OwnershipTest(test_util.TempDirTestCase): self.assertTrue(filesystem.has_same_ownership(path1, path2)) + @unittest.skipIf(POSIX_MODE, reason='Test specific to Windows security') + def test_copy_ownership_and_mode_windows(self): + src = self.probe_path + dst = _create_probe(self.tempdir, name='dst') + + filesystem.chmod(src, 0o700) + self.assertTrue(filesystem.check_mode(src, 0o700)) + self.assertTrue(filesystem.check_mode(dst, 0o744)) + + # Checking an actual change of owner is tricky during a unit test, since we do not know + # if any user exists beside the current one. So we mock _copy_win_ownership. It's behavior + # have been checked theoretically with test_copy_ownership_and_apply_mode_windows. + with mock.patch('certbot.compat.filesystem._copy_win_ownership') as mock_copy_owner: + filesystem.copy_ownership_and_mode(src, dst) + + mock_copy_owner.assert_called_once_with(src, dst) + self.assertTrue(filesystem.check_mode(dst, 0o700)) + class CheckPermissionsTest(test_util.TempDirTestCase): """Tests relative to functions that check modes.""" @@ -537,9 +555,9 @@ def _set_owner(target, security_owner, user): target, win32security.OWNER_SECURITY_INFORMATION, security_owner) -def _create_probe(tempdir): +def _create_probe(tempdir, name='probe'): filesystem.chmod(tempdir, 0o744) - probe_path = os.path.join(tempdir, 'probe') + probe_path = os.path.join(tempdir, name) util.safe_open(probe_path, 'w', chmod=0o744).close() return probe_path