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

Reply via email to