Skip to content

Dont store gid 0 for non-posix groups #8075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alexey-tikhonov merged 3 commits into from
Nov 12, 2025
Merged

Dont store gid 0 for non-posix groups #8075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
936dbfb
SYSDB: Add sysdb_add_bool()
justin-stephenson Oct 23, 2025
dc9188b
SYSDB: Dont store gid 0 for non-posix groups
justin-stephenson Aug 18, 2025
d953045
SDAP: Remove sdap_store_group_with_gid()
justin-stephenson Oct 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/db/sysdb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@
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");
}

Expand Down Expand Up @@ -1698,6 +1698,13 @@
return sysdb_ldb_msg_string_helper(msg, LDB_FLAG_MOD_DELETE, attr, value);
}

int sysdb_add_bool(struct ldb_message *msg,

Check warning on line 1701 in src/db/sysdb.c

View workflow job for this annotation

GitHub Actions / cppcheck 

The function 'sysdb_add_bool' is never used.
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)
{
Expand Down
6 changes: 4 additions & 2 deletions src/db/sysdb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")"
Copy link
Member

@alexey-tikhonov alexey-tikhonov Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this to be able to return non-posix groups via IFP?

Copy link
Contributor Author

@justin-stephenson justin-stephenson Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before the PR non-posix groups were stored with gidNumber: 0 so this filter made sense. Now we need to remove the gidNumber part of the search filter constraint to find the relevant nonPosix groups.

Is this to be able to return non-posix groups via IFP?

Only no if these groups are stored with gidNumber: 0 based on previous behavior of the PR. What about systems updated to this SSSD version and still have groups stored in the old format? Do we need to handle this case?

Copy link
Member

@alexey-tikhonov alexey-tikhonov Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we need to remove the gidNumber part of the search filter constraint to find the relevant nonPosix groups.

That's what I'm asking: who consumes non-posix groups in INITGR list?
Is it only IFP?

What about systems updated to this SSSD version and still have groups stored in the old format?

Your patch looks backward compatible, no?
If "no" then new cache-upgrade proc is needed :-/

Copy link
Contributor Author

@justin-stephenson justin-stephenson Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I'm asking: who consumes non-posix groups in INITGR list?
Is it only IFP?

As far as I know, yes. I hope infopipe system tests would provide coverage for this, but I could test it manually with ipa-tuura if needed.


#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))"
Expand Down Expand Up @@ -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,
Expand Down
66 changes: 30 additions & 36 deletions src/db/sysdb_ops.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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));
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions src/db/sysdb_private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
47 changes: 4 additions & 43 deletions src/providers/ldap/sdap_async_groups.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -586,7 +549,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) {
Expand Down Expand Up @@ -746,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;
}

Expand Down
7 changes: 2 additions & 5 deletions src/providers/ldap/sdap_async_initgroups.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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. "
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/providers/ldap/sdap_async_initgroups_ad.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 1 addition & 11 deletions src/providers/ldap/sdap_async_nested_groups.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 4 additions & 2 deletions src/tests/cmocka/test_sysdb_views.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 0 additions & 7 deletions src/tests/sysdb-tests.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -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)
Expand Down
Loading