From d6d06a3a22d441fbfb208eeb63caae7aafd434b4 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Thu, 8 Aug 2019 18:46:32 +0000 Subject: [PATCH 7/8] Lookup AD user by SID and not by hardcoded username Looking up the user user as Administrator@REALM is not portable because the administrator login name may be localized. --- .travis.yml | 2 +- src/ipahealthcheck/ipa/plugin.py | 8 ++++ src/ipahealthcheck/ipa/trust.py | 44 +++++++++++++++---- tests/test_ipa_trust.py | 72 ++++++++++++++++++-------------- 4 files changed, 85 insertions(+), 41 deletions(-) diff --git a/.travis.yml b/.travis.yml index 16722fe..25dbf1e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -19,4 +19,4 @@ install: script: - tox -epep8,flake8 - - docker run -v ${TRAVIS_BUILD_DIR}:/root/src/ fedora:29 /bin/sh -c "dnf -y install freeipa-server tox python3-pytest; cd /root/src; tox -epy3" + - docker run -v ${TRAVIS_BUILD_DIR}:/root/src/ fedora:29 /bin/sh -c "dnf -y install freeipa-server freeipa-server-trust-ad tox python3-pytest; cd /root/src; tox -epy3" diff --git a/src/ipahealthcheck/ipa/plugin.py b/src/ipahealthcheck/ipa/plugin.py index bd95c16..34cb8c9 100644 --- a/src/ipahealthcheck/ipa/plugin.py +++ b/src/ipahealthcheck/ipa/plugin.py @@ -57,6 +57,14 @@ class IPARegistry(Registry): logging.debug('Failed to connect to LDAP: %s', e) return + # This package is pulled in when the trust package is installed + # and is required to lookup trust users. If this is not installed + # then it can be inferred that trust is not enabled. + try: + import pysss_nss_idmap # noqa: F401 + except ImportError: + return + roles = ( ADtrustBasedRole(u"ad_trust_agent_server", u"AD trust agent"), diff --git a/src/ipahealthcheck/ipa/trust.py b/src/ipahealthcheck/ipa/trust.py index 2da20c0..6c5978d 100644 --- a/src/ipahealthcheck/ipa/trust.py +++ b/src/ipahealthcheck/ipa/trust.py @@ -16,6 +16,12 @@ from ipaplatform.paths import paths from ipapython import ipautil from ipapython.dn import DN +try: + import pysss_nss_idmap +except ImportError: + # agent and controller will be set to False in init, all tests will + # be skipped + pass try: from ipaserver.masters import ENABLED_SERVICE except ImportError: @@ -33,13 +39,19 @@ def get_trust_domains(): Get the list of AD trust domains from IPA The caller is expected to catch any exceptions. + + 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': - trust_domains.append(result.get('cn')[0]) + domain = dict() + domain['domain'] = result.get('cn')[0] + domain['domainsid'] = result.get('ipanttrusteddomainsid')[0] + domain['netbios'] = result.get('ipantflatname')[0] + trust_domains.append(domain) return trust_domains @@ -123,14 +135,17 @@ class IPATrustDomainsCheck(IPAPlugin): if 'implicit_files' in sssd_domains: sssd_domains.remove('implicit_files') + trust_domains = [] try: - trust_domains = get_trust_domains() + domains = get_trust_domains() except Exception as e: yield Result(self, constants.WARNING, key='trust-find', error=str(e), msg='Execution of {key} failed: {error}') - trust_domains = [] + else: + for entry in domains: + trust_domains.append(entry.get('domain')) if api.env.domain in sssd_domains: sssd_domains.remove(api.env.domain) @@ -204,17 +219,28 @@ class IPATrustCatalogCheck(IPAPlugin): msg='Execution of {key} failed: {error}') trust_domains = [] - for domain in trust_domains: + for trust_domain in trust_domains: + sid = trust_domain.get('domainsid') try: - ipautil.run(['/bin/id', "Administrator@%s" % domain], - capture_output=True) + id = pysss_nss_idmap.getnamebysid(sid + '-500') except Exception as e: - yield Result(self, constants.WARNING, - key='/bin/id', + yield Result(self, constants.ERROR, + key=sid, error=str(e), - msg='Execution of {key} failed: {error}') + msg='Look up of{key} failed: {error}') continue + if not id: + yield Result(self, constants.WARNING, + key=sid, + error='returned nothing', + msg='Look up of {key} {error}') + else: + yield Result(self, constants.SUCCESS, + key='Domain Security Identifier', + sid=sid) + + domain = trust_domain.get('domain') args = [paths.SSSCTL, "domain-status", domain, "--active-server"] try: result = ipautil.run(args, capture_output=True) diff --git a/tests/test_ipa_trust.py b/tests/test_ipa_trust.py index 0a1d58c..89d3bff 100644 --- a/tests/test_ipa_trust.py +++ b/tests/test_ipa_trust.py @@ -261,12 +261,14 @@ class TestTrustDomains(BaseTest): { 'cn': ['ad.example'], 'ipantflatname': ['ADROOT'], - "trusttype": ["Active Directory domain"], + 'ipanttrusteddomainsid': ['S-1-5-21-abc'], + 'trusttype': ['Active Directory domain'], }, { 'cn': ['child.example'], 'ipantflatname': ['ADROOT'], - "trusttype": ["Active Directory domain"], + 'ipanttrusteddomainsid': ['S-1-5-21-def'], + 'trusttype': ['Active Directory domain'], }, ] }] @@ -324,12 +326,14 @@ class TestTrustDomains(BaseTest): { 'cn': ['ad.example'], 'ipantflatname': ['ADROOT'], - "trusttype": ["Active Directory domain"], + 'ipanttrusteddomainsid': ['S-1-5-21-abc'], + 'trusttype': ['Active Directory domain'], }, { 'cn': ['child.example'], 'ipantflatname': ['ADROOT'], - "trusttype": ["Active Directory domain"], + 'ipanttrusteddomainsid': ['S-1-5-21-def'], + 'trusttype': ['Active Directory domain'], }, ] }] @@ -373,41 +377,27 @@ class TestTrustCatalog(BaseTest): # Zero because the call was skipped altogether assert len(self.results) == 0 + @patch('pysss_nss_idmap.getnamebysid') @patch('ipapython.ipautil.run') - def test_trust_catalog_ok(self, mock_run): + def test_trust_catalog_ok(self, mock_run, mock_getnamebysid): # id Administrator@ad.example - idresult = namedtuple('run', ['returncode', 'error_log']) - idresult.returncode = 0 - idresult.error_log = '' - idresult.output = '797600500(administrator@ad.example),' \ - '1797600520(group policy creator owners@ad.example),' \ - '1797600519(enterprise admins@ad.example),' \ - '1797600512(domain admins@ad.example),' \ - '1797600518(schema admins@ad.example)' \ - ',1797600513(domain users@ad.example)\n' dsresult = namedtuple('run', ['returncode', 'error_log']) dsresult.returncode = 0 dsresult.error_log = '' dsresult.output = 'Active servers:\nAD Global Catalog: ' \ 'root-dc.ad.vm\nAD Domain Controller: root-dc.ad.vm\n' \ 'IPA: master.ipa.vm\n\n' - # id Administrator@client.example - id2result = namedtuple('run', ['returncode', 'error_log']) - id2result.returncode = 0 - id2result.error_log = '' - id2result.output = '797600500(administrator@client.example),' \ - '1797600520(group policy creator owners@client.example),' \ - '1797600519(enterprise admins@client.example),' \ - '1797600512(domain admins@client.example),' \ - '1797600518(schema admins@client.example)' \ - ',1797600513(domain users@client.example)\n' ds2result = namedtuple('run', ['returncode', 'error_log']) ds2result.returncode = 0 ds2result.error_log = '' ds2result.output = 'Active servers:\nAD Global Catalog: ' \ 'root-dc.ad.vm\nAD Domain Controller: root-dc.ad.vm\n' \ - mock_run.side_effect = [idresult, dsresult, id2result, ds2result] + mock_run.side_effect = [dsresult, ds2result] + mock_getnamebysid.side_effect = [ + {'S-1-5-21-abc-500': {'name': 'admin@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 = [{ @@ -415,12 +405,14 @@ class TestTrustCatalog(BaseTest): { 'cn': ['ad.example'], 'ipantflatname': ['ADROOT'], - "trusttype": ["Active Directory domain"], + 'ipanttrusteddomainsid': ['S-1-5-21-abc'], + 'trusttype': ['Active Directory domain'], }, { 'cn': ['child.example'], 'ipantflatname': ['ADROOT'], - "trusttype": ["Active Directory domain"], + 'ipanttrusteddomainsid': ['S-1-5-21-def'], + 'trusttype': ['Active Directory domain'], }, ] }] @@ -433,31 +425,49 @@ class TestTrustCatalog(BaseTest): f.config = config.Config() self.results = capture_results(f) - assert len(self.results) == 4 + assert len(self.results) == 6 result = self.results.results[0] 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('key') == 'Domain Security Identifier' + assert result.kw.get('sid') == 'S-1-5-21-abc' result = self.results.results[1] 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('key') == 'AD Global Catalog' + assert result.kw.get('domain') == 'ad.example' result = self.results.results[2] 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') == 'ad.example' + + result = self.results.results[3] + 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-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' - result = self.results.results[1] + 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' + assert result.kw.get('domain') == 'child.example' class Testsidgen(BaseTest): -- 2.20.1