Jan Cholasta wrote:
On 29.10.2012 20:11, Rob Crittenden wrote:
Jan Cholasta wrote:
Hi,

On 24.10.2012 21:22, Rob Crittenden wrote:
If uninstall fails in certain ways it is possible that some
certificates
could still be tracked by certmonger (even if the NSS database is now
gone). This will loop through the directories we care about and warn
the
user if there is anything left over.

I added some basic test instructions to the ticket.

rob


You should check the return value of find_request_value, it can be None
in case of error.

I would prefer if you used "os.path.join(REQUEST_DIR, file)" instead of
"'%s/%s' % (REQUEST_DIR, file)".

There is a trailing whitespace in the patch on line 75.

Honza


fixed.

rob

This will still break if find_request_value returns None:

+    for file in fileList:
+        rv = find_request_value(os.path.join(REQUEST_DIR, file),
+                                'cert_storage_location')
+        if rv:
+            rv = os.path.abspath(rv)
+        if rv.rstrip() == dir:
+            id = find_request_value(os.path.join(REQUEST_DIR, file),
'id').rstrip()
+            if id is not None:
+                reqid.append(id)

I would suggest to do it like this instead:

+    for file in fileList:
+        rv = find_request_value(os.path.join(REQUEST_DIR, file),
+                                'cert_storage_location')
+        if rv is None:
+            continue
+        rv = os.path.abspath(rv).rstrip()
+        if rv != dir:
+            continue
+        id = find_request_value(os.path.join(REQUEST_DIR, file), 'id')
+        if id is not None:
+            reqid.append(id.rstrip())

Honza


OK, great suggestion, sorry I didn't grok it sooner. Updated patch attached.

rob
>From 0837ee45b95e4ea38ccbe038b2c1973eb764b0b0 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Tue, 23 Oct 2012 16:31:37 -0400
Subject: [PATCH] After unininstall see if certmonger is still tracking any of
 our certs.

Rather than providing a list of nicknames I'm going to look at the NSS
databases directly. Anything in there is suspect and this will help
future-proof us.

certmonger may be tracking other certificates but we only care about
a subset of them, so don't complain if there are other tracked certificates.

This reads the certmonger files directly so the service doesn't need
to be started.

https://fedorahosted.org/freeipa/ticket/2702
---
 install/tools/ipa-server-install | 10 +++++++++-
 ipapython/certmonger.py          | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 6d1e6998ca4accbcf3900d5f97f889290e330c6b..70e5153d7bf88f2ae71407e90dea2de010b97b1c 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -52,6 +52,7 @@ from ipaserver.install import sysupgrade
 
 from ipaserver.install import service, installutils
 from ipapython import version
+from ipapython import certmonger
 from ipaserver.install.installutils import *
 from ipaserver.plugins.ldap2 import ldap2
 
@@ -527,7 +528,14 @@ def uninstall():
             rv = 1
 
     if has_state:
-        root_logger.warning('Some installation state has not been restored.\nThis will cause re-installation to fail.\nIt should be safe to remove /var/lib/ipa/sysrestore.state but it may\nmean your system hasn\'t be restored to its pre-installation state.')
+        root_logger.error('Some installation state has not been restored.\nThis may cause re-installation to fail.\nIt should be safe to remove /var/lib/ipa/sysrestore.state but it may\nmean your system hasn\'t be restored to its pre-installation state.')
+
+    # Note that this name will be wrong after the first uninstall.
+    dirname = dsinstance.config_dirname(dsinstance.realm_to_serverid(api.env.realm))
+    dirs = [dirname, dogtag.configured_constants().ALIAS_DIR, certs.NSS_DIR]
+    ids = certmonger.check_state(dirs)
+    if ids:
+        root_logger.error('Some certificates may still be tracked by certmonger.\nThis will cause re-installation to fail.\nStart the certmonger service and list the certificates being tracked\n # getcert list\nThese may be untracked by executing\n # getcert stop-tracking -i <request_id>\nfor each id in: %s' % ', '.join(ids))
 
     return rv
 
diff --git a/ipapython/certmonger.py b/ipapython/certmonger.py
index 9cc4466c61108a863eb76b1ff67bef559a9228d0..695aa17d9c32b75e42a6c21a6904c32565510617 100644
--- a/ipapython/certmonger.py
+++ b/ipapython/certmonger.py
@@ -114,6 +114,27 @@ def get_request_id(criteria):
 
     return reqid
 
+def get_requests_for_dir(dir):
+    """
+    Return a list containing the request ids for a given NSS database
+    directory.
+    """
+    reqid=[]
+    fileList=os.listdir(REQUEST_DIR)
+    for file in fileList:
+        rv = find_request_value(os.path.join(REQUEST_DIR, file),
+                                'cert_storage_location')
+        if rv is None:
+            continue
+        rv = os.path.abspath(rv).rstrip()
+        if rv != dir:
+            continue 
+        id = find_request_value(os.path.join(REQUEST_DIR, file), 'id').rstrip()
+        if id is not None:
+            reqid.append(id)
+
+    return reqid
+
 def add_request_value(request_id, directive, value):
     """
     Add a new directive to a certmonger request file.
@@ -393,6 +414,21 @@ def dogtag_start_tracking(ca, nickname, pin, pinfile, secdir, command):
 
     (stdout, stderr, returncode) = ipautil.run(args, nolog=[pin])
 
+def check_state(dirs):
+    """
+    Given a set of directories and nicknames verify that we are no longer
+    tracking certificates.
+
+    dirs is a list of directories to test for. We will return a tuple
+    of nicknames for any tracked certificates found.
+
+    This can only check for NSS-based certificates.
+    """
+    reqids = []
+    for dir in dirs:
+        reqids.extend(get_requests_for_dir(dir))
+
+    return reqids
 
 if __name__ == '__main__':
     request_id = request_cert("/etc/httpd/alias", "Test", "cn=tiger.example.com,O=IPA", "HTTP/tiger.example....@example.com")
-- 
1.7.11.4

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to