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

Reply via email to