From 936dbfbed23c2c39204b8550a4ef8c7d127c06e4 Mon Sep 17 00:00:00 2001 From: Justin Stephenson Date: Thu, 23 Oct 2025 10:45:47 -0400 Subject: [PATCH 1/3] SYSDB: Add sysdb_add_bool() Reviewed-by: Alexey Tikhonov Reviewed-by: Sumit Bose --- src/db/sysdb.c | 9 ++++++++- src/db/sysdb_private.h | 2 ++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/db/sysdb.c b/src/db/sysdb.c index 1faa11b16e0..e4c9579b747 100644 --- a/src/db/sysdb.c +++ b/src/db/sysdb.c @@ -627,7 +627,7 @@ int sysdb_attrs_add_base64_blob(struct sysdb_attrs *attrs, const char *name, int sysdb_attrs_add_bool(struct sysdb_attrs *attrs, const char *name, bool value) { - if(value) { + if (value) { return sysdb_attrs_add_string(attrs, name, "TRUE"); } @@ -1698,6 +1698,13 @@ int sysdb_delete_string(struct ldb_message *msg, return sysdb_ldb_msg_string_helper(msg, LDB_FLAG_MOD_DELETE, attr, value); } +int sysdb_add_bool(struct ldb_message *msg, + const char *attr, bool value) +{ + return sysdb_ldb_msg_string_helper(msg, LDB_FLAG_MOD_ADD, attr, + value ? "TRUE" : "FALSE"); +} + static int sysdb_ldb_msg_ulong_helper(struct ldb_message *msg, int flags, const char *attr, unsigned long value) { diff --git a/src/db/sysdb_private.h b/src/db/sysdb_private.h index cd1d3ec9c8e..43fee5d2e47 100644 --- a/src/db/sysdb_private.h +++ b/src/db/sysdb_private.h @@ -189,6 +189,8 @@ int sysdb_replace_string(struct ldb_message *msg, const char *attr, const char *value); int sysdb_delete_string(struct ldb_message *msg, const char *attr, const char *value); +int sysdb_add_bool(struct ldb_message *msg, + const char *attr, bool value); int sysdb_add_ulong(struct ldb_message *msg, const char *attr, unsigned long value); int sysdb_replace_ulong(struct ldb_message *msg, From dc9188bcdefb054eef9f571c7253205bbae1ed66 Mon Sep 17 00:00:00 2001 From: Justin Stephenson Date: Mon, 18 Aug 2025 10:07:17 -0400 Subject: [PATCH 2/3] SYSDB: Dont store gid 0 for non-posix groups Remove logic to store 'gidNumber: 0' in the cache for non-posix groups. Instead do not add a gidNumber at all, this avoids performance hit due to huge GID=0 index when a large number of non-posix groups are stored. Reviewed-by: Alexey Tikhonov Reviewed-by: Sumit Bose --- src/db/sysdb.h | 6 +- src/db/sysdb_ops.c | 66 +++++++++---------- src/providers/ldap/sdap_async_groups.c | 1 - src/providers/ldap/sdap_async_initgroups.c | 7 +- src/providers/ldap/sdap_async_initgroups_ad.c | 2 +- src/providers/ldap/sdap_async_nested_groups.c | 12 +--- src/tests/cmocka/test_sysdb_views.c | 6 +- src/tests/sysdb-tests.c | 7 -- 8 files changed, 42 insertions(+), 65 deletions(-) diff --git a/src/db/sysdb.h b/src/db/sysdb.h index d0f10e2ccc6..4494f6ed2e6 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -241,7 +241,7 @@ #define SYSDB_GRGID_MPG_FILTER "(|(&("SYSDB_GC")("SYSDB_GIDNUM"=%lu))(&("SYSDB_UC")("SYSDB_GIDNUM"=%lu)("SYSDB_UIDNUM"=%lu)))" #define SYSDB_GRENT_MPG_FILTER "("SYSDB_MPGC")" -#define SYSDB_INITGR_FILTER "(&("SYSDB_GC")("SYSDB_GIDNUM"=*))" +#define SYSDB_INITGR_FILTER "("SYSDB_GC")" #define SYSDB_NETGR_FILTER "(&("SYSDB_NC")(|("SYSDB_NAME_ALIAS"=%s)("SYSDB_NAME_ALIAS"=%s)("SYSDB_NAME"=%s)))" #define SYSDB_NETGR_TRIPLES_FILTER "(|("SYSDB_NAME_ALIAS"=%s)("SYSDB_NAME"=%s)("SYSDB_NAME_ALIAS"=%s)("SYSDB_MEMBEROF"=%s))" @@ -1166,7 +1166,9 @@ int sysdb_add_user(struct sss_domain_info *domain, /* Add group (only basic attrs and w/o checks) */ int sysdb_add_basic_group(struct sss_domain_info *domain, - const char *name, gid_t gid); + const char *name, + bool is_posix, + gid_t gid); /* Add group (all checks) */ int sysdb_add_group(struct sss_domain_info *domain, diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index b7157364d9d..a248e4720f5 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -1959,11 +1959,6 @@ int sysdb_add_user(struct sss_domain_info *domain, ret = sysdb_attrs_get_bool(attrs, SYSDB_POSIX, &posix); if (ret == ENOENT) { posix = true; - ret = sysdb_attrs_add_bool(attrs, SYSDB_POSIX, true); - if (ret) { - DEBUG(SSSDBG_TRACE_LIBS, "Failed to add posix attribute.\n"); - goto done; - } } else if (ret != EOK) { DEBUG(SSSDBG_TRACE_LIBS, "Failed to get posix attribute.\n"); goto done; @@ -2023,12 +2018,20 @@ int sysdb_add_user(struct sss_domain_info *domain, /* =Add-Basic-Group-NO-CHECKS============================================= */ int sysdb_add_basic_group(struct sss_domain_info *domain, - const char *name, gid_t gid) + const char *name, + bool is_posix, + gid_t gid) { struct ldb_message *msg; int ret; TALLOC_CTX *tmp_ctx; + if (is_posix && gid == 0) { + DEBUG(SSSDBG_OP_FAILURE, "Failure adding [%s], POSIX groups with gid==0 " + "are not supported.\n", name); + return EINVAL; + } + tmp_ctx = talloc_new(NULL); if (!tmp_ctx) { return ENOMEM; @@ -2046,14 +2049,19 @@ int sysdb_add_basic_group(struct sss_domain_info *domain, ERROR_OUT(ret, ENOMEM, done); } + ret = sysdb_add_bool(msg, SYSDB_POSIX, is_posix); + if (ret) goto done; + ret = sysdb_add_string(msg, SYSDB_OBJECTCATEGORY, SYSDB_GROUP_CLASS); if (ret) goto done; ret = sysdb_add_string(msg, SYSDB_NAME, name); if (ret) goto done; - ret = sysdb_add_ulong(msg, SYSDB_GIDNUM, (unsigned long)gid); - if (ret) goto done; + if (is_posix) { + ret = sysdb_add_ulong(msg, SYSDB_GIDNUM, (unsigned long)gid); + if (ret) goto done; + } /* creation time */ ret = sysdb_add_ulong(msg, SYSDB_CREATE_TIME, (unsigned long)time(NULL)); @@ -2156,22 +2164,6 @@ int sysdb_add_group(struct sss_domain_info *domain, } } - /* try to add the group */ - ret = sysdb_add_basic_group(domain, name, gid); - if (ret) { - DEBUG(SSSDBG_TRACE_LIBS, - "sysdb_add_basic_group failed for: %s with gid: " - "[%"SPRIgid"].\n", name, gid); - goto done; - } - - ret = sysdb_create_ts_grp(domain, name, cache_timeout, now); - if (ret != EOK) { - DEBUG(SSSDBG_MINOR_FAILURE, - "Cannot set timestamp cache attributes for a group\n"); - /* Not fatal */ - } - if (!attrs) { attrs = sysdb_new_attrs(tmp_ctx); if (!attrs) { @@ -2184,22 +2176,27 @@ int sysdb_add_group(struct sss_domain_info *domain, ret = sysdb_attrs_get_bool(attrs, SYSDB_POSIX, &posix); if (ret == ENOENT) { posix = true; - ret = sysdb_attrs_add_bool(attrs, SYSDB_POSIX, true); - if (ret) { - DEBUG(SSSDBG_TRACE_LIBS, "Failed to add posix attribute.\n"); - goto done; - } } else if (ret != EOK) { DEBUG(SSSDBG_TRACE_LIBS, "Failed to get posix attribute.\n"); goto done; } - if (posix && gid == 0) { - DEBUG(SSSDBG_OP_FAILURE, "Can't store posix user with gid=0.\n"); - ret = EINVAL; + /* try to add the group */ + ret = sysdb_add_basic_group(domain, name, posix, gid); + if (ret) { + DEBUG(SSSDBG_OP_FAILURE, + "sysdb_add_basic_group failed for: %s with gid: " + "[%"SPRIgid"].\n", name, gid); goto done; } + ret = sysdb_create_ts_grp(domain, name, cache_timeout, now); + if (ret != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, + "Cannot set timestamp cache attributes for a group\n"); + /* Not fatal */ + } + if (!now) { now = time(NULL); } @@ -2286,7 +2283,7 @@ int sysdb_add_incomplete_group(struct sss_domain_info *domain, } /* try to add the group */ - ret = sysdb_add_basic_group(domain, name, gid); + ret = sysdb_add_basic_group(domain, name, posix, gid); if (ret) goto done; if (!now) { @@ -2315,9 +2312,6 @@ int sysdb_add_incomplete_group(struct sss_domain_info *domain, (now + domain->group_timeout) : (now-1)); if (ret) goto done; - ret = sysdb_attrs_add_bool(attrs, SYSDB_POSIX, posix); - if (ret) goto done; - if (original_dn) { ret = sysdb_attrs_add_string(attrs, SYSDB_ORIG_DN, original_dn); if (ret) goto done; diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c index f36e5c5837a..965a6726ed9 100644 --- a/src/providers/ldap/sdap_async_groups.c +++ b/src/providers/ldap/sdap_async_groups.c @@ -586,7 +586,6 @@ static int sdap_save_group(TALLOC_CTX *memctx, } if (need_filter) { posix_group = false; - gid = 0; ret = sysdb_attrs_add_bool(group_attrs, SYSDB_POSIX, false); if (ret != EOK) { diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c index 8ce1f6cd484..0a2a774ebe9 100644 --- a/src/providers/ldap/sdap_async_initgroups.c +++ b/src/providers/ldap/sdap_async_initgroups.c @@ -44,7 +44,7 @@ errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb, const char *original_dn; const char *uuid = NULL; char **missing; - gid_t gid; + gid_t gid = 0; int ret; errno_t sret; bool in_transaction = false; @@ -158,7 +158,6 @@ errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb, groupname, (unsigned long)gid); } else { posix = false; - gid = 0; DEBUG(SSSDBG_TRACE_INTERNAL, "Group [%s] cannot be mapped. " @@ -174,9 +173,8 @@ errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb, DEBUG(SSSDBG_TRACE_LIBS, "The group %s gid was %s\n", groupname, ret == ENOENT ? "missing" : "zero"); DEBUG(SSSDBG_TRACE_FUNC, - "Marking group %s as non-POSIX and setting GID=0!\n", + "Marking group %s as non-POSIX!\n", groupname); - gid = 0; posix = false; } else if (ret) { DEBUG(SSSDBG_CRIT_FAILURE, @@ -222,7 +220,6 @@ errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb, if (need_filter) { posix = false; - gid = 0; } DEBUG(SSSDBG_TRACE_INTERNAL, diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c b/src/providers/ldap/sdap_async_initgroups_ad.c index fb80c92429d..c8f82d7ed5f 100644 --- a/src/providers/ldap/sdap_async_initgroups_ad.c +++ b/src/providers/ldap/sdap_async_initgroups_ad.c @@ -640,7 +640,7 @@ errno_t sdap_ad_save_group_membership_with_idmapping(const char *username, } ret = sysdb_add_incomplete_group(domain, name, gid, - NULL, sid, NULL, false, now); + NULL, sid, NULL, gid != 0, now); if (ret == ERR_GID_DUPLICATED) { /* In case o group id-collision, do: * - Delete the group from sysdb diff --git a/src/providers/ldap/sdap_async_nested_groups.c b/src/providers/ldap/sdap_async_nested_groups.c index 8a97c9db888..54e0f9e271f 100644 --- a/src/providers/ldap/sdap_async_nested_groups.c +++ b/src/providers/ldap/sdap_async_nested_groups.c @@ -291,17 +291,7 @@ sdap_nested_group_hash_group(struct sdap_nested_group_ctx *group_ctx, DEBUG(SSSDBG_TRACE_ALL, "The group's gid was %s\n", ret == ENOENT ? "missing" : "zero"); DEBUG(SSSDBG_TRACE_INTERNAL, - "Marking group as non-POSIX and setting GID=0!\n"); - - if (ret == ENOENT || !posix_group) { - ret = sysdb_attrs_add_uint32(group, - map[SDAP_AT_GROUP_GID].sys_name, 0); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, - "Failed to add a GID to non-POSIX group!\n"); - return ret; - } - } + "Marking group as non-POSIX!\n"); ret = sysdb_attrs_add_bool(group, SYSDB_POSIX, false); if (ret != EOK) { diff --git a/src/tests/cmocka/test_sysdb_views.c b/src/tests/cmocka/test_sysdb_views.c index 62071f8f424..07190b2d30c 100644 --- a/src/tests/cmocka/test_sysdb_views.c +++ b/src/tests/cmocka/test_sysdb_views.c @@ -1253,13 +1253,15 @@ static void enum_test_add_groups(struct sysdb_test_ctx *test_ctx, char *gr_name; for (i = 0; groupnames[i] != NULL; i++) { - attrs = talloc(test_ctx, struct sysdb_attrs); + attrs = sysdb_new_attrs(test_ctx); assert_non_null(attrs); gr_name = sss_create_internal_fqname(test_ctx, groupnames[i], test_ctx->domain->name); + + sysdb_attrs_add_bool(attrs, SYSDB_POSIX, false); ret = sysdb_store_group(test_ctx->domain, gr_name, - 0, NULL, 1, 1234 + i); + 0, attrs, 1, 1234 + i); assert_int_equal(ret, EOK); enum_test_group_override(test_ctx, gr_name, diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c index c44a3ab457d..92fd9656569 100644 --- a/src/tests/sysdb-tests.c +++ b/src/tests/sysdb-tests.c @@ -1679,9 +1679,6 @@ START_TEST (test_sysdb_add_nonposix_user) id = ldb_msg_find_attr_as_uint64(res->msgs[0], SYSDB_UIDNUM, 123); ck_assert_msg(id == 0, "Wrong UID value"); - id = ldb_msg_find_attr_as_uint64(res->msgs[0], SYSDB_GIDNUM, 123); - ck_assert_msg(id == 0, "Wrong GID value"); - talloc_free(test_ctx); } END_TEST @@ -1696,7 +1693,6 @@ static void add_nonposix_incomplete_group(struct sysdb_test_ctx *test_ctx, const char *attrval; const char *fq_name; int ret; - uint64_t id; /* Create group */ fq_name = sss_create_internal_fqname(test_ctx, groupname, test_ctx->domain->name); @@ -1712,9 +1708,6 @@ static void add_nonposix_incomplete_group(struct sysdb_test_ctx *test_ctx, attrval = ldb_msg_find_attr_as_string(msg, SYSDB_POSIX, NULL); sss_ck_fail_if_msg(strcasecmp(attrval, "false") != 0, "Got bad attribute value."); - - id = ldb_msg_find_attr_as_uint64(msg, SYSDB_GIDNUM, 123); - ck_assert_msg(id == 0, "Wrong GID value"); } START_TEST (test_sysdb_add_nonposix_group) From d953045ff29a86ed1ba599b7dd57828835ee3d37 Mon Sep 17 00:00:00 2001 From: Justin Stephenson Date: Thu, 23 Oct 2025 11:25:42 -0400 Subject: [PATCH 3/3] SDAP: Remove sdap_store_group_with_gid() It is no longer needed as we no longer want to store 'gid: 0' for non-POSIX groups. Reviewed-by: Alexey Tikhonov Reviewed-by: Sumit Bose --- src/providers/ldap/sdap_async_groups.c | 46 +++----------------------- 1 file changed, 4 insertions(+), 42 deletions(-) diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c index 965a6726ed9..6d0c7e49907 100644 --- a/src/providers/ldap/sdap_async_groups.c +++ b/src/providers/ldap/sdap_async_groups.c @@ -335,43 +335,6 @@ static int sdap_fill_memberships(struct sdap_options *opts, return ret; } -/* ==Save-Group-Entry===================================================== */ - - /* FIXME: support non legacy */ - /* FIXME: support storing additional attributes */ - -static errno_t -sdap_store_group_with_gid(struct sss_domain_info *domain, - const char *name, - gid_t gid, - struct sysdb_attrs *group_attrs, - uint64_t cache_timeout, - bool posix_group, - time_t now) -{ - errno_t ret; - - /* make sure that non-POSIX (empty or explicit gid=0) groups have the - * gidNumber set to zero even if updating existing group */ - if (!posix_group) { - ret = sysdb_attrs_add_uint32(group_attrs, SYSDB_GIDNUM, 0); - if (ret) { - DEBUG(SSSDBG_OP_FAILURE, - "Could not set explicit GID 0 for %s\n", name); - return ret; - } - } - - ret = sysdb_store_group(domain, name, gid, group_attrs, - cache_timeout, now); - if (ret) { - DEBUG(SSSDBG_OP_FAILURE, "Could not store group %s\n", name); - return ret; - } - - return ret; -} - static errno_t sdap_process_ghost_members(struct sysdb_attrs *attrs, struct sdap_options *opts, @@ -745,13 +708,12 @@ static int sdap_save_group(TALLOC_CTX *memctx, } DEBUG(SSSDBG_TRACE_FUNC, "Storing info for group %s\n", group_name); - ret = sdap_store_group_with_gid(dom, group_name, gid, group_attrs, - dom->group_timeout, - posix_group, now); + ret = sysdb_store_group(dom, group_name, gid, group_attrs, + dom->group_timeout, now); if (ret) { DEBUG(SSSDBG_MINOR_FAILURE, - "Could not store group with GID: [%s]\n", - sss_strerror(ret)); + "Could not store group [%s] with GID [%u]: [%s]\n", + group_name, gid, sss_strerror(ret)); goto done; }