[autobackport: sssd-2-9] Dont store gid 0 for non-posix groups

src/db/sysdb.c

@@ -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");
     }
 
@@ -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,
+                   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)
 {

src/db/sysdb.h

@@ -242,7 +242,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))"
@@ -1169,7 +1169,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,

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;

src/db/sysdb_private.h

@@ -204,6 +204,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,

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,
@@ -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) {
@@ -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;
     }
 

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,

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

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) {

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,

src/tests/sysdb-tests.c

@@ -1625,9 +1625,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
@@ -1642,7 +1639,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);
@@ -1658,9 +1654,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)