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


--
David Kupka
From b81786e68fba8efd4bb0c3e86a4702084137e30c Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Wed, 20 Aug 2014 13:58:50 +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                |  13 +-
 ipa-client/ipa-install/ipa-client-install      |   2 +-
 ipa-client/ipaclient/ipa_certupdate.py         |   5 +-
 ipapython/certmonger.py                        | 521 ++++++++++++-------------
 ipaserver/install/cainstance.py                |  10 +-
 ipaserver/install/certs.py                     |  28 +-
 ipaserver/install/ipa_cacert_manage.py         |   4 +-
 ipaserver/install/plugins/ca_renewal_master.py |   4 +-
 9 files changed, 273 insertions(+), 316 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 6f22bc92f76f2e4bd732a995e392d6845dab27b7..15aab5b45c5688f11e0125299c2842f23d986749 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -121,7 +121,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 adf6c8d847b931744cbfc4fcd14b6fbea55c26f5..e15e24fe0ee8511b39501a7d8aaf5c2f33bf138c 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -692,24 +692,23 @@ 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 = dict(
+            ('cert-database', nss_dir),
+            ('cert-nickname', nickname),
+            ('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/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 08fefc86d31392e9abf66ee4f8fff54a88179795..4cbfae077decca079f24385035428ddf0be38926 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -537,7 +537,7 @@ def uninstall(options, env):
         log_service_error(cmonger.service_name, 'start', e)
 
     try:
-        certmonger.stop_tracking(paths.NSS_DB_DIR, nickname=client_nss_nickname)
+        certmonger.stop_tracking(paths.NSS_DB_DIR, cert_nickname=client_nss_nickname)
     except (CalledProcessError, RuntimeError), e:
         root_logger.error("%s failed to stop tracking certificate: %s",
             cmonger.service_name, str(e))
diff --git a/ipa-client/ipaclient/ipa_certupdate.py b/ipa-client/ipaclient/ipa_certupdate.py
index 4199a293b2652863a2e7835256efd9cb12c8c33a..b25a3c85a2390405b37c26293c6bf65c747d462c 100644
--- a/ipa-client/ipaclient/ipa_certupdate.py
+++ b/ipa-client/ipaclient/ipa_certupdate.py
@@ -123,9 +123,8 @@ 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),
+            ('cert-database', dogtag_constants.ALIAS_DIR),
+            ('cert-nickname', nickname),
         )
         request_id = certmonger.get_request_id(criteria)
         if request_id is not None:
diff --git a/ipapython/certmonger.py b/ipapython/certmonger.py
index 67370738427998497770561dee55be2de66ec479..336ae8f44be471262f05ed59bbc0f3ea9e8e710f 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
@@ -25,146 +26,201 @@ 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
+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:]
+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.
+    Constuctor takes:
+    bus - DBus bus object, result of dbus.SystemBus() or dbus.SessionBus()
+    object_path - path to requested object on DBus bus
+    object_dbus_interface
+    parent_dbus_interface
+    property_interface - create DBus property interface? True or False
+    """
+    # 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
+
+    def __init__(self, bus, object_path, object_dbus_interface,
+                 parent_dbus_interface=None, property_interface=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, dbus_interface=object_dbus_interface)
+        if property_interface:
+            self.prop_if = dbus.Interface(self.obj, dbus_interface=DBUS_PROPERTY_IF)
+
+def _start_certmonger():
+    """
+    Start certmonger daemon. If it's already running systemctl just ignores
+    the command.
+    """
+    try:
+        services.knownservices.certmonger.start()
+    except Exception, e:
+        root_logger.error('Failed to start certmonger: %s' % e)
+        raise
+
+def _connect_to_certmonger(systembus=True):
+    """
+    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:
+        if systembus:
+            bus = dbus.SystemBus()
+        else:
+            bus = dbus.SessionBus()
+        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.')
+    if 'nickname' in criteria and len(criteria) > 1:
+        root_logger.warning('"nickname" specified. Ignoring other criteria')
+
+    cm = _connect_to_certmonger()
+    requests = []
+    if 'nickname' in criteria:
+        request_path = cm.obj_if.find_request_by_nickname(criteria['nickname'])
+        request = _cm_dbus_object(cm.bus, request_path, DBUS_CM_REQUEST_IF, DBUS_CM_IF, property_interface=True)
+        requests.append(request)
+    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, property_interface=True)
+            for criterion in criteria:
+                if request.prop_if.Get(DBUS_CM_REQUEST_IF, criterion) != criteria[criterion]:
+                    break
             else:
-                if line.startswith(directive + '='):
-                    found = True
-                    value = line[len(directive)+1:]
+                requests.append(request)
+    return requests
 
-    return value
-
-def get_request_value(request_id, directive):
+def _get_request(criteria):
     """
-    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.
+    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.
     """
-    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)
+    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 None
+def get_request_value(request_nickname, directive):
+    """
+    Get property of request.
+    """
+    try:
+        request = _get_request(dict(nickname=request_nickname))
+    except RuntimeError, e:
+        root_logger.error('Failed to get request: %s' % e)
+        return None
+    return request.prop_if.Get(DBUS_CM_REQUEST_IF, directive)
 
 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
-
-    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
+    try:
+        request = _get_request(criteria)
+    except RuntimeError, e:
+        root_logger.error('Failed to get request: %s' % e)
+        return None
+    return request.prop_if.Get(DBUS_CM_REQUEST_IF, 'nickname')
 
 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):
+def add_request_value(request_nickname, 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_nickname})
+    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):
+def add_principal(request_nickname, principal):
     """
     In order for a certmonger request to be renewable it needs a 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_nickname, 'template-principal', [principal])
 
-def add_subject(request_id, subject):
+def add_subject(request_nickname, subject):
     """
     In order for a certmonger request to be renwable it needs the subject
     set in the request file.
@@ -172,112 +228,82 @@ 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_nickname, 'template-subject', subject)
 
-def request_cert(nssdb, nickname, subject, principal, passwd_fname=None):
+#TODO: Remove? Used only internaly if ever.
+def request_cert(nssdb, cert_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=cert_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
-
-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):
+        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.')
+        return None
+    return request.obj_if.get_nickname()
+
+def start_tracking(cert_nickname, secdir, password_file=None, command=None):
     """
-    Tell certmonger to track the given certificate nickname in NSS
+    Tell certmonger to track the given certificate cert_nickname in NSS
     database in secdir protected by optional password file password_file.
 
     command is an optional parameter which specifies a command for
     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'] = cert_nickname
+    params['cert-database'] = os.path.abspath(secdir)
+    params['cert-storage'] = 'NSSDB'
+    params['key-nickname'] = cert_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.')
+        return None
+    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):
+def stop_tracking(secdir, request_nickname=None, cert_nickname=None):
     """
-    Stop tracking the current request using either the request_id or nickname.
+    Stop tracking the current request using either the request_nickname or cert_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',
-    ]
-    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))
-
-    (stdout, stderr, returncode) = ipautil.run(args)
+    if request_nickname is None and cert_nickname is None:
+        raise RuntimeError('Both request_nickname and cert_nickname are missing.')
 
-    return (stdout, stderr, returncode)
+    criteria = {'cert-database': secdir}
+    if request_nickname:
+        criteria['nickname'] = request_nickname
+    if cert_nickname:
+        criteria['cert-nickname'] = cert_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)
 
 def modify(request_id, profile=None):
     args = [paths.GETCERT, 'start-tracking',
@@ -301,13 +327,9 @@ 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)
-
-    return None
+    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)
 
 def add_principal_to_cas(principal):
     """
@@ -317,58 +339,26 @@ 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
-
-    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()
+    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)
 
 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
-
-    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
+    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)
+
+#TODO is there a way to get this over D-Bus?
 def get_pin(token, dogtag_constants=None):
     """
     Dogtag stores its NSS pin in a file formatted as token:PIN.
@@ -384,7 +374,7 @@ def get_pin(token, dogtag_constants=None):
                 return pin.strip()
     return None
 
-def dogtag_start_tracking(ca, nickname, pin, pinfile, secdir, pre_command,
+def dogtag_start_tracking(ca, cert_nickname, pin, pinfile, secdir, pre_command,
                           post_command, profile=None):
     """
     Tell certmonger to start tracking a dogtag CA certificate. These
@@ -398,52 +388,41 @@ 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()
 
-    if pre_command is not None:
+    params = {'TRACK': True}
+    params['cert-nickname'] = cert_nickname
+    params['cert-database'] = os.path.abspath(secdir)
+    params['cert-storage'] = 'NSSDB'
+    params['key-nickname'] = cert_nickname
+    params['key-database'] = os.path.abspath(secdir)
+    params['key-storage'] = 'NSSDB'
+    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:
+        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)
-
+        params['cert-postsave-command'] = post_command
     if profile:
-        args.append("-T")
-        args.append(profile)
+        params['ca-profile'] = profile
 
-    (stdout, stderr, returncode) = ipautil.run(args, nolog=[pin])
+    cm.obj_if.add_request(params)
 
 def check_state(dirs):
     """
@@ -475,7 +454,7 @@ 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")
-    csr = get_request_value(request_id, 'csr')
+    request_nickname = request_cert(paths.HTTPD_ALIAS_DIR, "Test", "cn=tiger.example.com,O=IPA", "HTTP/tiger.example....@example.com")
+    csr = get_request_value(request_nickname, 'csr')
     print csr
-    stop_tracking(request_id)
+    stop_tracking(request_nickname)
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 2a8ecc00c3711665e88251cd1dfffa736d35b2b1..0fe1303e5ecaddd713514ab9e73740e125efa3a2 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -318,13 +318,13 @@ def stop_tracking_certificates(dogtag_constants):
                      'caSigningCert cert-pki-ca']:
         try:
             certmonger.stop_tracking(
-                dogtag_constants.ALIAS_DIR, nickname=nickname)
+                dogtag_constants.ALIAS_DIR, cert_nickname=nickname)
         except (ipautil.CalledProcessError, RuntimeError), e:
             root_logger.error(
                 "certmonger failed to stop tracking certificate: %s" % str(e))
 
     try:
-        certmonger.stop_tracking(paths.HTTPD_ALIAS_DIR, nickname='ipaCert')
+        certmonger.stop_tracking(paths.HTTPD_ALIAS_DIR, cert_nickname='ipaCert')
     except (ipautil.CalledProcessError, RuntimeError), e:
         root_logger.error(
             "certmonger failed to stop tracking certificate: %s" % str(e))
@@ -1443,7 +1443,7 @@ class CAInstance(service.Service):
         try:
             certmonger.dogtag_start_tracking(
                 ca='dogtag-ipa-ca-renew-agent',
-                nickname='ipaCert',
+                cert_nickname='ipaCert',
                 pin=None,
                 pinfile=paths.ALIAS_PWDFILE_TXT,
                 secdir=paths.HTTPD_ALIAS_DIR,
@@ -1475,7 +1475,7 @@ class CAInstance(service.Service):
             try:
                 certmonger.dogtag_start_tracking(
                     ca='dogtag-ipa-ca-renew-agent',
-                    nickname=nickname,
+                    cert_nickname=nickname,
                     pin=pin,
                     pinfile=None,
                     secdir=self.dogtag_constants.ALIAS_DIR,
@@ -1496,7 +1496,7 @@ class CAInstance(service.Service):
         try:
             certmonger.dogtag_start_tracking(
                 ca='dogtag-ipa-renew-agent',
-                nickname='Server-Cert cert-pki-ca',
+                cert_nickname='Server-Cert cert-pki-ca',
                 pin=pin,
                 pinfile=None,
                 secdir=self.dogtag_constants.ALIAS_DIR,
diff --git a/ipaserver/install/certs.py b/ipaserver/install/certs.py
index 6569f51444617b710da4569c04a7a549ae41cb05..7129a2c9e40a49c1410f56b1cca9b172e5c779db 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:
+            certmonger.stop_tracking(self.secdir, cert_nickname=nickname)
+        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..f073bf0ab5d9ebb99f6cc3a2eb21b5167d885f78 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..4a76c6f8948cef7d5fb50677d607e8a81410835f 100644
--- a/ipaserver/install/plugins/ca_renewal_master.py
+++ b/ipaserver/install/plugins/ca_renewal_master.py
@@ -53,8 +53,8 @@ class update_ca_renewal_master(PostUpdate):
             return (False, False, [])
 
         criteria = (
-            ('cert_storage_location', paths.HTTPD_ALIAS_DIR, certmonger.NPATH),
-            ('cert_nickname', 'ipaCert', None),
+            ('cert-storage', paths.HTTPD_ALIAS_DIR),
+            ('cert-nickname', 'ipaCert'),
         )
         request_id = certmonger.get_request_id(criteria)
         if request_id is not None:
-- 
1.9.3

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

Reply via email to