On 09/03/2014 04:05 PM, Jan Cholasta wrote:
Dne 3.9.2014 v 12:37 David Kupka napsal(a):
On 09/02/2014 01:56 PM, Jan Cholasta wrote:
Dne 29.8.2014 v 14:34 David Kupka napsal(a):
Hope, I've addressed all the issues (except 9 and 11, inline). Let's go
for another round :-)

On 08/27/2014 11:05 AM, Jan Cholasta wrote:
Hi,

Dne 25.8.2014 v 15:39 David Kupka napsal(a):
On 08/19/2014 05:44 PM, Rob Crittenden wrote:
David Kupka wrote:
On 08/19/2014 09:58 AM, Martin Kosek wrote:
On 08/19/2014 09:05 AM, David Kupka wrote:
FreeIPA will use certmonger D-Bus API as discussed in this thread
https://www.redhat.com/archives/freeipa-devel/2014-July/msg00304.html





This change should prevent hard-to-reproduce bugs like
https://fedorahosted.org/freeipa/ticket/4280

Thanks for this effort, the updated certmonger module looks much
better! This
will help us get rid of the non-standard communication with
certmonger.

Just couple initial comments from me by reading the code:

1) Testing needs fixed version of certmonger, right? This needs
to be
spelled
out right with the patch.
Yes, certmonger 0.75.13 and above should be fine according ticket
https://fedorahosted.org/certmonger/ticket/36. Added to patch
description.

You should update the spec to set the minimum version as well.
Sure, thanks.


2) Description text in patches is cheap, do not be afraid to
use it
and
describe what you did and why. Link to the ticket is missing in
the
description
as well:
Ok, increased verbosity a bit :-)

Subject: [PATCH] Use certmonger D-Bus API instead of messing with
its
files.

---

3) get_request_id API:

           criteria = (
-            ('cert_storage_location',
dogtag_constants.ALIAS_DIR,
-             certmonger.NPATH),
-            ('cert_nickname', nickname, None),
+            ('cert_storage_location',
dogtag_constants.ALIAS_DIR),
+            ('cert_nickname', nickname),
           )
           request_id = certmonger.get_request_id(criteria)

Do we want to continue using the "criteria" object or should we
rather
switch
to normal function options? I.e. rather using

request_id = certmonger.get_request_id(cert_nickname=nickname,
cert_storage_location=dogtag_constants.ALIAS_DIR)

? It would look more consistent with other calls. I am just
asking,
not insisting.
I've no preference here. It seems to be a very small change. Has
anyone
a reason to do it one way and not the other?

I think I used this criteria thing to avoid having a bazillion
optional
parameters and for future-proofing. I think at this point the
list is
probably pretty stable, so I'd base it on whether you care about
having
a whole ton of optional parameters or not (it has the advantage of
self-documenting itself).

The list is probably stable but also really excessive. I don't
think it
would help to have more than dozen optional parameters. So I
prefer to
leave as-is and change it in future if it is wanted.

3) Starting function:

+    try:
+        ipautil.run([paths.SYSTEMCTL, 'start', 'certmonger'],
skip_output=True)
+    except Exception, e:
+        root_logger.error('Failed to start certmonger: %s' % e)
+        raise e

I see 2 issues related to this code:
a) Do not call SYSTEMCTL directly. To be platform independent,
rather use
services.knownservices.messagebus.start() that is overridable by
someone else
porting to non-systemd platforms.
Is there anything that can't be done using
ipalib/ipapython/ipaplatform?

It can't make coffee (yet).

b) In this case, do not use "raise e", but just "raise" to keep
the
exception
stack trace intact for better debugging.
Every day there's something new to learn about python or FreeIPA.

Both a) and b) should be fixed in other occasions and places.
I found only one occurence of a) issue. Is there some hidden or are
you
talking about the whole FreeIPA project?

4) Feel free to add yourself to Authors section of this module.
You
refactored
it greatly to earn it :-)
Done.

You already import dbus, why also separately import DBusException?

Removed, thanks for noticing.
rob


1) The patch needs to be rebased.

I didn't notice the patch is targeted for 4.0. Can you please provide
patches for both ipa-4-0 and ipa-4-1/master?


Attached, 0008-5 works on master/ipa-4-1 and 0008-5-ipa40 works on
ipa-4-0.

There is a little bug in ipa-upgradeconfig in the 4.0 version of the
patch. This is wrong:

     for request in requests:
         nss_dir, nickname, ca_name, pre_command, post_command, profile
= request
         criteria = {
             'cert-database': nss_dir,
             'cert-nickname': nickname,
             'ca-name': ca_name,
             'template-profile': profile,
         }

It should be:

      for nss_dir, nickname, ca_name, pre_command, post_command in
requests:
         criteria = {
             'cert-database': nss_dir,
             'cert-nickname': nickname,
             'ca-name': ca_name,
         }




2) Please try to follow PEP8, at least in certmonger.py.


3) In certificate_renewal_update() in ipa-upgradeconfig you removed
ca_name from criteria.


4) IMO renaming nickname to cert_nickname in dogtag_start_tracking()
and
stop_tracking() is unnecessary. We can keep calling request nicknames
"request_id" and certificate nicknames "nickname" in our APIs.


5) I think it would be better to add a docstring to
_cm_dbus_object.__init__() instead of doing this:

     # object is accesible over this DBus bus instance
     bus = None
     # DBus object path
     path = None
     # the actual DBus object
     obj = None
     # object interface name
     obj_dbus_if = None
     # object parent interface name
     parent_dbus_if = None
     # object inteface
     obj_if = None
     # property interface
     prop_if = None

You removed the comments, but left the attributes there. You should
remove them as well, they are not necessary, as you set them all in
__init__().


Removed.



6) In _start_certmonger(), please check if certmonger is already
running
before starting it, what applies to systemd might not apply to other
init systems.


7) Do we ever need to connect to certmonger on the session bus? If
not,
there is no need to support it in _connect_to_certmonger().


8) You should not ignore other criteria when 'nickname' is
specified in
_get_requests(). Use find_request_by_nickname only if 'nickname' is
the
only criterion, otherwise filter the result of get_requests, including
'nickname'.


9) IMO you can indeed remove request_cert().

Not sure, it is used in self-test although I doubt anyone ever ran it.



10) You forgot to port modify() and resubmit().

You no longer check if the profile argument is set in modify() - either
re-introduce the check, or remove the default value of the argument to
make it mandatory.



11) As for get_pin(), it should be moved to ipapython.dogtag,
because it
is Dogtag related (you don't need to do it in this patch).


This patch is ugly enough. I will create a separate one for this.

OK, no need to include the TODO comment though.



I haven't done functional testing yet, expect more comments later.

Honza



12) ipa-client-install still uses ipa-getcert instead of certmonger API
to request the host certificate, but since we plan on stopping doing
that (https://fedorahosted.org/freeipa/ticket/4449), I guess you don't
have to do anything about it.

Ok. I will leave it on you since you are owner of this ticket.



13) You require dict for criteria in get_request_id, but you pass tuples
to it in ipa-upgradeconfig, ipa-cacert-manage and ipa-certupdate, which
makes them fail.

Good point, fixed.

I forgot about ipaserver.install.plugins.ca_renewal_master (sorry), it
should be fixed as well.

I need to check my patches more carefully, thank for the patience.

--
David Kupka
From d5ca6b6243706ec3b75bbb368ad0e9e2032685c1 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Wed, 3 Sep 2014 09:07:16 +0200
Subject: [PATCH] Use certmonger D-Bus API instead of messing with its files.

FreeIPA certmonger module changed to use D-Bus to communicate with certmonger.
Using the D-Bus API should be more stable and supported way of using cermonger than
tampering with its files.

>=certmonger-0.75.13 is needed for this to work.

https://fedorahosted.org/freeipa/ticket/4280
---
 freeipa.spec.in                                |   2 +-
 install/tools/ipa-upgradeconfig                |  17 +-
 ipapython/certmonger.py                        | 500 ++++++++++++-------------
 ipaserver/install/certs.py                     |  26 +-
 ipaserver/install/plugins/ca_renewal_master.py |   8 +-
 5 files changed, 266 insertions(+), 287 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 5b9fa8e40855c929930418ed6f5360086bd0c0c3..0e43ee755e5ada38b8c9260d133d3d8434591470 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -123,7 +123,7 @@ Requires: python-dns
 Requires: zip
 Requires: policycoreutils >= %{POLICYCOREUTILSVER}
 Requires: tar
-Requires(pre): certmonger >= 0.65
+Requires(pre): certmonger >= 0.75.13
 Requires(pre): 389-ds-base >= 1.3.2.20
 Requires: fontawesome-fonts
 Requires: open-sans-fonts
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index 54193e9e6f6c9e8e0ca56336ea86cee673893638..9d488754b0fb30fa541955f71b3d69b3fd5e266d 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -677,24 +677,25 @@ def certificate_renewal_update(ca):
         return False
 
     # State not set, lets see if we are already configured
-    for nss_dir, nickname, ca_name, pre_command, post_command in requests:
-        criteria = (
-            ('cert_storage_location', nss_dir, certmonger.NPATH),
-            ('cert_nickname', nickname, None),
-            ('ca_name', ca_name, None),
-        )
+    for request in requests:
+        nss_dir, nickname, ca_name, pre_command, post_command = request
+        criteria = {
+            'cert-database': nss_dir,
+            'cert-nickname': nickname,
+            'ca-name': ca_name,
+        }
         request_id = certmonger.get_request_id(criteria)
         if request_id is None:
             break
 
-        val = certmonger.get_request_value(request_id, 'pre_certsave_command')
+        val = certmonger.get_request_value(request_id, 'cert-presave-command')
         if val is not None:
             val = val.split(' ', 1)[0]
             val = os.path.basename(val)
         if pre_command != val:
             break
 
-        val = certmonger.get_request_value(request_id, 'post_certsave_command')
+        val = certmonger.get_request_value(request_id, 'post-certsave-command')
         if val is not None:
             val = val.split(' ', 1)[0]
             val = os.path.basename(val)
diff --git a/ipapython/certmonger.py b/ipapython/certmonger.py
index 0099d239d238c87330feabfeb71f372b299f9283..05438c5fa95573e8c100622b474483aa3e8c192a 100644
--- a/ipapython/certmonger.py
+++ b/ipapython/certmonger.py
@@ -1,4 +1,5 @@
 # Authors: Rob Crittenden <rcrit...@redhat.com>
+#          David Kupka <dku...@redhat.com>
 #
 # Copyright (C) 2010  Red Hat
 # see file 'COPYING' for use and warranty information
@@ -23,136 +24,193 @@
 
 import os
 import sys
-import re
 import time
+import dbus
 from ipapython import ipautil
 from ipapython import dogtag
 from ipaplatform.paths import paths
+from ipaplatform import services
+from ipapython.ipa_log_manager import root_logger
 
-REQUEST_DIR=paths.CERTMONGER_REQUESTS_DIR
-CA_DIR=paths.CERTMONGER_CAS_DIR
-
-# Normalizer types for critera in get_request_id()
-NPATH = 1
-
-def find_request_value(filename, directive):
-    """
-    Return a value from a certmonger request file for the requested directive
-
-    It tries to do this a number of times because sometimes there is a delay
-    when ipa-getcert returns and the file is fully updated, particularly
-    when doing a request. Generating a CSR is fast but not instantaneous.
-    """
-    tries = 1
-    value = None
-    found = False
-    while value is None and tries <= 5:
-        tries=tries + 1
-        time.sleep(1)
-        fp = open(filename, 'r')
-        lines = fp.readlines()
-        fp.close()
-
-        for line in lines:
-            if found:
-                # A value can span multiple lines. If it does then it has a
-                # leading space.
-                if not line.startswith(' '):
-                    # We hit the next directive, return now
-                    return value
-                else:
-                    value = value + line[1:]
+REQUEST_DIR = paths.CERTMONGER_REQUESTS_DIR
+CA_DIR = paths.CERTMONGER_CAS_DIR
+
+DBUS_CM_PATH = '/org/fedorahosted/certmonger'
+DBUS_CM_IF = 'org.fedorahosted.certmonger'
+DBUS_CM_REQUEST_IF = 'org.fedorahosted.certmonger.request'
+DBUS_CM_CA_IF = 'org.fedorahosted.certmonger.ca'
+DBUS_PROPERTY_IF = 'org.freedesktop.DBus.Properties'
+
+
+class _cm_dbus_object(object):
+    """
+    Auxiliary class for convenient DBus object handling.
+    """
+    def __init__(self, bus, object_path, object_dbus_interface,
+                 parent_dbus_interface=None, property_interface=False):
+        """
+        bus - DBus bus object, result of dbus.SystemBus() or dbus.SessionBus()
+              Object is accesible over this DBus bus instance.
+        object_path - path to requested object on DBus bus
+        object_dbus_interface
+        parent_dbus_interface
+        property_interface - create DBus property interface? True or False
+        """
+        if bus is None or object_path is None or object_dbus_interface is None:
+            raise RuntimeError(
+                "bus, object_path and dbus_interface must not be None.")
+        if parent_dbus_interface is None:
+            parent_dbus_interface = object_dbus_interface
+        self.bus = bus
+        self.path = object_path
+        self.obj_dbus_if = object_dbus_interface
+        self.parent_dbus_if = parent_dbus_interface
+        self.obj = bus.get_object(parent_dbus_interface, object_path)
+        self.obj_if = dbus.Interface(self.obj, object_dbus_interface)
+        if property_interface:
+            self.prop_if = dbus.Interface(self.obj, DBUS_PROPERTY_IF)
+
+
+def _start_certmonger():
+    """
+    Start certmonger daemon. If it's already running systemctl just ignores
+    the command.
+    """
+    if not services.knownservices.certmonger.is_running():
+        try:
+            services.knownservices.certmonger.start()
+        except Exception, e:
+            root_logger.error('Failed to start certmonger: %s' % e)
+            raise
+
+
+def _connect_to_certmonger():
+    """
+    Start certmonger daemon and connect to it via DBus.
+    """
+    try:
+        _start_certmonger()
+    except (KeyboardInterrupt, OSError), e:
+        root_logger.error('Failed to start certmonger: %s' % e)
+        raise
+
+    try:
+        bus = dbus.SystemBus()
+        cm = _cm_dbus_object(bus, DBUS_CM_PATH, DBUS_CM_IF)
+    except dbus.DBusException, e:
+        root_logger.error("Failed to access certmonger over DBus: %s", e)
+        raise
+    return cm
+
+
+def _get_requests(criteria=dict()):
+    """
+    Get all requests that matches the provided criteria.
+    """
+    if not isinstance(criteria, dict):
+        raise TypeError('"criteria" must be dict.')
+
+    cm = _connect_to_certmonger()
+    requests = []
+    requests_paths = []
+    if 'nickname' in criteria:
+        request_path = cm.obj_if.find_request_by_nickname(criteria['nickname'])
+        if request_path:
+            requests_paths = [request_path]
+    else:
+        requests_paths = cm.obj_if.get_requests()
+
+    for request_path in requests_paths:
+        request = _cm_dbus_object(cm.bus, request_path, DBUS_CM_REQUEST_IF,
+                                  DBUS_CM_IF, True)
+        for criterion in criteria:
+            if criterion == 'ca-name':
+                ca_path = request.obj_if.get_ca()
+                ca = _cm_dbus_object(cm.bus, ca_path, DBUS_CM_CA_IF,
+                                     DBUS_CM_IF)
+                value = ca.obj_if.get_nickname()
             else:
-                if line.startswith(directive + '='):
-                    found = True
-                    value = line[len(directive)+1:]
+                value = request.prop_if.Get(DBUS_CM_REQUEST_IF, criterion)
+            if value != criteria[criterion]:
+                break
+        else:
+            requests.append(request)
+    return requests
+
+
+def _get_request(criteria):
+    """
+    Find request that matches criteria.
+    If 'nickname' is specified other criteria are ignored because 'nickname'
+    uniquely identify single request.
+    When multiple or none request matches specified criteria RuntimeError is
+    raised.
+    """
+    requests = _get_requests(criteria)
+    if len(requests) != 1:
+        raise RuntimeError("Criteria expected to be met by 1 request, got %s."
+                           % len(requests))
+    else:
+        return requests[0]
 
-    return value
 
 def get_request_value(request_id, directive):
     """
-    There is no guarantee that the request_id will match the filename
-    in the certmonger requests directory, so open each one to find the
-    request_id.
+    Get property of request.
     """
-    fileList=os.listdir(REQUEST_DIR)
-    for file in fileList:
-        value = find_request_value('%s/%s' % (REQUEST_DIR, file), 'id')
-        if value is not None and value.rstrip() == request_id:
-            return find_request_value('%s/%s' % (REQUEST_DIR, file), directive)
+    try:
+        request = _get_request(dict(nickname=request_id))
+    except RuntimeError, e:
+        root_logger.error('Failed to get request: %s' % e)
+        raise
+    return request.prop_if.Get(DBUS_CM_REQUEST_IF, directive)
 
-    return None
 
 def get_request_id(criteria):
     """
     If you don't know the certmonger request_id then try to find it by looking
-    through all the request files. An alternative would be to parse the
-    ipa-getcert list output but this seems cleaner.
+    through all the requests.
 
-    criteria is a tuple of key/value/type to search for. The more specific
+    criteria is a tuple of key/value to search for. The more specific
     the better. An error is raised if multiple request_ids are returned for
     the same criteria.
 
     None is returned if none of the criteria match.
     """
-    assert type(criteria) is tuple
+    try:
+        request = _get_request(criteria)
+    except RuntimeError, e:
+        root_logger.error('Failed to get request: %s' % e)
+        raise
+    return request.prop_if.Get(DBUS_CM_REQUEST_IF, 'nickname')
 
-    reqid=None
-    fileList=os.listdir(REQUEST_DIR)
-    for file in fileList:
-        match = True
-        for (key, value, valtype) in criteria:
-            rv = find_request_value('%s/%s' % (REQUEST_DIR, file), key)
-            if rv and valtype == NPATH:
-                rv = os.path.abspath(rv)
-            if rv is None or rv.rstrip() != value:
-                match = False
-                break
-        if match and reqid is not None:
-            raise RuntimeError('multiple certmonger requests match the criteria')
-        if match:
-            reqid = find_request_value('%s/%s' % (REQUEST_DIR, file), 'id').rstrip()
-
-    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')
-        if id is not None:
-            reqid.append(id.rstrip())
+    reqid = []
+    criteria = {'cert-storage': 'NSSDB', 'key-storage': 'NSSDB',
+                'cert-database': dir, 'key-database': dir, }
+    requests = _get_requests(criteria)
+    for request in requests:
+        reqid.append(request.prop_if.Get(DBUS_CM_REQUEST_IF, 'nickname'))
 
     return reqid
 
+
 def add_request_value(request_id, directive, value):
     """
     Add a new directive to a certmonger request file.
-
-    The certmonger service MUST be stopped in order for this to work.
     """
-    fileList=os.listdir(REQUEST_DIR)
-    for file in fileList:
-        id = find_request_value('%s/%s' % (REQUEST_DIR, file), 'id')
-        if id is not None and id.rstrip() == request_id:
-            current_value = find_request_value('%s/%s' % (REQUEST_DIR, file), directive)
-            if not current_value:
-                fp = open('%s/%s' % (REQUEST_DIR, file), 'a')
-                fp.write('%s=%s\n' % (directive, value))
-                fp.close()
+    try:
+        request = _get_request({'nickname': request_id})
+    except RuntimeError, e:
+        root_logger.error('Failed to get request: %s' % e)
+        raise
+    request.obj_if.modify({directive: value})
 
-    return
 
 def add_principal(request_id, principal):
     """
@@ -161,7 +219,8 @@ def add_principal(request_id, principal):
     When an existing certificate is added via start-tracking it won't have
     a principal.
     """
-    return add_request_value(request_id, 'template_principal', principal)
+    add_request_value(request_id, 'template-principal', [principal])
+
 
 def add_subject(request_id, subject):
     """
@@ -171,47 +230,30 @@ def add_subject(request_id, subject):
     When an existing certificate is added via start-tracking it won't have
     a subject_template set.
     """
-    return add_request_value(request_id, 'template_subject', subject)
+    add_request_value(request_id, 'template-subject', subject)
 
+
+# TODO: Remove? Used only internaly if ever.
 def request_cert(nssdb, nickname, subject, principal, passwd_fname=None):
     """
-    Execute certmonger to request a server certificate
+    Execute certmonger to request a server certificate.
     """
-    args = [paths.IPA_GETCERT,
-            'request',
-            '-d', nssdb,
-            '-n', nickname,
-            '-N', subject,
-            '-K', principal,
-    ]
+    cm = _connect_to_certmonger()
+    request_parameters = dict(KEY_STORAGE='NSSDB', CERT_STORAGE='NSSDB',
+                              CERT_LOCATION=nssdb, CERT_NICKNAME=nickname,
+                              SUBJECT=subject, PRINCIPAL=principal,)
     if passwd_fname:
-        args.append('-p')
-        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)
-    request_id = m.group(1)
-    return request_id
+        request_parameters['KEY_PIN_FILE'] = passwd_fname
+    result = cm.obj_if.add_request(request_parameters)
+    try:
+        if result[0]:
+            request = _cm_dbus_object(cm.bus, result[1], DBUS_CM_REQUEST_IF,
+                                      DBUS_CM_IF, True)
+    except TypeError:
+        root_logger.error('Failed to get create new request.')
+        raise
+    return request.obj_if.get_nickname()
 
-def cert_exists(nickname, secdir):
-    """
-    See if a nickname exists in an NSS database.
-
-    Returns True/False
-
-    This isn't very sophisticated in that it doesn't differentiate between
-    a database that doesn't exist and a nickname that doesn't exist within
-    the database.
-    """
-    args = [paths.CERTUTIL, "-L",
-           "-d", os.path.abspath(secdir),
-           "-n", nickname
-          ]
-    (stdout, stderr, rc) = ipautil.run(args, raiseonerr=False)
-    if rc == 0:
-        return True
-    else:
-        return False
 
 def start_tracking(nickname, secdir, password_file=None, command=None):
     """
@@ -222,61 +264,53 @@ def start_tracking(nickname, secdir, password_file=None, command=None):
     certmonger to run when it renews a certificate. This command must
     reside in /usr/lib/ipa/certmonger to work with SELinux.
 
-    Returns the stdout, stderr and returncode from running ipa-getcert
-
-    This assumes that certmonger is already running.
+    Returns True or False
     """
-    if not cert_exists(nickname, os.path.abspath(secdir)):
-        raise RuntimeError('Nickname "%s" doesn\'t exist in NSS database "%s"' % (nickname, secdir))
-    args = [paths.IPA_GETCERT, "start-tracking",
-            "-d", os.path.abspath(secdir),
-            "-n", nickname]
-    if password_file:
-        args.append("-p")
-        args.append(os.path.abspath(password_file))
+    cm = _connect_to_certmonger()
+    params = {'TRACK': True}
+    params['cert-nickname'] = nickname
+    params['cert-database'] = os.path.abspath(secdir)
+    params['cert-storage'] = 'NSSDB'
+    params['key-nickname'] = nickname
+    params['key-database'] = os.path.abspath(secdir)
+    params['key-storage'] = 'NSSDB'
     if command:
-        args.append("-C")
-        args.append(command)
+        params['cert-postsave-command'] = command
+    if password_file:
+        params['KEY_PIN_FILE'] = os.path.abspath(password_file)
+    result = cm.obj_if.add_request(params)
+    try:
+        if result[0]:
+            request = _cm_dbus_object(cm.bus, result[1], DBUS_CM_REQUEST_IF,
+                                      DBUS_CM_IF, True)
+    except TypeError, e:
+        root_logger.error('Failed to add new request.')
+        raise
+    return request.prop_if.Get(DBUS_CM_REQUEST_IF, 'nickname')
 
-    (stdout, stderr, returncode) = ipautil.run(args)
-
-    return (stdout, stderr, returncode)
 
 def stop_tracking(secdir, request_id=None, nickname=None):
     """
     Stop tracking the current request using either the request_id or nickname.
 
-    This assumes that the certmonger service is running.
+    Returns True or False
     """
     if request_id is None and nickname is None:
         raise RuntimeError('Both request_id and nickname are missing.')
-    if nickname:
-        # Using the nickname find the certmonger request_id
-        criteria = (('cert_storage_location', os.path.abspath(secdir), NPATH),('cert_nickname', nickname, None))
-        try:
-            request_id = get_request_id(criteria)
-            if request_id is None:
-                return ('', '', 0)
-        except RuntimeError:
-            # This means that multiple requests matched, skip it for now
-            # Fall back to trying to stop tracking using nickname
-            pass
 
-    args = [paths.GETCERT,
-            'stop-tracking',
-    ]
+    criteria = {'cert-database': secdir}
     if request_id:
-        args.append('-i')
-        args.append(request_id)
-    else:
-        args.append('-n')
-        args.append(nickname)
-        args.append('-d')
-        args.append(os.path.abspath(secdir))
+        criteria['nickname'] = request_id
+    if nickname:
+        criteria['cert-nickname'] = nickname
+    try:
+        request = _get_request(criteria)
+    except RuntimeError, e:
+        root_logger.error('Failed to get request: %s' % e)
+        raise
+    cm = _connect_to_certmonger()
+    cm.obj_if.remove_request(request.path)
 
-    (stdout, stderr, returncode) = ipautil.run(args)
-
-    return (stdout, stderr, returncode)
 
 def _find_IPA_ca():
     """
@@ -286,13 +320,10 @@ def _find_IPA_ca():
     We can use find_request_value because the ca files have the
     same file format.
     """
-    fileList=os.listdir(CA_DIR)
-    for file in fileList:
-        value = find_request_value('%s/%s' % (CA_DIR, file), 'id')
-        if value is not None and value.strip() == 'IPA':
-            return '%s/%s' % (CA_DIR, file)
+    cm = _connect_to_certmonger()
+    ca_path = cm.obj_if.find_ca_by_nickname('IPA')
+    return _cm_dbus_object(cm.bus, ca_path, DBUS_CM_CA_IF, DBUS_CM_IF, True)
 
-    return None
 
 def add_principal_to_cas(principal):
     """
@@ -302,58 +333,27 @@ def add_principal_to_cas(principal):
     /usr/libexec/certmonger/ipa-submit.
 
     We also need to restore this on uninstall.
-
-    The certmonger service MUST be stopped in order for this to work.
     """
-    cafile = _find_IPA_ca()
-    if cafile is None:
-        return
+    ca = _find_IPA_ca()
+    if ca:
+        ext_helper = ca.prop_if.Get(DBUS_CM_CA_IF, 'external-helper')
+        if ext_helper and ext_helper.find('-k') == -1:
+            ext_helper = '%s -k %s' % (ext_helper.strip(), principal)
+            ca.prop_if.Set(DBUS_CM_CA_IF, 'external-helper', ext_helper)
 
-    update = False
-    fp = open(cafile, 'r')
-    lines = fp.readlines()
-    fp.close()
-
-    for i in xrange(len(lines)):
-        if lines[i].startswith('ca_external_helper') and \
-            lines[i].find('-k') == -1:
-            lines[i] = '%s -k %s\n' % (lines[i].strip(), principal)
-            update = True
-
-    if update:
-        fp = open(cafile, 'w')
-        for line in lines:
-            fp.write(line)
-        fp.close()
 
 def remove_principal_from_cas():
     """
     Remove any -k principal options from the ipa_submit helper.
-
-    The certmonger service MUST be stopped in order for this to work.
     """
-    cafile = _find_IPA_ca()
-    if cafile is None:
-        return
+    ca = _find_IPA_ca()
+    if ca:
+        ext_helper = ca.prop_if.Get(DBUS_CM_CA_IF, 'external-helper')
+        if ext_helper and ext_helper.find('-k'):
+            ext_helper = ext_helper.strip()[0]
+            ca.prop_if.Set(DBUS_CM_CA_IF, 'external-helper', ext_helper)
 
-    update = False
-    fp = open(cafile, 'r')
-    lines = fp.readlines()
-    fp.close()
 
-    for i in xrange(len(lines)):
-        if lines[i].startswith('ca_external_helper') and \
-            lines[i].find('-k') > 0:
-            lines[i] = lines[i].strip().split(' ')[0] + '\n'
-            update = True
-
-    if update:
-        fp = open(cafile, 'w')
-        for line in lines:
-            fp.write(line)
-        fp.close()
-
-# Routines specific to renewing dogtag CA certificates
 def get_pin(token, dogtag_constants=None):
     """
     Dogtag stores its NSS pin in a file formatted as token:PIN.
@@ -369,6 +369,7 @@ def get_pin(token, dogtag_constants=None):
                 return pin.strip()
     return None
 
+
 def dogtag_start_tracking(ca, nickname, pin, pinfile, secdir, pre_command,
                           post_command, profile=None):
     """
@@ -383,52 +384,46 @@ def dogtag_start_tracking(ca, nickname, pin, pinfile, secdir, pre_command,
     post_command is the script to execute after a renewal is done.
 
     Both commands can be None.
-
-    Returns the stdout, stderr and returncode from running ipa-getcert
-
-    This assumes that certmonger is already running.
     """
-    if not cert_exists(nickname, os.path.abspath(secdir)):
-        raise RuntimeError('Nickname "%s" doesn\'t exist in NSS database "%s"' % (nickname, secdir))
 
-    args = [paths.GETCERT, "start-tracking",
-            "-d", os.path.abspath(secdir),
-            "-n", nickname,
-            "-c", ca,
-           ]
+    cm = _connect_to_certmonger()
+    certmonger_cmd_template = paths.CERTMONGER_COMMAND_TEMPLATE
 
-    if pre_command is not None:
+    params = {'TRACK': True}
+    params['cert-nickname'] = nickname
+    params['cert-database'] = os.path.abspath(secdir)
+    params['cert-storage'] = 'NSSDB'
+    params['key-nickname'] = nickname
+    params['key-database'] = os.path.abspath(secdir)
+    params['key-storage'] = 'NSSDB'
+    ca_path = cm.obj_if.find_ca_by_nickname(ca)
+    if ca_path:
+        params['ca'] = ca_path
+    if pin:
+        params['KEY_PIN'] = pin
+    if pinfile:
+        params['KEY_PIN_FILE'] = os.path.abspath(pinfile)
+    if pre_command:
         if not os.path.isabs(pre_command):
             if sys.maxsize > 2**32L:
                 libpath = 'lib64'
             else:
                 libpath = 'lib'
-            pre_command = paths.CERTMONGER_COMMAND_TEMPLATE % (libpath, pre_command)
-        args.append("-B")
-        args.append(pre_command)
-
-    if post_command is not None:
+            pre_command = certmonger_cmd_template % (libpath, pre_command)
+        params['cert-presave-command'] = pre_command
+    if post_command:
         if not os.path.isabs(post_command):
             if sys.maxsize > 2**32L:
                 libpath = 'lib64'
             else:
                 libpath = 'lib'
-            post_command = paths.CERTMONGER_COMMAND_TEMPLATE % (libpath, post_command)
-        args.append("-C")
-        args.append(post_command)
-
-    if pinfile:
-        args.append("-p")
-        args.append(pinfile)
-    else:
-        args.append("-P")
-        args.append(pin)
-
+            post_command = certmonger_cmd_template % (libpath, post_command)
+        params['cert-postsave-command'] = post_command
     if profile:
-        args.append("-T")
-        args.append(profile)
+        params['ca-profile'] = profile
+
+    cm.obj_if.add_request(params)
 
-    (stdout, stderr, returncode) = ipautil.run(args, nolog=[pin])
 
 def check_state(dirs):
     """
@@ -446,8 +441,11 @@ def check_state(dirs):
 
     return reqids
 
+
 if __name__ == '__main__':
-    request_id = request_cert(paths.HTTPD_ALIAS_DIR, "Test", "cn=tiger.example.com,O=IPA", "HTTP/tiger.example....@example.com")
+    request_id = request_cert(paths.HTTPD_ALIAS_DIR, "Test",
+                              "cn=tiger.example.com,O=IPA",
+                              "HTTP/tiger.example....@example.com")
     csr = get_request_value(request_id, 'csr')
     print csr
     stop_tracking(request_id)
diff --git a/ipaserver/install/certs.py b/ipaserver/install/certs.py
index 6e01efb9c790f6eb8374b64dcc751ebf66c48e74..53d04723fb5c1face323f56ff08dc58eb1ac747b 100644
--- a/ipaserver/install/certs.py
+++ b/ipaserver/install/certs.py
@@ -510,46 +510,26 @@ class CertDB(object):
             else:
                 libpath = 'lib'
             command = paths.CERTMONGER_COMMAND_TEMPLATE % (libpath, command)
-        cmonger = services.knownservices.certmonger
-        cmonger.enable()
-        services.knownservices.messagebus.start()
-        cmonger.start()
         try:
-            (stdout, stderr, rc) = certmonger.start_tracking(nickname, self.secdir, password_file, command)
-        except (ipautil.CalledProcessError, RuntimeError), e:
+            request_id = certmonger.start_tracking(nickname, self.secdir, password_file, command)
+        except RuntimeError, e:
             root_logger.error("certmonger failed starting to track certificate: %s" % str(e))
             return
 
-        cmonger.stop()
         cert = self.get_cert_from_db(nickname)
         nsscert = x509.load_certificate(cert, dbdir=self.secdir)
         subject = str(nsscert.subject)
-        m = re.match('New tracking request "(\d+)" added', stdout)
-        if not m:
-            root_logger.error('Didn\'t get new %s request, got %s' % (cmonger.service_name, stdout))
-            raise RuntimeError('%s did not issue new tracking request for \'%s\' in \'%s\'. Use \'ipa-getcert list\' to list existing certificates.' % (cmonger.service_name, nickname, self.secdir))
-        request_id = m.group(1)
-
         certmonger.add_principal(request_id, principal)
         certmonger.add_subject(request_id, subject)
 
-        cmonger.start()
-
     def untrack_server_cert(self, nickname):
         """
         Tell certmonger to stop tracking the given certificate nickname.
         """
-
-        # Always start certmonger. We can't untrack something if it isn't
-        # running
-        cmonger = services.knownservices.certmonger
-        services.knownservices.messagebus.start()
-        cmonger.start()
         try:
             certmonger.stop_tracking(self.secdir, nickname=nickname)
-        except (ipautil.CalledProcessError, RuntimeError), e:
+        except RuntimeError, e:
             root_logger.error("certmonger failed to stop tracking certificate: %s" % str(e))
-        cmonger.stop()
 
     def create_server_cert(self, nickname, hostname, other_certdb=None, subject=None):
         """
diff --git a/ipaserver/install/plugins/ca_renewal_master.py b/ipaserver/install/plugins/ca_renewal_master.py
index 37b5487fe2fcdaa98397d4ac67d7934434d3e905..21d20614bb2eafe95f4aaec4996bac51e36b229d 100644
--- a/ipaserver/install/plugins/ca_renewal_master.py
+++ b/ipaserver/install/plugins/ca_renewal_master.py
@@ -52,10 +52,10 @@ class update_ca_renewal_master(PostUpdate):
             self.debug("found CA renewal master %s", entries[0].dn[1].value)
             return (False, False, [])
 
-        criteria = (
-            ('cert_storage_location', paths.HTTPD_ALIAS_DIR, certmonger.NPATH),
-            ('cert_nickname', 'ipaCert', None),
-        )
+        criteria = {
+            'cert-storage': paths.HTTPD_ALIAS_DIR,
+            'cert-nickname': 'ipaCert',
+        }
         request_id = certmonger.get_request_id(criteria)
         if request_id is not None:
             self.debug("found certmonger request for ipaCert")
-- 
1.9.3

From be7510c5abb5557bb296820fd4a948bbba8cd8d4 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Wed, 3 Sep 2014 09:07:16 +0200
Subject: [PATCH] Use certmonger D-Bus API instead of messing with its files.

FreeIPA certmonger module changed to use D-Bus to communicate with certmonger.
Using the D-Bus API should be more stable and supported way of using cermonger than
tampering with its files.

>=certmonger-0.75.13 is needed for this to work.

https://fedorahosted.org/freeipa/ticket/4280
---
 freeipa.spec.in                                |   2 +-
 install/tools/ipa-upgradeconfig                |  16 +-
 ipa-client/ipaclient/ipa_certupdate.py         |   9 +-
 ipapython/certmonger.py                        | 514 ++++++++++++-------------
 ipaserver/install/certs.py                     |  26 +-
 ipaserver/install/ipa_cacert_manage.py         |   4 +-
 ipaserver/install/plugins/ca_renewal_master.py |   8 +-
 7 files changed, 277 insertions(+), 302 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 24771ac8eea0390d3cc3db201ca9bc986e48dc53..1b9f3b5ac5798b147defb542424bbfd560cf75b2 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -123,7 +123,7 @@ Requires: python-dns
 Requires: zip
 Requires: policycoreutils >= %{POLICYCOREUTILSVER}
 Requires: tar
-Requires(pre): certmonger >= 0.75.6
+Requires(pre): certmonger >= 0.75.13
 Requires(pre): 389-ds-base >= 1.3.2.20
 Requires: fontawesome-fonts
 Requires: open-sans-fonts
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index 9c9de033c3fc910e30891ad67b54166800dc43d7..9535cedd8eeef75c225824f459dcab284efb7dac 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -696,24 +696,24 @@ def certificate_renewal_update(ca):
     # State not set, lets see if we are already configured
     for request in requests:
         nss_dir, nickname, ca_name, pre_command, post_command, profile = request
-        criteria = (
-            ('cert_storage_location', nss_dir, certmonger.NPATH),
-            ('cert_nickname', nickname, None),
-            ('ca_name', ca_name, None),
-            ('template_profile', profile, None),
-        )
+        criteria = {
+            'cert-database': nss_dir,
+            'cert-nickname': nickname,
+            'ca-name': ca_name,
+            'template-profile': profile,
+        }
         request_id = certmonger.get_request_id(criteria)
         if request_id is None:
             break
 
-        val = certmonger.get_request_value(request_id, 'pre_certsave_command')
+        val = certmonger.get_request_value(request_id, 'cert-presave-command')
         if val is not None:
             val = val.split(' ', 1)[0]
             val = os.path.basename(val)
         if pre_command != val:
             break
 
-        val = certmonger.get_request_value(request_id, 'post_certsave_command')
+        val = certmonger.get_request_value(request_id, 'post-certsave-command')
         if val is not None:
             val = val.split(' ', 1)[0]
             val = os.path.basename(val)
diff --git a/ipa-client/ipaclient/ipa_certupdate.py b/ipa-client/ipaclient/ipa_certupdate.py
index 4199a293b2652863a2e7835256efd9cb12c8c33a..8e7fe0470b8ca1d379707142f4a5903f32d38e89 100644
--- a/ipa-client/ipaclient/ipa_certupdate.py
+++ b/ipa-client/ipaclient/ipa_certupdate.py
@@ -122,11 +122,10 @@ class CertUpdate(admintool.AdminTool):
 
         dogtag_constants = dogtag.configured_constants()
         nickname = 'caSigningCert cert-pki-ca'
-        criteria = (
-            ('cert_storage_location', dogtag_constants.ALIAS_DIR,
-             certmonger.NPATH),
-            ('cert_nickname', nickname, None),
-        )
+        criteria = {
+            'cert-database': dogtag_constants.ALIAS_DIR,
+            'cert-nickname': nickname,
+        }
         request_id = certmonger.get_request_id(criteria)
         if request_id is not None:
             timeout = api.env.startup_timeout + 60
diff --git a/ipapython/certmonger.py b/ipapython/certmonger.py
index 67370738427998497770561dee55be2de66ec479..6b5fe0d5c922c02f9c6371f2d207d771376d759a 100644
--- a/ipapython/certmonger.py
+++ b/ipapython/certmonger.py
@@ -1,4 +1,5 @@
 # Authors: Rob Crittenden <rcrit...@redhat.com>
+#          David Kupka <dku...@redhat.com>
 #
 # Copyright (C) 2010  Red Hat
 # see file 'COPYING' for use and warranty information
@@ -23,137 +24,193 @@
 
 import os
 import sys
-import re
 import time
+import dbus
 from ipapython import ipautil
 from ipapython import dogtag
 from ipapython.ipa_log_manager import *
 from ipaplatform.paths import paths
+from ipaplatform import services
 
-REQUEST_DIR=paths.CERTMONGER_REQUESTS_DIR
-CA_DIR=paths.CERTMONGER_CAS_DIR
-
-# Normalizer types for critera in get_request_id()
-NPATH = 1
-
-def find_request_value(filename, directive):
-    """
-    Return a value from a certmonger request file for the requested directive
-
-    It tries to do this a number of times because sometimes there is a delay
-    when ipa-getcert returns and the file is fully updated, particularly
-    when doing a request. Generating a CSR is fast but not instantaneous.
-    """
-    tries = 1
-    value = None
-    found = False
-    while value is None and tries <= 5:
-        tries=tries + 1
-        time.sleep(1)
-        fp = open(filename, 'r')
-        lines = fp.readlines()
-        fp.close()
-
-        for line in lines:
-            if found:
-                # A value can span multiple lines. If it does then it has a
-                # leading space.
-                if not line.startswith(' '):
-                    # We hit the next directive, return now
-                    return value
-                else:
-                    value = value + line[1:]
+REQUEST_DIR = paths.CERTMONGER_REQUESTS_DIR
+CA_DIR = paths.CERTMONGER_CAS_DIR
+
+DBUS_CM_PATH = '/org/fedorahosted/certmonger'
+DBUS_CM_IF = 'org.fedorahosted.certmonger'
+DBUS_CM_REQUEST_IF = 'org.fedorahosted.certmonger.request'
+DBUS_CM_CA_IF = 'org.fedorahosted.certmonger.ca'
+DBUS_PROPERTY_IF = 'org.freedesktop.DBus.Properties'
+
+
+class _cm_dbus_object(object):
+    """
+    Auxiliary class for convenient DBus object handling.
+    """
+    def __init__(self, bus, object_path, object_dbus_interface,
+                 parent_dbus_interface=None, property_interface=False):
+        """
+        bus - DBus bus object, result of dbus.SystemBus() or dbus.SessionBus()
+              Object is accesible over this DBus bus instance.
+        object_path - path to requested object on DBus bus
+        object_dbus_interface
+        parent_dbus_interface
+        property_interface - create DBus property interface? True or False
+        """
+        if bus is None or object_path is None or object_dbus_interface is None:
+            raise RuntimeError(
+                "bus, object_path and dbus_interface must not be None.")
+        if parent_dbus_interface is None:
+            parent_dbus_interface = object_dbus_interface
+        self.bus = bus
+        self.path = object_path
+        self.obj_dbus_if = object_dbus_interface
+        self.parent_dbus_if = parent_dbus_interface
+        self.obj = bus.get_object(parent_dbus_interface, object_path)
+        self.obj_if = dbus.Interface(self.obj, object_dbus_interface)
+        if property_interface:
+            self.prop_if = dbus.Interface(self.obj, DBUS_PROPERTY_IF)
+
+
+def _start_certmonger():
+    """
+    Start certmonger daemon. If it's already running systemctl just ignores
+    the command.
+    """
+    if not services.knownservices.certmonger.is_running():
+        try:
+            services.knownservices.certmonger.start()
+        except Exception, e:
+            root_logger.error('Failed to start certmonger: %s' % e)
+            raise
+
+
+def _connect_to_certmonger():
+    """
+    Start certmonger daemon and connect to it via DBus.
+    """
+    try:
+        _start_certmonger()
+    except (KeyboardInterrupt, OSError), e:
+        root_logger.error('Failed to start certmonger: %s' % e)
+        raise
+
+    try:
+        bus = dbus.SystemBus()
+        cm = _cm_dbus_object(bus, DBUS_CM_PATH, DBUS_CM_IF)
+    except dbus.DBusException, e:
+        root_logger.error("Failed to access certmonger over DBus: %s", e)
+        raise
+    return cm
+
+
+def _get_requests(criteria=dict()):
+    """
+    Get all requests that matches the provided criteria.
+    """
+    if not isinstance(criteria, dict):
+        raise TypeError('"criteria" must be dict.')
+
+    cm = _connect_to_certmonger()
+    requests = []
+    requests_paths = []
+    if 'nickname' in criteria:
+        request_path = cm.obj_if.find_request_by_nickname(criteria['nickname'])
+        if request_path:
+            requests_paths = [request_path]
+    else:
+        requests_paths = cm.obj_if.get_requests()
+
+    for request_path in requests_paths:
+        request = _cm_dbus_object(cm.bus, request_path, DBUS_CM_REQUEST_IF,
+                                  DBUS_CM_IF, True)
+        for criterion in criteria:
+            if criterion == 'ca-name':
+                ca_path = request.obj_if.get_ca()
+                ca = _cm_dbus_object(cm.bus, ca_path, DBUS_CM_CA_IF,
+                                     DBUS_CM_IF)
+                value = ca.obj_if.get_nickname()
             else:
-                if line.startswith(directive + '='):
-                    found = True
-                    value = line[len(directive)+1:]
+                value = request.prop_if.Get(DBUS_CM_REQUEST_IF, criterion)
+            if value != criteria[criterion]:
+                break
+        else:
+            requests.append(request)
+    return requests
+
+
+def _get_request(criteria):
+    """
+    Find request that matches criteria.
+    If 'nickname' is specified other criteria are ignored because 'nickname'
+    uniquely identify single request.
+    When multiple or none request matches specified criteria RuntimeError is
+    raised.
+    """
+    requests = _get_requests(criteria)
+    if len(requests) != 1:
+        raise RuntimeError("Criteria expected to be met by 1 request, got %s."
+                           % len(requests))
+    else:
+        return requests[0]
 
-    return value
 
 def get_request_value(request_id, directive):
     """
-    There is no guarantee that the request_id will match the filename
-    in the certmonger requests directory, so open each one to find the
-    request_id.
+    Get property of request.
     """
-    fileList=os.listdir(REQUEST_DIR)
-    for file in fileList:
-        value = find_request_value('%s/%s' % (REQUEST_DIR, file), 'id')
-        if value is not None and value.rstrip() == request_id:
-            return find_request_value('%s/%s' % (REQUEST_DIR, file), directive)
+    try:
+        request = _get_request(dict(nickname=request_id))
+    except RuntimeError, e:
+        root_logger.error('Failed to get request: %s' % e)
+        raise
+    return request.prop_if.Get(DBUS_CM_REQUEST_IF, directive)
 
-    return None
 
 def get_request_id(criteria):
     """
     If you don't know the certmonger request_id then try to find it by looking
-    through all the request files. An alternative would be to parse the
-    ipa-getcert list output but this seems cleaner.
+    through all the requests.
 
-    criteria is a tuple of key/value/type to search for. The more specific
+    criteria is a tuple of key/value to search for. The more specific
     the better. An error is raised if multiple request_ids are returned for
     the same criteria.
 
     None is returned if none of the criteria match.
     """
-    assert type(criteria) is tuple
+    try:
+        request = _get_request(criteria)
+    except RuntimeError, e:
+        root_logger.error('Failed to get request: %s' % e)
+        raise
+    return request.prop_if.Get(DBUS_CM_REQUEST_IF, 'nickname')
 
-    reqid=None
-    fileList=os.listdir(REQUEST_DIR)
-    for file in fileList:
-        match = True
-        for (key, value, valtype) in criteria:
-            rv = find_request_value('%s/%s' % (REQUEST_DIR, file), key)
-            if rv and valtype == NPATH:
-                rv = os.path.abspath(rv)
-            if rv is None or rv.rstrip() != value:
-                match = False
-                break
-        if match and reqid is not None:
-            raise RuntimeError('multiple certmonger requests match the criteria')
-        if match:
-            reqid = find_request_value('%s/%s' % (REQUEST_DIR, file), 'id').rstrip()
-
-    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')
-        if id is not None:
-            reqid.append(id.rstrip())
+    reqid = []
+    criteria = {'cert-storage': 'NSSDB', 'key-storage': 'NSSDB',
+                'cert-database': dir, 'key-database': dir, }
+    requests = _get_requests(criteria)
+    for request in requests:
+        reqid.append(request.prop_if.Get(DBUS_CM_REQUEST_IF, 'nickname'))
 
     return reqid
 
+
 def add_request_value(request_id, directive, value):
     """
     Add a new directive to a certmonger request file.
-
-    The certmonger service MUST be stopped in order for this to work.
     """
-    fileList=os.listdir(REQUEST_DIR)
-    for file in fileList:
-        id = find_request_value('%s/%s' % (REQUEST_DIR, file), 'id')
-        if id is not None and id.rstrip() == request_id:
-            current_value = find_request_value('%s/%s' % (REQUEST_DIR, file), directive)
-            if not current_value:
-                fp = open('%s/%s' % (REQUEST_DIR, file), 'a')
-                fp.write('%s=%s\n' % (directive, value))
-                fp.close()
+    try:
+        request = _get_request({'nickname': request_id})
+    except RuntimeError, e:
+        root_logger.error('Failed to get request: %s' % e)
+        raise
+    request.obj_if.modify({directive: value})
 
-    return
 
 def add_principal(request_id, principal):
     """
@@ -162,7 +219,8 @@ def add_principal(request_id, principal):
     When an existing certificate is added via start-tracking it won't have
     a principal.
     """
-    return add_request_value(request_id, 'template_principal', principal)
+    add_request_value(request_id, 'template-principal', [principal])
+
 
 def add_subject(request_id, subject):
     """
@@ -172,47 +230,30 @@ def add_subject(request_id, subject):
     When an existing certificate is added via start-tracking it won't have
     a subject_template set.
     """
-    return add_request_value(request_id, 'template_subject', subject)
+    add_request_value(request_id, 'template-subject', subject)
 
+
+# TODO: Remove? Used only internaly if ever.
 def request_cert(nssdb, nickname, subject, principal, passwd_fname=None):
     """
-    Execute certmonger to request a server certificate
+    Execute certmonger to request a server certificate.
     """
-    args = [paths.IPA_GETCERT,
-            'request',
-            '-d', nssdb,
-            '-n', nickname,
-            '-N', subject,
-            '-K', principal,
-    ]
+    cm = _connect_to_certmonger()
+    request_parameters = dict(KEY_STORAGE='NSSDB', CERT_STORAGE='NSSDB',
+                              CERT_LOCATION=nssdb, CERT_NICKNAME=nickname,
+                              SUBJECT=subject, PRINCIPAL=principal,)
     if passwd_fname:
-        args.append('-p')
-        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)
-    request_id = m.group(1)
-    return request_id
+        request_parameters['KEY_PIN_FILE'] = passwd_fname
+    result = cm.obj_if.add_request(request_parameters)
+    try:
+        if result[0]:
+            request = _cm_dbus_object(cm.bus, result[1], DBUS_CM_REQUEST_IF,
+                                      DBUS_CM_IF, True)
+    except TypeError:
+        root_logger.error('Failed to get create new request.')
+        raise
+    return request.obj_if.get_nickname()
 
-def cert_exists(nickname, secdir):
-    """
-    See if a nickname exists in an NSS database.
-
-    Returns True/False
-
-    This isn't very sophisticated in that it doesn't differentiate between
-    a database that doesn't exist and a nickname that doesn't exist within
-    the database.
-    """
-    args = [paths.CERTUTIL, "-L",
-           "-d", os.path.abspath(secdir),
-           "-n", nickname
-          ]
-    (stdout, stderr, rc) = ipautil.run(args, raiseonerr=False)
-    if rc == 0:
-        return True
-    else:
-        return False
 
 def start_tracking(nickname, secdir, password_file=None, command=None):
     """
@@ -223,75 +264,66 @@ def start_tracking(nickname, secdir, password_file=None, command=None):
     certmonger to run when it renews a certificate. This command must
     reside in /usr/lib/ipa/certmonger to work with SELinux.
 
-    Returns the stdout, stderr and returncode from running ipa-getcert
-
-    This assumes that certmonger is already running.
+    Returns True or False
     """
-    if not cert_exists(nickname, os.path.abspath(secdir)):
-        raise RuntimeError('Nickname "%s" doesn\'t exist in NSS database "%s"' % (nickname, secdir))
-    args = [paths.IPA_GETCERT, "start-tracking",
-            "-d", os.path.abspath(secdir),
-            "-n", nickname]
-    if password_file:
-        args.append("-p")
-        args.append(os.path.abspath(password_file))
+    cm = _connect_to_certmonger()
+    params = {'TRACK': True}
+    params['cert-nickname'] = nickname
+    params['cert-database'] = os.path.abspath(secdir)
+    params['cert-storage'] = 'NSSDB'
+    params['key-nickname'] = nickname
+    params['key-database'] = os.path.abspath(secdir)
+    params['key-storage'] = 'NSSDB'
     if command:
-        args.append("-C")
-        args.append(command)
+        params['cert-postsave-command'] = command
+    if password_file:
+        params['KEY_PIN_FILE'] = os.path.abspath(password_file)
+    result = cm.obj_if.add_request(params)
+    try:
+        if result[0]:
+            request = _cm_dbus_object(cm.bus, result[1], DBUS_CM_REQUEST_IF,
+                                      DBUS_CM_IF, True)
+    except TypeError, e:
+        root_logger.error('Failed to add new request.')
+        raise
+    return request.prop_if.Get(DBUS_CM_REQUEST_IF, 'nickname')
 
-    (stdout, stderr, returncode) = ipautil.run(args)
-
-    return (stdout, stderr, returncode)
 
 def stop_tracking(secdir, request_id=None, nickname=None):
     """
     Stop tracking the current request using either the request_id or nickname.
 
-    This assumes that the certmonger service is running.
+    Returns True or False
     """
     if request_id is None and nickname is None:
         raise RuntimeError('Both request_id and nickname are missing.')
-    if nickname:
-        # Using the nickname find the certmonger request_id
-        criteria = (('cert_storage_location', os.path.abspath(secdir), NPATH),('cert_nickname', nickname, None))
-        try:
-            request_id = get_request_id(criteria)
-            if request_id is None:
-                return ('', '', 0)
-        except RuntimeError:
-            # This means that multiple requests matched, skip it for now
-            # Fall back to trying to stop tracking using nickname
-            pass
 
-    args = [paths.GETCERT,
-            'stop-tracking',
-    ]
+    criteria = {'cert-database': secdir}
     if request_id:
-        args.append('-i')
-        args.append(request_id)
-    else:
-        args.append('-n')
-        args.append(nickname)
-        args.append('-d')
-        args.append(os.path.abspath(secdir))
+        criteria['nickname'] = request_id
+    if nickname:
+        criteria['cert-nickname'] = nickname
+    try:
+        request = _get_request(criteria)
+    except RuntimeError, e:
+        root_logger.error('Failed to get request: %s' % e)
+        raise
+    cm = _connect_to_certmonger()
+    cm.obj_if.remove_request(request.path)
 
-    (stdout, stderr, returncode) = ipautil.run(args)
-
-    return (stdout, stderr, returncode)
 
 def modify(request_id, profile=None):
-    args = [paths.GETCERT, 'start-tracking',
-            '-i', request_id]
     if profile:
-        args += ['-T', profile]
-    return ipautil.run(args)
+        request = _get_request({'nickname': request_id})
+        request.obj_if.modify({'template-profile': profile})
+
 
 def resubmit_request(request_id, profile=None):
-    args = [paths.IPA_GETCERT, 'resubmit',
-            '-i', request_id]
+    request = _get_request({'nickname': request_id})
     if profile:
-        args += ['-T', profile]
-    return ipautil.run(args)
+        request.obj_if.modify({'template-profile': profile})
+    request.obj_if.resubmit()
+
 
 def _find_IPA_ca():
     """
@@ -301,13 +333,10 @@ def _find_IPA_ca():
     We can use find_request_value because the ca files have the
     same file format.
     """
-    fileList=os.listdir(CA_DIR)
-    for file in fileList:
-        value = find_request_value('%s/%s' % (CA_DIR, file), 'id')
-        if value is not None and value.strip() == 'IPA':
-            return '%s/%s' % (CA_DIR, file)
+    cm = _connect_to_certmonger()
+    ca_path = cm.obj_if.find_ca_by_nickname('IPA')
+    return _cm_dbus_object(cm.bus, ca_path, DBUS_CM_CA_IF, DBUS_CM_IF, True)
 
-    return None
 
 def add_principal_to_cas(principal):
     """
@@ -317,58 +346,27 @@ def add_principal_to_cas(principal):
     /usr/libexec/certmonger/ipa-submit.
 
     We also need to restore this on uninstall.
-
-    The certmonger service MUST be stopped in order for this to work.
     """
-    cafile = _find_IPA_ca()
-    if cafile is None:
-        return
+    ca = _find_IPA_ca()
+    if ca:
+        ext_helper = ca.prop_if.Get(DBUS_CM_CA_IF, 'external-helper')
+        if ext_helper and ext_helper.find('-k') == -1:
+            ext_helper = '%s -k %s' % (ext_helper.strip(), principal)
+            ca.prop_if.Set(DBUS_CM_CA_IF, 'external-helper', ext_helper)
 
-    update = False
-    fp = open(cafile, 'r')
-    lines = fp.readlines()
-    fp.close()
-
-    for i in xrange(len(lines)):
-        if lines[i].startswith('ca_external_helper') and \
-            lines[i].find('-k') == -1:
-            lines[i] = '%s -k %s\n' % (lines[i].strip(), principal)
-            update = True
-
-    if update:
-        fp = open(cafile, 'w')
-        for line in lines:
-            fp.write(line)
-        fp.close()
 
 def remove_principal_from_cas():
     """
     Remove any -k principal options from the ipa_submit helper.
-
-    The certmonger service MUST be stopped in order for this to work.
     """
-    cafile = _find_IPA_ca()
-    if cafile is None:
-        return
+    ca = _find_IPA_ca()
+    if ca:
+        ext_helper = ca.prop_if.Get(DBUS_CM_CA_IF, 'external-helper')
+        if ext_helper and ext_helper.find('-k'):
+            ext_helper = ext_helper.strip()[0]
+            ca.prop_if.Set(DBUS_CM_CA_IF, 'external-helper', ext_helper)
 
-    update = False
-    fp = open(cafile, 'r')
-    lines = fp.readlines()
-    fp.close()
 
-    for i in xrange(len(lines)):
-        if lines[i].startswith('ca_external_helper') and \
-            lines[i].find('-k') > 0:
-            lines[i] = lines[i].strip().split(' ')[0] + '\n'
-            update = True
-
-    if update:
-        fp = open(cafile, 'w')
-        for line in lines:
-            fp.write(line)
-        fp.close()
-
-# Routines specific to renewing dogtag CA certificates
 def get_pin(token, dogtag_constants=None):
     """
     Dogtag stores its NSS pin in a file formatted as token:PIN.
@@ -384,6 +382,7 @@ def get_pin(token, dogtag_constants=None):
                 return pin.strip()
     return None
 
+
 def dogtag_start_tracking(ca, nickname, pin, pinfile, secdir, pre_command,
                           post_command, profile=None):
     """
@@ -398,52 +397,46 @@ def dogtag_start_tracking(ca, nickname, pin, pinfile, secdir, pre_command,
     post_command is the script to execute after a renewal is done.
 
     Both commands can be None.
-
-    Returns the stdout, stderr and returncode from running ipa-getcert
-
-    This assumes that certmonger is already running.
     """
-    if not cert_exists(nickname, os.path.abspath(secdir)):
-        raise RuntimeError('Nickname "%s" doesn\'t exist in NSS database "%s"' % (nickname, secdir))
 
-    args = [paths.GETCERT, "start-tracking",
-            "-d", os.path.abspath(secdir),
-            "-n", nickname,
-            "-c", ca,
-           ]
+    cm = _connect_to_certmonger()
+    certmonger_cmd_template = paths.CERTMONGER_COMMAND_TEMPLATE
 
-    if pre_command is not None:
+    params = {'TRACK': True}
+    params['cert-nickname'] = nickname
+    params['cert-database'] = os.path.abspath(secdir)
+    params['cert-storage'] = 'NSSDB'
+    params['key-nickname'] = nickname
+    params['key-database'] = os.path.abspath(secdir)
+    params['key-storage'] = 'NSSDB'
+    ca_path = cm.obj_if.find_ca_by_nickname(ca)
+    if ca_path:
+        params['ca'] = ca_path
+    if pin:
+        params['KEY_PIN'] = pin
+    if pinfile:
+        params['KEY_PIN_FILE'] = os.path.abspath(pinfile)
+    if pre_command:
         if not os.path.isabs(pre_command):
             if sys.maxsize > 2**32L:
                 libpath = 'lib64'
             else:
                 libpath = 'lib'
-            pre_command = paths.CERTMONGER_COMMAND_TEMPLATE % (libpath, pre_command)
-        args.append("-B")
-        args.append(pre_command)
-
-    if post_command is not None:
+            pre_command = certmonger_cmd_template % (libpath, pre_command)
+        params['cert-presave-command'] = pre_command
+    if post_command:
         if not os.path.isabs(post_command):
             if sys.maxsize > 2**32L:
                 libpath = 'lib64'
             else:
                 libpath = 'lib'
-            post_command = paths.CERTMONGER_COMMAND_TEMPLATE % (libpath, post_command)
-        args.append("-C")
-        args.append(post_command)
-
-    if pinfile:
-        args.append("-p")
-        args.append(pinfile)
-    else:
-        args.append("-P")
-        args.append(pin)
-
+            post_command = certmonger_cmd_template % (libpath, post_command)
+        params['cert-postsave-command'] = post_command
     if profile:
-        args.append("-T")
-        args.append(profile)
+        params['ca-profile'] = profile
+
+    cm.obj_if.add_request(params)
 
-    (stdout, stderr, returncode) = ipautil.run(args, nolog=[pin])
 
 def check_state(dirs):
     """
@@ -461,6 +454,7 @@ def check_state(dirs):
 
     return reqids
 
+
 def wait_for_request(request_id, timeout=120):
     for i in range(0, timeout, 5):
         state = get_request_value(request_id, 'state').strip()
@@ -475,7 +469,9 @@ def wait_for_request(request_id, timeout=120):
     return state
 
 if __name__ == '__main__':
-    request_id = request_cert(paths.HTTPD_ALIAS_DIR, "Test", "cn=tiger.example.com,O=IPA", "HTTP/tiger.example....@example.com")
+    request_id = request_cert(paths.HTTPD_ALIAS_DIR, "Test",
+                              "cn=tiger.example.com,O=IPA",
+                              "HTTP/tiger.example....@example.com")
     csr = get_request_value(request_id, 'csr')
     print csr
     stop_tracking(request_id)
diff --git a/ipaserver/install/certs.py b/ipaserver/install/certs.py
index 6569f51444617b710da4569c04a7a549ae41cb05..4d508cde8511f95480cf74772e4b066414ea3c35 100644
--- a/ipaserver/install/certs.py
+++ b/ipaserver/install/certs.py
@@ -547,46 +547,26 @@ class CertDB(object):
             else:
                 libpath = 'lib'
             command = paths.CERTMONGER_COMMAND_TEMPLATE % (libpath, command)
-        cmonger = services.knownservices.certmonger
-        cmonger.enable()
-        services.knownservices.messagebus.start()
-        cmonger.start()
         try:
-            (stdout, stderr, rc) = certmonger.start_tracking(nickname, self.secdir, password_file, command)
-        except (ipautil.CalledProcessError, RuntimeError), e:
+            request_id = certmonger.start_tracking(nickname, self.secdir, password_file, command)
+        except RuntimeError, e:
             root_logger.error("certmonger failed starting to track certificate: %s" % str(e))
             return
 
-        cmonger.stop()
         cert = self.get_cert_from_db(nickname)
         nsscert = x509.load_certificate(cert, dbdir=self.secdir)
         subject = str(nsscert.subject)
-        m = re.match('New tracking request "(\d+)" added', stdout)
-        if not m:
-            root_logger.error('Didn\'t get new %s request, got %s' % (cmonger.service_name, stdout))
-            raise RuntimeError('%s did not issue new tracking request for \'%s\' in \'%s\'. Use \'ipa-getcert list\' to list existing certificates.' % (cmonger.service_name, nickname, self.secdir))
-        request_id = m.group(1)
-
         certmonger.add_principal(request_id, principal)
         certmonger.add_subject(request_id, subject)
 
-        cmonger.start()
-
     def untrack_server_cert(self, nickname):
         """
         Tell certmonger to stop tracking the given certificate nickname.
         """
-
-        # Always start certmonger. We can't untrack something if it isn't
-        # running
-        cmonger = services.knownservices.certmonger
-        services.knownservices.messagebus.start()
-        cmonger.start()
         try:
             certmonger.stop_tracking(self.secdir, nickname=nickname)
-        except (ipautil.CalledProcessError, RuntimeError), e:
+        except RuntimeError, e:
             root_logger.error("certmonger failed to stop tracking certificate: %s" % str(e))
-        cmonger.stop()
 
     def create_server_cert(self, nickname, hostname, other_certdb=None, subject=None):
         """
diff --git a/ipaserver/install/ipa_cacert_manage.py b/ipaserver/install/ipa_cacert_manage.py
index 64602c835d1ffc6f6729eef59aca0ca42086f3b9..c681261e8baded28301397893e35ccd8ec4a03b0 100644
--- a/ipaserver/install/ipa_cacert_manage.py
+++ b/ipaserver/install/ipa_cacert_manage.py
@@ -153,8 +153,8 @@ class CACertManage(admintool.AdminTool):
             raise admintool.ScriptError("CA is not configured on this system")
 
         nss_dir = ca.dogtag_constants.ALIAS_DIR
-        criteria = (('cert_storage_location', nss_dir, certmonger.NPATH),
-                    ('cert_nickname', self.cert_nickname, None))
+        criteria = {'cert-database': nss_dir,
+                    'cert-nickname': self.cert_nickname}
         self.request_id = certmonger.get_request_id(criteria)
         if self.request_id is None:
             raise admintool.ScriptError(
diff --git a/ipaserver/install/plugins/ca_renewal_master.py b/ipaserver/install/plugins/ca_renewal_master.py
index 37b5487fe2fcdaa98397d4ac67d7934434d3e905..21d20614bb2eafe95f4aaec4996bac51e36b229d 100644
--- a/ipaserver/install/plugins/ca_renewal_master.py
+++ b/ipaserver/install/plugins/ca_renewal_master.py
@@ -52,10 +52,10 @@ class update_ca_renewal_master(PostUpdate):
             self.debug("found CA renewal master %s", entries[0].dn[1].value)
             return (False, False, [])
 
-        criteria = (
-            ('cert_storage_location', paths.HTTPD_ALIAS_DIR, certmonger.NPATH),
-            ('cert_nickname', 'ipaCert', None),
-        )
+        criteria = {
+            'cert-storage': paths.HTTPD_ALIAS_DIR,
+            'cert-nickname': 'ipaCert',
+        }
         request_id = certmonger.get_request_id(criteria)
         if request_id is not None:
             self.debug("found certmonger request for ipaCert")
-- 
1.9.3

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

Reply via email to