URL: https://github.com/freeipa/freeipa/pull/584 Author: martbab Title: #584: Improve the implementation of PKINIT certificate retrieval Action: opened
PR body: """ The original PKINIT cert request code contained numerous defects, namely: * nearly absent handling of rejected requests and CA errors which resulted e.g. in an unusable WebUI after replica installation and * certificate request logic that was not consistent with the rest of the installers (DS, HTTP for example): what caused hard errors in their case went unnoticed in PKINIT setup This PR consolidates this code so that errors arising from CA rejecting the PKINIT cert request cause the installers to abort immediately. The PKINIT step was also split into a separate method executed before LDAP updates. The name was chosen to be `enable_ssl` in order to make the planned refactoring of certificate requesting code (https://pagure.io/freeipa/issue/6429) easier: the method name is not accurate but at least it is consistent with e.g. LDAP installer co the common code can be grepper with greater ease. https://pagure.io/freeipa/issue/6739 """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/584/head:pr584 git checkout pr584
From 596e3e22a4436d75973c44e48378b1b509c30867 Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Tue, 14 Mar 2017 09:56:07 +0100 Subject: [PATCH 1/4] Make PKINIT certificate request logic consistent with other installers The certmonger request handling code during pkinit setup actually never correctly handled situations when certificate request was rejected by the CA or CA was unreachable. This led to subtle errors caused by broken anonymous pkinit (e.g. failing WebUI logins) which are hard to debug. The code should behave as other service installers, e. g. use `request_and_wait_for_cert` method which raises hard error when request times out or is not granted by CA. On master contact Dogtag CA endpoint directly as is done in DS installation. https://pagure.io/freeipa/issue/6739 --- ipaserver/install/krbinstance.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py index 08d39e2..c74fe40 100644 --- a/ipaserver/install/krbinstance.py +++ b/ipaserver/install/krbinstance.py @@ -357,10 +357,15 @@ def setup_pkinit(self): subject = str(DN(('cn', self.fqdn), self.subject_base)) krbtgt = "krbtgt/" + self.realm + "@" + self.realm certpath = (paths.KDC_CERT, paths.KDC_KEY) + try: - reqid = certmonger.request_cert(certpath, subject, krbtgt, - dns=self.fqdn, storage='FILE', - profile='KDCs_PKINIT_Certs') + certmonger.request_and_wait_for_cert( + certpath, + subject, + krbtgt, + dns=self.fqdn, + storage='FILE', + profile='KDCs_PKINIT_Certs') except dbus.DBusException as e: # if the certificate is already tracked, ignore the error name = e.get_dbus_name() @@ -368,11 +373,6 @@ def setup_pkinit(self): root_logger.error("Failed to initiate the request: %s", e) return - try: - certmonger.wait_for_request(reqid) - except RuntimeError as e: - root_logger.error("Failed to wait for request: %s", e) - # Finally copy the cacert in the krb directory so we don't # have any selinux issues with the file context shutil.copyfile(paths.IPA_CA_CRT, paths.CACERT_PEM) From 73f8676e8b49e8f76883c57fd1e6618e00cab94c Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Tue, 14 Mar 2017 13:16:07 +0100 Subject: [PATCH 2/4] Request PKINIT cert directly from Dogtag API on first master On the first master the framework may not be fully functional to server certificate requests. It is safer to configure helper that contacts Dogtag REST API directly. https://pagure.io/freeipa/issue/6739 --- ipaserver/install/krbinstance.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py index c74fe40..5f2a4b1 100644 --- a/ipaserver/install/krbinstance.py +++ b/ipaserver/install/krbinstance.py @@ -68,6 +68,7 @@ def __init__(self, fstore=None): self.kdc_password = None self.sub_dict = None self.pkcs12_info = None + self.master_fqdn = None suffix = ipautil.dn_attribute_property('_suffix') subject_base = ipautil.dn_attribute_property('_subject_base') @@ -359,6 +360,18 @@ def setup_pkinit(self): certpath = (paths.KDC_CERT, paths.KDC_KEY) try: + prev_helper = None + if self.master_fqdn is None: + ca_args = [ + paths.CERTMONGER_DOGTAG_SUBMIT, + '--ee-url', 'https://%s:8443/ca/ee/ca' % self.fqdn, + '--certfile', paths.RA_AGENT_PEM, + '--keyfile', paths.RA_AGENT_KEY, + '--cafile', paths.IPA_CA_CRT, + '--agent-submit' + ] + helper = " ".join(ca_args) + prev_helper = certmonger.modify_ca_helper('IPA', helper) certmonger.request_and_wait_for_cert( certpath, subject, @@ -372,6 +385,9 @@ def setup_pkinit(self): if name != 'org.fedorahosted.certmonger.duplicate': root_logger.error("Failed to initiate the request: %s", e) return + finally: + if prev_helper is not None: + certmonger.modify_ca_helper('IPA', prev_helper) # Finally copy the cacert in the krb directory so we don't # have any selinux issues with the file context From 2a30acb1653845418496e50bad0d761e1ab1a140 Mon Sep 17 00:00:00 2001 From: Simo Sorce <s...@redhat.com> Date: Thu, 9 Mar 2017 12:49:54 -0500 Subject: [PATCH 3/4] Configure KDC to use certs after they are deployed Certmonger needs to access the KDC when it tries to obtain certs, so make sure the KDC can run, then reconfigure it to use pkinit anchors once certs are deployed. Signed-off-by: Simo Sorce <s...@redhat.com> https://pagure.io/freeipa/issue/6739 --- install/share/kdc.conf.template | 4 ++-- ipaserver/install/krbinstance.py | 28 +++++++++++++++++++--------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/install/share/kdc.conf.template b/install/share/kdc.conf.template index ec53a1f..c9d5c28 100644 --- a/install/share/kdc.conf.template +++ b/install/share/kdc.conf.template @@ -12,6 +12,6 @@ dict_file = $DICT_WORDS default_principal_flags = +preauth ; admin_keytab = $KRB5KDC_KADM5_KEYTAB - pkinit_identity = FILE:$KDC_CERT,$KDC_KEY - pkinit_anchors = FILE:$CACERT_PEM +$NOPK pkinit_identity = FILE:$KDC_CERT,$KDC_KEY +$NOPK pkinit_anchors = FILE:$CACERT_PEM } diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py index 5f2a4b1..8eea155 100644 --- a/ipaserver/install/krbinstance.py +++ b/ipaserver/install/krbinstance.py @@ -69,6 +69,7 @@ def __init__(self, fstore=None): self.sub_dict = None self.pkcs12_info = None self.master_fqdn = None + self.config_pkinit = None suffix = ipautil.dn_attribute_property('_suffix') subject_base = ipautil.dn_attribute_property('_subject_base') @@ -142,11 +143,15 @@ def __common_setup(self, realm_name, host_name, domain_name, admin_password): def __common_post_setup(self): self.step("starting the KDC", self.__start_instance) self.step("configuring KDC to start on boot", self.__enable) + if self.config_pkinit: + self.step("installing X509 Certificate for PKINIT", + self.setup_pkinit) def create_instance(self, realm_name, host_name, domain_name, admin_password, master_password, setup_pkinit=False, pkcs12_info=None, subject_base=None): self.master_password = master_password self.pkcs12_info = pkcs12_info self.subject_base = subject_base + self.config_pkinit = setup_pkinit self.__common_setup(realm_name, host_name, domain_name, admin_password) @@ -161,10 +166,6 @@ def create_instance(self, realm_name, host_name, domain_name, admin_password, ma self.__common_post_setup() - if setup_pkinit: - self.step("installing X509 Certificate for PKINIT", - self.setup_pkinit) - self.start_creation() self.kpasswd = KpasswdInstance() @@ -179,14 +180,12 @@ def create_replica(self, realm_name, self.pkcs12_info = pkcs12_info self.subject_base = subject_base self.master_fqdn = master_fqdn + self.config_pkinit = setup_pkinit self.__common_setup(realm_name, host_name, domain_name, admin_password) self.step("configuring KDC", self.__configure_instance) self.step("adding the password extension to the directory", self.__add_pwd_extop_module) - if setup_pkinit: - self.step("installing X509 Certificate for PKINIT", - self.setup_pkinit) self.__common_post_setup() @@ -221,6 +220,7 @@ def __setup_sub_dict(self): KRB5KDC_KADM5_ACL=paths.KRB5KDC_KADM5_ACL, DICT_WORDS=paths.DICT_WORDS, KRB5KDC_KADM5_KEYTAB=paths.KRB5KDC_KADM5_KEYTAB, + NOPK=';', KDC_CERT=paths.KDC_CERT, KDC_KEY=paths.KDC_KEY, CACERT_PEM=paths.CACERT_PEM) @@ -256,11 +256,12 @@ def __add_krb_container(self): def __add_default_acis(self): self._ldap_mod("default-aci.ldif", self.sub_dict) - def __template_file(self, path, chmod=0o644): + def __template_file(self, path, chmod=0o644, backup=True): template = os.path.join(paths.USR_SHARE_IPA_DIR, os.path.basename(path) + ".template") conf = ipautil.template_file(template, self.sub_dict) - self.fstore.backup_file(path) + if backup: + self.fstore.backup_file(path) fd = open(path, "w+") fd.write(conf) fd.close() @@ -393,6 +394,15 @@ def setup_pkinit(self): # have any selinux issues with the file context shutil.copyfile(paths.IPA_CA_CRT, paths.CACERT_PEM) + # Now modify configuration to add pkinit anchors and restart KDC + self.sub_dict['NOPK'] = '' + self.__template_file(paths.KRB5KDC_KDC_CONF, chmod=None, backup=False) + try: + self.restart() + except Exception: + root_logger.critical("krb5kdc service failed to restart") + raise + def get_anonymous_principal_name(self): return "%s@%s" % (ANON_USER, self.realm) From b129c93a260f43093d7c58e2594c0a892a79d6d2 Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Tue, 14 Mar 2017 12:18:35 +0100 Subject: [PATCH 4/4] Move PKINIT configuration to a later stage of server/replica install This is to ensure that we can request PKINIT certs once all the following requirements are in place: * CA is configured or PKCS#12 file is provided * LDAP, KDC and Apache are configured and the master role is thus completed and enabled https://pagure.io/freeipa/issue/6739 --- ipaserver/install/krbinstance.py | 11 ++++++++--- ipaserver/install/server/install.py | 3 +++ ipaserver/install/server/replicainstall.py | 3 +++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py index 8eea155..78cafe8 100644 --- a/ipaserver/install/krbinstance.py +++ b/ipaserver/install/krbinstance.py @@ -143,9 +143,6 @@ def __common_setup(self, realm_name, host_name, domain_name, admin_password): def __common_post_setup(self): self.step("starting the KDC", self.__start_instance) self.step("configuring KDC to start on boot", self.__enable) - if self.config_pkinit: - self.step("installing X509 Certificate for PKINIT", - self.setup_pkinit) def create_instance(self, realm_name, host_name, domain_name, admin_password, master_password, setup_pkinit=False, pkcs12_info=None, subject_base=None): self.master_password = master_password @@ -403,6 +400,14 @@ def setup_pkinit(self): root_logger.critical("krb5kdc service failed to restart") raise + def enable_ssl(self): + if self.config_pkinit: + self.steps = [] + self.step("installing X509 Certificate for PKINIT", + self.setup_pkinit) + + self.start_creation() + def get_anonymous_principal_name(self): return "%s@%s" % (ANON_USER, self.realm) diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py index d9710dc..de6b5b3 100644 --- a/ipaserver/install/server/install.py +++ b/ipaserver/install/server/install.py @@ -836,6 +836,9 @@ def install(installer): ca.set_subject_base_in_config(options.subject_base) + # configure PKINIT now that all required services are in place + krb.enable_ssl() + # Apply any LDAP updates. Needs to be done after the configuration file # is created. DS is restarted in the process. service.print_msg("Applying LDAP updates") diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py index d7f0307..b4463fd 100644 --- a/ipaserver/install/server/replicainstall.py +++ b/ipaserver/install/server/replicainstall.py @@ -1461,6 +1461,9 @@ def install(installer): options.dm_password = config.dirman_password ca.install(False, config, options) + # configure PKINIT now that all required services are in place + krb.enable_ssl() + # Apply any LDAP updates. Needs to be done after the replica is synced-up service.print_msg("Applying LDAP updates") ds.apply_updates()
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code