[SSSD] [patches] SDAP: "ldap_group_nesting_level = 0" does not appear to work

Stephen Gallagher sgallagh at redhat.com
Fri May 30 13:38:46 UTC 2014


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

On 05/30/2014 08:52 AM, Pavel Reichl wrote:
> Thank you Jakub for comments. I proposed new version of text to
> man pages. I updated 2nd patch and generally speaking returned to
> solution which I proposed in 1st or 2nd iteration of this patches.
> But this time I traded off code beauty for performance, but I think
> it's OK.
> 
> Regards,
> 
> PR
> 
> 
> On Tue, 2014-05-27 at 21:37 +0200, Jakub Hrozek wrote:
>> On Mon, May 26, 2014 at 07:28:37PM +0200, Pavel Reichl wrote:
>>>>> Hello,
>>>>> 
>>>>> I have updated patches and in 3rd patch I added option to
>>>>> disable Token-Groups for AD provider and thus make
>>>>> nesting_level = 0 work.
>>>>> 
>>>>> 2nd patch may be simplified even more if kind reviewer (or
>>>>> Lukas:-) ) thinks that call of is_group_dn should be
>>>>> replaced by:
>>>>> 
>>>>> "strstr(strdn, "cn=groups") != NULL"
>>>>> 
>>>> Yes, please.
>>>> 
>>>> For group with 100 users without any nesting group, you will
>>>> save 700 allocations,
>>>> 
>>>> strstr approach is much more lightweight :-)
>>>> 
>>>> LS
>>> 
>>> OK, whatever.
>>> 
>>> Updated patches attached.
>>> 
>>> 
>>>> _______________________________________________ sssd-devel
>>>> mailing list sssd-devel at lists.fedorahosted.org 
>>>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>>> 
>> 
>>> From 328def1e84e5d5baa81c8beca3484a659c353a6a Mon Sep 17
>>> 00:00:00 2001 From: Pavel Reichl <preichl at redhat.com> Date:
>>> Mon, 12 May 2014 22:45:00 +0000 Subject: [PATCH 1/3] MAN:
>>> Detailed ldap_group_nesting_level option
>>> 
>>> Resolves: https://fedorahosted.org/sssd/ticket/2294 --- 
>>> src/man/sssd-ldap.5.xml | 12 ++++++++++++ 1 file changed, 12
>>> insertions(+)
>>> 
>>> diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml 
>>> index
>>> 6426fe4fca5dc9bb9bc84fcbf633404144052d01..d7bb3475d38ca93044632673bb9c7ac6d83c02f6
>>> 100644 --- a/src/man/sssd-ldap.5.xml +++
>>> b/src/man/sssd-ldap.5.xml @@ -928,6 +928,18 @@ RFC2307 schema. 
>>> </para> <para> +                            Note: This option
>>> specifies the minimal level of +
>>> nesting that will be processed by the LDAP +
>>> provider. However, nested groups beyond this limit +
>>> <emphasis>may be</emphasis> returned.
>> 
>> I think this needs further clarification, something like "may be 
>> returned if previous lookups already resolved the deeper nesting 
>> levels". As with any man page change, please ask some native
>> English speaker for language check.
>> 
>>> +                        </para> +
>>> <para> +                            If ldap_group_nesting_level
>>> is set to 0 and LDAP +                            provider is
>>> used then no nested groups are processed +
>>> at all. Same behavior may be achieved for AD +
>>> provider if ldap_use_tokengroups is set to false.
>> 
>> I think the last sentence is misleading, the way I read it,
>> disabling tokengroups would always disable nesting, which is not
>> the case, is it?
>> 
>>> +                        </para> +
>>> <para> Default: 2 </para> </listitem> -- 1.8.4.2
>>> 
>> 
>>> From 484bda4041269e0e52211a1660ff5e5172cb208f Mon Sep 17
>>> 00:00:00 2001 From: Pavel Reichl <preichl at redhat.com> Date:
>>> Mon, 12 May 2014 15:00:26 +0000 Subject: [PATCH 2/3] SDAP: Make
>>> nesting_level = 0 to ignore nested groups
>>> 
>>> Make ldap_group_nesting_level = 0 to ignore group nesting
>>> entirely.
>>> 
>>> Resolves: https://fedorahosted.org/sssd/ticket/2294 --- 
>>> src/providers/ldap/sdap_async_groups.c | 11 +++++++++++ 1 file
>>> changed, 11 insertions(+)
>>> 
>>> diff --git a/src/providers/ldap/sdap_async_groups.c
>>> b/src/providers/ldap/sdap_async_groups.c index
>>> 89a5afb91e8fe666f834720c6b97e74e266822a6..c3c4e72c0b60cd5e689b5d9b0c42612c1feefa8b
>>> 100644 --- a/src/providers/ldap/sdap_async_groups.c +++
>>> b/src/providers/ldap/sdap_async_groups.c @@ -1211,6 +1211,9 @@
>>> sdap_process_group_members_2307bis(struct tevent_req *req, char
>>> *strdn; int ret; int i; +    int nesting_level; + +
>>> nesting_level = dp_opt_get_int(state->opts->basic,
>>> SDAP_NESTING_LEVEL);
>>> 
>>> for (i=0; i < memberel->num_values; i++) { member_dn = (char
>>> *)memberel->values[i].data; @@ -1220,7 +1223,15 @@
>>> sdap_process_group_members_2307bis(struct tevent_req *req, 
>>> state->dom, member_dn, &strdn); + if (ret == EOK) { +
>>> if (nesting_level == 0) { +                /* Ignore group
>>> members which are groups themselves. */ +                if
>>> (strstr(strdn, SYSDB_GROUPS_CONTAINER) != 0 ) {
>> 
>> While I agree with Lukas that saving allocations for these
>> insensive operations with potentially many loops should be done,
>> could we check that the dn contains the domain group container
>> (cn=Groups,cn=$domname I think) and ideally also that the DN ends
>> with this string? Computing the group container and its lenght
>> could be done just once per group, so it wouldn't be too
>> expensive.
>> 
>>> +                    continue; +                } +
>>> } + /* * User already cached in sysdb. Remember the sysdb DN
>>> for later * use by sdap_save_groups() -- 1.8.4.2
>>> 
>> 
>>> From 94722009db7fee6320af1cef5adfad9e0e3356e2 Mon Sep 17
>>> 00:00:00 2001 From: Pavel Reichl <preichl at redhat.com> Date:
>>> Wed, 21 May 2014 09:30:13 +0100 Subject: [PATCH 3/3] SDAP: Add
>>> option to disable use of Token-Groups
>>> 
>>> Disabling use of Token-Groups is mandatory if expansion of
>>> nested groups is not desired (ldap_group_nesting_level = 0) for
>>> AD provider.
>>> 
>>> Resolves: https://fedorahosted.org/sssd/ticket/2294 --- 
>>> src/config/SSSDConfig/__init__.py.in       |  1 + 
>>> src/config/etc/sssd.api.d/sssd-ad.conf     |  1 + 
>>> src/config/etc/sssd.api.d/sssd-ipa.conf    |  1 + 
>>> src/config/etc/sssd.api.d/sssd-ldap.conf   |  1 + 
>>> src/man/sssd-ldap.5.xml                    | 13 +++++++++++++ 
>>> src/providers/ad/ad_opts.h                 |  1 + 
>>> src/providers/ipa/ipa_opts.h               |  1 + 
>>> src/providers/ldap/ldap_opts.h             |  1 + 
>>> src/providers/ldap/sdap.h                  |  1 + 
>>> src/providers/ldap/sdap_async_initgroups.c |  7 +++++-- 10
>>> files changed, 26 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/src/config/SSSDConfig/__init__.py.in
>>> b/src/config/SSSDConfig/__init__.py.in index
>>> e221eba278518b07ead0c39a6ecd7527f86d9b5e..d9b186f73220363f429b51260e6b83bd00a9cdb6
>>> 100644 --- a/src/config/SSSDConfig/__init__.py.in +++
>>> b/src/config/SSSDConfig/__init__.py.in @@ -321,6 +321,7 @@
>>> option_strings = {
>>> 
>>> 'ldap_groups_use_matching_rule_in_chain' : _('Use
>>> LDAP_MATCHING_RULE_IN_CHAIN for group lookups'), 
>>> 'ldap_initgroups_use_matching_rule_in_chain' : _('Use
>>> LDAP_MATCHING_RULE_IN_CHAIN for initgroup lookups'), +
>>> 'ldap_use_tokengroups' : _('Whether to use Token-Groups'), 
>>> 'ldap_min_id' : _('Set lower boundary for allowed IDs from the
>>> LDAP server'), 'ldap_max_id' : _('Set upper boundary for
>>> allowed IDs from the LDAP server'),
>>> 
>>> diff --git a/src/config/etc/sssd.api.d/sssd-ad.conf
>>> b/src/config/etc/sssd.api.d/sssd-ad.conf index
>>> b70e74c0ac17da95574756f54538fd4414f6f807..33d460e828607f3640b9d19572b1319d4c0cc001
>>> 100644 --- a/src/config/etc/sssd.api.d/sssd-ad.conf +++
>>> b/src/config/etc/sssd.api.d/sssd-ad.conf @@ -111,6 +111,7 @@
>>> ldap_idmap_default_domain = str, None, false 
>>> ldap_idmap_default_domain_sid = str, None, false 
>>> ldap_groups_use_matching_rule_in_chain = bool, None, false 
>>> ldap_initgroups_use_matching_rule_in_chain = bool, None, false 
>>> +ldap_use_tokengroups = bool, None, false 
>>> ldap_rfc2307_fallback_to_local_users = bool, None, false
>>> 
>>> [provider/ad/auth] diff --git
>>> a/src/config/etc/sssd.api.d/sssd-ipa.conf
>>> b/src/config/etc/sssd.api.d/sssd-ipa.conf index
>>> 3a3f6a4cf9586b2a81844e38498ba86b504f1f52..11484e7d4bd9a9da15be38ef2a0fe3a9d931cb50
>>> 100644 --- a/src/config/etc/sssd.api.d/sssd-ipa.conf +++
>>> b/src/config/etc/sssd.api.d/sssd-ipa.conf @@ -129,6 +129,7 @@
>>> ldap_idmap_default_domain = str, None, false 
>>> ldap_idmap_default_domain_sid = str, None, false 
>>> ldap_groups_use_matching_rule_in_chain = bool, None, false 
>>> ldap_initgroups_use_matching_rule_in_chain = bool, None, false 
>>> +ldap_use_tokengroups = bool, None, false 
>>> ldap_rfc2307_fallback_to_local_users = bool, None, false 
>>> ipa_server_mode = bool, None, false
>>> 
>>> diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf
>>> b/src/config/etc/sssd.api.d/sssd-ldap.conf index
>>> 0057c080f84d4b5f75107903b03a3cc91f03cc83..fa9cdd69846f142cbeef9b0192360816c9b554fc
>>> 100644 --- a/src/config/etc/sssd.api.d/sssd-ldap.conf +++
>>> b/src/config/etc/sssd.api.d/sssd-ldap.conf @@ -118,6 +118,7 @@
>>> ldap_idmap_default_domain = str, None, false 
>>> ldap_idmap_default_domain_sid = str, None, false 
>>> ldap_groups_use_matching_rule_in_chain = bool, None, false 
>>> ldap_initgroups_use_matching_rule_in_chain = bool, None, false 
>>> +ldap_use_tokengroups = bool, None, false 
>>> ldap_rfc2307_fallback_to_local_users = bool, None, false 
>>> ldap_min_id = int, None, false ldap_max_id = int, None, false 
>>> diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml 
>>> index
>>> d7bb3475d38ca93044632673bb9c7ac6d83c02f6..f33e138009f40ed56860d3c5f7ab5fe4772d5746
>>> 100644 --- a/src/man/sssd-ldap.5.xml +++
>>> b/src/man/sssd-ldap.5.xml @@ -1005,6 +1005,19 @@ 
>>> </varlistentry>
>>> 
>>> <varlistentry> +
>>> <term>ldap_use_tokengroups</term> +
>>> <listitem> +                        <para> +
>>> This options enables of disables use of Token-Groups +
>>> attribute. It is used by AD provider only.
>> 
>> Are you sure? According to the code, the TG lookup is used also
>> if you set ldap_schema to ad..
>> 
>>> +                        </para> +
>>> <para> +                            Default: True +
>>> </para> +                    </listitem> +
>>> </varlistentry> + +                <varlistentry> 
>>> <term>ldap_netgroup_object_class (string)</term> <listitem> 
>>> <para> diff --git a/src/providers/ad/ad_opts.h
>>> b/src/providers/ad/ad_opts.h
>> 
>> The rest of the patches looks good to me but I haven't ran any
>> tests yet.

Patch 0001: Ack to the manpage contents.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlOIieYACgkQeiVVYja6o6NgRgCcCWoaLTE6sCTOcV8nTdBgTJpU
bysAn1Zx1NfbAfBNxYcUEJ9zmrxArdd3
=vvGo
-----END PGP SIGNATURE-----



More information about the sssd-devel mailing list