From ca10bedb3ca2b7736dc95b7529eb261050b71188 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Tue, 17 Nov 2020 20:46:34 -0500 Subject: [PATCH] Include upstream patches to fix trust collection and make check - Include upstream patch to fix collection of AD trust domains - Include upstream patch to fix failing not-valid-after test --- ...nd-trustdomain-find-to-identify-all-.patch | 303 ++++++++++++++++++ ...erate-a-proper-not-valid-after-field.patch | 141 ++++++++ freeipa-healthcheck.spec | 8 +- 3 files changed, 451 insertions(+), 1 deletion(-) create mode 100644 0002-Use-trust-find-and-trustdomain-find-to-identify-all-.patch create mode 100644 0003-tests-Generate-a-proper-not-valid-after-field.patch diff --git a/0002-Use-trust-find-and-trustdomain-find-to-identify-all-.patch b/0002-Use-trust-find-and-trustdomain-find-to-identify-all-.patch new file mode 100644 index 0000000..f643aa0 --- /dev/null +++ b/0002-Use-trust-find-and-trustdomain-find-to-identify-all-.patch @@ -0,0 +1,303 @@ +From 886153da7dd1ca1f5d37dd9c1e2141850b7177b2 Mon Sep 17 00:00:00 2001 +From: Rob Crittenden +Date: Tue, 17 Nov 2020 20:37:52 -0500 +Subject: [PATCH] Use trust-find and trustdomain-find to identify all AD trusts + +Not all AD domains are visible to trust-find. For each trust +iterate over trustdomain-find to find the complete +list of domains. + +Signed-off-by: Rob Crittenden +--- + src/ipahealthcheck/ipa/trust.py | 20 +++-- + tests/test_ipa_trust.py | 155 ++++++++++++++++++++------------ + 2 files changed, 108 insertions(+), 67 deletions(-) + +diff --git a/src/ipahealthcheck/ipa/trust.py b/src/ipahealthcheck/ipa/trust.py +index 0abe5cd..00971c4 100644 +--- a/src/ipahealthcheck/ipa/trust.py ++++ b/src/ipahealthcheck/ipa/trust.py +@@ -42,16 +42,18 @@ def get_trust_domains(): + + Each entry is a dictionary representating an AD domain. + """ +- result = api.Command.trust_find() +- results = result['result'] + trust_domains = [] +- for result in results: +- if result.get('trusttype')[0] == 'Active Directory domain': +- domain = dict() +- domain['domain'] = result.get('cn')[0] +- domain['domainsid'] = result.get('ipanttrusteddomainsid')[0] +- domain['netbios'] = result.get('ipantflatname')[0] +- trust_domains.append(domain) ++ trusts = api.Command.trust_find(pkey_only=True, raw=True) ++ for trust in trusts['result']: ++ for cn in trust.get('cn'): ++ trustdomains = api.Command.trustdomain_find(cn, raw=True) ++ for trustdomain in trustdomains['result']: ++ domain = dict() ++ domain['domain'] = trustdomain.get('cn')[0] ++ domain['domainsid'] = trustdomain.get( ++ 'ipanttrusteddomainsid')[0] ++ domain['netbios'] = trustdomain.get('ipantflatname')[0] ++ trust_domains.append(domain) + return trust_domains + + +diff --git a/tests/test_ipa_trust.py b/tests/test_ipa_trust.py +index 3c4b947..f3a9f27 100644 +--- a/tests/test_ipa_trust.py ++++ b/tests/test_ipa_trust.py +@@ -72,6 +72,56 @@ class mock_ldap_conn: + return tuple() + + ++# ++# Construct a setup with two direct trusts and one sub domain ++# ++def trust_find(): ++ return [{ ++ 'result': [ ++ { ++ 'cn': ['ad.example'], ++ }, ++ { ++ 'cn': ['child.example'], ++ }, ++ ] ++ }] ++ ++ ++def trustdomain_find(): ++ return [ ++ { ++ "result": [ ++ { ++ "cn": ["ad.example"], ++ "ipantflatname": ["ADROOT"], ++ "ipanttrusteddomainsid": ["S-1-5-21-abc"], ++ "ipanttrusttype": ["2"], ++ "ipanttrustattributes": ["8"], ++ }, ++ { ++ "cn": ["child.ad.example"], ++ "ipantflatname": ["CHILD.ADROOT"], ++ "ipanttrusteddomainsid": ["S-1-5-22-def"], ++ "ipanttrusttype": ["2"], ++ "ipanttrustattributes": ["1"], ++ }, ++ ], ++ }, ++ { ++ "result": [ ++ { ++ "cn": ["child.example"], ++ "ipantflatname": ["CHILD"], ++ "ipanttrusteddomainsid": ["S-1-5-21-ghi"], ++ "ipanttrusttype": ["2"], ++ "ipanttrustattributes": ["8"], ++ }, ++ ], ++ }, ++ ] ++ ++ + class SSSDDomain: + def __init__(self, return_ipa_server_mode=True, provider='ipa'): + self.return_ipa_server_mode = return_ipa_server_mode +@@ -246,31 +296,17 @@ class TestTrustDomains(BaseTest): + dlresult.returncode = 0 + dlresult.error_log = '' + dlresult.output = 'implicit_files\nipa.example\nad.example\n' \ +- 'child.example\n' ++ 'child.ad.example\nchild.example\n' + olresult = namedtuple('run', ['returncode', 'error_log']) + olresult.returncode = 0 + olresult.error_log = '' + olresult.output = 'Online status: Online\n\n' + +- mock_run.side_effect = [dlresult, olresult, olresult] ++ mock_run.side_effect = [dlresult, olresult, olresult, olresult] + + # get_trust_domains() +- m_api.Command.trust_find.side_effect = [{ +- 'result': [ +- { +- 'cn': ['ad.example'], +- 'ipantflatname': ['ADROOT'], +- 'ipanttrusteddomainsid': ['S-1-5-21-abc'], +- 'trusttype': ['Active Directory domain'], +- }, +- { +- 'cn': ['child.example'], +- 'ipantflatname': ['ADROOT'], +- 'ipanttrusteddomainsid': ['S-1-5-21-def'], +- 'trusttype': ['Active Directory domain'], +- }, +- ] +- }] ++ m_api.Command.trust_find.side_effect = trust_find() ++ m_api.Command.trustdomain_find.side_effect = trustdomain_find() + + framework = object() + registry.initialize(framework, config.Config) +@@ -279,15 +315,17 @@ class TestTrustDomains(BaseTest): + + self.results = capture_results(f) + +- assert len(self.results) == 3 ++ assert len(self.results) == 4 + + result = self.results.results[0] + assert result.result == constants.SUCCESS + assert result.source == 'ipahealthcheck.ipa.trust' + assert result.check == 'IPATrustDomainsCheck' + assert result.kw.get('key') == 'domain-list' +- assert result.kw.get('trust_domains') == 'ad.example, child.example' +- assert result.kw.get('sssd_domains') == 'ad.example, child.example' ++ assert result.kw.get('trust_domains') == \ ++ 'ad.example, child.ad.example, child.example' ++ assert result.kw.get('sssd_domains') == \ ++ 'ad.example, child.ad.example, child.example' + + result = self.results.results[1] + assert result.result == constants.SUCCESS +@@ -301,6 +339,13 @@ class TestTrustDomains(BaseTest): + assert result.source == 'ipahealthcheck.ipa.trust' + assert result.check == 'IPATrustDomainsCheck' + assert result.kw.get('key') == 'domain-status' ++ assert result.kw.get('domain') == 'child.ad.example' ++ ++ result = self.results.results[3] ++ assert result.result == constants.SUCCESS ++ assert result.source == 'ipahealthcheck.ipa.trust' ++ assert result.check == 'IPATrustDomainsCheck' ++ assert result.kw.get('key') == 'domain-status' + assert result.kw.get('domain') == 'child.example' + + @patch('ipapython.ipautil.run') +@@ -319,22 +364,8 @@ class TestTrustDomains(BaseTest): + mock_run.side_effect = [dlresult, olresult, olresult] + + # get_trust_domains() +- m_api.Command.trust_find.side_effect = [{ +- 'result': [ +- { +- 'cn': ['ad.example'], +- 'ipantflatname': ['ADROOT'], +- 'ipanttrusteddomainsid': ['S-1-5-21-abc'], +- 'trusttype': ['Active Directory domain'], +- }, +- { +- 'cn': ['child.example'], +- 'ipantflatname': ['ADROOT'], +- 'ipanttrusteddomainsid': ['S-1-5-21-def'], +- 'trusttype': ['Active Directory domain'], +- }, +- ] +- }] ++ m_api.Command.trust_find.side_effect = trust_find() ++ m_api.Command.trustdomain_find.side_effect = trustdomain_find() + + framework = object() + registry.initialize(framework, config.Config) +@@ -350,7 +381,8 @@ class TestTrustDomains(BaseTest): + assert result.source == 'ipahealthcheck.ipa.trust' + assert result.check == 'IPATrustDomainsCheck' + assert result.kw.get('key') == 'domain-list' +- assert result.kw.get('trust_domains') == 'ad.example, child.example' ++ assert result.kw.get('trust_domains') == \ ++ 'ad.example, child.ad.example, child.example' + assert result.kw.get('sssd_domains') == 'child.example' + + result = self.results.results[1] +@@ -428,29 +460,16 @@ class TestTrustCatalog(BaseTest): + ds2result.output = 'Active servers:\nAD Global Catalog: ' \ + 'root-dc.ad.vm\nAD Domain Controller: root-dc.ad.vm\n' \ + +- mock_run.side_effect = [dsresult, ds2result] ++ mock_run.side_effect = [dsresult, ds2result, ds2result] + mock_getnamebysid.side_effect = [ + {'S-1-5-21-abc-500': {'name': 'admin@ad.example', 'type': 3}}, ++ {'S-1-5-21-ghi-500': {'name': 'admin@child.ad.example', 'type': 3}}, + {'S-1-5-21-def-500': {'name': 'admin@child.example', 'type': 3}} + ] + + # get_trust_domains() +- m_api.Command.trust_find.side_effect = [{ +- 'result': [ +- { +- 'cn': ['ad.example'], +- 'ipantflatname': ['ADROOT'], +- 'ipanttrusteddomainsid': ['S-1-5-21-abc'], +- 'trusttype': ['Active Directory domain'], +- }, +- { +- 'cn': ['child.example'], +- 'ipantflatname': ['ADROOT'], +- 'ipanttrusteddomainsid': ['S-1-5-21-def'], +- 'trusttype': ['Active Directory domain'], +- }, +- ] +- }] ++ m_api.Command.trust_find.side_effect = trust_find() ++ m_api.Command.trustdomain_find.side_effect = trustdomain_find() + + framework = object() + registry.initialize(framework, config.Config) +@@ -459,7 +478,7 @@ class TestTrustCatalog(BaseTest): + + self.results = capture_results(f) + +- assert len(self.results) == 6 ++ assert len(self.results) == 9 + + result = self.results.results[0] + assert result.result == constants.SUCCESS +@@ -487,20 +506,40 @@ class TestTrustCatalog(BaseTest): + assert result.source == 'ipahealthcheck.ipa.trust' + assert result.check == 'IPATrustCatalogCheck' + assert result.kw.get('key') == 'Domain Security Identifier' +- assert result.kw.get('sid') == 'S-1-5-21-def' ++ assert result.kw.get('sid') == 'S-1-5-22-def' + + result = self.results.results[4] + assert result.result == constants.SUCCESS + assert result.source == 'ipahealthcheck.ipa.trust' + assert result.check == 'IPATrustCatalogCheck' + assert result.kw.get('key') == 'AD Global Catalog' +- assert result.kw.get('domain') == 'child.example' ++ assert result.kw.get('domain') == 'child.ad.example' + + result = self.results.results[5] + assert result.result == constants.SUCCESS + assert result.source == 'ipahealthcheck.ipa.trust' + assert result.check == 'IPATrustCatalogCheck' + assert result.kw.get('key') == 'AD Domain Controller' ++ ++ result = self.results.results[6] ++ assert result.result == constants.SUCCESS ++ assert result.source == 'ipahealthcheck.ipa.trust' ++ assert result.check == 'IPATrustCatalogCheck' ++ assert result.kw.get('key') == 'Domain Security Identifier' ++ assert result.kw.get('sid') == 'S-1-5-21-ghi' ++ ++ result = self.results.results[7] ++ assert result.result == constants.SUCCESS ++ assert result.source == 'ipahealthcheck.ipa.trust' ++ assert result.check == 'IPATrustCatalogCheck' ++ assert result.kw.get('key') == 'AD Global Catalog' ++ assert result.kw.get('domain') == 'child.example' ++ ++ result = self.results.results[8] ++ assert result.result == constants.SUCCESS ++ assert result.source == 'ipahealthcheck.ipa.trust' ++ assert result.check == 'IPATrustCatalogCheck' ++ assert result.kw.get('key') == 'AD Domain Controller' + assert result.kw.get('domain') == 'child.example' + + +-- +2.25.4 + diff --git a/0003-tests-Generate-a-proper-not-valid-after-field.patch b/0003-tests-Generate-a-proper-not-valid-after-field.patch new file mode 100644 index 0000000..ba56475 --- /dev/null +++ b/0003-tests-Generate-a-proper-not-valid-after-field.patch @@ -0,0 +1,141 @@ +From 90f0b7c16c68d1dd876fc88b56b58c04bc565230 Mon Sep 17 00:00:00 2001 +From: Stanislav Levin +Date: Fri, 6 Nov 2020 15:18:33 +0300 +Subject: [PATCH] tests: Generate a proper `not-valid-after` field + +Some tests assume that the mocked certificate will be valid in N +days from now(). There was a hardcoded `not-valid-after` value +which pointed to 20201205214850Z. So, from Nov 06 2020 the assertion +20201205214850Z - now() < cert_expiration_days(30days) fails. + +Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/159 +Signed-off-by: Stanislav Levin +--- + tests/mock_certmonger.py | 18 ++++++++++++++++-- + tests/test_ipa_certfile_expiration.py | 12 ++++++++---- + tests/test_ipa_expiration.py | 8 ++++++-- + 3 files changed, 30 insertions(+), 8 deletions(-) + +diff --git a/tests/mock_certmonger.py b/tests/mock_certmonger.py +index ab53620..8fa4d36 100644 +--- a/tests/mock_certmonger.py ++++ b/tests/mock_certmonger.py +@@ -3,6 +3,7 @@ + # + + import copy ++from datetime import datetime, timedelta, timezone + + from ipaplatform.paths import paths + +@@ -10,6 +11,8 @@ from ipaplatform.paths import paths + # distinct from the value from the overrident get_defaults() method. + template = paths.CERTMONGER_COMMAND_TEMPLATE + ++CERT_EXPIRATION_DAYS = 30 ++ + pristine_cm_requests = [ + { + 'nickname': '1234', +@@ -20,7 +23,11 @@ pristine_cm_requests = [ + 'cert-storage': 'FILE', + 'cert-presave-command': template % 'renew_ra_cert_pre', + 'cert-postsave-command': template % 'renew_ra_cert', +- 'not-valid-after': 1024, ++ 'not-valid-after': ( ++ int( ++ datetime(1970, 1, 1, 0, 17, 4, tzinfo=timezone.utc).timestamp() ++ ) ++ ), + }, + { + 'nickname': '5678', +@@ -30,7 +37,14 @@ pristine_cm_requests = [ + 'template_profile': 'caIPAserviceCert', + 'cert-storage': 'FILE', + 'cert-postsave-command': template % 'restart_httpd', +- 'not-valid-after': 1607204930, ++ 'not-valid-after': ( ++ int( ++ ( ++ datetime.now(timezone.utc) + ++ timedelta(days=CERT_EXPIRATION_DAYS + 1) ++ ).timestamp() ++ ) ++ ), + }, + ] + +diff --git a/tests/test_ipa_certfile_expiration.py b/tests/test_ipa_certfile_expiration.py +index 23fc9d9..51e848a 100644 +--- a/tests/test_ipa_certfile_expiration.py ++++ b/tests/test_ipa_certfile_expiration.py +@@ -9,7 +9,11 @@ from ipahealthcheck.ipa.plugin import registry + from ipahealthcheck.ipa.certs import IPACertfileExpirationCheck + from unittest.mock import Mock, patch + from mock_certmonger import create_mock_dbus, _certmonger +-from mock_certmonger import get_expected_requests, set_requests ++from mock_certmonger import ( ++ get_expected_requests, ++ set_requests, ++ CERT_EXPIRATION_DAYS, ++) + + from datetime import datetime, timedelta + +@@ -37,7 +41,7 @@ class TestIPACertificateFile(BaseTest): + set_requests(remove=1) + + cert = IPACertificate(not_valid_after=datetime.utcnow() + +- timedelta(days=30)) ++ timedelta(days=CERT_EXPIRATION_DAYS)) + mock_load_cert.return_value = cert + + framework = object() +@@ -67,7 +71,7 @@ class TestIPACertificateFile(BaseTest): + registry.initialize(framework, config.Config) + f = IPACertfileExpirationCheck(registry) + +- f.config.cert_expiration_days = '30' ++ f.config.cert_expiration_days = str(CERT_EXPIRATION_DAYS) + self.results = capture_results(f) + + assert len(self.results) == 1 +@@ -91,7 +95,7 @@ class TestIPACertificateFile(BaseTest): + registry.initialize(framework, config.Config) + f = IPACertfileExpirationCheck(registry) + +- f.config.cert_expiration_days = '30' ++ f.config.cert_expiration_days = str(CERT_EXPIRATION_DAYS) + self.results = capture_results(f) + + assert len(self.results) == 1 +diff --git a/tests/test_ipa_expiration.py b/tests/test_ipa_expiration.py +index ff3564b..fb7105b 100644 +--- a/tests/test_ipa_expiration.py ++++ b/tests/test_ipa_expiration.py +@@ -11,7 +11,11 @@ from ipahealthcheck.ipa.certs import IPACertmongerExpirationCheck + from ipahealthcheck.ipa.certs import IPACAChainExpirationCheck + from unittest.mock import Mock, patch + from mock_certmonger import create_mock_dbus, _certmonger +-from mock_certmonger import get_expected_requests, set_requests ++from mock_certmonger import ( ++ get_expected_requests, ++ set_requests, ++ CERT_EXPIRATION_DAYS, ++) + + from datetime import datetime, timedelta, timezone + +@@ -67,7 +71,7 @@ class TestExpiration(BaseTest): + registry.initialize(framework, config.Config) + f = IPACertmongerExpirationCheck(registry) + +- f.config.cert_expiration_days = '30' ++ f.config.cert_expiration_days = str(CERT_EXPIRATION_DAYS) + self.results = capture_results(f) + + assert len(self.results) == 2 +-- +2.25.4 + diff --git a/freeipa-healthcheck.spec b/freeipa-healthcheck.spec index 4996c84..0b89a3c 100644 --- a/freeipa-healthcheck.spec +++ b/freeipa-healthcheck.spec @@ -9,7 +9,7 @@ Name: %{project}-%{shortname} Version: 0.7 -Release: 1%{?dist} +Release: 2%{?dist} Summary: Health check tool for %{projectname} BuildArch: noarch License: GPLv3 @@ -18,6 +18,8 @@ Source0: https://github.com/freeipa/freeipa-healthcheck/archive/%{version Source1: %{longname}.conf Patch0001: 0001-Remove-ipaclustercheck.patch +Patch0002: 0002-Use-trust-find-and-trustdomain-find-to-identify-all-.patch +Patch0003: 0003-tests-Generate-a-proper-not-valid-after-field.patch Requires: %{name}-core = %{version}-%{release} Requires: %{project}-server @@ -133,6 +135,10 @@ install -p -m644 %{_builddir}/%{project}-%{shortname}-%{version}/man/man5/%{long %changelog +* Tue Nov 17 2020 Rob Crittenden - 0.7-2 +- Include upstream patch to fix collection of AD trust domains +- Include upstream patch to fix failing not-valid-after test + * Thu Oct 29 2020 Rob Crittenden - 0.7-1 - Update to upstream 0.7