URL: https://github.com/freeipa/freeipa/pull/584 Author: martbab Title: #584: Improve the implementation of PKINIT certificate retrieval Action: synchronized
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 89186ef9f4e6e7278f0c1fe36cf40b74cac217d1 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/7] 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 20c2797223f4b4fc23e49c00782d7b7ae7ba7e6e 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/7] 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 1145ce1a7ecd4e368f02e64f4454c224a7dfe99d Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Thu, 9 Mar 2017 12:49:54 -0500 Subject: [PATCH 3/7] 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 | 24 +++++++++++++++++------- ipaserver/install/server/install.py | 3 +++ ipaserver/install/server/replicainstall.py | 3 +++ 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py index 5f2a4b1..04cf681 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') @@ -147,6 +148,7 @@ def create_instance(self, realm_name, host_name, domain_name, admin_password, ma 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 +163,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 +177,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() @@ -393,6 +389,20 @@ def setup_pkinit(self): # have any selinux issues with the file context shutil.copyfile(paths.IPA_CA_CRT, paths.CACERT_PEM) + try: + self.restart() + except Exception: + 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() From 18e22fd5b181b41cdb1b721c5b52d926d1aef11a Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Wed, 15 Mar 2017 13:31:27 +0100 Subject: [PATCH 4/7] Make wait_for_entry raise exceptions Instead of only logging errors when timeout is reached or query for the entry fails for other reasons, `wait_for_entry` should raise exceptions so that we can handle them in caller or let them propagate and fail early. https://pagure.io/freeipa/issue/6739 --- ipaserver/install/replication.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py index 1f13783..3cd871e 100644 --- a/ipaserver/install/replication.py +++ b/ipaserver/install/replication.py @@ -177,7 +177,7 @@ def wait_for_entry(connection, dn, timeout=7200, attr='', quiet=True): pass # no entry yet except Exception as e: # badness root_logger.error("Error reading entry %s: %s", dn, e) - break + raise if not entry: if not quiet: sys.stdout.write(".") @@ -185,13 +185,10 @@ def wait_for_entry(connection, dn, timeout=7200, attr='', quiet=True): time.sleep(1) if not entry and int(time.time()) > timeout: - root_logger.error( - "wait_for_entry timeout for %s for %s", connection, dn) + raise errors.NotFound( + reason="wait_for_entry timeout for %s for %s" % (connection, dn)) elif entry and not quiet: root_logger.error("The waited for entry is: %s", entry) - elif not entry: - root_logger.error( - "Error: could not read entry %s from %s", dn, connection) class ReplicationManager(object): From 2748d2c53dd7bf2b62584b7a7a56ec876d7bc575 Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Wed, 15 Mar 2017 14:00:49 +0100 Subject: [PATCH 5/7] check that the master requesting PKINIT cert has KDC enabled https://pagure.io/freeipa/issue/6739 --- ipaserver/plugins/cert.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py index 8b9b863..47c10f3 100644 --- a/ipaserver/plugins/cert.py +++ b/ipaserver/plugins/cert.py @@ -215,11 +215,23 @@ def caacl_check(principal, ca, profile_id): ) -def ca_kdc_check(ldap, hostname): - result = api.Command.config_show()['result'] - if hostname not in result['ipa_master_server']: +def ca_kdc_check(api_instance, hostname): + master_dn = api_instance.Object.server.get_dn(unicode(hostname)) + kdc_dn = DN(('cn', 'KDC'), master_dn) + + try: + kdc_entry = api_instance.Backend.ldap2.get_entry( + kdc_dn, ['ipaConfigString']) + + ipaconfigstring = {val.lower() for val in kdc_entry['ipaConfigString']} + + if 'enabledservice' not in ipaconfigstring: + raise errors.NotFound() + + except errors.NotFound: raise errors.ACIError(info=_( - "Host '%(hostname)s' is not a KDC") % dict(hostname=hostname)) + "Host '%(hostname)s' is not an active KDC") + % dict(hostname=hostname)) def validate_certificate(value): @@ -604,7 +616,7 @@ def execute(self, csr, all=False, raw=False, chain=False, **kw): if not bypass_caacl: if principal_type == KRBTGT: - ca_kdc_check(ldap, bind_principal.hostname) + ca_kdc_check(self.api, bind_principal.hostname) else: caacl_check(principal, ca, profile_id) From 824ccd3cf439acfb7746367ad93b025a18f80568 Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Wed, 15 Mar 2017 14:03:19 +0100 Subject: [PATCH 6/7] check for replica's KDC entry on master before requesting PKINIT cert This prevents replication-based race conditions to break PKINIT certificate requests on replica installation. https://pagure.io/freeipa/issue/6739 --- ipaserver/install/krbinstance.py | 15 +++++++++++++++ ipaserver/plugins/cert.py | 6 +++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py index 04cf681..36d1588 100644 --- a/ipaserver/install/krbinstance.py +++ b/ipaserver/install/krbinstance.py @@ -30,6 +30,7 @@ from ipaserver.install import service from ipaserver.install import installutils +from ipapython import ipaldap from ipapython import ipautil from ipapython import kernel_keyring from ipalib import api @@ -342,6 +343,17 @@ def __create_host_keytab(self): self.move_service_to_host(host_principal) + def _wait_for_replica_kdc_entry(self): + master_dn = self.api.Object.server.get_dn(self.fqdn) + kdc_dn = DN(('cn', 'KDC'), master_dn) + + ldap_uri = 'ldap://{}'.format(self.master_fqdn) + + with ipaldap.LDAPClient( + ldap_uri, cacert=paths.IPA_CA_CRT) as remote_ldap: + remote_ldap.gssapi_bind() + replication.wait_for_entry(remote_ldap, kdc_dn, timeout=60) + def setup_pkinit(self): if self.pkcs12_info: certs.install_pem_from_p12(self.pkcs12_info[0], @@ -368,6 +380,9 @@ def setup_pkinit(self): ] helper = " ".join(ca_args) prev_helper = certmonger.modify_ca_helper('IPA', helper) + else: + self._wait_for_replica_kdc_entry() + certmonger.request_and_wait_for_cert( certpath, subject, diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py index 47c10f3..9f90107 100644 --- a/ipaserver/plugins/cert.py +++ b/ipaserver/plugins/cert.py @@ -229,9 +229,9 @@ def ca_kdc_check(api_instance, hostname): raise errors.NotFound() except errors.NotFound: - raise errors.ACIError(info=_( - "Host '%(hostname)s' is not an active KDC") - % dict(hostname=hostname)) + raise errors.ACIError( + info=_("Host '%(hostname)s' is not an active KDC") + % dict(hostname=hostname)) def validate_certificate(value): From 6a86ff2c2bac94a665dd782418b170490a4fc9b3 Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Wed, 15 Mar 2017 14:04:56 +0100 Subject: [PATCH 7/7] Try out anonymous PKINIT after it is configured After PKINIT certificate is requested and everything is set up, we should attempt to perform anonymous PKINIT and fail hard if it does not work for some reason. https://pagure.io/freeipa/issue/6739 --- ipaserver/install/krbinstance.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py index 36d1588..d936cc5 100644 --- a/ipaserver/install/krbinstance.py +++ b/ipaserver/install/krbinstance.py @@ -410,6 +410,12 @@ def setup_pkinit(self): root_logger.critical("krb5kdc service failed to restart") raise + with ipautil.private_ccache() as anon_ccache: + try: + ipautil.run([paths.KINIT, '-n', '-c', anon_ccache]) + except ipautil.CalledProcessError as e: + raise RuntimeError("Failed to configure anonymous PKINIT") + def enable_ssl(self): if self.config_pkinit: self.steps = []
-- 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
