From 96a780d75274031fd97167c1109fcb63efa7d5cd Mon Sep 17 00:00:00 2001

From: Rob Crittenden <rcritten@redhat.com>

Date: Tue, 5 Apr 2022 09:25:45 -0400

Subject: [PATCH] Allow multiple file modes in the FileChecker

There are some cases where a strict file mode is not

necessary. The kadmind.log is one.

It is owned root:root so there is no real difference

between 0600 and 0640. So allow both.

https://bugzilla.redhat.com/show_bug.cgi?id=2058239

Signed-off-by: Rob Crittenden <rcritten@redhat.com>

---

 src/ipahealthcheck/core/files.py | 31 +++++++++++++++++++++++--------

 src/ipahealthcheck/ipa/files.py  |  3 ++-

 tests/test_core_files.py         | 32 ++++++++++++++++++++++++++++----

 3 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/src/ipahealthcheck/core/files.py b/src/ipahealthcheck/core/files.py

index 1b208d1..af03243 100644

--- a/src/ipahealthcheck/core/files.py

+++ b/src/ipahealthcheck/core/files.py

@@ -16,13 +16,15 @@ class FileCheck:

        files is a tuple of tuples. Each tuple consists of:

            (path, expected_perm, expected_owner, expected_group)

-       perm is in the form of a POSIX ACL: e.g. 0440, 0770.

+       perm is a POSIX ACL as either a string or tuple: e.g. 0440, (0770,).

        Owner and group are names, not uid/gid.

     """

     @duration

     def check(self):

         for (path, owner, group, mode) in self.files:

+            if not isinstance(mode, tuple):

+                mode = tuple((mode,))

             if not os.path.exists(path):

                 for type in ('mode', 'owner', 'group'):

                     key = '%s_%s' % (path.replace('/', '_'), type)

@@ -33,19 +35,32 @@ class FileCheck:

             stat = os.stat(path)

             fmode = str(oct(stat.st_mode)[-4:])

             key = '%s_mode' % path.replace('/', '_')

-            if mode != fmode:

-                if mode < fmode:

+            if fmode not in mode:

+                if len(mode) == 1:

+                    modes = mode[0]

+                else:

+                    modes = 'one of {}'.format(','.join(mode))

+                if all(m < fmode for m in mode):

                     yield Result(self, constants.WARNING, key=key,

-                                 path=path, type='mode', expected=mode,

+                                 path=path, type='mode', expected=modes,

                                  got=fmode,

                                  msg='Permissions of %s are too permissive: '

-                                 '%s and should be %s' % (path, fmode, mode))

-                if mode > fmode:

+                                 '%s and should be %s' %

+                                 (path, fmode, modes))

+                elif all(m > fmode for m in mode):

                     yield Result(self, constants.ERROR, key=key,

-                                 path=path, type='mode', expected=mode,

+                                 path=path, type='mode', expected=modes,

                                  got=fmode,

                                  msg='Permissions of %s are too restrictive: '

-                                 '%s and should be %s' % (path, fmode, mode))

+                                 '%s and should be %s' %

+                                 (path, fmode, modes))

+                else:

+                    yield Result(self, constants.ERROR, key=key,

+                                 path=path, type='mode', expected=modes,

+                                 got=fmode,

+                                 msg='Permissions of %s are unexpected: '

+                                 '%s and should be %s' %

+                                 (path, fmode, modes))

             else:

                 yield Result(self, constants.SUCCESS, key=key,

                              type='mode', path=path)

diff --git a/src/ipahealthcheck/ipa/files.py b/src/ipahealthcheck/ipa/files.py

index abfa52f..a372992 100644

--- a/src/ipahealthcheck/ipa/files.py

+++ b/src/ipahealthcheck/ipa/files.py

@@ -118,7 +118,8 @@ class IPAFileCheck(IPAPlugin, FileCheck):

         self.files.append((paths.IPA_CUSTODIA_AUDIT_LOG,

                           'root', 'root', '0644'))

-        self.files.append((paths.KADMIND_LOG, 'root', 'root', '0600'))

+        self.files.append((paths.KADMIND_LOG, 'root', 'root',

+                          ('0600', '0640')))

         self.files.append((paths.KRB5KDC_LOG, 'root', 'root', '0640'))

         inst = api.env.realm.replace('.', '-')

diff --git a/tests/test_core_files.py b/tests/test_core_files.py

index 8257f40..509e28e 100644

--- a/tests/test_core_files.py

+++ b/tests/test_core_files.py

@@ -15,7 +15,8 @@ nobody = pwd.getpwnam('nobody')

 # Mock files to test

 files = (('foo', 'root', 'root', '0660'),

-         ('bar', 'nobody', 'nobody', '0664'),)

+         ('bar', 'nobody', 'nobody', '0664'),

+         ('zap', 'root', 'root', ('0664', '0640'),))

 def make_stat(mode=33200, uid=0, gid=0):

@@ -25,6 +26,13 @@ def make_stat(mode=33200, uid=0, gid=0):

             mode = 0660

             owner = root

             group = root

+

+       Cheat sheet equivalents:

+           0600 = 33152

+           0640 = 33184

+           0644 = 33188

+           0660 = 33200

+           0666 = 33206

     """

     return posix.stat_result((mode, 1, 42, 1, uid, gid, 0, 1, 1, 1,))

@@ -80,6 +88,11 @@ def test_files_group(mock_stat):

     assert my_results.results[0].result == constants.WARNING

     assert my_results.results[1].result == constants.SUCCESS

+    assert my_results.results[2].result == constants.WARNING

+    assert my_results.results[2].kw.get('got') == 'nobody'

+    assert my_results.results[2].kw.get('expected') == 'root'

+    assert my_results.results[2].kw.get('type') == 'group'

+

 @patch('os.stat')

 def test_files_mode(mock_stat):

@@ -94,17 +107,28 @@ def test_files_mode(mock_stat):

     assert my_results.results[0].result == constants.SUCCESS

     assert my_results.results[1].result == constants.ERROR

-    mock_stat.return_value = make_stat(mode=33152)

+    mock_stat.return_value = make_stat(mode=33152)  # 0600

     results = capture_results(f)

     my_results = get_results(results, 'mode')

     assert my_results.results[0].result == constants.ERROR

     assert my_results.results[1].result == constants.ERROR

+    assert my_results.results[2].result == constants.ERROR

-    mock_stat.return_value = make_stat(mode=33206)

+    # Too restrictive

+    mock_stat.return_value = make_stat(mode=33206)  # 0666

     results = capture_results(f)

     my_results = get_results(results, 'mode')

     assert my_results.results[0].result == constants.WARNING

     assert my_results.results[1].result == constants.WARNING

+    assert my_results.results[2].result == constants.WARNING

+

+    # Too restrictive with allowed multi-mode value

+    mock_stat.return_value = make_stat(mode=33184)  # 0640

+    results = capture_results(f)

+    my_results = get_results(results, 'mode')

+    assert my_results.results[0].result == constants.ERROR

+    assert my_results.results[1].result == constants.ERROR

+    assert my_results.results[2].result == constants.SUCCESS

 @patch('os.path.exists')

@@ -118,7 +142,7 @@ def test_files_not_found(mock_exists):

     for type in ('mode', 'group', 'owner'):

         my_results = get_results(results, type)

-        assert len(my_results.results) == 2

+        assert len(my_results.results) == len(f.files)

         for result in my_results.results:

             assert result.result == constants.SUCCESS

             assert result.kw.get('msg') == 'File does not exist'

-- 

2.31.1