rpms/ipa-healthcheck
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Include upstream patches to fix trust collection and make check

Browse Source 
- Include upstream patch to fix collection of AD trust domains
- Include upstream patch to fix failing not-valid-after test
This commit is contained in:
Rob Crittenden 2020-11-17 20:46:34 -05:00
parent de38a7080f
commit ca10bedb3c
3 changed files with 451 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

303
0002-Use-trust-find-and-trustdomain-find-to-identify-all-.patch Normal file
View File

@ -0,0 +1,303 @@
From 886153da7dd1ca1f5d37dd9c1e2141850b7177b2 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
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 <domain> to find the complete
list of domains.
Signed-off-by: Rob Crittenden <rcritten@redhat.com>
---
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

141
0003-tests-Generate-a-proper-not-valid-after-field.patch Normal file
View File

@ -0,0 +1,141 @@
From 90f0b7c16c68d1dd876fc88b56b58c04bc565230 Mon Sep 17 00:00:00 2001
From: Stanislav Levin <slev@altlinux.org>
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 <slev@altlinux.org>
---
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

8
freeipa-healthcheck.spec
View File

@ -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 <rcritten@redhat.com> - 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 <rcritten@redhat.com> - 0.7-1
- Update to upstream 0.7