URL: https://github.com/freeipa/freeipa/pull/484
Author: stlaz
 Title: #484: FIPS: Remove pkispawn cruft
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/484/head:pr484
git checkout pr484
From 89a7de36231081856e59f64f0a6217beaabbcf37 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Tue, 14 Feb 2017 16:54:43 +0100
Subject: [PATCH 1/2] Remove ra_db argument from CAInstance init

The ra_db argument to CAInstance init is a constant so it can
be removed. This constant corresponds to the default CertDB directory
and since CertDB now passes passwords to its inner NSSDatabase instance
we do need to care about having our own run_certutil() method.

https://fedorahosted.org/freeipa/ticket/5695
---
 ipaserver/install/ca.py             |  8 +++-----
 ipaserver/install/cainstance.py     | 32 ++++++++++----------------------
 ipaserver/install/server/upgrade.py |  2 +-
 3 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/ipaserver/install/ca.py b/ipaserver/install/ca.py
index 8e92ef0..e346a2b 100644
--- a/ipaserver/install/ca.py
+++ b/ipaserver/install/ca.py
@@ -265,8 +265,7 @@ def install_step_0(standalone, replica_config, options):
         'certmap.conf', 'subject_base', str(subject_base))
     dsinstance.write_certmap_conf(realm_name, ca_subject)
 
-    ca = cainstance.CAInstance(realm_name, paths.IPA_RADB_DIR,
-                               host_name=host_name)
+    ca = cainstance.CAInstance(realm_name, host_name=host_name)
     ca.configure_instance(host_name, dm_password, dm_password,
                           subject_base=subject_base,
                           ca_subject=ca_subject,
@@ -293,8 +292,7 @@ def install_step_1(standalone, replica_config, options):
     subject_base = options._subject_base
     basedn = ipautil.realm_to_suffix(realm_name)
 
-    ca = cainstance.CAInstance(realm_name, paths.IPA_RADB_DIR,
-                               host_name=host_name)
+    ca = cainstance.CAInstance(realm_name, host_name=host_name)
 
     ca.stop('pki-tomcat')
 
@@ -356,7 +354,7 @@ def install_step_1(standalone, replica_config, options):
 
 
 def uninstall():
-    ca_instance = cainstance.CAInstance(api.env.realm, paths.IPA_RADB_DIR)
+    ca_instance = cainstance.CAInstance(api.env.realm)
     ca_instance.stop_tracking_certificates()
     if ca_instance.is_configured():
         ca_instance.uninstall()
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 52485b9..0b78139 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -294,7 +294,7 @@ class CAInstance(DogtagInstance):
                      ('caSigningCert cert-pki-ca', 'ipaCACertRenewal'))
     server_cert_name = 'Server-Cert cert-pki-ca'
 
-    def __init__(self, realm=None, ra_db=None, host_name=None):
+    def __init__(self, realm=None, host_name=None):
         super(CAInstance, self).__init__(
             realm=realm,
             subsystem="CA",
@@ -313,11 +313,8 @@ def __init__(self, realm=None, ra_db=None, host_name=None):
             self.canickname = get_ca_nickname(realm)
         else:
             self.canickname = None
-        self.ra_agent_db = ra_db
-        if self.ra_agent_db is not None:
-            self.ra_agent_pwd = self.ra_agent_db + "/pwdfile.txt"
-        else:
-            self.ra_agent_pwd = None
+        self.ra_agent_db = paths.IPA_RADB_DIR
+        self.ra_agent_pwd = os.path.join(self.ra_agent_db, "pwdfile.txt")
         self.ra_cert = None
         self.requestId = None
         self.log = log_mgr.get_logger(self)
@@ -742,16 +739,6 @@ def __create_ca_agent(self):
 
         conn.disconnect()
 
-    def __run_certutil(self, args, database=None, pwd_file=None, stdin=None,
-                       **kwargs):
-        if not database:
-            database = self.ra_agent_db
-        if not pwd_file:
-            pwd_file = self.ra_agent_pwd
-        new_args = [paths.CERTUTIL, "-d", database, "-f", pwd_file]
-        new_args = new_args + args
-        return ipautil.run(new_args, stdin, nolog=(pwd_file,), **kwargs)
-
     def __get_ca_chain(self):
         try:
             return dogtag.get_ca_certchain(ca_host=self.fqdn)
@@ -791,7 +778,7 @@ def __import_ca_chain(self):
                 else:
                     nick = str(subject_dn)
                     trust_flags = ',,'
-                self.__run_certutil(
+                certdb.run_certutil(
                     ['-A', '-t', trust_flags, '-n', nick, '-a',
                      '-i', chain_file.name]
                 )
@@ -852,7 +839,8 @@ def __request_ra_certificate(self):
                 post_command='renew_ra_cert')
 
             self.requestId = str(reqId)
-            result = self.__run_certutil(
+            certdb = certs.CertDB(self.realm)
+            result = certdb.run_certutil(
                 ['-L', '-n', 'ipaCert', '-a'], capture_output=True)
             self.ra_cert = x509.strip_header(result.output)
             self.ra_cert = "\n".join(
@@ -1017,8 +1005,8 @@ def configure_agent_renewal(self):
                 ca='dogtag-ipa-ca-renew-agent',
                 nickname='ipaCert',
                 pin=None,
-                pinfile=os.path.join(paths.IPA_RADB_DIR, 'pwdfile.txt'),
-                secdir=paths.IPA_RADB_DIR,
+                pinfile=self.ra_agent_pwd,
+                secdir=self.ra_agent_db,
                 pre_command='renew_ra_cert_pre',
                 post_command='renew_ra_cert')
         except RuntimeError as e:
@@ -1037,7 +1025,7 @@ def stop_tracking_certificates(self):
                 certmonger.stop_tracking(self.nss_db, nickname=nickname)
 
         try:
-            certmonger.stop_tracking(paths.IPA_RADB_DIR, nickname='ipaCert')
+            certmonger.stop_tracking(self.ra_agent_db, nickname='ipaCert')
         except RuntimeError as e:
             root_logger.error(
                 "certmonger failed to stop tracking certificate: %s", e)
@@ -1863,5 +1851,5 @@ def update_ipa_conf():
     standard_logging_setup("install.log")
     ds = dsinstance.DsInstance()
 
-    ca = CAInstance("EXAMPLE.COM", paths.HTTPD_ALIAS_DIR)
+    ca = CAInstance("EXAMPLE.COM")
     ca.configure_instance("catest.example.com", "password", "password")
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index e65592c..23e5555 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -1540,7 +1540,7 @@ def upgrade_configuration():
         sub_dict['SUBJECT_BASE'] = subject_base
 
     ca = cainstance.CAInstance(
-            api.env.realm, paths.IPA_RADB_DIR, host_name=api.env.host)
+            api.env.realm, host_name=api.env.host)
     ca_running = ca.is_running()
 
     # create passswd.txt file in PKI_TOMCAT_ALIAS_DIR if it does not exist

From 5796207bff4bd8e6aaed85c1a4bf3571d5f3ba8d Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Tue, 14 Feb 2017 16:55:11 +0100
Subject: [PATCH 2/2] Remove DM password files after successfull pkispawn run

https://fedorahosted.org/freeipa/ticket/5695
---
 ipaserver/install/cainstance.py     | 16 +++++++++++-----
 ipaserver/install/dogtaginstance.py | 13 +++++++++----
 ipaserver/install/krainstance.py    | 14 ++++++++++----
 ipaserver/install/server/upgrade.py | 22 ++++++++++++++++++++++
 4 files changed, 52 insertions(+), 13 deletions(-)

diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 0b78139..19f4b02 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -449,7 +449,10 @@ def configure_instance(self, host_name, dm_password, admin_password,
                 self.step("configuring certmonger renewal for lightweight CAs",
                           self.__add_lightweight_ca_tracking_requests)
 
-        self.start_creation(runtime=210)
+        try:
+            self.start_creation(runtime=210)
+        finally:
+            self.clean_pkispawn_files()
 
     def __spawn_instance(self):
         """
@@ -463,6 +466,9 @@ def __spawn_instance(self):
         os.close(cfg_fd)
         pent = pwd.getpwnam(self.service_user)
         os.chown(cfg_file, pent.pw_uid, pent.pw_gid)
+        self.tmp_agent_db = tempfile.mkdtemp(
+                prefix="tmp-", dir=paths.VAR_LIB_IPA)
+        self.tmp_agent_pwd = ipautil.ipa_generate_password()
 
         # Create CA configuration
         config = ConfigParser()
@@ -482,8 +488,8 @@ def __spawn_instance(self):
                 ipautil.format_netloc(api.env.domain)))
 
         # Client security database
-        config.set("CA", "pki_client_database_dir", self.agent_db)
-        config.set("CA", "pki_client_database_password", self.admin_password)
+        config.set("CA", "pki_client_database_dir", self.tmp_agent_db)
+        config.set("CA", "pki_client_database_password", self.tmp_agent_pwd)
         config.set("CA", "pki_client_database_purge", "False")
         config.set("CA", "pki_client_pkcs12_password", self.admin_password)
 
@@ -791,7 +797,7 @@ def __request_ra_certificate(self):
         # create a temp file storing the pwd
         agent_file = tempfile.NamedTemporaryFile(
             mode="w", dir=paths.VAR_LIB_IPA, delete=False)
-        agent_file.write(self.admin_password)
+        agent_file.write(self.tmp_agent_pwd)
         agent_file.close()
 
         # create a temp pem file storing the CA chain
@@ -811,7 +817,7 @@ def __request_ra_certificate(self):
              ], stdin=data, capture_output=False)
 
         agent_args = [paths.DOGTAG_IPA_CA_RENEW_AGENT_SUBMIT,
-                      "--dbdir", self.agent_db,
+                      "--dbdir", self.tmp_agent_db,
                       "--nickname", "ipa-ca-agent",
                       "--cafile", chain_file.name,
                       "--ee-url", 'http://%s:8080/ca/ee/ca/' % self.fqdn,
diff --git a/ipaserver/install/dogtaginstance.py b/ipaserver/install/dogtaginstance.py
index 968f4b2..cbaaa25 100644
--- a/ipaserver/install/dogtaginstance.py
+++ b/ipaserver/install/dogtaginstance.py
@@ -127,7 +127,7 @@ def __init__(self, realm, subsystem, service_desc, host_name=None,
         self.admin_dn = DN(('uid', self.admin_user),
                            ('ou', 'people'), ('o', 'ipaca'))
         self.admin_groups = None
-        self.agent_db = tempfile.mkdtemp(prefix="tmp-", dir=paths.VAR_LIB_IPA)
+        self.tmp_agent_db = None
         self.subsystem = subsystem
         self.security_domain_name = "IPA"
         # replication parameters
@@ -138,9 +138,6 @@ def __init__(self, realm, subsystem, service_desc, host_name=None,
 
         self.log = log_mgr.get_logger(self)
 
-    def __del__(self):
-        shutil.rmtree(self.agent_db, ignore_errors=True)
-
     def is_installed(self):
         """
         Determine if subsystem instance has been installed.
@@ -171,6 +168,14 @@ def spawn_instance(self, cfg_file, nolog_list=()):
         except ipautil.CalledProcessError as e:
             self.handle_setup_error(e)
 
+    def clean_pkispawn_files(self):
+        if self.tmp_agent_db is not None:
+            shutil.rmtree(self.tmp_agent_db, ignore_errors=True)
+
+        shutil.rmtree('/root/.dogtag/pki-tomcat/{subsystem}/'
+                      .format(subsystem=self.subsystem.lower()),
+                      ignore_errors=True)
+
     def restart_instance(self):
         try:
             self.restart('pki-tomcat')
diff --git a/ipaserver/install/krainstance.py b/ipaserver/install/krainstance.py
index ec38801..2f56a19 100644
--- a/ipaserver/install/krainstance.py
+++ b/ipaserver/install/krainstance.py
@@ -134,7 +134,10 @@ def configure_instance(self, realm_name, host_name, dm_password,
 
             self.step("enabling KRA instance", self.__enable_instance)
 
-        self.start_creation(runtime=126)
+        try:
+            self.start_creation(runtime=126)
+        finally:
+            self.clean_pkispawn_files()
 
     def __spawn_instance(self):
         """
@@ -148,6 +151,8 @@ def __spawn_instance(self):
         os.close(cfg_fd)
         pent = pwd.getpwnam(self.service_user)
         os.chown(cfg_file, pent.pw_uid, pent.pw_gid)
+        self.tmp_agent_db = tempfile.mkdtemp(
+                prefix="tmp-", dir=paths.VAR_LIB_IPA)
 
         # Create KRA configuration
         config = ConfigParser()
@@ -170,9 +175,10 @@ def __spawn_instance(self):
         config.set("KRA", "pki_backup_password", self.admin_password)
 
         # Client security database
-        config.set("KRA", "pki_client_database_dir", self.agent_db)
-        config.set("KRA", "pki_client_database_password", self.admin_password)
-        config.set("KRA", "pki_client_database_purge", "False")
+        config.set("KRA", "pki_client_database_dir", self.tmp_agent_db)
+        config.set("KRA", "pki_client_database_password",
+                   ipautil.ipa_generate_password())
+        config.set("KRA", "pki_client_database_purge", "True")
         config.set("KRA", "pki_client_pkcs12_password", self.admin_password)
 
         # Administrator
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index 23e5555..f1c722c 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -283,6 +283,27 @@ def cleanup_adtrust(fstore):
             root_logger.debug('Removing %s from backup', backed_up_file)
 
 
+def cleanup_dogtag():
+    """
+    pkispawn leaves some mess we were not cleaning up until recently. Try
+    to clean up what we can.
+    """
+    subsystems = []
+    if api.Command.ca_is_enabled()['result']:
+        subsystems.append('CA')
+        if api.Command.kra_is_enabled()['result']:
+            subsystems.append('KRA')
+
+    for system in subsystems:
+        root_logger.debug(
+            "Cleaning up after pkispawn for the {sub} subsystem"
+            .format(sub=system))
+        instance = dogtaginstance.DogtagInstance(
+            api.env.realm, system, service_desc=None,
+        )
+        instance.clean_pkispawn_files()
+
+
 def upgrade_adtrust_config():
     """
     Upgrade 'dedicated keytab file' in smb.conf to omit FILE: prefix
@@ -1672,6 +1693,7 @@ def upgrade_configuration():
 
     cleanup_kdc(fstore)
     cleanup_adtrust(fstore)
+    cleanup_dogtag()
     upgrade_adtrust_config()
 
     bind = bindinstance.BindInstance(fstore)
-- 
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