[SSSD] [PATCHES] Netgroup support for LDAP provider

Stephen Gallagher sgallagh at redhat.com
Mon Oct 11 18:32:59 UTC 2010


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/05/2010 10:21 AM, Sumit Bose wrote:
> Hi,
> 
> this series of patches continue the work Stephen has started in
> "[PATCHES] Support for netgroups in the NSS client and responder".
> 
> We decided to try to be as compatible to nss_ldap as possible, i.e. we
> do not any group unrolling or loop detection inside of sssd, but rely on
> glibc. To achieve this I added support to return netgroup member groups
> to the client and glibc. This is mostly done in 0003 and 0006. 0007 and
> 0008 add the necessary support to the LDAP provider.
> 
> There is one difference to nss_ldap. If a netgroup member is not
> specified by a plain name but by a DN nss_ldap just returns the DN
> string to glibc and then glibc searches for a netgroup where the name is
> the returned DN. Even nss_ldap cannot find a matching netgroup for this
> name. If sssd detects a DN in the member list it tries to translate it
> to the corresponding name of the netgroup. If this fails it will return
> the full DN.
> 
> The patches 0001 and 0002 fixes two errors in Stephen's patches.


Patch 0001: Nack. Please reverse the order of these two functions. The
request should be set to "done" before we post it.

Patch 0002: Ack

Patch 0003: Please update the comment about the wire protocol to mention
that byte 8 describes the 8-bit value for the reply type.

Also, I'm not sure that this is safe for aligned platforms like MIPS. It
might be wisest to just make this a 32-bit value and use the alignment
code here.

Patch 0004: Ack

Patch 0005: Ack

Patch 0006: To avoid doing two calls to talloc_realloc() (which can be
expensive, especially for very large netgroups) it might be best to do
both element checks first and realloc with the results for both ahead of
populating them.

In lookup_netgr_step() you're still using &netgr->triples as the return
variable. I think you were correct elsewhere to change this to entries,
and the same should be done here.

Also, I'm not sure if we want to return EINVAL for an empty member. It's
probably enough to just skip it.

Patch 0007: Ack

Patch 0008: Nack
netgr_translate_members_send():
After calling sysdb_search_entry, please free sysdb_filter and
netgr_basedn since we won't need them again.
Spelling mistake: all_resovled should be all_resolved. Also in
update_dn_list()

You can't return from netgr_translate_members_ldap_step() with
tevent_req_done() (such as when we hit ENOMEM) safely.

If it happens on the first call (from within
netgr_translate_members_send()) then the callback will never fire. You
should change netgr_translate_members_ldap_step() to return errno_t types.

EOK should mean immediately call tevent_req_done() and
tevent_req_post(). If it returns EAGAIN, reenter the mainloop. Any other
error should call tevent_req_error() and tevent_req_post().

Please steal cn_attr onto the subreq so that it's cleaned up once the
subreq is freed in netgr_translate_members_ldap_done(). Otherwise, we're
carrying an extra cn_attrs array around in memory for every nested
netgroup we're processing.


Patch 0009: Ack



- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkyzWFsACgkQeiVVYja6o6MtXQCfcy91N7g1iINZ5Ss8/NpqE+uU
lOAAnRZ1xnLiNNzhHtaijS7axY/396eQ
=h8/P
-----END PGP SIGNATURE-----



More information about the sssd-devel mailing list