Blob Blame History Raw
From ed5322daad5dc456e4958228835b33a32c7d1608 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
Date: Tue, 29 Mar 2022 12:58:01 -0400
Subject: [PATCH] Use the subject base from the IPA configuration, not REALM

The expected certificates were hardcoded with O={REALM} which
would return false-positives if the customer defined their
own certificate subject base.

Also add a search filter to only retrieve the certificate(s) we
want to examine rather than the entire contents.

Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/253

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
---
 src/ipahealthcheck/ipa/certs.py |  24 ++--
 tests/test_ipa_cert_match.py    | 202 ++++++++++++++++++++++++--------
 2 files changed, 166 insertions(+), 60 deletions(-)

diff --git a/src/ipahealthcheck/ipa/certs.py b/src/ipahealthcheck/ipa/certs.py
index 2ef33f9..fd5e180 100644
--- a/src/ipahealthcheck/ipa/certs.py
+++ b/src/ipahealthcheck/ipa/certs.py
@@ -707,12 +707,14 @@ class IPADogtagCertsMatchCheck(IPAPlugin):
 
         def match_ldap_nss_certs_by_subject(plugin, ldap, db, dn,
                                             expected_nicks_subjects):
-            entries = ldap.get_entries(dn)
             all_ok = True
             for nick, subject in expected_nicks_subjects.items():
+                entries = ldap.get_entries(
+                    dn,
+                    filter=f'subjectname={subject}'
+                )
                 cert = db.get_cert_from_db(nick)
-                ok = any([cert in entry['userCertificate'] and
-                          subject == entry['subjectName'][0]
+                ok = any([cert in entry['userCertificate']
                           for entry in entries
                           if 'userCertificate' in entry])
                 if not ok:
@@ -740,26 +742,28 @@ class IPADogtagCertsMatchCheck(IPAPlugin):
                                                       db, dn, 'CACertificate',
                                                       casigning_nick)
 
+        config = api.Command.config_show()
+        subject_base = config['result']['ipacertificatesubjectbase'][0]
         expected_nicks_subjects = {
             'ocspSigningCert cert-pki-ca':
-                'CN=OCSP Subsystem,O=%s' % api.env.realm,
+                f'CN=OCSP Subsystem,{subject_base}',
             'subsystemCert cert-pki-ca':
-                'CN=CA Subsystem,O=%s' % api.env.realm,
+                f'CN=CA Subsystem,{subject_base}',
             'auditSigningCert cert-pki-ca':
-                'CN=CA Audit,O=%s' % api.env.realm,
+                f'CN=CA Audit,{subject_base}',
             'Server-Cert cert-pki-ca':
-                'CN=%s,O=%s' % (api.env.host, api.env.realm),
+                f'CN={api.env.host},{subject_base}',
         }
 
         kra = krainstance.KRAInstance(api.env.realm)
         if kra.is_installed():
             kra_expected_nicks_subjects = {
                 'transportCert cert-pki-kra':
-                    'CN=KRA Transport Certificate,O=%s' % api.env.realm,
+                    f'CN=KRA Transport Certificate,{subject_base}',
                 'storageCert cert-pki-kra':
-                    'CN=KRA Storage Certificate,O=%s' % api.env.realm,
+                    f'CN=KRA Storage Certificate,{subject_base}',
                 'auditSigningCert cert-pki-kra':
-                    'CN=KRA Audit,O=%s' % api.env.realm,
+                    f'CN=KRA Audit,{subject_base}',
             }
             expected_nicks_subjects.update(kra_expected_nicks_subjects)
 
diff --git a/tests/test_ipa_cert_match.py b/tests/test_ipa_cert_match.py
index 460e61a..70ef59e 100644
--- a/tests/test_ipa_cert_match.py
+++ b/tests/test_ipa_cert_match.py
@@ -44,8 +44,15 @@ class mock_ldap:
     def get_entries(self, base_dn, scope=SCOPE_SUBTREE, filter=None,
                     attrs_list=None, get_effective_rights=False, **kwargs):
         if self.results is None:
-            raise errors.NotFound(reason='test')
-        return self.results.values()
+            raise errors.NotFound(reason='None')
+        if filter:
+            (attr, value) = filter.split('=', maxsplit=1)
+            for result in self.results.values():
+                if result.get(attr)[0] == value:
+                    return [result]
+            raise errors.NotFound(reason='Not found %s' % filter)
+
+        return self.results
 
 
 class mock_ldap_conn:
@@ -82,6 +89,10 @@ class TestIPACertMatch(BaseTest):
         Mock(return_value=mock_ldap_conn())
     }
 
+    trust = {
+        ('%s IPA CA' % m_api.env.realm): 'u,u,u'
+    }
+
     @patch('ipalib.x509.load_certificate_list_from_file')
     @patch('ipaserver.install.certs.CertDB')
     def test_certs_match_ok(self, mock_certdb, mock_load_cert):
@@ -92,11 +103,8 @@ class TestIPACertMatch(BaseTest):
                                    'cn=certificates,cn=ipa,cn=etc',
                                     m_api.env.basedn),
                                 CACertificate=[IPACertificate()])
-        trust = {
-            ('%s IPA CA' % m_api.env.realm): 'u,u,u'
-        }
 
-        mock_certdb.return_value = mock_CertDB(trust)
+        mock_certdb.return_value = mock_CertDB(self.trust)
         mock_load_cert.return_value = [IPACertificate()]
 
         framework = object()
@@ -121,11 +129,8 @@ class TestIPACertMatch(BaseTest):
                                    'cn=certificates,cn=ipa,cn=etc',
                                     m_api.env.basedn),
                                 CACertificate=[IPACertificate()])
-        trust = {
-            ('%s IPA CA' % m_api.env.realm): 'u,u,u'
-        }
 
-        mock_certdb.return_value = mock_CertDB(trust)
+        mock_certdb.return_value = mock_CertDB(self.trust)
         mock_load_cert.return_value = [IPACertificate(serial_number=2)]
 
         framework = object()
@@ -155,15 +160,54 @@ class TestIPACertMatch(BaseTest):
         assert len(self.results) == 0
 
 
+default_subject_base = [{
+    'result':
+        {
+            'ipacertificatesubjectbase': [f'O={m_api.env.realm}'],
+        },
+}]
+
+custom_subject_base = [{
+    'result':
+        {
+            'ipacertificatesubjectbase': ['OU=Eng,O=ACME'],
+        },
+}]
+
+
 class TestIPADogtagCertMatch(BaseTest):
     patches = {
         'ipaserver.install.krainstance.KRAInstance':
         Mock(return_value=KRAInstance()),
     }
+    trust = {
+        'ocspSigningCert cert-pki-ca': 'u,u,u',
+        'caSigningCert cert-pki-ca': 'u,u,u',
+        'subsystemCert cert-pki-ca': 'u,u,u',
+        'auditSigningCert cert-pki-ca': 'u,u,Pu',
+        'Server-Cert cert-pki-ca': 'u,u,u',
+        'transportCert cert-pki-kra': 'u,u,u',
+        'storageCert cert-pki-kra': 'u,u,u',
+        'auditSigningCert cert-pki-kra': 'u,u,Pu',
+    }
+
+    def get_dogtag_subjects(self, hostname, base):
+        subject_base = base[0]['result']['ipacertificatesubjectbase'][0]
+        return (
+            f'CN=OCSP Subsystem,{subject_base}',
+            f'CN=CA Subsystem,{subject_base}',
+            f'CN=CA Audit,{subject_base}',
+            f'CN=%s,{subject_base}',
+            f'CN=KRA Transport Certificate,{subject_base}',
+            f'CN=KRA Storage Certificate,{subject_base}',
+            f'CN=KRA Audit,{subject_base}',
+            f'CN={hostname},{subject_base}',
+        )
 
     @patch('ipaserver.install.certs.CertDB')
     def test_certs_match_ok(self, mock_certdb):
         """ Ensure match check is ok"""
+        m_api.Command.config_show.side_effect = default_subject_base
         fake_conn = LDAPClient('ldap://localhost', no_schema=True)
         pkidbentry = LDAPEntry(fake_conn,
                                DN('uid=pkidbuser,ou=people,o=ipaca'),
@@ -177,25 +221,9 @@ class TestIPADogtagCertMatch(BaseTest):
                                 userCertificate=[IPACertificate()],
                                 subjectName=['test'])
         ldap_entries = [pkidbentry, casignentry]
-        trust = {
-            'ocspSigningCert cert-pki-ca': 'u,u,u',
-            'caSigningCert cert-pki-ca': 'u,u,u',
-            'subsystemCert cert-pki-ca': 'u,u,u',
-            'auditSigningCert cert-pki-ca': 'u,u,Pu',
-            'Server-Cert cert-pki-ca': 'u,u,u',
-            'transportCert cert-pki-kra': 'u,u,u',
-            'storageCert cert-pki-kra': 'u,u,u',
-            'auditSigningCert cert-pki-kra': 'u,u,Pu',
-        }
-
-        dogtag_entries_subjects = (
-            'CN=OCSP Subsystem,O=%s' % m_api.env.realm,
-            'CN=CA Subsystem,O=%s' % m_api.env.realm,
-            'CN=CA Audit,O=%s' % m_api.env.realm,
-            'CN=%s,O=%s' % (m_api.env.host, m_api.env.realm),
-            'CN=KRA Transport Certificate,O=%s' % m_api.env.realm,
-            'CN=KRA Storage Certificate,O=%s' % m_api.env.realm,
-            'CN=KRA Audit,O=%s' % m_api.env.realm,
+
+        dogtag_entries_subjects = self.get_dogtag_subjects(
+            m_api.env.host, default_subject_base
         )
 
         for i, subject in enumerate(dogtag_entries_subjects):
@@ -206,7 +234,7 @@ class TestIPADogtagCertMatch(BaseTest):
                               subjectName=[subject])
             ldap_entries.append(entry)
 
-        mock_certdb.return_value = mock_CertDB(trust)
+        mock_certdb.return_value = mock_CertDB(self.trust)
 
         framework = object()
         registry.initialize(framework, config.Config())
@@ -223,6 +251,7 @@ class TestIPADogtagCertMatch(BaseTest):
     @patch('ipaserver.install.certs.CertDB')
     def test_certs_mismatch(self, mock_certdb):
         """ Ensure mismatches are detected"""
+        m_api.Command.config_show.side_effect = default_subject_base
         fake_conn = LDAPClient('ldap://localhost', no_schema=True)
         pkidbentry = LDAPEntry(fake_conn,
                                DN('uid=pkidbuser,ou=people,o=ipaca'),
@@ -238,25 +267,9 @@ class TestIPADogtagCertMatch(BaseTest):
                                 userCertificate=[IPACertificate()],
                                 subjectName=['test'])
         ldap_entries = [pkidbentry, casignentry]
-        trust = {
-            'ocspSigningCert cert-pki-ca': 'u,u,u',
-            'caSigningCert cert-pki-ca': 'u,u,u',
-            'subsystemCert cert-pki-ca': 'u,u,u',
-            'auditSigningCert cert-pki-ca': 'u,u,Pu',
-            'Server-Cert cert-pki-ca': 'u,u,u',
-            'transportCert cert-pki-kra': 'u,u,u',
-            'storageCert cert-pki-kra': 'u,u,u',
-            'auditSigningCert cert-pki-kra': 'u,u,Pu',
-        }
-
-        dogtag_entries_subjects = (
-            'CN=OCSP Subsystem,O=%s' % m_api.env.realm,
-            'CN=CA Subsystem,O=%s' % m_api.env.realm,
-            'CN=CA Audit,O=%s' % m_api.env.realm,
-            'CN=%s,O=%s' % (m_api.env.host, m_api.env.realm),
-            'CN=KRA Transport Certificate,O=%s' % m_api.env.realm,
-            'CN=KRA Storage Certificate,O=%s' % m_api.env.realm,
-            'CN=KRA Audit,O=%s' % m_api.env.realm,
+
+        dogtag_entries_subjects = self.get_dogtag_subjects(
+            m_api.env.host, default_subject_base
         )
 
         for i, subject in enumerate(dogtag_entries_subjects):
@@ -267,7 +280,7 @@ class TestIPADogtagCertMatch(BaseTest):
                               subjectName=[subject])
             ldap_entries.append(entry)
 
-        mock_certdb.return_value = mock_CertDB(trust)
+        mock_certdb.return_value = mock_CertDB(self.trust)
 
         framework = object()
         registry.initialize(framework, config.Config())
@@ -280,3 +293,92 @@ class TestIPADogtagCertMatch(BaseTest):
         assert result.result == constants.ERROR
         assert result.source == 'ipahealthcheck.ipa.certs'
         assert result.check == 'IPADogtagCertsMatchCheck'
+
+    @patch('ipaserver.install.certs.CertDB')
+    def test_certs_match_ok_subject(self, mock_certdb):
+        """ Ensure match check is ok"""
+        m_api.Command.config_show.side_effect = custom_subject_base
+        fake_conn = LDAPClient('ldap://localhost', no_schema=True)
+        pkidbentry = LDAPEntry(fake_conn,
+                               DN('uid=pkidbuser,ou=people,o=ipaca'),
+                               userCertificate=[IPACertificate()],
+                               subjectName=['test'])
+        casignentry = LDAPEntry(fake_conn,
+                                DN('cn=%s IPA CA' % m_api.env.realm,
+                                   'cn=certificates,cn=ipa,cn=etc',
+                                    m_api.env.basedn),
+                                CACertificate=[IPACertificate()],
+                                userCertificate=[IPACertificate()],
+                                subjectName=['test'])
+        ldap_entries = [pkidbentry, casignentry]
+
+        dogtag_entries_subjects = self.get_dogtag_subjects(
+            m_api.env.host, custom_subject_base
+        )
+
+        for i, subject in enumerate(dogtag_entries_subjects):
+            entry = LDAPEntry(fake_conn,
+                              DN('cn=%i,ou=certificateRepository' % i,
+                                 'ou=ca,o=ipaca'),
+                              userCertificate=[IPACertificate()],
+                              subjectName=[subject])
+            ldap_entries.append(entry)
+
+        mock_certdb.return_value = mock_CertDB(self.trust)
+
+        framework = object()
+        registry.initialize(framework, config.Config())
+        f = IPADogtagCertsMatchCheck(registry)
+        f.conn = mock_ldap(ldap_entries)
+        self.results = capture_results(f)
+
+        assert len(self.results) == 3
+        for result in self.results.results:
+            assert result.result == constants.SUCCESS
+            assert result.source == 'ipahealthcheck.ipa.certs'
+            assert result.check == 'IPADogtagCertsMatchCheck'
+
+    @patch('ipaserver.install.certs.CertDB')
+    def test_certs_mismatch_subject(self, mock_certdb):
+        """ Ensure mismatches are detected"""
+        m_api.Command.config_show.side_effect = custom_subject_base
+        fake_conn = LDAPClient('ldap://localhost', no_schema=True)
+        pkidbentry = LDAPEntry(fake_conn,
+                               DN('uid=pkidbuser,ou=people,o=ipaca'),
+                               userCertificate=[IPACertificate(
+                                   serial_number=2
+                               )],
+                               subjectName=['test'])
+        casignentry = LDAPEntry(fake_conn,
+                                DN('cn=%s IPA CA' % m_api.env.realm,
+                                   'cn=certificates,cn=ipa,cn=etc',
+                                    m_api.env.basedn),
+                                CACertificate=[IPACertificate()],
+                                userCertificate=[IPACertificate()],
+                                subjectName=['test'])
+        ldap_entries = [pkidbentry, casignentry]
+
+        dogtag_entries_subjects = self.get_dogtag_subjects(
+            m_api.env.host, custom_subject_base
+        )
+
+        for i, subject in enumerate(dogtag_entries_subjects):
+            entry = LDAPEntry(fake_conn,
+                              DN('cn=%i,ou=certificateRepository' % i,
+                                 'ou=ca,o=ipaca'),
+                              userCertificate=[IPACertificate()],
+                              subjectName=[subject])
+            ldap_entries.append(entry)
+
+        mock_certdb.return_value = mock_CertDB(self.trust)
+
+        framework = object()
+        registry.initialize(framework, config.Config())
+        f = IPADogtagCertsMatchCheck(registry)
+        f.conn = mock_ldap(ldap_entries)
+        self.results = capture_results(f)
+
+        assert len(self.results) == 3
+        result = self.results.results[0]
+        assert result.result == constants.ERROR
+        assert result.source == 'ipahealthcheck.ipa.certs'
-- 
2.31.1