[SSSD] [PATCHES] filter domain local groups and fix cross-domain groupmembership

Jakub Hrozek jhrozek at redhat.com
Thu Dec 19 15:40:54 UTC 2013


On Thu, Dec 19, 2013 at 03:17:36PM +0100, Sumit Bose wrote:
> On Thu, Dec 19, 2013 at 10:56:47AM +0100, Jakub Hrozek wrote:
> > On Wed, Dec 18, 2013 at 04:48:44PM +0100, Sumit Bose wrote:
> > > Hi,
> > > 
> > > the follwing patch set contains two different fixes but since they are
> > > related and depend on each other I thought it might be easier to just
> > > send them together. The first three patches should fix
> > > https://fedorahosted.org/sssd/ticket/2178 while the last one should fix
> > > cross-domain group-memberships which was broken by a previous patch by
> > > me.
> > > 
> > > bye,
> > > Sumit
> > 
> > I'm still reviewing the last patch, but I wanted to give some comments
> > so we can work in parallel:
> > 
> > > From f410f0869d090048f4ab5aec5190bae0c82d5b2f Mon Sep 17 00:00:00 2001
> > > From: Sumit Bose <sbose at redhat.com>
> > > Date: Mon, 9 Dec 2013 12:17:43 +0100
> > > Subject: [PATCH 1/4] Add new option ldap_group_type
> > > 
> > > index 72586fb..82dcbba 100644
> > > --- a/src/man/sssd-ldap.5.xml
> > > +++ b/src/man/sssd-ldap.5.xml
> > > @@ -849,6 +849,27 @@
> > >                  </varlistentry>
> > >  
> > >                  <varlistentry>
> > > +                    <term>ldap_group_type (integer)</term>
> > > +                    <listitem>
> > > +                        <para>
> > > +                            The LDAP attribute that contains an integer value
> > > +                            indicating the type of the group and maybe other
> > > +                            flags.
> > > +                        </para>
> > > +                        <para>
> > > +                            This attribute is currently only used by the AD
> > > +                            privder to determine if a group is a domain local
> > 
> >                               ^^^^^
> >                               typo
> > > +                            groups and has to be filtered out for trusted
> > > +                            domains.
> > > +                        </para>
> > > +                        <para>
> > > +                            Default: groupType in the AD provider, othewise not
> > > +                            set
> > > +                        </para>
> > > +                    </listitem>
> > > +                </varlistentry>
> > > +
> > > +                <varlistentry>
> > >                      <term>ldap_group_nesting_level (integer)</term>
> > >                      <listitem>
> > >                          <para>
> > 
> > Otherwise ACK
> > 
> > > From 717c1f39324e0d718021873f20f31154d7bf58f5 Mon Sep 17 00:00:00 2001
> > > From: Sumit Bose <sbose at redhat.com>
> > > Date: Tue, 10 Dec 2013 10:14:02 +0100
> > > Subject: [PATCH 2/4] Add sysdb_attrs_get_int32_t
> > 
> > > --- a/src/db/sysdb.h
> > > +++ b/src/db/sysdb.h
> > > @@ -294,6 +294,10 @@ errno_t sysdb_attrs_get_bool(struct sysdb_attrs *attrs, const char *name,
> > >                               bool *value);
> > >  int sysdb_attrs_get_uint16_t(struct sysdb_attrs *attrs, const char *name,
> > >                               uint16_t *value);
> > > +int sysdb_attrs_get_string(struct sysdb_attrs *attrs, const char *name,
> > > +                           const char **string);
> > 
> > This ^^ addition is not needed, sysdb_attrs_get_string() was already
> > declared in sysdb.h earlier.
> > 
> > > +int sysdb_attrs_get_int32_t(struct sysdb_attrs *attrs, const char *name,
> > > +                             int32_t *value);
> > >  int sysdb_attrs_get_uint32_t(struct sysdb_attrs *attrs, const char *name,
> > >                               uint32_t *value);
> > 
> > > From 4b82b6439042550fd63023b23e876ba8ba35490e Mon Sep 17 00:00:00 2001
> > > From: Sumit Bose <sbose at redhat.com>
> > > Date: Tue, 10 Dec 2013 10:14:28 +0100
> > > Subject: [PATCH 3/4] AD: filter domain local groups for trusted/sub domains
> > > 
> > > In Active Directory groups with a domain local scope should only be used
> > > inside of the specific domain. Since SSSD read the group memberships
> > > from LDAP server of the user's domain the domain local groups are
> > > included in the LDAP result. Those groups should be filtered out if the
> > > domain is a sub/trusted domain, i.e. is not the domain the client
> > > running SSSD is joined to.
> > > 
> > > The groups will still be in the cache but marked as non-POSIX groups and
> > > no GID will be assigned.
> > > 
> > > Fixes https://fedorahosted.org/sssd/ticket/2178
> > 
> > ACK, the code looks good and seems to work fine.
> 
> Thank you for the review, new versions attached.
> 
> byem
> Sumit

ACK to these patches.

I'm waiting for Coverity scan to finish before pushing them. I'll be
crafting the release notes in the meantime.



More information about the sssd-devel mailing list