[SSSD] [PATCH] Two minor patches for tokenGroups nested group processing

Lukas Slebodnik lslebodn at redhat.com
Mon Aug 11 08:05:56 UTC 2014


On (22/07/14 18:43), Jakub Hrozek wrote:
>On Thu, Mar 13, 2014 at 12:00:47PM +0100, Sumit Bose wrote:
>> On Wed, Mar 12, 2014 at 11:20:42PM +0100, Jakub Hrozek wrote:
>> > Hi,
>> > 
>> > the attached two patches are not strictly related to tokenGroups
>> > processing, but it's very easy to reproduce the problem that way. The
>> > issue is only confusing DEBUG messages, but it has already cost me
>> > several hours in processing logs from an SSSD user, so I think a fix is
>> > due, at least for master.
>> > 
>> > See the patches and the commit messages for more details.
>> 
>> as a first note, the second patch makes nestedgroups-tests fail because
>> sdap_idmap_domain_has_algorithmic_mapping() must be made available to
>> the test and sdap_idmap_ctx must be initialized.
>> 
>> But since this 'only' influences the tests I will run some tests with
>> the current version of the patches.
>> 
>> bye,
>> Sumit
>
>I completely forgot about these patches until I was doing cleanup of my
>git branches. Attached is a new revision..

>From 3c4ce4a2b9f58e0b50f9ad48d07110b30bc74cd9 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Tue, 11 Mar 2014 17:39:31 +0100
>Subject: [PATCH 2/2] Only check GID if ID-mapping
>
>---
> Makefile.am                                   | 11 ++++++++---
> src/providers/ldap/sdap_async_nested_groups.c |  9 ++++++++-
> src/tests/cmocka/test_nested_groups.c         | 20 ++++++++++++++++++++
> 3 files changed, 36 insertions(+), 4 deletions(-)
>
>diff --git a/Makefile.am b/Makefile.am
>index e3592868ce29b569a71f6ad74bc24cc617301b34..783a8922ede16ea3cc3eb6df568773126c5661fb 100644
>--- a/Makefile.am
>+++ b/Makefile.am
>@@ -1740,15 +1740,20 @@ fqnames_tests_LDADD = \
> nestedgroups_tests_SOURCES = \
>     $(TEST_MOCK_OBJ) \
>     $(TEST_MOCK_PROVIDER_OBJ) \
>+    src/providers/ldap/sdap_idmap.c \
>     src/tests/cmocka/test_nested_groups.c \
>-    src/providers/ldap/sdap_async_nested_groups.c
>+    src/providers/ldap/sdap_async_nested_groups.c \
>+    $(NULL)
> nestedgroups_tests_CFLAGS = \
>-    $(AM_CFLAGS)
>+    $(AM_CFLAGS) \
>+    $(NULL)
> nestedgroups_tests_LDADD = \
>     $(CMOCKA_LIBS) \
>     $(SSSD_LIBS) \
>     $(SSSD_INTERNAL_LTLIBS) \
>-    libsss_test_common.la
>+    libsss_idmap.la \
>+    libsss_test_common.la \
>+    $(NULL)
> 
> test_sss_idmap_SOURCES = \
>     src/tests/cmocka/test_sss_idmap.c
>diff --git a/src/providers/ldap/sdap_async_nested_groups.c b/src/providers/ldap/sdap_async_nested_groups.c
>index 305afbc9d4f153fe2daa6289cc318b1183be72ca..5398b14bc0e163c122def1c3d729b28d72873b82 100644
>--- a/src/providers/ldap/sdap_async_nested_groups.c
>+++ b/src/providers/ldap/sdap_async_nested_groups.c
>@@ -34,6 +34,7 @@
> #include "providers/ldap/ldap_common.h"
> #include "providers/ldap/sdap_async.h"
> #include "providers/ldap/sdap_async_private.h"
>+#include "providers/ldap/sdap_idmap.h"
> 
> #define sdap_nested_group_sysdb_search_users(domain, filter) \
>     sdap_nested_group_sysdb_search((domain), (filter), true)
>@@ -242,6 +243,7 @@ sdap_nested_group_hash_group(struct sdap_nested_group_ctx *group_ctx,
>     errno_t ret;
>     int32_t ad_group_type;
>     bool posix_group = true;
>+    bool use_id_mapping;
> 
>     if (group_ctx->opts->schema_type == SDAP_SCHEMA_AD) {
>         ret = sysdb_attrs_get_int32_t(group, SYSDB_GROUP_TYPE, &ad_group_type);
>@@ -265,7 +267,12 @@ sdap_nested_group_hash_group(struct sdap_nested_group_ctx *group_ctx,
>         }
>     }
> 
>-    if (posix_group) {
>+    use_id_mapping = sdap_idmap_domain_has_algorithmic_mapping(
>+                                                          group_ctx->opts->idmap_ctx,
>+                                                          group_ctx->domain->name,
>+                                                          group_ctx->domain->domain_id);
>+
>+    if (posix_group && !use_id_mapping) {
>         ret = sysdb_attrs_get_uint32_t(group, map[SDAP_AT_GROUP_GID].sys_name,
>                                        &gid);
>     }

270 use_id_mapping = sdap_idmap_domain_has_algorithmic_mapping(
271 group_ctx->opts->idmap_ctx,
272 group_ctx->domain->name,
273 group_ctx->domain->domain_id);
274
275 if (posix_group && !use_id_mapping) {
        // 'posix_group' can be true and 'use_id_mapping' is not equal to 0
        // sysdb_attrs_get_uint32_t will not be called.
276     ret = sysdb_attrs_get_uint32_t(group, map[SDAP_AT_GROUP_GID].sys_name,
277                                    &gid);
278 }
279 if (!posix_group || ret == ENOENT || (ret == EOK && gid == 0)) {
                        ^^^^^^^^^^^^^
            'ret' needn't be ENOENT because function 'sysdb_attrs_get_int32_t'
             could be called on line 249

            'gid' will not be initialised.

LS



More information about the sssd-devel mailing list