Attachment 909203 Details for Bug 1089738 – Redesign automatic GID allocation

[patch] Redesign automatic GID allocation

0001-Redesign-automatic-GID-allocation.patch (text/plain), 19.90 KB, created by Stephen Gallagher on 2014-06-16 17:45:12 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stephen Gallagher

Created: 2014-06-16 17:45:12 UTC

Size: 19.90 KB

patch

obsolete

>From e551be23be24508ecf5c8afdf74fd69b88832ecd 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 first iterate through all entries
>in the local database with gr_next(), recording the IDs that are in
>use. We then start from the highest presumed-available entry and
>call getgrgid() to see if it is available. We continue this until
>we come to the first unused GID. We then select that and return it.
>
>If we make it through all the remaining IDs without finding a free
>one, we start over from the beginning of the range and try to find
>room in one of the gaps in the range.
>---
> libmisc/find_new_gid.c | 533 +++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 407 insertions(+), 126 deletions(-)
>
>diff --git a/libmisc/find_new_gid.c b/libmisc/find_new_gid.c
>index 05f5622edb79069d9a43d3f9c69a463b6b71141a..25900dd12874e46e5efdfcf7c895f6b814763a16 100644
>--- a/libmisc/find_new_gid.c
>+++ b/libmisc/find_new_gid.c
>@@ -39,6 +39,118 @@
> #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);
>+
>+		/* Check that the ranges make sense */
>+		if (*max_id < *min_id) {
>+			(void) fprintf (stderr,
>+                            _("%s: Invalid configuration: SYS_GID_MIN (%lu), "
>+                              "GID_MIN (%lu), SYS_GID_MAX (%lu)\n"),
>+                            Prog, (unsigned long) *min_id,
>+                            getdef_ulong ("GID_MIN", 1000UL),
>+                            (unsigned long) *max_id);
>+			return EINVAL;
>+		}
>+	} 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) {
>+			(void) fprintf(stderr,
>+					_("%s: Invalid configuration: GID_MIN (%lu), "
>+					  "GID_MAX (%lu)\n"),
>+					Prog, (unsigned long) *min_id,
>+					(unsigned long) *max_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,
>+		             bool *used_gids)
>+{
>+	/* First test that the preferred ID is in the range */
>+	if (gid < gid_min || gid > gid_max) {
>+		return ERANGE;
>+	}
>+
>+	/*
>+	 * Check whether we already detected this GID
>+	 * using the gr_next() loop
>+	 */
>+	if (used_gids != NULL && used_gids[gid]) {
>+		return EEXIST;
>+	}
>+	/* 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;
>+		}
>+	}
>+
>+	/* 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
>@@ -48,166 +160,335 @@
>  * 
>  * Return 0 on success, -1 if no unused GIDs are available.
>  */
>-int find_new_gid (bool sys_group,
>-                  gid_t *gid,
>-                  /*@null@*/gid_t const *preferred_gid)
>+int find_new_gid(bool sys_group,
>+                 gid_t *gid,
>+                 /*@null@*/gid_t const *preferred_gid)
> {
>-	const struct group *grp;
>-	gid_t gid_min, gid_max, group_id;
> 	bool *used_gids;
>+	const struct group *grp;
>+	gid_t gid_min, gid_max, preferred_min;
>+	gid_t group_id, id;
>+	gid_t lowest_found, highest_found;
>+	int result;
>+	int nospam = 0;
> 
>-	assert (gid != NULL);
>+	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);
>+	/*
>+	 * 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) {
>+		return -1;
>+	}
>+
>+	/* Check if the preferred GID is available */
>+	if (preferred_gid) {
>+		result = check_gid(*preferred_gid, preferred_min, gid_max, NULL);
>+		if (result == 0) {
>+			/*
>+			 * Make sure the GID isn't queued for use already
>+			 */
>+			if (gr_locate_gid (preferred_gid) == NULL) {
>+				*gid = *preferred_gid;
>+				return 0;
>+			}
>+			/*
>+			 * gr_locate_gid() found the GID in an as-yet uncommitted
>+			 * entry. We'll proceed below and auto-set a GID.
>+			 */
>+		} 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;
> 		}
> 	}
>+
>+	/*
>+	 * Search the entire group file,
>+	 * looking for the next unused value.
>+	 *
>+	 * We first check the local database with gr_rewind/gr_next to find
>+	 * all local values that are in use.
>+	 *
>+	 * We then compare the next free value to all databases (local and
>+	 * remote) and iterate until we find a free one. If there are free
>+	 * values beyond the lowest (system groups) or highest (non-system
>+	 * groups), we will prefer those and avoid potentially reclaiming a
>+	 * deleted group (which can be a security issue, since it may grant
>+	 * access to files belonging to that former group).
>+	 *
>+	 * If there are no GIDs available at the end of the search, we will
>+	 * have no choice but to iterate through the range looking for gaps.
>+	 *
>+	 */
>+
>+	/* Create an array to hold all of the discovered GIDs */
> 	used_gids = malloc (sizeof (bool) * (gid_max +1));
> 	if (NULL == used_gids) {
> 		fprintf (stderr,
>-		         _("%s: failed to allocate memory: %s\n"),
>-		         Prog, strerror (errno));
>+				 _("%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;
>-	}
>+	/* First look for the lowest and highest value in the local database */
>+	(void) gr_rewind ();
>+	highest_found = gid_min;
>+	lowest_found = gid_max;
>+	while ((grp = gr_next ()) != NULL) {
>+		/*
>+		 * Does this entry have a lower GID than the lowest we've found
>+		 * so far?
>+		 */
>+		if ((grp->gr_gid <= lowest_found) && (grp->gr_gid >= gid_min)) {
>+			lowest_found = grp->gr_gid - 1;
>+		}
>+
>+		/*
>+		 * Does this entry have a higher GID than the highest we've found
>+		 * so far?
>+		 */
>+		if ((grp->gr_gid >= highest_found) && (grp->gr_gid <= gid_max)) {
>+			highest_found = grp->gr_gid + 1;
>+		}
>+
>+		/* create index of used GIDs */
>+		if (grp->gr_gid >= gid_min
>+			&& grp->gr_gid <= gid_max) {
> 
>-        /* 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);
>+			used_gids[grp->gr_gid] = true;
>+		}
>+	}
> 
>-	/*
>-	 * Search the entire group file,
>-	 * looking for the largest 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.
>+		/*
>+		 * For system groups, we want to start from the
>+		 * top of the range and work downwards.
> 		 */
>-		group_id = gid_max;
>-		for (id = gid_max; id >= gid_min; id--) {
>-			if (getgrgid (id) != NULL) {
>-				group_id = id - 1;
>-				used_gids[id] = true;
>-			}
>+
>+		/*
>+		 * At the conclusion of the gr_next() search, we will either
>+		 * have a presumed-free GID or we will be at GID_MIN - 1.
>+		 */
>+		if (lowest_found < gid_min) {
>+			/*
>+			 * In this case, a GID is in use at GID_MIN.
>+			 *
>+			 * We will reset the search to GID_MAX and proceed down
>+			 * through all the GIDs (skipping those we detected with
>+			 * used_gids) for a free one. It is a known issue that
>+			 * this may result in reusing a previously-deleted GID,
>+			 * so administrators should be instructed to use this
>+			 * auto-detection with care (and prefer to assign GIDs
>+			 * explicitly).
>+			 */
>+			lowest_found = gid_max;
> 		}
> 
>-		(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;
>+		/* Search through all of the IDs in the range */
>+		for (id = lowest_found; id >= gid_min; id--) {
>+			result = check_gid(id, gid_min, gid_max, used_gids);
>+			if (result == 0) {
>+				/* This GID is available. Return it. */
>+				*gid = id;
>+				free(used_gids);
>+				return 0;
>+			} else if (result == EEXIST) {
>+				/* This GID is in use, we'll continue to the next */
>+			} else {
>+				/*
>+				 * An unexpected error occurred.
>+				 *
>+				 * Only report it the first time to avoid spamming
>+				 * the logs
>+				 *
>+				 */
>+				if (!nospam) {
>+					fprintf(stderr,
>+							_("%s: Can't get unique system GID (%s). "
>+							  "Suppressing additional messages.\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 {
>-		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;
>+
>+		/*
>+		 * If we get all the way through the loop, try again from GID_MAX,
>+		 * unless that was where we previously started. (NOTE: the worst-case
>+		 * scenario here is that we will run through (GID_MAX - GID_MIN - 1)
>+		 * cycles *again* if we fall into this case with lowest_found as
>+		 * GID_MAX - 1, all groups in the range in use and maintained by
>+		 * network services such as LDAP.)
>+		 */
>+		if (lowest_found != gid_max) {
>+			for (id = gid_max; id >= gid_min; id--) {
>+				result = check_gid(id, gid_min, gid_max, used_gids);
>+				if (result == 0) {
>+					/* This GID is available. Return it. */
>+					*gid = id;
>+					free(used_gids);
>+					return 0;
>+				} else if (result == EEXIST) {
>+					/* This GID is in use, we'll continue to the next */
>+				} else {
>+					/*
>+					 * An unexpected error occurred.
>+					 *
>+					 * Only report it the first time to avoid spamming
>+					 * the logs
>+					 *
>+					 */
>+					if (!nospam) {
>+						fprintf(stderr,
>+								_("%s: Can't get unique system GID (%s). "
>+								  "Suppressing additional messages.\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.
>+					 */
>+				}
> 			}
> 		}
>-		endgrent ();
>+	} else { /* !sys_group */
>+		/*
>+		 * For non-system groups, we want to start from the
>+		 * bottom of the range and work upwards.
>+		 */
> 
>-		(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;
>-			}
>+		/*
>+		 * At the conclusion of the gr_next() search, we will either
>+		 * have a presumed-free GID or we will be at GID_MAX + 1.
>+		 */
>+		if (highest_found > gid_max) {
>+			/*
>+			 * In this case, a GID is in use at GID_MAX.
>+			 *
>+			 * We will reset the search to GID_MIN and proceed up
>+			 * through all the GIDs (skipping those we detected with
>+			 * used_gids) for a free one. It is a known issue that
>+			 * this may result in reusing a previously-deleted GID,
>+			 * so administrators should be instructed to use this
>+			 * auto-detection with care (and prefer to assign GIDs
>+			 * explicitly).
>+			 */
>+			highest_found = gid_min;
> 		}
>-	}
> 
>-	/*
>-	 * 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;
>+		/* Search through all of the IDs in the range */
>+		for (id = highest_found; id <= gid_max; id++) {
>+			result = check_gid(id, gid_min, gid_max, used_gids);
>+			if (result == 0) {
>+				/* This GID is available. Return it. */
>+				*gid = id;
>+				free(used_gids);
>+				return 0;
>+			} else if (result == EEXIST) {
>+				/* This GID is in use, we'll continue to the next */
>+			} else {
>+				/*
>+				 * An unexpected error occurred.
>+				 *
>+				 * Only report it the first time to avoid spamming
>+				 * the logs
>+				 *
>+				 */
>+				if (!nospam) {
>+					fprintf(stderr,
>+							_("%s: Can't get unique GID (%s). "
>+							  "Suppressing additional messages.\n"),
>+							Prog, strerror(result));
>+					SYSLOG((LOG_ERR,
>+							"Error checking available GIDs: %s",
>+							strerror(result)));
> 				}
>-			}
>-			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;
>+				/*
>+				 * 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 we get all the way through the loop, try again from GID_MIN,
>+		 * unless that was where we previously started. (NOTE: the worst-case
>+		 * scenario here is that we will run through (GID_MAX - GID_MIN - 1)
>+		 * cycles *again* if we fall into this case with highest_found as
>+		 * GID_MIN + 1, all groups in the range in use and maintained by
>+		 * network services such as LDAP.)
>+		 */
>+		if (highest_found != gid_min) {
>+			for (id = gid_min; id <= gid_max; id++) {
>+				result = check_gid(id, gid_min, gid_max, used_gids);
>+				if (result == 0) {
>+					/* This GID is available. Return it. */
>+					*gid = id;
>+					free(used_gids);
>+					return 0;
>+				} else if (result == EEXIST) {
>+					/* This GID is in use, we'll continue to the next */
>+				} else {
>+					/*
>+					 * An unexpected error occurred.
>+					 *
>+					 * Only report it the first time to avoid spamming
>+					 * the logs
>+					 *
>+					 */
>+					if (!nospam) {
>+						fprintf(stderr,
>+								_("%s: Can't get unique GID (%s). "
>+								  "Suppressing additional messages.\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.
>+					 */
> 				}
> 			}
>-			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;
>-			}
> 		}
> 	}
> 
>-	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 GID (no more available GIDs)\n"),
>+			Prog);
>+	SYSLOG((LOG_WARN, "no more available GIDs on the system"));
>+	free(used_gids);
>+	return -1;
> }
> 
>-- 
>1.9.3
>

Flags:

tmraz: review+

Actions: View | Diff

Attachments on bug 1089738: 904763 | 905729 | 909203