[SSSD] [PATCHES] PAC responder improvements

Sumit Bose sbose at redhat.com
Mon Aug 26 09:26:41 UTC 2013


On Mon, Aug 26, 2013 at 11:02:56AM +0200, Jakub Hrozek wrote:
> On Fri, Aug 16, 2013 at 06:25:02PM +0200, Sumit Bose wrote:
> > Hi,
> > 
> > this series of patches contains improvements for the PAC responder
> > related to the support of UIDs and GIDs managed by AD.
> > 
> > The first patch is a fix for https://fedorahosted.org/sssd/ticket/1996.
> > The original idea in the ticket was to modify an existing user entry
> > instead of deleting and recreating it. But since the PAC does not
> > contain any useful information which would improve the entry I decided to
> > not touch existing user entries at all and only update the group
> > memberships.
> > 
> > Please find details about the other patches in the commit messages.
> > 
> > bye,
> > Sumit
> 
> Hi,
> 
> The patches work well with one addition I sent to the list separately. I
> don't have any change requests, just some questions before the patches
> are applied, see below:
> 
> > From 1e54d2061d72d2f72c30d3fced9b43d4ec369a28 Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose at redhat.com>
> > Date: Thu, 1 Aug 2013 12:40:24 +0200
> > Subject: [PATCH 1/6] PAC: if user entry already exists keep it
> > 
> > Currently the PAC responder deletes a user entry and recreates it if
> > some attributes seems to be different.
> > 
> > Two of the attributes where the home directory and the shell of the
> > user. Those two attributes are not available from the PAC but where
> > generates by the PAC responder. The corresponding ID provider might have
> > better means to determine those attributes, e.g. read them from LDAP, so
> > we shouldn't change them here.
> > 
> > The third attribute is the user name. Since the PAC responder does
> > lookups only based on the UID we can wait until the ID provider updates
> > the entry.
> 
> The current logic doesn't detect if any of the extra attributes we put
> into the "_attrs" variable of get_pwd_from_pac() changed. I think this
> can only apply to UPN as the alias is derived from the name and the SID
> should never change. Can this be a problem?

I think not. If it is the generated UPN, i.e. username + realm, they
are the same and if the UPN in the cache was extracted from the Kerberos
ticket during password authentication it would be the more correct one.

> 
> Also maybe we should lowercase the alias explicitly as we do in the IPA
> extdom handler code. But this patch is good as-is, I think, if there are
> any issues, they can be solved separately.

I agree this is a good idea. Maybe we should consider to introduce a new
sysdb call like sysdb_attrs_add_lower_case_string()? Would you mind to
open a suitable ticket?

Thank you for the review.

bye,
Sumit
> 
> > From: Sumit Bose <sbose at redhat.com>
> > Date: Tue, 6 Aug 2013 11:10:10 +0200
> > Subject: [PATCH 2/6] PAC: do not create users with missing GID
> ACK
> 
> > From: Sumit Bose <sbose at redhat.com>
> > Date: Thu, 8 Aug 2013 12:35:12 +0200
> > Subject: [PATCH 3/6] PAC: handle non-POSIX groups in cache
> ACK
> 
> > From: Sumit Bose <sbose at redhat.com>
> > Date: Thu, 8 Aug 2013 14:09:42 +0200
> > Subject: [PATCH 4/6] PAC: read user DN instead of constructing it
> ACK.
> 
> > From 966390e5436d2eed565774adddf33f6ce3f85b24 Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose at redhat.com>
> > Date: Thu, 8 Aug 2013 16:56:06 +0200
> > Subject: [PATCH 5/6] PAC: do not fail if a single group cannot be
> >  added/removed
> ACK
> 
> > From: Sumit Bose <sbose at redhat.com>
> > Date: Thu, 8 Aug 2013 18:29:48 +0200
> > Subject: [PATCH 6/6] PAC: use SID instead of GID to search for groups
> ACK
> 
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel



More information about the sssd-devel mailing list