Skip to content

Commit

Permalink
Allow multiple file modes in the FileChecker
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
rcritten committed Apr 5, 2022
1 parent 715bd0d commit 798001c
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 11 deletions.
31 changes: 23 additions & 8 deletions src/ipahealthcheck/core/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ 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 or a tuple of names, not uid/gid.
If owner and/or group are tuples then all names are checked.
Expand All @@ -33,6 +33,8 @@ def check(self):
owner = tuple((owner,))
if not isinstance(group, tuple):
group = tuple((group,))
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)
Expand All @@ -43,19 +45,32 @@ def check(self):
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)
Expand Down
3 changes: 2 additions & 1 deletion src/ipahealthcheck/ipa/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ def check(self):
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('.', '-')
Expand Down
5 changes: 3 additions & 2 deletions tests/test_core_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
files = (('foo', 'root', 'root', '0660'),
('bar', 'nobody', 'nobody', '0664'),
('baz', ('root', 'nobody'), ('root', 'nobody'), '0664'),
('fiz', ('root', 'bin'), ('root', 'bin'), '0664'),)
('fiz', ('root', 'bin'), ('root', 'bin'), '0664'),
('zap', ('root', 'bin'), ('root', 'bin'), ('0664', '0644'),))


def make_stat(mode=33200, uid=0, gid=0):
Expand Down Expand Up @@ -157,7 +158,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) == 4
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'

0 comments on commit 798001c

Please sign in to comment.