From cae2fcc0135c26811b1ce353ea28e4a93900c138 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Mon, 10 Feb 2014 09:02:16 -0500 Subject: POSIX: Fix verification of symlinks * Stat the link itself, not its target * Get SELinux context from the link, not the target * Don't get ACLs at all; symlinks don't have their own ACLs The first issue listed wasn't actually a bug, because none of the information queried from the target by the stat call was actually used in verification, but it's been fixed for completeness. --- src/lib/Bcfg2/Client/Tools/POSIX/base.py | 6 +-- .../TestClient/TestTools/TestPOSIX/Testbase.py | 50 +++++++++++++--------- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/lib/Bcfg2/Client/Tools/POSIX/base.py b/src/lib/Bcfg2/Client/Tools/POSIX/base.py index e593e0a0a..12f7f8a56 100644 --- a/src/lib/Bcfg2/Client/Tools/POSIX/base.py +++ b/src/lib/Bcfg2/Client/Tools/POSIX/base.py @@ -419,7 +419,7 @@ class POSIXTool(Bcfg2.Client.Tools.Tool): """ Get data on the existing state of -- e.g., whether or not it exists, owner, group, permissions, etc. """ try: - ondisk = os.stat(path) + ondisk = os.lstat(path) except OSError: self.logger.debug("POSIX: %s does not exist" % path) return (False, None, None, None, None, None) @@ -456,7 +456,7 @@ class POSIXTool(Bcfg2.Client.Tools.Tool): if HAS_SELINUX: try: - secontext = selinux.getfilecon(path)[1].split(":")[2] + secontext = selinux.lgetfilecon(path)[1].split(":")[2] except (OSError, KeyError): err = sys.exc_info()[1] self.logger.debug("POSIX: Could not get current SELinux " @@ -465,7 +465,7 @@ class POSIXTool(Bcfg2.Client.Tools.Tool): else: secontext = None - if HAS_ACLS: + if HAS_ACLS and not stat.S_ISLNK(ondisk): acls = self._list_file_acls(path) else: acls = None diff --git a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py index d2f383f42..ab6e2fe54 100644 --- a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py +++ b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py @@ -563,16 +563,16 @@ class TestPOSIXTool(TestTool): self.assertEqual(6, ptool._norm_acl_perms("rwa")) self.assertEqual(4, ptool._norm_acl_perms("rr")) - @patch('os.stat') - def test__gather_data(self, mock_stat): + @patch('os.lstat') + def test__gather_data(self, mock_lstat): ptool = self.get_obj() path = '/test' - mock_stat.side_effect = OSError + mock_lstat.side_effect = OSError self.assertFalse(ptool._gather_data(path)[0]) - mock_stat.assert_called_with(path) + mock_lstat.assert_called_with(path) - mock_stat.reset_mock() - mock_stat.side_effect = None + mock_lstat.reset_mock() + mock_lstat.side_effect = None # create a return value stat_rv = MagicMock() def stat_getitem(key): @@ -585,7 +585,7 @@ class TestPOSIXTool(TestTool): # and ensure that they're stripped return int('060660', 8) stat_rv.__getitem__ = Mock(side_effect=stat_getitem) - mock_stat.return_value = stat_rv + mock_lstat.return_value = stat_rv # disable selinux and acls for this call -- we test them # separately so that we can skip those tests as appropriate @@ -597,7 +597,7 @@ class TestPOSIXTool(TestTool): (stat_rv, '0', '10', '0660', None, None)) Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX, \ Bcfg2.Client.Tools.POSIX.base.HAS_ACLS = states - mock_stat.assert_called_with(path) + mock_lstat.assert_called_with(path) @skipUnless(HAS_SELINUX, "SELinux not found, skipping") def test__gather_data_selinux(self): @@ -605,12 +605,12 @@ class TestPOSIXTool(TestTool): context = 'system_u:object_r:root_t:s0' path = '/test' - @patch('os.stat') - @patchIf(HAS_SELINUX, "selinux.getfilecon") - def inner(mock_getfilecon, mock_stat): - mock_getfilecon.return_value = [len(context) + 1, context] - mock_stat.return_value = MagicMock() - mock_stat.return_value.__getitem__.return_value = MagicMock() + @patch('os.lstat') + @patchIf(HAS_SELINUX, "selinux.lgetfilecon") + def inner(mock_lgetfilecon, mock_lstat): + mock_lgetfilecon.return_value = [len(context) + 1, context] + mock_lstat.return_value = MagicMock() + mock_lstat.return_value.__getitem__.return_value = MagicMock() # disable acls for this call and test them separately state = (Bcfg2.Client.Tools.POSIX.base.HAS_ACLS, Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX) @@ -619,30 +619,40 @@ class TestPOSIXTool(TestTool): self.assertEqual(ptool._gather_data(path)[4], 'root_t') Bcfg2.Client.Tools.POSIX.base.HAS_ACLS, \ Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX = state - mock_getfilecon.assert_called_with(path) + mock_lgetfilecon.assert_called_with(path) inner() @skipUnless(HAS_ACLS, "ACLS not found, skipping") - @patch('os.stat') - def test__gather_data_acls(self, mock_stat): + @patch('os.lstat') + @patch('stat.S_ISLNK') + def test__gather_data_acls(self, mock_S_ISLNK, mock_lstat): ptool = self.get_obj() ptool._list_file_acls = Mock() acls = {("default", posix1e.ACL_USER, "testuser"): "rwx", ("access", posix1e.ACL_GROUP, "testgroup"): "rx"} ptool._list_file_acls.return_value = acls path = '/test' - mock_stat.return_value = MagicMock() - mock_stat.return_value.__getitem__.return_value = MagicMock() + mock_lstat.return_value = MagicMock() + mock_lstat.return_value.__getitem__.return_value = MagicMock() + mock_S_ISLNK.return_value = False # disable selinux for this call and test it separately state = (Bcfg2.Client.Tools.POSIX.base.HAS_ACLS, Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX) Bcfg2.Client.Tools.POSIX.base.HAS_ACLS = True Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX = False self.assertItemsEqual(ptool._gather_data(path)[5], acls) + ptool._list_file_acls.assert_called_with(path) + + # symlinks can't have their own ACLs, so ensure that the + # _list_file_acls call is skipped and no ACLs are returned + mock_S_ISLNK.return_value = True + ptool._list_file_acls.reset_mock() + self.assertEqual(ptool._gather_data(path)[5], None) + self.assertFalse(ptool._list_file_acls.called) + Bcfg2.Client.Tools.POSIX.base.HAS_ACLS, \ Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX = state - ptool._list_file_acls.assert_called_with(path) @patchIf(HAS_SELINUX, "selinux.matchpathcon") def test_verify_metadata(self, mock_matchpathcon): -- cgit v1.2.3-1-g7c22