Attachment 905729 Details for Bug 1089738 – Redesign automatic GID allocation

[patch] Redesign automatic GID allocation

0001-Redesign-automatic-GID-allocation.patch (text/plain), 13.76 KB, created by Stephen Gallagher on 2014-06-09 18:46:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stephen Gallagher

Created: 2014-06-09 18:46:25 UTC

Size: 13.76 KB

patch

obsolete

>From 93811bbe4a7fed5e3af971e0b320f09c6cf154d7 Mon Sep 17 00:00:00 2001
>From: Stephen Gallagher <sgallagh@redhat.com>
>Date: Mon, 9 Jun 2014 10:34:02 -0400
>Subject: [PATCH] Redesign automatic GID allocation
>
>Previously, this allocation was optimized for an outdated
>deployment style (that of /etc/group alongside nss_db). The issue
>here is that this results in extremely poor performance when using
>SSSD, Winbind or nss_ldap.
>
>There were actually three serious bugs here that have been addressed:
>
>1) Running getgrent() loops won't work in most SSSD or Winbind
>environments, as full group enumeration is disabled by default.
>This could easily result in auto-allocating a group that was
>already in use. (This might result in a security issue as well, if
>the shared GID is a privileged group).
>
>2) For system groups, the loop was always iterating through the
>complete SYS_GID_MIN->SYS_GID_MAX range. On SSSD and Winbind, this
>means hundreds of round-trips to LDAP (unless the GIDs were
>specifically configured to be ignored by the SSSD or winbindd).
>To a user with a slow connection to their LDAP server, this would
>appear as if groupadd -r was hung. (Though it would eventually
>complete).
>
>3) This patch also adds better error-handling for errno from
>getgrgid(), since if this function returns an unexpected error, we
>should not be treating it as "ID is available". This could result
>in assigning a GID that was already in use, with all the same
>issues as 1) above.
>
>This patch changes the algorithm to be more favorable for LDAP
>environments, at the expense of some performance when using nss_db.
>Given that the DB is a local service, this should have a negligible
>effect from a user's perspective.
>
>With the new algorithm, we simply iterate down from [SYS_]GID_MAX
>with getgrgid() (and gr_locate_gid()) until we come to the first
>unused GID. We then select that and return it.
>---
> libmisc/find_new_gid.c | 332 ++++++++++++++++++++++++++++---------------------
> 1 file changed, 193 insertions(+), 139 deletions(-)
>
>diff --git a/libmisc/find_new_gid.c b/libmisc/find_new_gid.c
>index 05f5622edb79069d9a43d3f9c69a463b6b71141a..eafc914d7ffbaf6cac0e9db513eebfca773f49ee 100644
>--- a/libmisc/find_new_gid.c
>+++ b/libmisc/find_new_gid.c
>@@ -39,6 +39,99 @@
> #include "getdef.h"
> 
> /*
>+ * _get_ranges - Get the minimum and maximum ID ranges for the search
>+ *
>+ * This function will return the minimum and maximum ranges for IDs
>+ *
>+ * 0: The function completed successfully
>+ * EINVAL: The provided ranges are impossible (such as maximum < minimum)
>+ *
>+ * preferred_min: The special-case minimum value for a specifically-
>+ * requested ID, which may be lower than the standard min_id
>+ */
>+static int _get_ranges(bool sys_group, gid_t *min_id, gid_t *max_id, gid_t *preferred_min)
>+{
>+    gid_t gid_def_max = 0;
>+
>+    if (sys_group) {
>+        /* System groups */
>+
>+        /* A requested ID is allowed to be below the autoselect range */
>+        *preferred_min = (gid_t) 1;
>+
>+        /* Get the minimum ID range from login.defs or default to 101 */
>+        *min_id = (gid_t) getdef_ulong ("SYS_GID_MIN", 101UL);
>+
>+        /*
>+         * If SYS_GID_MAX is unspecified, we should assume it to be one
>+         * less than the GID_MIN (which is reserved for non-system accounts)
>+         */
>+        gid_def_max = (gid_t) getdef_ulong ("GID_MIN", 1000UL) - 1;
>+        *max_id = (gid_t) getdef_ulong ("SYS_GID_MAX", (unsigned long) gid_def_max);
>+    } else {
>+        /* Non-system groups */
>+
>+        /* Get the values from login.defs or use reasonable defaults */
>+        *min_id = (gid_t) getdef_ulong ("GID_MIN", 1000UL);
>+        *max_id = (gid_t) getdef_ulong ("GID_MAX", 60000UL);
>+
>+        /*
>+         * The preferred minimum should match the standard ID minimum
>+         * for non-system groups.
>+         */
>+        *preferred_min = *min_id;
>+    }
>+
>+    /* Check that the ranges make sense */
>+    if (*max_id < *min_id) {
>+        return EINVAL;
>+    }
>+
>+    return 0;
>+}
>+
>+/*
>+ * _check_gid - See if the requested GID is available
>+ *
>+ * On success, return 0
>+ * If the ID is in use, return EEXIST
>+ * If the ID is outside the range, return ERANGE
>+ * In other cases, return errno from getgrgid()
>+ */
>+static int _check_gid(const gid_t gid,
>+                      const gid_t gid_min,
>+                      const gid_t gid_max)
>+{
>+    /* First test that the preferred ID is in the range */
>+    if (gid < gid_min
>+        || gid > gid_max) {
>+        return ERANGE;
>+    }
>+
>+        /* Check if the GID exists according to NSS */
>+    errno = 0;
>+    if (getgrgid (gid) != NULL) {
>+        return EEXIST;
>+    } else {
>+        /* getgrgid() was NULL, check whether this was
>+         * due to an error, so we can report it.
>+         */
>+        if (errno != 0) {
>+            return errno;
>+        }
>+
>+        /* Check also the local database in case of uncommitted
>+         * changes */
>+        if (gr_locate_gid (gid) != NULL) {
>+            return EEXIST;
>+        }
>+    }
>+
>+    /* If we've made it here, the GID must be available */
>+    return 0;
>+}
>+
>+/*
>  * find_new_gid - Find a new unused GID.
>  *
>  * If successful, find_new_gid provides an unused group ID in the
>@@ -53,161 +146,122 @@ int find_new_gid (bool sys_group,
>                   /*@null@*/gid_t const *preferred_gid)
> {
> 	const struct group *grp;
>-	gid_t gid_min, gid_max, group_id;
>-	bool *used_gids;
>+	gid_t gid_min, gid_max, preferred_min, group_id, id;
>+	int result;
> 
> 	assert (gid != NULL);
> 
>-	if (!sys_group) {
>-		gid_min = (gid_t) getdef_ulong ("GID_MIN", 1000UL);
>-		gid_max = (gid_t) getdef_ulong ("GID_MAX", 60000UL);
>-		if (gid_max < gid_min) {
>-			(void) fprintf (stderr,
>-			                _("%s: Invalid configuration: GID_MIN (%lu), GID_MAX (%lu)\n"),
>-			                Prog, (unsigned long) gid_min, (unsigned long) gid_max);
>-			return -1;
>-		}
>-	} else {
>-		gid_min = (gid_t) 1;
>-		gid_max = (gid_t) getdef_ulong ("GID_MIN", 1000UL) - 1;
>-		gid_max = (gid_t) getdef_ulong ("SYS_GID_MAX", (unsigned long) gid_max);
>-		if (gid_max < gid_min) {
>-			(void) fprintf (stderr,
>-			                _("%s: Invalid configuration: SYS_GID_MIN (%lu), GID_MIN (%lu), SYS_GID_MAX (%lu)\n"),
>-			                Prog, (unsigned long) gid_min, getdef_ulong ("GID_MIN", 1000UL), (unsigned long) gid_max);
>-			return -1;
>-		}
>+	/*
>+	 * First, figure out what ID range is appropriate for
>+	 * automatic assignment
>+	 */
>+	result = _get_ranges(sys_group, &gid_min, &gid_max, &preferred_min);
>+	if (result == EINVAL) {
>+        (void) fprintf (stderr,
>+                        _("%s: Invalid configuration: GID_MIN (%lu), GID_MAX (%lu)\n"),
>+                        Prog, (unsigned long) gid_min, (unsigned long) gid_max);
>+        return -1;
> 	}
>-	used_gids = malloc (sizeof (bool) * (gid_max +1));
>-	if (NULL == used_gids) {
>-		fprintf (stderr,
>-		         _("%s: failed to allocate memory: %s\n"),
>-		         Prog, strerror (errno));
>-		return -1;
>-	}
>-	memset (used_gids, false, sizeof (bool) * (gid_max + 1));
> 
>-	if (   (NULL != preferred_gid)
>-	    && (*preferred_gid >= gid_min)
>-	    && (*preferred_gid <= gid_max)
>-	    /* Check if the user exists according to NSS */
>-	    && (getgrgid (*preferred_gid) == NULL)
>-	    /* Check also the local database in case of uncommitted
>-	     * changes */
>-	    && (gr_locate_gid (*preferred_gid) == NULL)) {
>-		*gid = *preferred_gid;
>-		free (used_gids);
>-		return 0;
>+	/* Check if the preferred GID is available */
>+	if (preferred_gid) {
>+        result = _check_gid(*preferred_gid, preferred_min, gid_max);
>+        if (result == 0) {
>+            *gid = *preferred_gid;
>+            return 0;
>+        } else if (result == EEXIST || result == ERANGE) {
>+            /*
>+             * Continue on below. At this time, we won't
>+             * treat these two cases differently.
>+             */
>+        } else {
>+            /*
>+             * An unexpected error occurred. We should report
>+             * this and fail the group creation.
>+             * This differs from the automatic creation
>+             * behavior below, since if a specific GID was
>+             * requested and generated an error, the user is
>+             * more likely to want to stop and address the
>+             * issue.
>+             */
>+            fprintf (stderr,
>+                     _("%s: Encountered error attempting to use "
>+                        "preferred GID: %s\n"),
>+                        Prog, strerror(result));
>+            return -1;
>+        }
> 	}
> 
>-        /* if we did not find free preffered system gid, we start to look for
>-         * one in the range assigned to dynamic system IDs */
>-        if (sys_group)
>-                gid_min = (gid_t) getdef_ulong ("SYS_GID_MIN", 101UL);
>-
> 	/*
> 	 * Search the entire group file,
>-	 * looking for the largest unused value.
>+	 * looking for the next unused value.
> 	 *
> 	 * We check the list of groups according to NSS (setgrent/getgrent),
> 	 * but we also check the local database (gr_rewind/gr_next) in case
> 	 * some groups were created but the changes were not committed yet.
> 	 */
> 	if (sys_group) {
>-		gid_t id;
>-		/* setgrent / getgrent / endgrent can be very slow with
>-		 * LDAP configurations (and many accounts).
>-		 * Since there is a limited amount of IDs to be tested
>-		 * for system accounts, we just check the existence
>-		 * of IDs with getgrgid.
>-		 */
>-		group_id = gid_max;
>-		for (id = gid_max; id >= gid_min; id--) {
>-			if (getgrgid (id) != NULL) {
>-				group_id = id - 1;
>-				used_gids[id] = true;
>-			}
>-		}
>-
>-		(void) gr_rewind ();
>-		while ((grp = gr_next ()) != NULL) {
>-			if ((grp->gr_gid <= group_id) && (grp->gr_gid >= gid_min)) {
>-				group_id = grp->gr_gid - 1;
>-			}
>-			/* create index of used GIDs */
>-			if (grp->gr_gid <= gid_max) {
>-				used_gids[grp->gr_gid] = true;
>-			}
>-		}
>-	} else {
>-		group_id = gid_min;
>-		setgrent ();
>-		while ((grp = getgrent ()) != NULL) {
>-			if ((grp->gr_gid >= group_id) && (grp->gr_gid <= gid_max)) {
>-				group_id = grp->gr_gid + 1;
>-			}
>-			/* create index of used GIDs */
>-			if (grp->gr_gid <= gid_max) {
>-				used_gids[grp->gr_gid] = true;
>-			}
>-		}
>-		endgrent ();
>-
>-		(void) gr_rewind ();
>-		while ((grp = gr_next ()) != NULL) {
>-			if ((grp->gr_gid >= group_id) && (grp->gr_gid <= gid_max)) {
>-				group_id = grp->gr_gid + 1;
>-			}
>-			/* create index of used GIDs */
>-			if (grp->gr_gid <= gid_max) {
>-				used_gids[grp->gr_gid] = true;
>-			}
>-		}
>-	}
>-
>-	/*
>-	 * If a group (resp. system group) with GID equal to GID_MAX (resp.
>-	 * GID_MIN) exists, the above algorithm will give us GID_MAX+1
>-	 * (resp. GID_MIN-1) even if not unique. Search for the first free
>-	 * GID starting with GID_MIN (resp. GID_MAX).
>-	 */
>-	if (sys_group) {
>-		if (group_id < gid_min) {
>-			for (group_id = gid_max; group_id >= gid_min; group_id--) {
>-				if (false == used_gids[group_id]) {
>-					break;
>-				}
>-			}
>-			if (group_id < gid_min) {
>-				fprintf (stderr,
>-				         _("%s: Can't get unique system GID (no more available GIDs)\n"),
>-				         Prog);
>-				SYSLOG ((LOG_WARN,
>-				         "no more available GID on the system"));
>-				free (used_gids);
>-				return -1;
>-			}
>-		}
>+	    /*
>+	     * For system groups, we want to start from the
>+	     * top of the range and work downwards.
>+	     */
>+        for (id = gid_max; id >= gid_min; id--) {
>+            result = _check_gid(id, gid_min, gid_max);
>+            if (result == 0) {
>+                /* This GID is available. Return it. */
>+                *gid = id;
>+                return 0;
>+            } else if (result == EEXIST) {
>+                /* This GID is in use, we'll continue to the next */
>+            } else {
>+                /* An unexpected error occurred. Report it. */
>+                fprintf (stderr,
>+                         _("%s: Can't get unique system GID (%s)\n"),
>+                         Prog, strerror(result));
>+                SYSLOG ((LOG_ERR,
>+                         "Error checking available GIDs: %s",
>+                         strerror(result)));
>+                /*
>+                 * We will continue anyway. Hopefully a later GID
>+                 * will work properly.
>+                 */
>+            }
>+        }
> 	} else {
>-		if (group_id > gid_max) {
>-			for (group_id = gid_min; group_id <= gid_max; group_id++) {
>-				if (false == used_gids[group_id]) {
>-					break;
>-				}
>-			}
>-			if (group_id > gid_max) {
>-				fprintf (stderr,
>-				         _("%s: Can't get unique GID (no more available GIDs)\n"),
>-				         Prog);
>-				SYSLOG ((LOG_WARN, "no more available GID on the system"));
>-				free (used_gids);
>-				return -1;
>-			}
>-		}
>-	}
>+        /*
>+         * For non-system groups, we want to start from the
>+         * bottom of the range and work upwards.
>+         */
>+        for (id = gid_min; id <= gid_max; id++) {
>+            result = _check_gid(id, gid_min, gid_max);
>+            if (result == 0) {
>+                /* This GID is available. Return it. */
>+                *gid = id;
>+                return 0;
>+            } else if (result == EEXIST) {
>+                /* This GID is in use, we'll continue to the next */
>+            } else {
>+                /* An unexpected error occurred. Report it. */
>+                fprintf (stderr,
>+                         _("%s: Can't get unique system GID (%s)\n"),
>+                         Prog, strerror(result));
>+                SYSLOG ((LOG_ERR,
>+                         "Error checking available GIDs: %s",
>+                         strerror(result)));
>+                /*
>+                 * We will continue anyway. Hopefully a later GID
>+                 * will work properly.
>+                 */
>+            }
>+        }
>+    }
> 
>-	free (used_gids);
>-	*gid = group_id;
>-	return 0;
>+	/* The code reached here and found no available IDs in the range */
>+    fprintf (stderr,
>+             _("%s: Can't get unique system GID (no more available GIDs)\n"),
>+             Prog);
>+    SYSLOG ((LOG_WARN,
>+             "no more available GID on the system"));
>+    return -1;
> }
> 
>-- 
>1.9.3
>

Actions: View | Diff

Attachments on bug 1089738: 904763 | 905729 | 909203