Martin Kosek wrote:
On Tue, 2011-11-08 at 18:33 -0500, Rob Crittenden wrote:
There are times we need to hunt through the certmonger request files
trying (such as trying to stop tracking a cert). One criteria is the
cert database and they need to match exactly. We weren't normalizing
this so something as simple as a trailing slash would cause a match to fail.

Normalize both values to address this.

rob

Looks good.

I just found few nitpicks that may be fixed before push:

1) I didn't like constructs like this one:
'%s' % os.path.abspath(secdir)

OK


Simple "os.path.abspath(secdir)" would be enough

2) I think get_request_id() function documentation should contain
recognized value types:
     None: Any type
     1: File path

Not sure what you mean. The function takes a tuple and seems fairly well documented to me.

3) We may want also to normalize path to PW file. This is what we use
now in ipa-server-install:

2011-12-07T10:11:25Z DEBUG args=/usr/bin/ipa-getcert start-tracking
-d /etc/dirsrv/slapd-PKI-IPA -n    Server-Cert
-p /etc/dirsrv/slapd-PKI-IPA//pwdfile.txt
This shoud be enough:
-        args.append(passwd_fname)
+        args.append(os.path.abspath(passwd_fname))

done

rob

>From 1b187303eaa345a80c6c8e19d2f3d81fabc48bb8 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Wed, 7 Dec 2011 15:03:00 -0500
Subject: [PATCH] Use absolute paths when trying to find certmonger request
 id.

The value stored in certmonger is not guaranteed to be normalized
nor is the value passed-in (could be a relative path and may or not
contain trailing slash). We do direct string compares so they need
to match exactly or we won't find the request.

https://fedorahosted.org/freeipa/ticket/1942
---
 ipapython/certmonger.py |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ipapython/certmonger.py b/ipapython/certmonger.py
index 1ed9076..a6418c2 100644
--- a/ipapython/certmonger.py
+++ b/ipapython/certmonger.py
@@ -157,7 +157,7 @@ def request_cert(nssdb, nickname, subject, principal, passwd_fname=None):
     ]
     if passwd_fname:
         args.append('-p')
-        args.append(passwd_fname)
+        args.append(os.path.abspath(passwd_fname))
     (stdout, stderr, returncode) = ipautil.run(args)
     # FIXME: should be some error handling around this
     m = re.match('New signing request "(\d+)" added', stdout)
@@ -200,7 +200,7 @@ def start_tracking(nickname, secdir, password_file=None):
             "-n", nickname]
     if password_file:
         args.append("-p")
-        args.append(password_file)
+        args.append(os.path.abspath(password_file))
 
     (stdout, stderr, returncode) = ipautil.run(args)
 
@@ -216,7 +216,7 @@ def stop_tracking(secdir, request_id=None, nickname=None):
         raise RuntimeError('Both request_id and nickname are missing.')
     if nickname:
         # Using the nickname find the certmonger request_id
-        criteria = (('cert_storage_location','%s' % secdir),('cert_nickname', '%s' % nickname))
+        criteria = (('cert_storage_location', secdir),('cert_nickname', nickname))
         try:
             request_id = get_request_id(criteria)
             if request_id is None:
-- 
1.7.6

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

Reply via email to