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 <sb...@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 <sb...@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 <sb...@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 <sb...@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?
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
