From fa46b41af42797dba8dcd04b8cacbc78d602ab80 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Wed, 24 Jan 2024 09:23:22 +0100 Subject: [PATCH 1/2] Compatibility fix for PyCA cryptography 42.0.0 Cryptography 42.0.0 introduced two new abstract properties `not_valid_before_utc` and `not_valid_after_utc`, which are non-naive UTC variants of the `not_valid_before` and `not_valid_after` properties. The old properties are deprecated. The changeset also modifies code and tests to use the new `_utc` variants. Fixes: https://pagure.io/freeipa/issue/9518 Signed-off-by: Christian Heimes Reviewed-By: Florence Blanc-Renaud --- ipaclient/install/client.py | 4 ++-- ipalib/x509.py | 22 +++++++++++++++++++ ipapython/certdb.py | 15 +++++++------ ipaserver/install/ipa_cacert_manage.py | 2 +- ipaserver/install/ipa_cert_fix.py | 12 +++++----- ipaserver/plugins/cert.py | 4 ++-- ipaserver/plugins/dogtag.py | 12 +++++----- ipaserver/plugins/service.py | 5 +++-- ipatests/test_integration/test_acme.py | 4 ++-- .../test_integration/test_installation.py | 2 +- .../test_integration/test_ipa_cert_fix.py | 2 +- .../test_integration/test_ipahealthcheck.py | 2 +- ipatests/test_ipalib/test_x509.py | 6 +++++ 13 files changed, 61 insertions(+), 31 deletions(-) diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py index 976d3821d..5b97a37f2 100644 --- a/ipaclient/install/client.py +++ b/ipaclient/install/client.py @@ -1727,8 +1727,8 @@ def cert_summary(msg, certs, indent=' '): for cert in certs: s += '%sSubject: %s\n' % (indent, DN(cert.subject)) s += '%sIssuer: %s\n' % (indent, DN(cert.issuer)) - s += '%sValid From: %s\n' % (indent, cert.not_valid_before) - s += '%sValid Until: %s\n' % (indent, cert.not_valid_after) + s += '%sValid From: %s\n' % (indent, cert.not_valid_before_utc) + s += '%sValid Until: %s\n' % (indent, cert.not_valid_after_utc) s += '\n' s = s[:-1] diff --git a/ipalib/x509.py b/ipalib/x509.py index 769d48007..daeea8195 100644 --- a/ipalib/x509.py +++ b/ipalib/x509.py @@ -272,6 +272,28 @@ class IPACertificate(crypto_x509.Certificate): def not_valid_after(self): return self._cert.not_valid_after.replace(tzinfo=datetime.timezone.utc) + if hasattr(crypto_x509.Certificate, "not_valid_before_utc"): + # added in python-cryptography 42.0.0 + @property + def not_valid_before_utc(self): + return self._cert.not_valid_before_utc + + @property + def not_valid_after_utc(self): + return self._cert.not_valid_after_utc + else: + @property + def not_valid_before_utc(self): + return self._cert.not_valid_before.replace( + tzinfo=datetime.timezone.utc + ) + + @property + def not_valid_after_utc(self): + return self._cert.not_valid_after.replace( + tzinfo=datetime.timezone.utc + ) + @property def tbs_certificate_bytes(self): return self._cert.tbs_certificate_bytes diff --git a/ipapython/certdb.py b/ipapython/certdb.py index 21af42d23..e3a80bcec 100644 --- a/ipapython/certdb.py +++ b/ipapython/certdb.py @@ -944,19 +944,20 @@ class NSSDatabase: """Common checks for cert validity """ utcnow = datetime.datetime.now(tz=datetime.timezone.utc) - if cert.not_valid_before > utcnow: + if cert.not_valid_before_utc > utcnow: raise ValueError( - f"not valid before {cert.not_valid_before} UTC is in the " - "future." + f"not valid before {cert.not_valid_before_utc} UTC is in " + "the future." ) - if cert.not_valid_after < utcnow: + if cert.not_valid_after_utc < utcnow: raise ValueError( - f"has expired {cert.not_valid_after} UTC" + f"has expired {cert.not_valid_after_utc} UTC" ) # make sure the cert does not expire during installation - if cert.not_valid_after + datetime.timedelta(hours=1) < utcnow: + if cert.not_valid_after_utc + datetime.timedelta(hours=1) < utcnow: raise ValueError( - f"expires in less than one hour ({cert.not_valid_after} UTC)" + f"expires in less than one hour ({cert.not_valid_after_utc} " + "UTC)" ) def verify_server_cert_validity(self, nickname, hostname): diff --git a/ipaserver/install/ipa_cacert_manage.py b/ipaserver/install/ipa_cacert_manage.py index d371a854b..f6ab736fa 100644 --- a/ipaserver/install/ipa_cacert_manage.py +++ b/ipaserver/install/ipa_cacert_manage.py @@ -558,7 +558,7 @@ class CACertManage(admintool.AdminTool): now = datetime.datetime.now(tz=datetime.timezone.utc) for ca_cert, ca_nickname, _ca_trust_flags in ca_certs: - if ca_cert.not_valid_after < now: + if ca_cert.not_valid_after_utc < now: expired_certs.append(ca_nickname) diff --git a/ipaserver/install/ipa_cert_fix.py b/ipaserver/install/ipa_cert_fix.py index 834e9557d..8e02d1e75 100644 --- a/ipaserver/install/ipa_cert_fix.py +++ b/ipaserver/install/ipa_cert_fix.py @@ -208,7 +208,7 @@ def expired_dogtag_certs(now): except RuntimeError: pass # unfortunately certdb doesn't give us a better exception else: - if cert.not_valid_after <= now: + if cert.not_valid_after_utc <= now: certs.append((certid, cert)) return certs @@ -226,12 +226,12 @@ def expired_ipa_certs(now): # IPA RA cert = x509.load_certificate_from_file(paths.RA_AGENT_PEM) - if cert.not_valid_after <= now: + if cert.not_valid_after_utc <= now: certs.append((IPACertType.IPARA, cert)) # Apache HTTPD cert = x509.load_certificate_from_file(paths.HTTPD_CERT_FILE) - if cert.not_valid_after <= now: + if cert.not_valid_after_utc <= now: if not is_ipa_issued_cert(api, cert): non_renewed.append((IPACertType.HTTPS, cert)) else: @@ -244,7 +244,7 @@ def expired_ipa_certs(now): ds_nickname = ds.get_server_cert_nickname(serverid) db = NSSDatabase(nssdir=ds_dbdir) cert = db.get_cert(ds_nickname) - if cert.not_valid_after <= now: + if cert.not_valid_after_utc <= now: if not is_ipa_issued_cert(api, cert): non_renewed.append((IPACertType.LDAPS, cert)) else: @@ -252,7 +252,7 @@ def expired_ipa_certs(now): # KDC cert = x509.load_certificate_from_file(paths.KDC_CERT) - if cert.not_valid_after <= now: + if cert.not_valid_after_utc <= now: if not is_ipa_issued_cert(api, cert): non_renewed.append((IPACertType.HTTPS, cert)) else: @@ -286,7 +286,7 @@ def print_cert_info(context, desc, cert): print("{} {} certificate:".format(context, desc)) print(" Subject: {}".format(DN(cert.subject))) print(" Serial: {}".format(cert.serial_number)) - print(" Expires: {}".format(cert.not_valid_after)) + print(" Expires: {}".format(cert.not_valid_after_utc)) print() diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py index 4fb85069a..d52cdc1e2 100644 --- a/ipaserver/plugins/cert.py +++ b/ipaserver/plugins/cert.py @@ -486,9 +486,9 @@ class BaseCertObject(Object): obj['serial_number'] = str(cert.serial_number) obj['serial_number_hex'] = '0x%X' % cert.serial_number obj['valid_not_before'] = x509.format_datetime( - cert.not_valid_before) + cert.not_valid_before_utc) obj['valid_not_after'] = x509.format_datetime( - cert.not_valid_after) + cert.not_valid_after_utc) if full: obj['sha1_fingerprint'] = x509.to_hex_with_colons( cert.fingerprint(hashes.SHA1())) diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py index 7cd51ae58..23667c3dd 100644 --- a/ipaserver/plugins/dogtag.py +++ b/ipaserver/plugins/dogtag.py @@ -1475,14 +1475,14 @@ class ra(rabase.rabase, RestClient): if issuer_dn: response_request['issuer'] = issuer_dn - not_valid_before = cert.get('NotValidBefore') - if not_valid_before: + not_valid_before_utc = cert.get('NotValidBefore') + if not_valid_before_utc: response_request['valid_not_before'] = ( - not_valid_before) + not_valid_before_utc) - not_valid_after = cert.get('NotValidAfter') - if not_valid_after: - response_request['valid_not_after'] = (not_valid_after) + not_valid_after_utc = cert.get('NotValidAfter') + if not_valid_after_utc: + response_request['valid_not_after'] = (not_valid_after_utc) status = cert.get('Status') if status: diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py index f4b107213..075a1be8a 100644 --- a/ipaserver/plugins/service.py +++ b/ipaserver/plugins/service.py @@ -303,8 +303,9 @@ def set_certificate_attrs(entry_attrs): entry_attrs['serial_number_hex'] = u'0x%X' % cert.serial_number entry_attrs['issuer'] = unicode(DN(cert.issuer)) entry_attrs['valid_not_before'] = x509.format_datetime( - cert.not_valid_before) - entry_attrs['valid_not_after'] = x509.format_datetime(cert.not_valid_after) + cert.not_valid_before_utc) + entry_attrs['valid_not_after'] = x509.format_datetime( + cert.not_valid_after_utc) entry_attrs['sha1_fingerprint'] = x509.to_hex_with_colons( cert.fingerprint(hashes.SHA1())) entry_attrs['sha256_fingerprint'] = x509.to_hex_with_colons( diff --git a/ipatests/test_integration/test_acme.py b/ipatests/test_integration/test_acme.py index ee13eb4a0..8e6243d4c 100644 --- a/ipatests/test_integration/test_acme.py +++ b/ipatests/test_integration/test_acme.py @@ -670,7 +670,7 @@ class TestACMERenew(IntegrationTest): f'/etc/letsencrypt/live/{self.clients[0].hostname}/cert.pem' ) cert = x509.load_pem_x509_certificate(data, backend=default_backend()) - initial_expiry = cert.not_valid_after + initial_expiry = cert.not_valid_after_utc self.clients[0].run_command(['certbot', 'renew']) @@ -678,7 +678,7 @@ class TestACMERenew(IntegrationTest): f'/etc/letsencrypt/live/{self.clients[0].hostname}/cert.pem' ) cert = x509.load_pem_x509_certificate(data, backend=default_backend()) - renewed_expiry = cert.not_valid_after + renewed_expiry = cert.not_valid_after_utc assert initial_expiry != renewed_expiry diff --git a/ipatests/test_integration/test_installation.py b/ipatests/test_integration/test_installation.py index 36142447f..217aae019 100644 --- a/ipatests/test_integration/test_installation.py +++ b/ipatests/test_integration/test_installation.py @@ -1565,7 +1565,7 @@ class TestKRAinstallAfterCertRenew(IntegrationTest): certs = x509.load_certificate_list(cmd.stdout_text.encode('utf-8')) # get expiry date of agent cert - cert_expiry = certs[0].not_valid_after + cert_expiry = certs[0].not_valid_after_utc # move date to grace period so that certs get renewed self.master.run_command(['systemctl', 'stop', 'chronyd']) diff --git a/ipatests/test_integration/test_ipa_cert_fix.py b/ipatests/test_integration/test_ipa_cert_fix.py index ec9456e51..219e7d0e1 100644 --- a/ipatests/test_integration/test_ipa_cert_fix.py +++ b/ipatests/test_integration/test_ipa_cert_fix.py @@ -92,7 +92,7 @@ def get_cert_expiry(host, nssdb_path, cert_nick): ]) data = host.get_file_contents('/root/cert.pem') cert = x509.load_pem_x509_certificate(data, backend=default_backend()) - return cert.not_valid_after + return cert.not_valid_after_utc @pytest.fixture diff --git a/ipatests/test_integration/test_ipahealthcheck.py b/ipatests/test_integration/test_ipahealthcheck.py index 40c848988..28200e096 100644 --- a/ipatests/test_integration/test_ipahealthcheck.py +++ b/ipatests/test_integration/test_ipahealthcheck.py @@ -1595,7 +1595,7 @@ class TestIpaHealthCheck(IntegrationTest): # Pick a cert to find the upcoming expiration certfile = self.master.get_file_contents(paths.RA_AGENT_PEM) cert = x509.load_certificate_list(certfile) - cert_expiry = cert[0].not_valid_after + cert_expiry = cert[0].not_valid_after_utc # Stop chronyd so it doesn't freak out with time so off restart_service(self.master, 'chronyd') diff --git a/ipatests/test_ipalib/test_x509.py b/ipatests/test_ipalib/test_x509.py index 74287c84a..8ab2ea8c3 100644 --- a/ipatests/test_ipalib/test_x509.py +++ b/ipatests/test_ipalib/test_x509.py @@ -246,6 +246,8 @@ class test_x509: assert cert.serial_number == 1093 assert cert.not_valid_before == not_before assert cert.not_valid_after == not_after + assert cert.not_valid_before_utc == not_before + assert cert.not_valid_after_utc == not_after assert cert.san_general_names == [] assert cert.san_a_label_dns_names == [] assert cert.extended_key_usage == {'1.3.6.1.5.5.7.3.1'} @@ -277,6 +279,8 @@ class test_x509: # ensure the timezone doesn't mess with not_before and not_after assert cert.not_valid_before == not_before assert cert.not_valid_after == not_after + assert cert.not_valid_before_utc == not_before + assert cert.not_valid_after_utc == not_after def test_load_pkcs7_pem(self): certlist = x509.pkcs7_to_certs(good_pkcs7, datatype=x509.PEM) @@ -312,6 +316,8 @@ class test_x509: datetime.timezone.utc) assert cert.not_valid_before == not_before assert cert.not_valid_after == not_after + assert cert.not_valid_before_utc == not_before + assert cert.not_valid_after_utc == not_after assert cert.san_general_names == [DNSName('ipa.demo1.freeipa.org')] assert cert.san_a_label_dns_names == ['ipa.demo1.freeipa.org'] assert cert.extended_key_usage == { -- 2.43.0 From 18244d7ec1103ec6fba0f94c385e62dba774ed3d Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Thu, 25 Jan 2024 08:56:11 +0100 Subject: [PATCH 2/2] test_acme: Use ipalib.x509 Use IPA's x509 module instead of `cryptography.x509`. This fixes a regression which was introduced in commit a45a7a20. Related: https://pagure.io/freeipa/issue/9518 Signed-off-by: Christian Heimes Reviewed-By: Florence Blanc-Renaud Reviewed-By: Mohammad Rizwan Yusuf --- ipatests/test_integration/test_acme.py | 9 ++++----- ipatests/test_integration/test_ipa_cert_fix.py | 5 ++--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/ipatests/test_integration/test_acme.py b/ipatests/test_integration/test_acme.py index 8e6243d4c..4032d266a 100644 --- a/ipatests/test_integration/test_acme.py +++ b/ipatests/test_integration/test_acme.py @@ -4,11 +4,10 @@ import time -from cryptography.hazmat.backends import default_backend -from cryptography import x509 import pytest from ipalib.constants import IPA_CA_RECORD +from ipalib import x509 from ipatests.test_integration.base import IntegrationTest from ipatests.pytest_ipa.integration.firewall import Firewall from ipatests.pytest_ipa.integration import tasks @@ -278,7 +277,7 @@ class TestACME(CALessBase): cert_path = \ f'/etc/letsencrypt/live/{self.clients[0].hostname}/cert.pem' data = self.clients[0].get_file_contents(cert_path) - cert = x509.load_pem_x509_certificate(data, backend=default_backend()) + cert = x509.load_pem_x509_certificate(data) # revoke cert via ACME self.clients[0].run_command( @@ -669,7 +668,7 @@ class TestACMERenew(IntegrationTest): data = self.clients[0].get_file_contents( f'/etc/letsencrypt/live/{self.clients[0].hostname}/cert.pem' ) - cert = x509.load_pem_x509_certificate(data, backend=default_backend()) + cert = x509.load_pem_x509_certificate(data) initial_expiry = cert.not_valid_after_utc self.clients[0].run_command(['certbot', 'renew']) @@ -677,7 +676,7 @@ class TestACMERenew(IntegrationTest): data = self.clients[0].get_file_contents( f'/etc/letsencrypt/live/{self.clients[0].hostname}/cert.pem' ) - cert = x509.load_pem_x509_certificate(data, backend=default_backend()) + cert = x509.load_pem_x509_certificate(data) renewed_expiry = cert.not_valid_after_utc assert initial_expiry != renewed_expiry diff --git a/ipatests/test_integration/test_ipa_cert_fix.py b/ipatests/test_integration/test_ipa_cert_fix.py index 219e7d0e1..e6ec30de1 100644 --- a/ipatests/test_integration/test_ipa_cert_fix.py +++ b/ipatests/test_integration/test_ipa_cert_fix.py @@ -5,13 +5,12 @@ """ Module provides tests for ipa-cert-fix CLI. """ -from cryptography.hazmat.backends import default_backend -from cryptography import x509 from datetime import datetime, date import pytest import time import logging +from ipalib import x509 from ipaplatform.paths import paths from ipapython.ipaldap import realm_to_serverid from ipatests.pytest_ipa.integration import tasks @@ -91,7 +90,7 @@ def get_cert_expiry(host, nssdb_path, cert_nick): '-o', '/root/cert.pem' ]) data = host.get_file_contents('/root/cert.pem') - cert = x509.load_pem_x509_certificate(data, backend=default_backend()) + cert = x509.load_pem_x509_certificate(data) return cert.not_valid_after_utc -- 2.43.0