From 10eb7f52b799e6b36deeebb9b78f5d0734d9f05b Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 7 Nov 2012 08:44:05 -0500 Subject: POSIX: ensure that automatically-created parent dirs have appropriate +x perms --- src/lib/Bcfg2/Client/Tools/POSIX/base.py | 20 ++++++++++-- .../TestClient/TestTools/TestPOSIX/Testbase.py | 38 +++++++++++++--------- 2 files changed, 41 insertions(+), 17 deletions(-) diff --git a/src/lib/Bcfg2/Client/Tools/POSIX/base.py b/src/lib/Bcfg2/Client/Tools/POSIX/base.py index 35dc57612..3873c6d98 100644 --- a/src/lib/Bcfg2/Client/Tools/POSIX/base.py +++ b/src/lib/Bcfg2/Client/Tools/POSIX/base.py @@ -5,6 +5,7 @@ import sys import pwd import grp import stat +import copy import shutil import Bcfg2.Client.Tools import Bcfg2.Client.XML @@ -672,7 +673,8 @@ class POSIXTool(Bcfg2.Client.Tools.Tool): us, but it sets permissions according to umask, which is probably wrong. we need to find out which directories were created and set permissions on those - (http://trac.mcs.anl.gov/projects/bcfg2/ticket/1125) """ + (http://trac.mcs.anl.gov/projects/bcfg2/ticket/1125 and + http://trac.mcs.anl.gov/projects/bcfg2/ticket/1134) """ created = [] if path is None: path = entry.get("name") @@ -689,8 +691,22 @@ class POSIXTool(Bcfg2.Client.Tools.Tool): self.logger.error('POSIX: Failed to create directory %s: %s' % (path, err)) rv = False + + # we need to make sure that we give +x to everyone who needs + # it. E.g., if the file that's been distributed is 0600, we + # can't make the parent directories 0600 also; that'd be + # pretty useless. They need to be 0700. + tmpentry = copy.deepcopy(entry) + newmode = int(entry.get('mode'), 8) + for i in range(0, 3): + if newmode & (6 * pow(8, i)): + newmode |= 1 * pow(8, i) + tmpentry.set('mode', oct(newmode)) + for acl in tmpentry.findall('ACL'): + acl.set('perms', + oct(self._norm_acl_perms(acl.get('perms')) | ACL_MAP['x'])) for cpath in created: - rv &= self._set_perms(entry, path=cpath) + rv &= self._set_perms(tmpentry, path=cpath) return rv diff --git a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py index 0e0234719..ec194d401 100644 --- a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py +++ b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py @@ -87,7 +87,7 @@ class TestPOSIXTool(Bcfg2TestCase): @patch("Bcfg2.Client.Tools.POSIX.base.%s._set_perms" % test_obj.__name__) def test_install(self, mock_set_perms, mock_walk): entry = lxml.etree.Element("Path", name="/test", type="file") - + mock_set_perms.return_value = True self.assertTrue(self.ptool.install(entry)) mock_set_perms.assert_called_with(entry) @@ -705,7 +705,7 @@ class TestPOSIXTool(Bcfg2TestCase): for attr, idx, val in expected: self.assertEqual(entry.get(attr), val) self.assertEqual(entry.get("current_mtime"), str(mtime)) - + # failure modes for each checked datum. tuple of changed attr, # return value index, new (failing) value failures = [('current_owner', 1, '10'), @@ -713,7 +713,7 @@ class TestPOSIXTool(Bcfg2TestCase): ('current_mode', 3, '0660')] if HAS_SELINUX: failures.append(('current_secontext', 4, 'root_t')) - + for fail_attr, fail_idx, fail_val in failures: entry = reset() entry.set("mtime", str(mtime)) @@ -749,7 +749,7 @@ class TestPOSIXTool(Bcfg2TestCase): for attr, idx, val in expected: self.assertEqual(entry.get(attr), val) self.assertEqual(entry.get("current_mtime"), str(fail_mtime)) - + if HAS_SELINUX: # test success and failure for __default__ secontext entry = reset() @@ -905,7 +905,7 @@ class TestPOSIXTool(Bcfg2TestCase): # _verify_acls code, and I don't feel like doing that, so eff # it. let's just test to make sure that failures are # identified at all for now. - + acls = {("access", posix1e.ACL_USER, "user"): 7, ("default", posix1e.ACL_GROUP, "group"): 5} extra_acls = copy.deepcopy(acls) @@ -950,7 +950,9 @@ class TestPOSIXTool(Bcfg2TestCase): @patch("Bcfg2.Client.Tools.POSIX.base.%s._set_perms" % test_obj.__name__) def test_makedirs(self, mock_set_perms, mock_exists, mock_makedirs): entry = lxml.etree.Element("Path", name="/test/foo/bar", - type="directory") + type="directory", mode="0644") + parent_entry = lxml.etree.Element("Path", name="/test/foo/bar", + type="directory", mode="0755") def reset(): mock_exists.reset_mock() @@ -968,17 +970,21 @@ class TestPOSIXTool(Bcfg2TestCase): self.assertItemsEqual(mock_exists.call_args_list, [call("/test"), call("/test/foo"), call("/test/foo/bar")]) - self.assertItemsEqual(mock_set_perms.call_args_list, - [call(entry, path="/test/foo"), - call(entry, path="/test/foo/bar")]) + for args in mock_set_perms.call_args_list: + self.assertXMLEqual(args[0][0], parent_entry) + self.assertItemsEqual([a[1] for a in mock_set_perms.call_args_list], + [dict(path="/test/foo"), + dict(path="/test/foo/bar")]) mock_makedirs.assert_called_with(entry.get("name")) reset() mock_makedirs.side_effect = OSError self.assertFalse(self.ptool._makedirs(entry)) - self.assertItemsEqual(mock_set_perms.call_args_list, - [call(entry, path="/test/foo"), - call(entry, path="/test/foo/bar")]) + for args in mock_set_perms.call_args_list: + self.assertXMLEqual(args[0][0], parent_entry) + self.assertItemsEqual([a[1] for a in mock_set_perms.call_args_list], + [dict(path="/test/foo"), + dict(path="/test/foo/bar")]) reset() mock_makedirs.side_effect = None @@ -992,7 +998,9 @@ class TestPOSIXTool(Bcfg2TestCase): self.assertItemsEqual(mock_exists.call_args_list, [call("/test"), call("/test/foo"), call("/test/foo/bar")]) - self.assertItemsEqual(mock_set_perms.call_args_list, - [call(entry, path="/test/foo"), - call(entry, path="/test/foo/bar")]) + for args in mock_set_perms.call_args_list: + self.assertXMLEqual(args[0][0], parent_entry) + self.assertItemsEqual([a[1] for a in mock_set_perms.call_args_list], + [dict(path="/test/foo"), + dict(path="/test/foo/bar")]) mock_makedirs.assert_called_with(entry.get("name")) -- cgit v1.2.3-1-g7c22