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

Sumit Bose sbose at redhat.com
Tue Sep 22 09:40:46 UTC 2015


On Mon, Sep 21, 2015 at 02:49:53PM +0200, Petr Cech wrote:
> On 09/21/2015 01:27 PM, Sumit Bose wrote:
> >>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.
> >
> 
> Hello Sumit,
> 
> thank you for your patience. There is fixed patch attached. I used the
> predefined mapping.
> 
> >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);
> 
> I saw another 2 occurrences of calling 'sysdb_attrs_add_string()' in for
> loop. Maybe there should be 'safe' variant too.

The 'safe' variant comes with a cost and should be only used when really
needed. I think the other cases are safe (famous last words :-).

The patches are looking good and fixed the netgroups issues with group
and nested group memberships. CI passes
http://sssd-ci.duckdns.org/logs/job/27/43/summary.html except the debian
test but this is an unrelated known issue, so ACK.

Thank you for your patience as well.

bye,
Sumit

> 
> Regards
> Petr
> 
> >+            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