From 6661eb1f6ac3873fea26d0ceb2c8560b70641c22 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Fri, 18 Jul 2025 11:23:44 -0400 Subject: [PATCH] Report if an expiring certificate is externally signed This is intended for externally-signed IPA CA certificates. It should not duplicate user-provided certificats because this check, IPACertmongerExpirationCheck, is based on certmonger tracking and we don't track, by default, user-provided certs. Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/104 Signed-off-by: Rob Crittenden --- src/ipahealthcheck/ipa/certs.py | 41 +++++++++++++--- tests/mock_certmonger.py | 2 + tests/test_ipa_expiration.py | 87 +++++++++++++++++++++++++++++++-- tests/test_ipa_tracking.py | 3 +- 4 files changed, 123 insertions(+), 10 deletions(-) diff --git a/src/ipahealthcheck/ipa/certs.py b/src/ipahealthcheck/ipa/certs.py index fc626a6f..434dadd3 100644 --- a/src/ipahealthcheck/ipa/certs.py +++ b/src/ipahealthcheck/ipa/certs.py @@ -304,6 +304,9 @@ def check(self): 'nickname')) notafter = int(request.prop_if.Get(certmonger.DBUS_CM_REQUEST_IF, 'not-valid-after')) + cert = str(request.prop_if.Get(certmonger.DBUS_CM_REQUEST_IF, + 'cert')) + if notafter == 0: yield Result(self, constants.ERROR, key=id, @@ -312,27 +315,53 @@ def check(self): 'has not been issued yet.') continue + # if we have notafter then we should have a cert but lets not + # assume. + is_ipa_issued = None + if cert: + try: + cert = x509.load_certificate_list(cert.encode('utf-8'))[0] + except Exception as e: + logger.debug("Failed to load certificate: ", e) + else: + is_ipa_issued = is_ipa_issued_cert(api, cert) + + if is_ipa_issued is False: + external_msg = ( + 'This is not an IPA-issued certificate and ' + 'will not auto-renew.') + else: + external_msg = '' + nafter = datetime.fromtimestamp(notafter, timezone.utc) now = datetime.now(timezone.utc) if now > nafter: + msg = ( + 'Request id {key} expired on {expiration_date}. ' + + external_msg) yield Result(self, constants.ERROR, key=id, expiration_date=generalized_time(nafter), - msg='Request id {key} expired on ' - '{expiration_date}') + msg=msg) else: delta = nafter - now diff = int(delta.total_seconds() / DAY) if diff < int(self.config.cert_expiration_days): + msg = 'Request id {key} expires in {days} days. ' + if external_msg: + msg = msg + external_msg + else: + msg = msg + ( + 'certmonger should renew this ' + 'automatically. Watch the status with ' + 'getcert list -i {key}.' + ) yield Result(self, constants.WARNING, key=id, expiration_date=generalized_time(nafter), days=diff, - msg='Request id {key} expires in {days} ' - 'days. certmonger should renew this ' - 'automatically. Watch the status with ' - 'getcert list -i {key}.') + msg=msg) else: yield Result(self, constants.SUCCESS, key=id) diff --git a/tests/mock_certmonger.py b/tests/mock_certmonger.py index 0303aef8..1117e238 100644 --- a/tests/mock_certmonger.py +++ b/tests/mock_certmonger.py @@ -23,6 +23,7 @@ 'cert-storage': 'FILE', 'cert-presave-command': template % 'renew_ra_cert_pre', 'cert-postsave-command': template % 'renew_ra_cert', + 'cert': '----- BEGIN -----', 'not-valid-after': ( int( datetime(1970, 1, 1, 0, 17, 4, tzinfo=timezone.utc).timestamp() @@ -37,6 +38,7 @@ 'template_profile': 'caIPAserviceCert', 'cert-storage': 'FILE', 'cert-postsave-command': template % 'restart_httpd', + 'cert': '----- BEGIN -----', 'not-valid-after': ( int( ( diff --git a/tests/test_ipa_expiration.py b/tests/test_ipa_expiration.py index e19aa7b5..3a168fda 100644 --- a/tests/test_ipa_expiration.py +++ b/tests/test_ipa_expiration.py @@ -29,8 +29,21 @@ class TestExpiration(BaseTest): 'ipalib.install.certmonger._certmonger': Mock(return_value=_certmonger()) } + root_ca = 'CN=Certificate Authority,O=EXAMPLE.TEST' + subject = 'CN=Certificate Authority,O=EXAMPLE.TEST' - def test_expiration(self): + @patch('ipalib.x509.load_certificate_list') + @patch('ipahealthcheck.ipa.certs.is_ipa_issued_cert') + def test_expiration(self, mock_external, mock_load): + mock_external.return_value = True + mock_load.return_value = [ + FakeIPACertificate( + None, + subject=self.subject, + issuer=self.root_ca, + not_after=datetime.now(timezone.utc) + timedelta(days=20) + ), + ] set_requests() framework = object() @@ -55,7 +68,67 @@ def test_expiration(self): assert result.check == 'IPACertmongerExpirationCheck' assert result.kw.get('key') == '5678' - def test_expiration_warning(self): + @patch('ipalib.x509.load_certificate_list') + @patch('ipahealthcheck.ipa.certs.is_ipa_issued_cert') + def test_expiration_warning(self, mock_external, mock_load): + mock_external.return_value = True + mock_load.return_value = [ + FakeIPACertificate( + None, + subject=self.subject, + issuer=self.root_ca, + not_after=datetime.now(timezone.utc) + timedelta(days=20) + ), + ] + warning = datetime.now(timezone.utc) + timedelta(days=20) + replaceme = { + 'nickname': '7777', + 'cert-file': paths.RA_AGENT_PEM, + 'key-file': paths.RA_AGENT_KEY, + 'ca-name': 'dogtag-ipa-ca-renew-agent', + 'not-valid-after': int(warning.timestamp()), + 'cert': '----- BEGIN -----' + } + + set_requests(remove=0, add=replaceme) + + framework = object() + registry.initialize(framework, config.Config) + f = IPACertmongerExpirationCheck(registry) + + f.config.cert_expiration_days = str(CERT_EXPIRATION_DAYS) + self.results = capture_results(f) + + assert len(self.results) == 2 + + result = self.results.results[0] + assert result.result == constants.SUCCESS + assert result.source == 'ipahealthcheck.ipa.certs' + assert result.check == 'IPACertmongerExpirationCheck' + assert result.kw.get('key') == '5678' + + result = self.results.results[1] + assert result.result == constants.WARNING + assert result.source == 'ipahealthcheck.ipa.certs' + assert result.check == 'IPACertmongerExpirationCheck' + assert result.kw.get('key') == '7777' + assert result.kw.get('days') == 19 + assert 'This is not an IPA-issued cert' not in result.kw.get('msg') + + @patch('ipalib.x509.load_certificate_list') + @patch('ipahealthcheck.ipa.certs.is_ipa_issued_cert') + def test_external_expiration_warning(self, mock_external, mock_load): + root_ca = 'CN=Certificate Shack Root CA,O=Certificate Shack Ltd' + subject = 'CN=Certificate Authority,O=EXAMPLE.TEST' + mock_external.return_value = False + mock_load.return_value = [ + FakeIPACertificate( + None, + subject=subject, + issuer=root_ca, + not_after=datetime.now(timezone.utc) + timedelta(days=20) + ), + ] warning = datetime.now(timezone.utc) + timedelta(days=20) replaceme = { 'nickname': '7777', @@ -63,6 +136,7 @@ def test_expiration_warning(self): 'key-file': paths.RA_AGENT_KEY, 'ca-name': 'dogtag-ipa-ca-renew-agent', 'not-valid-after': int(warning.timestamp()), + 'cert': '----- BEGIN -----' } set_requests(remove=0, add=replaceme) @@ -88,17 +162,24 @@ def test_expiration_warning(self): assert result.check == 'IPACertmongerExpirationCheck' assert result.kw.get('key') == '7777' assert result.kw.get('days') == 19 + assert 'This is not an IPA-issued cert' in result.kw.get('msg') class FakeIPACertificate: - def __init__(self, cert, backend=None, subject=None, not_after=None): + def __init__(self, cert, backend=None, subject=None, issuer=None, + not_after=None): self.subj = subject + self._issuer = issuer self.not_after = not_after @property def subject(self): return self.subj + @property + def issuer(self): + return self._issuer + @property def not_valid_after_utc(self): return self.not_after diff --git a/tests/test_ipa_tracking.py b/tests/test_ipa_tracking.py index 58d97b7f..70d36ef5 100644 --- a/tests/test_ipa_tracking.py +++ b/tests/test_ipa_tracking.py @@ -59,7 +59,8 @@ def test_missing_cert_tracking(self): "cert-presave-command=" \ "/usr/libexec/ipa/certmonger/renew_ra_cert_pre, " \ "cert-postsave-command=" \ - "/usr/libexec/ipa/certmonger/renew_ra_cert" + "/usr/libexec/ipa/certmonger/renew_ra_cert, " \ + "cert=----- BEGIN -----" def test_unknown_cert_tracking(self): # Add a custom, unknown request