|
|
d27d41 |
From 621a32bb892fa36cb2fd78a3f30f6080300b0bef Mon Sep 17 00:00:00 2001
|
|
|
d27d41 |
From: Rob Crittenden <rcritten@redhat.com>
|
|
|
d27d41 |
Date: Tue, 5 Apr 2022 09:25:45 -0400
|
|
|
d27d41 |
Subject: [PATCH] Allow multiple file modes in the FileChecker
|
|
|
d27d41 |
|
|
|
d27d41 |
There are some cases where a strict file mode is not
|
|
|
d27d41 |
necessary. The kadmind.log is one.
|
|
|
d27d41 |
|
|
|
d27d41 |
It is owned root:root so there is no real difference
|
|
|
d27d41 |
between 0600 and 0640. So allow both.
|
|
|
d27d41 |
|
|
|
d27d41 |
https://bugzilla.redhat.com/show_bug.cgi?id=2058239
|
|
|
d27d41 |
|
|
|
d27d41 |
Signed-off-by: Rob Crittenden <rcritten@redhat.com>
|
|
|
d27d41 |
---
|
|
|
d27d41 |
src/ipahealthcheck/core/files.py | 31 ++++++++++++++++------
|
|
|
d27d41 |
src/ipahealthcheck/ipa/files.py | 3 ++-
|
|
|
d27d41 |
tests/test_core_files.py | 44 +++++++++++++++++++++++++++++---
|
|
|
d27d41 |
3 files changed, 65 insertions(+), 13 deletions(-)
|
|
|
d27d41 |
|
|
|
d27d41 |
diff --git a/src/ipahealthcheck/core/files.py b/src/ipahealthcheck/core/files.py
|
|
|
d27d41 |
index 0a7641e..59e8b76 100644
|
|
|
d27d41 |
--- a/src/ipahealthcheck/core/files.py
|
|
|
d27d41 |
+++ b/src/ipahealthcheck/core/files.py
|
|
|
d27d41 |
@@ -16,7 +16,7 @@ class FileCheck:
|
|
|
d27d41 |
files is a tuple of tuples. Each tuple consists of:
|
|
|
d27d41 |
(path, expected_perm, expected_owner, expected_group)
|
|
|
d27d41 |
|
|
|
d27d41 |
- perm is in the form of a POSIX ACL: e.g. 0440, 0770.
|
|
|
d27d41 |
+ perm is a POSIX ACL as either a string or tuple: e.g. 0440, (0770,).
|
|
|
d27d41 |
owner and group are names or a tuple of names, not uid/gid.
|
|
|
d27d41 |
|
|
|
d27d41 |
If owner and/or group are tuples then all names are checked.
|
|
|
d27d41 |
@@ -33,6 +33,8 @@ class FileCheck:
|
|
|
d27d41 |
owner = tuple((owner,))
|
|
|
d27d41 |
if not isinstance(group, tuple):
|
|
|
d27d41 |
group = tuple((group,))
|
|
|
d27d41 |
+ if not isinstance(mode, tuple):
|
|
|
d27d41 |
+ mode = tuple((mode,))
|
|
|
d27d41 |
if not os.path.exists(path):
|
|
|
d27d41 |
for type in ('mode', 'owner', 'group'):
|
|
|
d27d41 |
key = '%s_%s' % (path.replace('/', '_'), type)
|
|
|
d27d41 |
@@ -43,19 +45,32 @@ class FileCheck:
|
|
|
d27d41 |
stat = os.stat(path)
|
|
|
d27d41 |
fmode = str(oct(stat.st_mode)[-4:])
|
|
|
d27d41 |
key = '%s_mode' % path.replace('/', '_')
|
|
|
d27d41 |
- if mode != fmode:
|
|
|
d27d41 |
- if mode < fmode:
|
|
|
d27d41 |
+ if fmode not in mode:
|
|
|
d27d41 |
+ if len(mode) == 1:
|
|
|
d27d41 |
+ modes = mode[0]
|
|
|
d27d41 |
+ else:
|
|
|
d27d41 |
+ modes = 'one of {}'.format(','.join(mode))
|
|
|
d27d41 |
+ if all(m < fmode for m in mode):
|
|
|
d27d41 |
yield Result(self, constants.WARNING, key=key,
|
|
|
d27d41 |
- path=path, type='mode', expected=mode,
|
|
|
d27d41 |
+ path=path, type='mode', expected=modes,
|
|
|
d27d41 |
got=fmode,
|
|
|
d27d41 |
msg='Permissions of %s are too permissive: '
|
|
|
d27d41 |
- '%s and should be %s' % (path, fmode, mode))
|
|
|
d27d41 |
- if mode > fmode:
|
|
|
d27d41 |
+ '%s and should be %s' %
|
|
|
d27d41 |
+ (path, fmode, modes))
|
|
|
d27d41 |
+ elif all(m > fmode for m in mode):
|
|
|
d27d41 |
yield Result(self, constants.ERROR, key=key,
|
|
|
d27d41 |
- path=path, type='mode', expected=mode,
|
|
|
d27d41 |
+ path=path, type='mode', expected=modes,
|
|
|
d27d41 |
got=fmode,
|
|
|
d27d41 |
msg='Permissions of %s are too restrictive: '
|
|
|
d27d41 |
- '%s and should be %s' % (path, fmode, mode))
|
|
|
d27d41 |
+ '%s and should be %s' %
|
|
|
d27d41 |
+ (path, fmode, modes))
|
|
|
d27d41 |
+ else:
|
|
|
d27d41 |
+ yield Result(self, constants.ERROR, key=key,
|
|
|
d27d41 |
+ path=path, type='mode', expected=modes,
|
|
|
d27d41 |
+ got=fmode,
|
|
|
d27d41 |
+ msg='Permissions of %s are unexpected: '
|
|
|
d27d41 |
+ '%s and should be %s' %
|
|
|
d27d41 |
+ (path, fmode, modes))
|
|
|
d27d41 |
else:
|
|
|
d27d41 |
yield Result(self, constants.SUCCESS, key=key,
|
|
|
d27d41 |
type='mode', path=path)
|
|
|
d27d41 |
diff --git a/src/ipahealthcheck/ipa/files.py b/src/ipahealthcheck/ipa/files.py
|
|
|
d27d41 |
index 4728171..c86be0d 100644
|
|
|
d27d41 |
--- a/src/ipahealthcheck/ipa/files.py
|
|
|
d27d41 |
+++ b/src/ipahealthcheck/ipa/files.py
|
|
|
d27d41 |
@@ -123,7 +123,8 @@ class IPAFileCheck(IPAPlugin, FileCheck):
|
|
|
d27d41 |
self.files.append((paths.IPA_CUSTODIA_AUDIT_LOG,
|
|
|
d27d41 |
'root', 'root', '0644'))
|
|
|
d27d41 |
|
|
|
d27d41 |
- self.files.append((paths.KADMIND_LOG, 'root', 'root', '0600'))
|
|
|
d27d41 |
+ self.files.append((paths.KADMIND_LOG, 'root', 'root',
|
|
|
d27d41 |
+ ('0600', '0640')))
|
|
|
d27d41 |
self.files.append((paths.KRB5KDC_LOG, 'root', 'root', '0640'))
|
|
|
d27d41 |
|
|
|
d27d41 |
inst = api.env.realm.replace('.', '-')
|
|
|
d27d41 |
diff --git a/tests/test_core_files.py b/tests/test_core_files.py
|
|
|
d27d41 |
index 10824ab..6e3ec38 100644
|
|
|
d27d41 |
--- a/tests/test_core_files.py
|
|
|
d27d41 |
+++ b/tests/test_core_files.py
|
|
|
d27d41 |
@@ -17,7 +17,8 @@ nobody = pwd.getpwnam('nobody')
|
|
|
d27d41 |
files = (('foo', 'root', 'root', '0660'),
|
|
|
d27d41 |
('bar', 'nobody', 'nobody', '0664'),
|
|
|
d27d41 |
('baz', ('root', 'nobody'), ('root', 'nobody'), '0664'),
|
|
|
d27d41 |
- ('fiz', ('root', 'bin'), ('root', 'bin'), '0664'),)
|
|
|
d27d41 |
+ ('fiz', ('root', 'bin'), ('root', 'bin'), '0664'),
|
|
|
d27d41 |
+ ('zap', ('root', 'bin'), ('root', 'bin'), ('0664', '0640'),))
|
|
|
d27d41 |
|
|
|
d27d41 |
|
|
|
d27d41 |
def make_stat(mode=33200, uid=0, gid=0):
|
|
|
d27d41 |
@@ -27,6 +28,13 @@ def make_stat(mode=33200, uid=0, gid=0):
|
|
|
d27d41 |
mode = 0660
|
|
|
d27d41 |
owner = root
|
|
|
d27d41 |
group = root
|
|
|
d27d41 |
+
|
|
|
d27d41 |
+ Cheat sheet equivalents:
|
|
|
d27d41 |
+ 0600 = 33152
|
|
|
d27d41 |
+ 0640 = 33184
|
|
|
d27d41 |
+ 0644 = 33188
|
|
|
d27d41 |
+ 0660 = 33200
|
|
|
d27d41 |
+ 0666 = 33206
|
|
|
d27d41 |
"""
|
|
|
d27d41 |
# (mode, ino, dev, nlink, uid, gid, size, atime, mtime, ctime)
|
|
|
d27d41 |
return posix.stat_result((mode, 1, 42, 1, uid, gid, 0, 1, 1, 1,))
|
|
|
d27d41 |
@@ -81,6 +89,11 @@ def test_files_owner(mock_stat):
|
|
|
d27d41 |
assert my_results.results[3].kw.get('msg') == \
|
|
|
d27d41 |
'Ownership of fiz is nobody and should be one of root,bin'
|
|
|
d27d41 |
|
|
|
d27d41 |
+ assert my_results.results[4].result == constants.WARNING
|
|
|
d27d41 |
+ assert my_results.results[4].kw.get('got') == 'nobody'
|
|
|
d27d41 |
+ assert my_results.results[4].kw.get('expected') == 'root,bin'
|
|
|
d27d41 |
+ assert my_results.results[4].kw.get('type') == 'owner'
|
|
|
d27d41 |
+
|
|
|
d27d41 |
|
|
|
d27d41 |
@patch('os.stat')
|
|
|
d27d41 |
def test_files_group(mock_stat):
|
|
|
d27d41 |
@@ -119,6 +132,11 @@ def test_files_group(mock_stat):
|
|
|
d27d41 |
assert my_results.results[3].kw.get('msg') == \
|
|
|
d27d41 |
'Group of fiz is nobody and should be one of root,bin'
|
|
|
d27d41 |
|
|
|
d27d41 |
+ assert my_results.results[4].result == constants.WARNING
|
|
|
d27d41 |
+ assert my_results.results[4].kw.get('got') == 'nobody'
|
|
|
d27d41 |
+ assert my_results.results[4].kw.get('expected') == 'root,bin'
|
|
|
d27d41 |
+ assert my_results.results[4].kw.get('type') == 'group'
|
|
|
d27d41 |
+
|
|
|
d27d41 |
|
|
|
d27d41 |
@patch('os.stat')
|
|
|
d27d41 |
def test_files_mode(mock_stat):
|
|
|
d27d41 |
@@ -133,17 +151,35 @@ def test_files_mode(mock_stat):
|
|
|
d27d41 |
assert my_results.results[0].result == constants.SUCCESS
|
|
|
d27d41 |
assert my_results.results[1].result == constants.ERROR
|
|
|
d27d41 |
|
|
|
d27d41 |
- mock_stat.return_value = make_stat(mode=33152)
|
|
|
d27d41 |
+ # Too restrictive
|
|
|
d27d41 |
+ mock_stat.return_value = make_stat(mode=33152) # 0600
|
|
|
d27d41 |
results = capture_results(f)
|
|
|
d27d41 |
my_results = get_results(results, 'mode')
|
|
|
d27d41 |
assert my_results.results[0].result == constants.ERROR
|
|
|
d27d41 |
assert my_results.results[1].result == constants.ERROR
|
|
|
d27d41 |
+ assert my_results.results[2].result == constants.ERROR
|
|
|
d27d41 |
+ assert my_results.results[3].result == constants.ERROR
|
|
|
d27d41 |
+ assert my_results.results[4].result == constants.ERROR
|
|
|
d27d41 |
|
|
|
d27d41 |
- mock_stat.return_value = make_stat(mode=33206)
|
|
|
d27d41 |
+ # Too permissive
|
|
|
d27d41 |
+ mock_stat.return_value = make_stat(mode=33206) # 0666
|
|
|
d27d41 |
results = capture_results(f)
|
|
|
d27d41 |
my_results = get_results(results, 'mode')
|
|
|
d27d41 |
assert my_results.results[0].result == constants.WARNING
|
|
|
d27d41 |
assert my_results.results[1].result == constants.WARNING
|
|
|
d27d41 |
+ assert my_results.results[2].result == constants.WARNING
|
|
|
d27d41 |
+ assert my_results.results[3].result == constants.WARNING
|
|
|
d27d41 |
+ assert my_results.results[4].result == constants.WARNING
|
|
|
d27d41 |
+
|
|
|
d27d41 |
+ # Too restrictive with allowed multi-mode value
|
|
|
d27d41 |
+ mock_stat.return_value = make_stat(mode=33184) # 0640
|
|
|
d27d41 |
+ results = capture_results(f)
|
|
|
d27d41 |
+ my_results = get_results(results, 'mode')
|
|
|
d27d41 |
+ assert my_results.results[0].result == constants.ERROR
|
|
|
d27d41 |
+ assert my_results.results[1].result == constants.ERROR
|
|
|
d27d41 |
+ assert my_results.results[2].result == constants.ERROR
|
|
|
d27d41 |
+ assert my_results.results[3].result == constants.ERROR
|
|
|
d27d41 |
+ assert my_results.results[4].result == constants.SUCCESS
|
|
|
d27d41 |
|
|
|
d27d41 |
|
|
|
d27d41 |
@patch('os.path.exists')
|
|
|
d27d41 |
@@ -157,7 +193,7 @@ def test_files_not_found(mock_exists):
|
|
|
d27d41 |
|
|
|
d27d41 |
for type in ('mode', 'group', 'owner'):
|
|
|
d27d41 |
my_results = get_results(results, type)
|
|
|
d27d41 |
- assert len(my_results.results) == 4
|
|
|
d27d41 |
+ assert len(my_results.results) == len(f.files)
|
|
|
d27d41 |
for result in my_results.results:
|
|
|
d27d41 |
assert result.result == constants.SUCCESS
|
|
|
d27d41 |
assert result.kw.get('msg') == 'File does not exist'
|
|
|
d27d41 |
--
|
|
|
d27d41 |
2.31.1
|
|
|
d27d41 |
|