SOURCES/0006-Fixes-log-file-permissions-as-per-CIS-benchmark.patch

@@ -0,0 +1,47 @@
+From e0c09f9f1388bbce43775f40a39266e692e231da Mon Sep 17 00:00:00 2001
+From: Thorsten Scherf <tscherf@redhat.com>
+Date: Wed, 13 Mar 2024 12:57:34 +0100
+Subject: [PATCH 1/4] Fixes log file permissions as per CIS benchmark
+
+As per CIS benchmark the log file permissions should be 640 for some log
+files but if we change /var/log/ipa-custodia.audit.log permissions to
+640 then "ipa-healthcheck" reports a permission issue.
+
+Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/325
+Signed-off-by: Thorsten Scherf <tscherf@redhat.com>
+---
+ src/ipahealthcheck/ipa/files.py | 8 +++++---
+ 1 file changed, 5 insertions(+), 3 deletions(-)
+
+diff --git a/src/ipahealthcheck/ipa/files.py b/src/ipahealthcheck/ipa/files.py
+index b7ca116..d914014 100644
+--- a/src/ipahealthcheck/ipa/files.py
++++ b/src/ipahealthcheck/ipa/files.py
+@@ -121,7 +121,7 @@ class IPAFileCheck(IPAPlugin, FileCheck):
+                 self.files.append((filename, 'root', 'root', '0600'))
+ 
+         self.files.append((paths.IPA_CUSTODIA_AUDIT_LOG,
+-                          'root', 'root', '0644'))
++                          'root', 'root', '0644', '0640'))
+ 
+         self.files.append((paths.KADMIND_LOG, 'root', 'root',
+                           ('0600', '0640')))
+@@ -133,11 +133,13 @@ class IPAFileCheck(IPAPlugin, FileCheck):
+         self.files.append((paths.SLAPD_INSTANCE_ERROR_LOG_TEMPLATE % inst,
+                            constants.DS_USER, constants.DS_GROUP, '0600'))
+ 
+-        self.files.append((paths.VAR_LOG_HTTPD_ERROR, 'root', 'root', '0644'))
++        self.files.append((paths.VAR_LOG_HTTPD_ERROR, 'root', 'root',
++                           '0644', '0640'))
+ 
+         for globpath in glob.glob("%s/debug*.log" % paths.TOMCAT_CA_DIR):
+             self.files.append(
+-                (globpath, constants.PKI_USER, constants.PKI_GROUP, "0644")
++                (globpath, constants.PKI_USER, constants.PKI_GROUP,
++                 "0644", "0640")
+             )
+ 
+         for globpath in glob.glob(
+-- 
+2.45.0
+

SOURCES/0007-Fix-some-file-mode-format-issues.patch

@@ -0,0 +1,189 @@
+From 54e2e9b8bff0bc84b6179eac44993b460f02ad02 Mon Sep 17 00:00:00 2001
+From: Rob Crittenden <rcritten@redhat.com>
+Date: Fri, 21 Jun 2024 15:15:36 -0400
+Subject: [PATCH 1/2] Fix some file mode format issues
+
+When specifying multiple possible modes for a file the values must
+be a tuple. There were two occurances where they were listed
+separately.
+
+Add in a pre-check on the formatting to raise an error for badly
+formatted files. This may be annoying for users if one sneaks in
+again but the CI should catch it.
+
+Related: https://github.com/freeipa/freeipa-healthcheck/issues/325
+
+Signed-off-by: Rob Crittenden <rcritten@redhat.com>
+---
+ src/ipahealthcheck/core/files.py | 12 +++++-
+ src/ipahealthcheck/ipa/files.py  |  6 +--
+ tests/test_core_files.py         | 71 +++++++++++++++++++++++++++++++-
+ tests/util.py                    |  1 +
+ 4 files changed, 85 insertions(+), 5 deletions(-)
+
+diff --git a/src/ipahealthcheck/core/files.py b/src/ipahealthcheck/core/files.py
+index 59e8b76..58dd74a 100644
+--- a/src/ipahealthcheck/core/files.py
++++ b/src/ipahealthcheck/core/files.py
+@@ -28,7 +28,17 @@ class FileCheck:
+ 
+     @duration
+     def check(self):
+-        for (path, owner, group, mode) in self.files:
++        # first validate that the list of files to check is in the correct
++        # format
++        process_files = []
++        for file in self.files:
++            if len(file) == 4:
++                process_files.append(file)
++            else:
++                yield Result(self, constants.ERROR, key=file,
++                             msg='Code format is incorrect for file')
++
++        for (path, owner, group, mode) in process_files:
+             if not isinstance(owner, tuple):
+                 owner = tuple((owner,))
+             if not isinstance(group, tuple):
+diff --git a/src/ipahealthcheck/ipa/files.py b/src/ipahealthcheck/ipa/files.py
+index d914014..c80fd5b 100644
+--- a/src/ipahealthcheck/ipa/files.py
++++ b/src/ipahealthcheck/ipa/files.py
+@@ -121,7 +121,7 @@ class IPAFileCheck(IPAPlugin, FileCheck):
+                 self.files.append((filename, 'root', 'root', '0600'))
+ 
+         self.files.append((paths.IPA_CUSTODIA_AUDIT_LOG,
+-                          'root', 'root', '0644', '0640'))
++                          'root', 'root', ('0644', '0640')))
+ 
+         self.files.append((paths.KADMIND_LOG, 'root', 'root',
+                           ('0600', '0640')))
+@@ -134,12 +134,12 @@ class IPAFileCheck(IPAPlugin, FileCheck):
+                            constants.DS_USER, constants.DS_GROUP, '0600'))
+ 
+         self.files.append((paths.VAR_LOG_HTTPD_ERROR, 'root', 'root',
+-                           '0644', '0640'))
++                           ('0644', '0640')))
+ 
+         for globpath in glob.glob("%s/debug*.log" % paths.TOMCAT_CA_DIR):
+             self.files.append(
+                 (globpath, constants.PKI_USER, constants.PKI_GROUP,
+-                 "0644", "0640")
++                 ("0644", "0640"))
+             )
+ 
+         for globpath in glob.glob(
+diff --git a/tests/test_core_files.py b/tests/test_core_files.py
+index 6e3ec38..09fc216 100644
+--- a/tests/test_core_files.py
++++ b/tests/test_core_files.py
+@@ -2,14 +2,22 @@
+ # Copyright (C) 2019 FreeIPA Contributors see COPYING for license
+ #
+ 
++from ldap import OPT_X_SASL_SSF_MIN
+ import pwd
+ import posix
++from util import m_api
++from util import capture_results
++
++from ipahealthcheck.core import config
+ from ipahealthcheck.core.files import FileCheck
+ from ipahealthcheck.core import constants
+ from ipahealthcheck.core.plugin import Results
++from ipahealthcheck.ipa.files import IPAFileCheck
++from ipahealthcheck.system.plugin import registry
+ from unittest.mock import patch
++from ipapython.dn import DN
++from ipapython.ipaldap import LDAPClient, LDAPEntry
+ 
+-from util import capture_results
+ 
+ nobody = pwd.getpwnam('nobody')
+ 
+@@ -20,6 +28,37 @@ files = (('foo', 'root', 'root', '0660'),
+          ('fiz', ('root', 'bin'), ('root', 'bin'), '0664'),
+          ('zap', ('root', 'bin'), ('root', 'bin'), ('0664', '0640'),))
+ 
++bad_modes = (('biz', ('root', 'bin'), ('root', 'bin'), '0664', '0640'),)
++
++
++class mock_ldap:
++    SCOPE_BASE = 1
++    SCOPE_ONELEVEL = 2
++    SCOPE_SUBTREE = 4
++
++    def __init__(self, ldapentry):
++        """Initialize the results that we will return from get_entries"""
++        self.results = ldapentry
++
++    def get_entry(self, dn, attrs_list=None, time_limit=None,
++                  size_limit=None, get_effective_rights=False):
++        return []  # the call doesn't check the value
++
++
++class mock_ldap_conn:
++    def set_option(self, option, invalue):
++        pass
++
++    def get_option(self, option):
++        if option == OPT_X_SASL_SSF_MIN:
++            return 256
++
++        return None
++
++    def search_s(self, base, scope, filterstr=None,
++                 attrlist=None, attrsonly=0):
++        return tuple()
++
+ 
+ def make_stat(mode=33200, uid=0, gid=0):
+     """Return a mocked-up stat.
+@@ -197,3 +236,33 @@ def test_files_not_found(mock_exists):
+         for result in my_results.results:
+             assert result.result == constants.SUCCESS
+             assert result.kw.get('msg') == 'File does not exist'
++
++
++def test_bad_modes():
++    f = FileCheck()
++    f.files = bad_modes
++
++    results = capture_results(f)
++
++    for result in results.results:
++        assert result.result == constants.ERROR
++        assert result.kw.get('msg') == 'Code format is incorrect for file'
++
++
++@patch('ipaserver.install.krbinstance.is_pkinit_enabled')
++def test_ipa_files_format(mock_pkinit):
++    mock_pkinit.return_value = True
++
++    fake_conn = LDAPClient('ldap://localhost', no_schema=True)
++    ldapentry = LDAPEntry(fake_conn, DN(m_api.env.container_dns,
++                          m_api.env.basedn))
++    framework = object()
++    registry.initialize(framework, config.Config)
++    f = IPAFileCheck(registry)
++
++    f.conn = mock_ldap(ldapentry)
++
++    results = capture_results(f)
++
++    for result in results.results:
++        assert result.result == constants.SUCCESS
+diff --git a/tests/util.py b/tests/util.py
+index 8081595..5dcb0cd 100644
+--- a/tests/util.py
++++ b/tests/util.py
+@@ -140,6 +140,7 @@ m_api.env.container_host = DN(('cn', 'computers'), ('cn', 'accounts'))
+ m_api.env.container_sysaccounts = DN(('cn', 'sysaccounts'), ('cn', 'etc'))
+ m_api.env.container_service = DN(('cn', 'services'), ('cn', 'accounts'))
+ m_api.env.container_masters = DN(('cn', 'masters'))
++m_api.env.container_dns = DN(('cn', 'dns'))
+ m_api.Backend = Mock()
+ m_api.Command = Mock()
+ m_api.Command.ping.return_value = {
+-- 
+2.45.0
+

SOURCES/0008-Allow-WARNING-in-the-files-test.patch

@@ -0,0 +1,28 @@
+From 79cca342b3c440a045cadbff871ff977e35222c6 Mon Sep 17 00:00:00 2001
+From: Rob Crittenden <rcritten@redhat.com>
+Date: Thu, 20 Jun 2024 14:27:16 -0400
+Subject: [PATCH] Allow WARNING in the files test
+
+We are only validating the format and don't need to actually
+enforce the results in CI. The validation raises ERROR.
+
+Related: https://github.com/freeipa/freeipa-healthcheck/issues/325
+
+Signed-off-by: Rob Crittenden <rcritten@redhat.com>
+---
+ tests/test_core_files.py | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/tests/test_core_files.py b/tests/test_core_files.py
+index e7010a9..d308410 100644
+--- a/tests/test_core_files.py
++++ b/tests/test_core_files.py
+@@ -302,4 +302,4 @@ def test_ipa_files_format(mock_pkinit):
+     results = capture_results(f)
+ 
+     for result in results.results:
+-        assert result.result == constants.SUCCESS
++        assert result.result in (constants.SUCCESS, constants.WARNING)
+-- 
+2.45.0
+

SOURCES/0009-Address-issues-uncovered-by-pylint-2.15.5.patch

@@ -0,0 +1,70 @@
+From 18178ba09b221eef7f0bb869980e1c043a8e764f Mon Sep 17 00:00:00 2001
+From: Rob Crittenden <rcritten@redhat.com>
+Date: Wed, 31 May 2023 17:21:55 -0400
+Subject: [PATCH] Address issues uncovered by pylint 2.15.5
+
+Two variables used before assignment
+
+Three Useless suppression of 'unexpected-keyword-arg'
+
+Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/295
+
+Signed-off-by: Rob Crittenden <rcritten@redhat.com>
+---
+ src/ipahealthcheck/ipa/certs.py | 5 +----
+ src/ipahealthcheck/ipa/trust.py | 2 +-
+ 2 files changed, 2 insertions(+), 5 deletions(-)
+
+diff --git a/src/ipahealthcheck/ipa/certs.py b/src/ipahealthcheck/ipa/certs.py
+index 11ac0c1..4ea5112 100644
+--- a/src/ipahealthcheck/ipa/certs.py
++++ b/src/ipahealthcheck/ipa/certs.py
+@@ -343,7 +343,6 @@ class IPACertfileExpirationCheck(IPAPlugin):
+ 
+                 try:
+                     if 'pwd_file' in signature(certdb.NSSDatabase).parameters:
+-                        # pylint: disable=unexpected-keyword-arg
+                         db = certdb.NSSDatabase(
+                             dbdir, token=token,
+                             pwd_file=pwd_file.name if pwd_file else None)
+@@ -624,7 +623,6 @@ class IPACertNSSTrust(IPAPlugin):
+             pwd_file = get_token_password_file(self.ca.hsm_enabled,
+                                                token)
+ 
+-            # pylint: disable=unexpected-keyword-arg
+             db = certdb.NSSDatabase(
+                 paths.PKI_TOMCAT_ALIAS_DIR, token=token,
+                 pwd_file=pwd_file.name if pwd_file else None)
+@@ -987,7 +985,7 @@ class IPANSSChainValidation(IPAPlugin):
+                         key=key,
+                         dbdir=dbdir,
+                         nickname=nickname,
+-                        reason=response.output_error,
++                        reason=str(e),
+                         msg='Validation of {nickname} in {dbdir} failed: '
+                             '{reason}')
+                 else:
+@@ -1251,7 +1249,6 @@ class IPACertRevocation(IPAPlugin):
+                 dbdir = request.get('cert-database')
+                 try:
+                     if 'pwd_file' in signature(certdb.NSSDatabase).parameters:
+-                        # pylint: disable=unexpected-keyword-arg
+                         db = certdb.NSSDatabase(
+                             dbdir, token=token,
+                             pwd_file=pwd_file.name if pwd_file else None
+diff --git a/src/ipahealthcheck/ipa/trust.py b/src/ipahealthcheck/ipa/trust.py
+index b962807..243502f 100644
+--- a/src/ipahealthcheck/ipa/trust.py
++++ b/src/ipahealthcheck/ipa/trust.py
+@@ -307,7 +307,7 @@ class IPATrustCatalogCheck(IPAPlugin):
+                 id = pysss_nss_idmap.getnamebysid(sid + '-500')
+             except Exception as e:
+                 yield Result(self, constants.ERROR,
+-                             key=id,
++                             key='getnamebysid',
+                              domain=domain,
+                              error=str(e),
+                              msg='Look up of ID {key} for {domain} failed: '
+-- 
+2.48.1
+

SOURCES/0010-Don-t-rely-on-order-in-trust-agent-controller-role-c.patch

@@ -0,0 +1,139 @@
+From 7539b4aee19c7e28539ec853369a3230f2ae08f3 Mon Sep 17 00:00:00 2001
+From: Rob Crittenden <rcritten@redhat.com>
+Date: Mon, 23 Jun 2025 13:30:26 -0400
+Subject: [PATCH] Don't rely on order in trust agent/controller role check
+
+The code expected that the local server would always be the
+first one returned. Instead loop through the returned list
+to find the current server and set the state based on that.
+
+Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/356
+
+Signed-off-by: Rob Crittenden <rcritten@redhat.com>
+---
+ src/ipahealthcheck/ipa/plugin.py | 15 ++++---
+ tests/test_ipa_trust.py          | 71 +++++++++++++++++++++++++++++++-
+ 2 files changed, 79 insertions(+), 7 deletions(-)
+
+diff --git a/src/ipahealthcheck/ipa/plugin.py b/src/ipahealthcheck/ipa/plugin.py
+index f1a325c..efaa947 100644
+--- a/src/ipahealthcheck/ipa/plugin.py
++++ b/src/ipahealthcheck/ipa/plugin.py
+@@ -35,6 +35,13 @@ class IPARegistry(Registry):
+         self.trust_controller = False
+         self.ca_configured = False
+
++    def has_role(self, roles):
++        for role in roles:
++            if role.get('server_server') == api.env.host:
++                if role.get('status') == 'enabled':
++                    return True
++        return False
++
+     def initialize(self, framework, config, options=None):
+         super().initialize(framework, config)
+         # deferred import for mock
+@@ -81,12 +88,8 @@ class IPARegistry(Registry):
+                 component_services=['ADTRUST']
+             ),
+         )
+-        role = roles[0].status(api)[0]
+-        if role.get('status') == 'enabled':
+-            self.trust_agent = True
+-        role = roles[1].status(api)[0]
+-        if role.get('status') == 'enabled':
+-            self.trust_controller = True
++        self.trust_agent = self.has_role(roles[0].status(api))
++        self.trust_controller = self.has_role(roles[1].status(api))
+
+
+ registry = IPARegistry()
+diff --git a/tests/test_ipa_trust.py b/tests/test_ipa_trust.py
+index 6c4754a..0faa702 100644
+--- a/tests/test_ipa_trust.py
++++ b/tests/test_ipa_trust.py
+@@ -11,7 +11,8 @@ from util import capture_results
+ from util import m_api
+
+ from ipahealthcheck.core import config, constants
+-from ipahealthcheck.ipa.plugin import registry
++from ipahealthcheck.core.plugin import Results
++from ipahealthcheck.ipa.plugin import registry, IPARegistry
+ from ipahealthcheck.ipa.trust import (IPATrustAgentCheck,
+                                       IPATrustDomainsCheck,
+                                       IPADomainCheck,
+@@ -1287,3 +1288,71 @@ class TestPackageCheck(BaseTest):
+         assert result.source == 'ipahealthcheck.ipa.trust'
+         assert result.check == 'IPATrustPackageCheck'
+         sys.modules['ipaserver.install'] = save
++
++
++class TestHasRole(BaseTest):
++    """Verify that the output of server-role-find which is used to
++       determine whether a host is a trust agent or controller
++      (or neither) isn't dependent upon the order the hosts are
++      returned.
++
++      Only trust agent is tested here but there is no difference
++      between an agent and a trust in the way they are stored in
++      a server role.
++    """
++    def test_role_last(self):
++        self.results = Results()
++        reg = IPARegistry()
++
++        roles = [
++            {
++                "role_servrole": "AD trust agent",
++                "server_server": "replica.ipa.example",
++                "status": "absent",
++            },
++            {
++                "role_servrole": "AD trust agent",
++                "server_server": "server.ipa.example",
++                "status": "enabled",
++            },
++        ]
++
++        assert reg.has_role(roles) is True
++
++    def test_role_first(self):
++        self.results = Results()
++        reg = IPARegistry()
++
++        roles = [
++            {
++                "role_servrole": "AD trust agent",
++                "server_server": "server.ipa.example",
++                "status": "enabled",
++            },
++            {
++                "role_servrole": "AD trust agent",
++                "server_server": "replica.ipa.example",
++                "status": "absent",
++            },
++        ]
++
++        assert reg.has_role(roles) is True
++
++    def test_no_role(self):
++        self.results = Results()
++        reg = IPARegistry()
++
++        roles = [
++            {
++                "role_servrole": "AD trust agent",
++                "server_server": "server.ipa.example",
++                "status": "absent",
++            },
++            {
++                "role_servrole": "AD trust agent",
++                "server_server": "replica.ipa.example",
++                "status": "enabled",
++            },
++        ]
++
++        assert reg.has_role(roles) is False
+-- 
+2.49.0
+

SPECS/ipa-healthcheck.spec

@@ -8,7 +8,7 @@
 
 Name:           ipa-healthcheck
 Version:        0.12
-Release:        3%{?dist}
+Release:        6%{?dist}
 Summary:        Health check tool for IdM
 BuildArch:      noarch
 License:        GPLv3
@@ -21,12 +21,18 @@ Patch0002:      0002-Disable-two-failing-tests.patch
 Patch0003:      0003-Fix-logging-issue-related-to-dtype.patch
 Patch0004:      0004-Skip-AD-domains-with-posix-ranges-in-the-catalog-che.patch
 Patch0005:      0005-Don-t-error-in-DogtagCertsConnectivityCheck-with-ext.patch
+Patch0006:      0006-Fixes-log-file-permissions-as-per-CIS-benchmark.patch
+Patch0007:      0007-Fix-some-file-mode-format-issues.patch
+Patch0008:      0008-Allow-WARNING-in-the-files-test.patch
+Patch0009:      0009-Address-issues-uncovered-by-pylint-2.15.5.patch
+Patch0010:      0010-Don-t-rely-on-order-in-trust-agent-controller-role-c.patch
 
 Requires:       %{name}-core = %{version}-%{release}
 Requires:       ipa-server
 Requires:       python3-ipalib
 Requires:       python3-ipaserver
 Requires:       python3-lib389
+Requires:       python3-libsss_nss_idmap
 # cronie-anacron provides anacron
 Requires:       anacron
 Requires:       logrotate
@@ -124,6 +130,16 @@ install -p -m644 %{_builddir}/%{project}-%{shortname}-%{version}/man/man5/%{long
 
 
 %changelog
+* Mon Jun 23 2025 Rob Crittenden <rcritten@redhat.com> - 0.12-6
+- Don't rely on order in trust roles (RHEL-99487)
+
+* Thu Feb 27 2025 Rob Crittenden <rcritten@redhat.com> - 0.12-5
+- Pull in lint fixes. Prevents exception when testing for AD trust (RHEL-79081)
+- Add direct requires on python3-libsss_nss_idmap.
+
+* Fri Jun 21 2024 Rob Crittenden <rcritten@redhat.com> - 0.12-4
+- Change log file permissions of IPA as per CIS benchmark (RHEL-38929)
+
 * Mon Jul 24 2023 Rob Crittenden <rcritten@redhat.com> - 0.12-3
 - Error in DogtagCertsConnectivityCheckCA with external CA (#2223942)