From 8c2c6f825337aebc3bbd07ffa875547f4bb6a40b Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Fri, 19 Feb 2021 20:36:03 +1100 Subject: [PATCH] ipa-cert-fix: improve handling of 'pki-server cert-fix' failure 'pki-server cert-fix' has a known and expected failure when the DS certificate is expired. 'ipa-cert-fix' handles this by optimistically ignoring the CalledProcessError and continuing when the DS certificate was up for renewal. This heuristic is a bit too optimistic. If 'pki-server cert-fix' fails due and returns nonzero due to some other, more serious error (as has been seen in the wild[1]), 'ipa-cert-fix' continues then fails later with a more confusing error, for example: [Errno 2] No such file or directory: '/etc/pki/pki-tomcat/certs/27-renewed.crt' [1] https://bugzilla.redhat.com/show_bug.cgi?id=1930586 Improve the heuristic by also checking whether output files corresponding ot all of the "extra" certificate that we asked 'ipa-cert-fix' to renew, do indeed exist and are X.509 certificates. Fixes: https://pagure.io/freeipa/issue/8721 Reviewed-By: Florence Blanc-Renaud --- ipaserver/install/ipa_cert_fix.py | 44 ++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/ipaserver/install/ipa_cert_fix.py b/ipaserver/install/ipa_cert_fix.py index 210cf80f1..36de2ac34 100644 --- a/ipaserver/install/ipa_cert_fix.py +++ b/ipaserver/install/ipa_cert_fix.py @@ -57,6 +57,8 @@ and keys, is STRONGLY RECOMMENDED. """ +RENEWED_CERT_PATH_TEMPLATE = "/etc/pki/pki-tomcat/certs/{}-renewed.crt" + logger = logging.getLogger(__name__) @@ -145,11 +147,18 @@ class IPACertFix(AdminTool): x[0] is IPACertType.LDAPS for x in extra_certs + non_renewed ): - # The DS cert was expired. This will cause - # 'pki-server cert-fix' to fail at the final - # restart. Therefore ignore the CalledProcessError - # and proceed to installing the IPA-specific certs. - pass + # The DS cert was expired. This will cause 'pki-server + # cert-fix' to fail at the final restart, and return nonzero. + # So this exception *might* be OK to ignore. + # + # If 'pki-server cert-fix' has written new certificates + # corresponding to all the extra_certs, then ignore the + # CalledProcessError and proceed to installing the IPA-specific + # certs. Otherwise re-raise. + if check_renewed_ipa_certs(extra_certs): + pass + else: + raise else: raise # otherwise re-raise @@ -365,11 +374,32 @@ def replicate_dogtag_certs(subject_base, ca_subject_dn, certs): replicate_cert(subject_base, ca_subject_dn, cert) +def check_renewed_ipa_certs(certs): + """ + Check whether all expected IPA-specific certs (extra_certs) were renewed + successfully. + + For now this subroutine just checks that the files that we expect + ``pki-server cert-fix`` to have written do exist and contain an X.509 + certificate. + + Return ``True`` if everything seems to be as expected, otherwise ``False``. + + """ + for _certtype, oldcert in certs: + cert_path = RENEWED_CERT_PATH_TEMPLATE.format(oldcert.serial_number) + try: + x509.load_certificate_from_file(cert_path) + except (IOError, ValueError): + return False + + return True + + def install_ipa_certs(subject_base, ca_subject_dn, certs): """Print details and install renewed IPA certificates.""" for certtype, oldcert in certs: - cert_path = "/etc/pki/pki-tomcat/certs/{}-renewed.crt" \ - .format(oldcert.serial_number) + cert_path = RENEWED_CERT_PATH_TEMPLATE.format(oldcert.serial_number) cert = x509.load_certificate_from_file(cert_path) print_cert_info("Renewed IPA", certtype.value, cert)