[SSSD] Fix #2275 nested netgroups do not work in IPA provider

Sumit Bose sbose at redhat.com
Mon Sep 21 11:27:37 UTC 2015


On Fri, Sep 18, 2015 at 02:33:03PM +0200, Petr Cech wrote:
> On 09/14/2015 01:53 PM, Sumit Bose wrote:
> >Hi Petr,
> >
> >thank you for your patience. Both versions of your patches work as
> >expected and fixes the issues with group and nested group memberships.
> >
> >After testing a reading the current sources I think the best solution
> >might be a slight variant of your first patch where you check for both
> >memberOf and originalMemberOf but instead of always checking both I
> >would suggest to pass down the attribute to check by adding a new member
> >to struct extract_state and set it with the help of a new attribute to
> >extract_members(). Since extract_members() is called individually for
> >users and hosts in ipa_netgr_process_all() you can always pass the right
> >attribute.Btw. instead of using SYSDB_ORIG_MEMBEROF and
> >SYSDB_MEMBEROF explicitly you might want to use the configured mapping
> >which e.g. is available in ipa_netgr_process_all() as
> >
> >state->ipa_opts->id->user_map[SDAP_AT_USER_MEMBEROF].sys_name
> >
> >bye,
> >Sumit
> 
> Hello Sumit,
> 
> there is next version of patch addresing your comments.
> 
> I didn't use statement like
> # state->ipa_opts->id->user_map[SDAP_AT_USER_MEMBEROF].sys_name
> because we don't have any for originalMemberOf. I'm not sure if I added

we have,

state->ipa_opts->host_map[IPA_AT_HOST_MEMBER_OF].sys_name

> originalMemberOf version of statement above wheter it is not more like the
> second version of patch. If I understood the problem right, we wouldn't have
> saved both kind of memberOf in our DB.

yes, we do not save it in the cache, but I think I was confused myself
when I mentioned the cache earlier in the thread. I thought we used this
data from the cache to create the netgroup data, but the IPA provider
already saves netgroup triples to the cache which makes sense because
otherwise the responder must have knowledge about how netgroups are
stored on the server. So no kind of cached memberOf data is used to
create the netgroup entries, sorry for guiding you in the wrong
direction here.

However, the server side names aren't used as well. Since the most
common use case for the LDAP data we read from the server is to write
them into the cache and to make further processing more easy the
attribute names are translated automatically into the internal names
based on the various struct sdap_attr_map maps. Based on those
translated names the netgroups evaluation is done. As you can see in
e.g. src/providers/ipa/ipa_opts.h ""memberOf" of a user entry is
translated into "SYSDB_MEMBEROF" and ""memberOf" of a host entry is
translated into "SYSDB_ORIG_MEMBEROF". I hope this helps to make it more
clear.

Nevertheless the patch looks good and works as expected. I only found an
issues when there are duplicated entries dues to memberships in multiple
groups. I had to add

diff --git a/src/providers/ipa/ipa_netgroups.c
b/src/providers/ipa/ipa_netgroups.c
index 2c39257..bc89006 100644
--- a/src/providers/ipa/ipa_netgroups.c
+++ b/src/providers/ipa/ipa_netgroups.c
@@ -121,9 +121,9 @@ static errno_t ipa_save_netgroup(TALLOC_CTX
*mem_ctx,
         }
     } else {
         for(c = 0; c < el->num_values; c++) {
-            ret = sysdb_attrs_add_string(netgroup_attrs,
-                                         SYSDB_NETGROUP_TRIPLE,
-                                         (const
                                          char*)el->values[c].data);
+            ret = sysdb_attrs_add_string_safe(netgroup_attrs,
+                                              SYSDB_NETGROUP_TRIPLE,
+                                              (const char*)el->values[c].data);
             if (ret) {
                 goto fail;
             }

to make it work in my tests.

bye,
Sumit

> 
> Regards
> Petr


More information about the sssd-devel mailing list