On Mon, Jun 06, 2016 at 11:06:06AM +0200, Sumit Bose wrote:
> On Fri, Jun 03, 2016 at 02:56:08PM +0200, Jakub Hrozek wrote:
> > 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.
>
> Thank you for reviewing and testing.
>
> >
> > > 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) {
> > }
>
> fixed
>
> >
> > > + 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.
>
> fixed
>
> >
> >
> > > 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?
>
> fixed
>
> >
> >
> > > 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?
>
> I would says yes.
>
> New version attached.
Thank you, can you rebase the patches on top of origin/master?
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org