Updated patch attached; comments inline below.

On Mon, Apr 25, 2016 at 07:55:46AM +0200, Jan Cholasta wrote:
> I think it would be better to merge the `client` and `client_servicename`
> into a single `client_principal` argument, as both of the arguments are used
> only to specify the principal name of the client.

> Also I would prefer if the keyfile and keytab arguments were required,
> because it's better if you can explicitly see what values are used at the
> call site.

> Why is init_creds() now called from __init__()? Why is it still called from
> _auth_header()?
I invoke it from __init__ for more eager failure if there's a
problem (e.g. service is not in keytab), giving better error

It remains necessary to invoke it from _auth_header in case of
short-lived credentials.  I added some comments to the source.

> Why is ldap_uri now passed to IPAKEMKeys()?
It tries to use LDAPI by default, so ldap_uri needs to be passed
through if process owner cannot access LDAPI socket.  I added
commentary to source.

Regarding your suggestion to make a base class and override class
variables, I prefer to pass values around.  There are very few
instantiations of CustodiaClient so it is easy enough to follow.

From ba34a8f5c9b3e31a511e01266e3d3fedd53bcca6 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Fri, 8 Apr 2016 15:21:19 +1000
Subject: [PATCH] Allow CustodiaClient to be used by arbitrary principals

Currently CustodiaClient assumes that the client is the host
principal, and it is hard-coded to read the host keytab and server

For the Lightweight CAs feature, Dogtag on CA replicas will use
CustodiaClient to retrieve signing keys from the originating
replica.  Because this process runs as 'pkiuser', the host keys
cannot be used; instead, each Dogtag replica will have a service
principal to use for Custodia authentication.

Update CustodiaClient to require specifying the client keytab and
Custodia keyfile to use, and change the client argument to be a full
GSS service name (instead of hard-coding host service) to load from
the keytab.  Update call sites accordingly.

Also pass the given 'ldap_uri' argument through to IPAKEMKeys
because without it, the client tries to use LDAPI, but may not have

Part of: https://fedorahosted.org/freeipa/ticket/4559
 ipapython/secrets/client.py           | 20 +++++++++++++-------
 ipaserver/install/custodiainstance.py | 14 +++++++++++---
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/ipapython/secrets/client.py b/ipapython/secrets/client.py
--- a/ipapython/secrets/client.py
+++ b/ipapython/secrets/client.py
@@ -41,16 +41,22 @@ class CustodiaClient(object):
         return iSecStore(config)
-    def __init__(self, client, server, realm, ldap_uri=None, auth_type=None):
-        self.client = client
-        self.creds = None
+    def __init__(
+            self, client_service, keyfile, keytab, server, realm,
+            ldap_uri=None, auth_type=None):
+        self.client_service = client_service
+        self.keytab = keytab
+        # Init creds immediately to make sure they are valid.  Creds
+        # can also be re-inited by _auth_header to avoid expiry.
+        #
+        self.creds = self.init_creds()
         self.service_name = gssapi.Name('HTTP@%s' % (server,),
         self.server = server
-        keyfile = os.path.join(paths.IPA_CUSTODIA_CONF_DIR, 'server.keys')
-        self.ikk = IPAKEMKeys({'server_keys': keyfile})
+        self.ikk = IPAKEMKeys({'server_keys': keyfile, 'ldap_uri': ldap_uri})
         self.kemcli = KEMClient(self._server_keys(server, realm),
@@ -61,9 +67,9 @@ class CustodiaClient(object):
     def init_creds(self):
-        name = gssapi.Name('host@%s' % (self.client,),
+        name = gssapi.Name(self.client_service,
-        store = {'client_keytab': paths.KRB5_KEYTAB,
+        store = {'client_keytab': self.keytab,
                  'ccache': 'MEMORY:Custodia_%s' % b64encode(os.urandom(8))}
         return gssapi.Credentials(name=name, store=store, usage='initiate')
diff --git a/ipaserver/install/custodiainstance.py 
--- a/ipaserver/install/custodiainstance.py
+++ b/ipaserver/install/custodiainstance.py
@@ -12,6 +12,7 @@ from ipaserver.install import ldapupdate
 from ipaserver.install import sysupgrade
 from base64 import b64encode, b64decode
 from jwcrypto.common import json_decode
+import functools
 import shutil
 import os
 import tempfile
@@ -28,6 +29,13 @@ class CustodiaInstance(SimpleServiceInstance):
         self.fqdn = host_name
         self.realm = realm
         self.ca_is_configured = ca_is_configured
+        self.__CustodiaClient = functools.partial(
+            CustodiaClient,
+            client_service='host@' + self.fqdn,
+            keyfile=self.server_keys,
+            keytab=paths.KRB5_KEYTAB,
+            realm=realm,
+        )
     def __config_file(self):
         template_file = os.path.basename(self.config_file) + '.template'
@@ -94,11 +102,11 @@ class CustodiaInstance(SimpleServiceInstance):
         updater.update([os.path.join(paths.UPDATES_DIR, '73-custodia.update')])
     def __import_ra_key(self):
-        cli = CustodiaClient(self.fqdn, self.master_host_name, self.realm)
+        cli = self.__CustodiaClient(server=self.master_host_name)
     def import_dm_password(self, master_host_name):
-        cli = CustodiaClient(self.fqdn, master_host_name, self.realm)
+        cli = self.__CustodiaClient(server=master_host_name)
     def __get_keys(self, ca_host, cacerts_file, cacerts_pwd, data):
@@ -108,7 +116,7 @@ class CustodiaInstance(SimpleServiceInstance):
         prefix = data['prefix']
         certlist = data['list']
-        cli = CustodiaClient(self.fqdn, ca_host, self.realm)
+        cli = self.__CustodiaClient(server=ca_host)
         # Temporary nssdb
         tmpnssdir = tempfile.mkdtemp(dir=paths.TMP)

Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to