On Fri, May 20, 2016 at 09:13:29PM +0200, Sumit Bose wrote:
Hi,
this set of patches should resolve
https://fedorahosted.org/sssd/ticket/2897 "Smart Cards: Certificate in
the ID View" and cover all other use cases from
https://fedorahosted.org/sssd/wiki/DesignDocs/LookupUsersByCertificatePart2
as well. So basically certificates can be read from IPA and local
overrides and from AD with direct in indirect integration.
The patches are all about lookups, so Smartcards and authentication is
not needed to test them. All is needed is a certificate which can be
added to an AD user object or an override object and then try to lookup
the user with
dbus-send --system --print-reply --dest=org.freedesktop.sssd.infopipe \
/org/freedesktop/sssd/infopipe/Users \
org.freedesktop.sssd.infopipe.Users.FindByCertificate
string:"BASE64_CERTIFICATE_STRING"
from a IPA client, IPA server or AD client with AD provier.
If the certificate is store in the AD user object and the lookup is
started on an IPA client a patch for the IPA server is needed, because
the request has to run via the extdom plugin. I'll send a patch to
freeipa-devel which will use the sss_nss_getnamebycert() call added by
one of the patches to allow the extdom plugin to do lookups by
certificate. This means that SSSD on the IPA server must used the
attached patches as well.
bye,
Sumit
Hi,
so far I read the patches, see comments inline. I haven't tested them
yet, feel free to postpone sending new patches until the final ack/nack.
From b081fc781a4bb22f3fdc6200256086f6b2cb811f Mon Sep 17 00:00:00
2001
From: Sumit Bose <sbose(a)redhat.com>
Date: Wed, 6 Apr 2016 11:12:30 +0200
Subject: [PATCH 02/12] sysdb: add searches by certificate with overrides
+ /* If there are views we have to check if override values must be added to
+ * the original object. */
+ if (DOM_HAS_VIEWS(domain) && orig_obj->count == 1) {
+ ret = sysdb_add_overrides_to_object(domain, orig_obj->msgs[0],
+ override_obj == NULL ? NULL : override_obj->msgs[0],
+ NULL);
As a style nitpick, I would prefer to use:
if (ret == ENOENT) {
} else if (ret != EOK) {
}
+ if (ret != EOK && ret != ENOENT) {
+ DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_overrides_to_object
failed.\n");
+ goto done;
+ }
From 2a2a021aa6ff9f3651c389201292c4066af4319d Mon Sep 17 00:00:00
2001
From: Sumit Bose <sbose(a)redhat.com>
Date: Fri, 8 Apr 2016 13:22:24 +0200
Subject: [PATCH 09/12] sss_override: add certificate support
---
src/db/sysdb.h | 1 +
src/tests/intg/ldap_local_override_test.py | 8 +++----
src/tools/sss_override.c | 38 ++++++++++++++++++++++++++----
It would be nice to amend the sss_override manpage, too.
From d564862024bb208614b2f3b547d99b72a9787a45 Mon Sep 17 00:00:00
2001
From: Sumit Bose <sbose(a)redhat.com>
Date: Mon, 25 Apr 2016 16:09:48 +0200
Subject: [PATCH 11/12] NSS: add SSS_NSS_GETNAMEBYCERT request
+static void ifp_users_find_by_cert_done(struct tevent_req *req);
I guess using the ifp prefix here is a copy-and-paste error?
From f7647dbf154ce5cb082b391269f9b674ca47e712 Mon Sep 17 00:00:00
2001
From: Sumit Bose <sbose(a)redhat.com>
Date: Tue, 26 Apr 2016 13:13:43 +0200
Subject: [PATCH 12/12] nss-idmap: add sss_nss_getnamebycert()
---
Makefile.am | 2 +-
src/python/pysss_nss_idmap.c | 47 ++++++++++++++++++++++++++++--
src/responder/nss/nsssrv_cmd.c | 1 +
src/sss_client/idmap/sss_nss_idmap.c | 26 ++++++++++++++++-
src/sss_client/idmap/sss_nss_idmap.exports | 6 ++++
src/sss_client/idmap/sss_nss_idmap.h | 15 ++++++++++
6 files changed, 93 insertions(+), 4 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index d20a10fa61f8a2fc296f839318503960382a9879..58255b0dc766e3755d70c12ee312a9aa52d6a724
100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -989,7 +989,7 @@ libsss_nss_idmap_la_LIBADD = \
$(CLIENT_LIBS)
libsss_nss_idmap_la_LDFLAGS = \
-Wl,--version-script,$(srcdir)/src/sss_client/idmap/sss_nss_idmap.exports \
- -version-info 1:0:1
+ -version-info 2:0:2
The change itself is good, do we expect this is the final change to the
library in this release?