shadow-utils-newxidmap/shadow-4.1.5.1-id-alloc.patch

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.

The patch was originally written by Stephen Gallagher and applied
identically also to the user allocation by Tomáš Mráz.

diff -up shadow-4.1.5.1/libmisc/find_new_gid.c.id-alloc shadow-4.1.5.1/libmisc/find_new_gid.c
--- shadow-4.1.5.1/libmisc/find_new_gid.c.id-alloc	2014-09-10 10:25:41.165524986 +0200
+++ shadow-4.1.5.1/libmisc/find_new_gid.c	2014-09-10 10:25:41.195525677 +0200
@@ -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.
+		 */
+		/* ignore errors for now * 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,339 @@
  * 
  * 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;
-	}
-
-        /* 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);
+	/* 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) {
+
+			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.
-		 */
-		group_id = gid_max;
-		for (id = gid_max; id >= gid_min; id--) {
-			if (getgrgid (id) != NULL) {
-				group_id = id - 1;
-				used_gids[id] = true;
-			}
+		/*
+		 * For system groups, we want to start from the
+		 * top of the range and work downwards.
+		 */
+
+		/*
+		 * 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)));
+					nospam = 1;
+				}
+				/*
+				 * 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)));
+						nospam = 1;
+					}
+					/*
+					 * 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)));
+					nospam = 1;
 				}
-			}
-			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)));
+						nospam = 1;
+					}
+					/*
+					 * 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;
 }
 
diff -up shadow-4.1.5.1/libmisc/find_new_uid.c.id-alloc shadow-4.1.5.1/libmisc/find_new_uid.c
--- shadow-4.1.5.1/libmisc/find_new_uid.c.id-alloc	2011-07-29 17:39:16.000000000 +0200
+++ shadow-4.1.5.1/libmisc/find_new_uid.c	2014-10-17 16:52:30.481217270 +0200
@@ -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_user, uid_t *min_id, uid_t *max_id,
+	uid_t *preferred_min)
+{
+	uid_t uid_def_max = 0;
+
+	if (sys_user) {
+		/* System users */
+
+		/* A requested ID is allowed to be below the autoselect range */
+		*preferred_min = (uid_t) 1;
+
+		/* Get the minimum ID range from login.defs or default to 101 */
+		*min_id = (uid_t) getdef_ulong("SYS_UID_MIN", 101UL);
+
+		/*
+		 * If SYS_UID_MAX is unspecified, we should assume it to be one
+		 * less than the UID_MIN (which is reserved for non-system accounts)
+		 */
+		uid_def_max = (uid_t) getdef_ulong("UID_MIN", 1000UL) - 1;
+		*max_id = (uid_t) getdef_ulong("SYS_UID_MAX",
+				(unsigned long) uid_def_max);
+
+		/* Check that the ranges make sense */
+		if (*max_id < *min_id) {
+			(void) fprintf (stderr,
+                            _("%s: Invalid configuration: SYS_UID_MIN (%lu), "
+                              "UID_MIN (%lu), SYS_UID_MAX (%lu)\n"),
+                            Prog, (unsigned long) *min_id,
+                            getdef_ulong ("UID_MIN", 1000UL),
+                            (unsigned long) *max_id);
+			return EINVAL;
+		}
+	} else {
+		/* Non-system users */
+
+		/* Get the values from login.defs or use reasonable defaults */
+		*min_id = (uid_t) getdef_ulong("UID_MIN", 1000UL);
+		*max_id = (uid_t) getdef_ulong("UID_MAX", 60000UL);
+
+		/*
+		 * The preferred minimum should match the standard ID minimum
+		 * for non-system users.
+		 */
+		*preferred_min = *min_id;
+
+		/* Check that the ranges make sense */
+		if (*max_id < *min_id) {
+			(void) fprintf(stderr,
+					_("%s: Invalid configuration: UID_MIN (%lu), "
+					  "UID_MAX (%lu)\n"),
+					Prog, (unsigned long) *min_id,
+					(unsigned long) *max_id);
+			return EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * check_uid - See if the requested UID 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 getpwuid()
+ */
+static int check_uid(const uid_t uid,
+		             const uid_t uid_min,
+		             const uid_t uid_max,
+		             bool *used_uids)
+{
+	/* First test that the preferred ID is in the range */
+	if (uid < uid_min || uid > uid_max) {
+		return ERANGE;
+	}
+
+	/*
+	 * Check whether we already detected this UID
+	 * using the pw_next() loop
+	 */
+	if (used_uids != NULL && used_uids[uid]) {
+		return EEXIST;
+	}
+	/* Check if the UID exists according to NSS */
+	errno = 0;
+	if (getpwuid(uid) != NULL) {
+		return EEXIST;
+	} else {
+		/* getpwuid() was NULL, check whether this was
+		 * due to an error, so we can report it.
+		 */
+		/* ignore errors for now * if (errno != 0) {
+			return errno;
+		} */
+	}
+
+	/* If we've made it here, the UID must be available */
+	return 0;
+}
+
+/*
  * find_new_uid - Find a new unused UID.
  *
  * If successful, find_new_uid provides an unused user ID in the
@@ -48,162 +160,339 @@
  * 
  * Return 0 on success, -1 if no unused UIDs are available.
  */
-int find_new_uid (bool sys_user,
-                  uid_t *uid,
-                  /*@null@*/uid_t const *preferred_uid)
+int find_new_uid(bool sys_user,
+                 uid_t *uid,
+                 /*@null@*/uid_t const *preferred_uid)
 {
-	const struct passwd *pwd;
-	uid_t uid_min, uid_max, user_id;
 	bool *used_uids;
+	const struct passwd *pwd;
+	uid_t uid_min, uid_max, preferred_min;
+	uid_t user_id, id;
+	uid_t lowest_found, highest_found;
+	int result;
+	int nospam = 0;
 
 	assert (uid != NULL);
 
-	if (!sys_user) {
-		uid_min = (uid_t) getdef_ulong ("UID_MIN", 1000UL);
-		uid_max = (uid_t) getdef_ulong ("UID_MAX", 60000UL);
-		if (uid_max < uid_min) {
-			(void) fprintf (stderr,
-			                _("%s: Invalid configuration: UID_MIN (%lu), UID_MAX (%lu)\n"),
-			                Prog, (unsigned long) uid_min, (unsigned long) uid_max);
-			return -1;
-		}
-	} else {
-		uid_min = (uid_t) getdef_ulong ("SYS_UID_MIN", 101UL);
-		uid_max = (uid_t) getdef_ulong ("UID_MIN", 1000UL) - 1;
-		uid_max = (uid_t) getdef_ulong ("SYS_UID_MAX", (unsigned long) uid_max);
-		if (uid_max < uid_min) {
-			(void) fprintf (stderr,
-			                _("%s: Invalid configuration: SYS_UID_MIN (%lu), UID_MIN (%lu), SYS_UID_MAX (%lu)\n"),
-			                Prog, (unsigned long) uid_min, getdef_ulong ("UID_MIN", 1000UL), (unsigned long) uid_max);
+	/*
+	 * First, figure out what ID range is appropriate for
+	 * automatic assignment
+	 */
+	result = get_ranges(sys_user, &uid_min, &uid_max, &preferred_min);
+	if (result == EINVAL) {
+		return -1;
+	}
+
+	/* Check if the preferred UID is available */
+	if (preferred_uid) {
+		result = check_uid(*preferred_uid, preferred_min, uid_max, NULL);
+		if (result == 0) {
+			/*
+			 * Make sure the UID isn't queued for use already
+			 */
+			if (pw_locate_uid (*preferred_uid) == NULL) {
+				*uid = *preferred_uid;
+				return 0;
+			}
+			/*
+			 * pw_locate_uid() found the UID in an as-yet uncommitted
+			 * entry. We'll proceed below and auto-set an UID.
+			 */
+		} 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 user creation.
+			 * This differs from the automatic creation
+			 * behavior below, since if a specific UID 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 UID: %s\n"),
+					Prog, strerror(result));
 			return -1;
 		}
 	}
+
+	/*
+	 * Search the entire passwd file,
+	 * looking for the next unused value.
+	 *
+	 * We first check the local database with pw_rewind/pw_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 users) or highest (non-system
+	 * users), we will prefer those and avoid potentially reclaiming a
+	 * deleted user (which can be a security issue, since it may grant
+	 * access to files belonging to that former user).
+	 *
+	 * If there are no UIDs 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 UIDs */
 	used_uids = malloc (sizeof (bool) * (uid_max +1));
 	if (NULL == used_uids) {
 		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_uids, false, sizeof (bool) * (uid_max + 1));
 
-	if (   (NULL != preferred_uid)
-	    && (*preferred_uid >= uid_min)
-	    && (*preferred_uid <= uid_max)
-	    /* Check if the user exists according to NSS */
-	    && (getpwuid (*preferred_uid) == NULL)
-	    /* Check also the local database in case of uncommitted
-	     * changes */
-	    && (pw_locate_uid (*preferred_uid) == NULL)) {
-		*uid = *preferred_uid;
-		free (used_uids);
-		return 0;
-	}
+	/* First look for the lowest and highest value in the local database */
+	(void) pw_rewind ();
+	highest_found = uid_min;
+	lowest_found = uid_max;
+	while ((pwd = pw_next ()) != NULL) {
+		/*
+		 * Does this entry have a lower UID than the lowest we've found
+		 * so far?
+		 */
+		if ((pwd->pw_uid <= lowest_found) && (pwd->pw_uid >= uid_min)) {
+			lowest_found = pwd->pw_uid - 1;
+		}
 
+		/*
+		 * Does this entry have a higher UID than the highest we've found
+		 * so far?
+		 */
+		if ((pwd->pw_uid >= highest_found) && (pwd->pw_uid <= uid_max)) {
+			highest_found = pwd->pw_uid + 1;
+		}
+
+		/* create index of used UIDs */
+		if (pwd->pw_uid >= uid_min
+			&& pwd->pw_uid <= uid_max) {
+
+			used_uids[pwd->pw_uid] = true;
+		}
+	}
 
-	/*
-	 * Search the entire password file,
-	 * looking for the largest unused value.
-	 *
-	 * We check the list of users according to NSS (setpwent/getpwent),
-	 * but we also check the local database (pw_rewind/pw_next) in case
-	 * some users were created but the changes were not committed yet.
-	 */
 	if (sys_user) {
-		uid_t id;
-		/* setpwent / getpwent / endpwent 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 getpwuid.
-		 */
-		user_id = uid_max;
-		for (id = uid_max; id >= uid_min; id--) {
-			if (getpwuid (id) != NULL) {
-				user_id = id - 1;
-				used_uids[id] = true;
-			}
+		/*
+		 * For system users, we want to start from the
+		 * top of the range and work downwards.
+		 */
+
+		/*
+		 * At the conclusion of the pw_next() search, we will either
+		 * have a presumed-free UID or we will be at UID_MIN - 1.
+		 */
+		if (lowest_found < uid_min) {
+			/*
+			 * In this case, an UID is in use at UID_MIN.
+			 *
+			 * We will reset the search to UID_MAX and proceed down
+			 * through all the UIDs (skipping those we detected with
+			 * used_uids) for a free one. It is a known issue that
+			 * this may result in reusing a previously-deleted UID,
+			 * so administrators should be instructed to use this
+			 * auto-detection with care (and prefer to assign UIDs
+			 * explicitly).
+			 */
+			lowest_found = uid_max;
 		}
 
-		(void) pw_rewind ();
-		while ((pwd = pw_next ()) != NULL) {
-			if ((pwd->pw_uid <= user_id) && (pwd->pw_uid >= uid_min)) {
-				user_id = pwd->pw_uid - 1;
-			}
-			/* create index of used UIDs */
-			if (pwd->pw_uid <= uid_max) {
-				used_uids[pwd->pw_uid] = true;
+		/* Search through all of the IDs in the range */
+		for (id = lowest_found; id >= uid_min; id--) {
+			result = check_uid(id, uid_min, uid_max, used_uids);
+			if (result == 0) {
+				/* This UID is available. Return it. */
+				*uid = id;
+				free(used_uids);
+				return 0;
+			} else if (result == EEXIST) {
+				/* This UID 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 UID (%s). "
+							  "Suppressing additional messages.\n"),
+							Prog, strerror(result));
+					SYSLOG((LOG_ERR,
+							"Error checking available UIDs: %s",
+							strerror(result)));
+					nospam = 1;
+				}
+				/*
+				 * We will continue anyway. Hopefully a later UID
+				 * will work properly.
+				 */
 			}
 		}
-	} else {
-		user_id = uid_min;
-		setpwent ();
-		while ((pwd = getpwent ()) != NULL) {
-			if ((pwd->pw_uid >= user_id) && (pwd->pw_uid <= uid_max)) {
-				user_id = pwd->pw_uid + 1;
-			}
-			/* create index of used UIDs */
-			if (pwd->pw_uid <= uid_max) {
-				used_uids[pwd->pw_uid] = true;
+
+		/*
+		 * If we get all the way through the loop, try again from UID_MAX,
+		 * unless that was where we previously started. (NOTE: the worst-case
+		 * scenario here is that we will run through (UID_MAX - UID_MIN - 1)
+		 * cycles *again* if we fall into this case with lowest_found as
+		 * UID_MAX - 1, all users in the range in use and maintained by
+		 * network services such as LDAP.)
+		 */
+		if (lowest_found != uid_max) {
+			for (id = uid_max; id >= uid_min; id--) {
+				result = check_uid(id, uid_min, uid_max, used_uids);
+				if (result == 0) {
+					/* This UID is available. Return it. */
+					*uid = id;
+					free(used_uids);
+					return 0;
+				} else if (result == EEXIST) {
+					/* This UID 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 UID (%s). "
+								  "Suppressing additional messages.\n"),
+								Prog, strerror(result));
+						SYSLOG((LOG_ERR,
+								"Error checking available UIDs: %s",
+								strerror(result)));
+						nospam = 1;
+					}
+					/*
+					 * We will continue anyway. Hopefully a later UID
+					 * will work properly.
+					 */
+				}
 			}
 		}
-		endpwent ();
+	} else { /* !sys_user */
+		/*
+		 * For non-system users, we want to start from the
+		 * bottom of the range and work upwards.
+		 */
 
-		(void) pw_rewind ();
-		while ((pwd = pw_next ()) != NULL) {
-			if ((pwd->pw_uid >= user_id) && (pwd->pw_uid <= uid_max)) {
-				user_id = pwd->pw_uid + 1;
-			}
-			/* create index of used UIDs */
-			if (pwd->pw_uid <= uid_max) {
-				used_uids[pwd->pw_uid] = true;
-			}
+		/*
+		 * At the conclusion of the pw_next() search, we will either
+		 * have a presumed-free UID or we will be at UID_MAX + 1.
+		 */
+		if (highest_found > uid_max) {
+			/*
+			 * In this case, a UID is in use at UID_MAX.
+			 *
+			 * We will reset the search to UID_MIN and proceed up
+			 * through all the UIDs (skipping those we detected with
+			 * used_uids) for a free one. It is a known issue that
+			 * this may result in reusing a previously-deleted UID,
+			 * so administrators should be instructed to use this
+			 * auto-detection with care (and prefer to assign UIDs
+			 * explicitly).
+			 */
+			highest_found = uid_min;
 		}
-	}
 
-	/*
-	 * If a user (resp. system user) with UID equal to UID_MAX (resp.
-	 * UID_MIN) exists, the above algorithm will give us UID_MAX+1
-	 * (resp. UID_MIN-1) even if not unique. Search for the first free
-	 * UID starting with UID_MIN (resp. UID_MAX).
-	 */
-	if (sys_user) {
-		if (user_id < uid_min) {
-			for (user_id = uid_max; user_id >= uid_min; user_id--) {
-				if (false == used_uids[user_id]) {
-					break;
+		/* Search through all of the IDs in the range */
+		for (id = highest_found; id <= uid_max; id++) {
+			result = check_uid(id, uid_min, uid_max, used_uids);
+			if (result == 0) {
+				/* This UID is available. Return it. */
+				*uid = id;
+				free(used_uids);
+				return 0;
+			} else if (result == EEXIST) {
+				/* This UID 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 UID (%s). "
+							  "Suppressing additional messages.\n"),
+							Prog, strerror(result));
+					SYSLOG((LOG_ERR,
+							"Error checking available UIDs: %s",
+							strerror(result)));
+					nospam = 1;
 				}
-			}
-			if (user_id < uid_min ) {
-				fprintf (stderr,
-				         _("%s: Can't get unique system UID (no more available UIDs)\n"),
-				         Prog);
-				SYSLOG ((LOG_WARN,
-				         "no more available UID on the system"));
-				free (used_uids);
-				return -1;
+				/*
+				 * We will continue anyway. Hopefully a later UID
+				 * will work properly.
+				 */
 			}
 		}
-	} else {
-		if (user_id > uid_max) {
-			for (user_id = uid_min; user_id <= uid_max; user_id++) {
-				if (false == used_uids[user_id]) {
-					break;
+
+		/*
+		 * If we get all the way through the loop, try again from UID_MIN,
+		 * unless that was where we previously started. (NOTE: the worst-case
+		 * scenario here is that we will run through (UID_MAX - UID_MIN - 1)
+		 * cycles *again* if we fall into this case with highest_found as
+		 * UID_MIN + 1, all users in the range in use and maintained by
+		 * network services such as LDAP.)
+		 */
+		if (highest_found != uid_min) {
+			for (id = uid_min; id <= uid_max; id++) {
+				result = check_uid(id, uid_min, uid_max, used_uids);
+				if (result == 0) {
+					/* This UID is available. Return it. */
+					*uid = id;
+					free(used_uids);
+					return 0;
+				} else if (result == EEXIST) {
+					/* This UID 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 UID (%s). "
+								  "Suppressing additional messages.\n"),
+								Prog, strerror(result));
+						SYSLOG((LOG_ERR,
+								"Error checking available UIDs: %s",
+								strerror(result)));
+						nospam = 1;
+					}
+					/*
+					 * We will continue anyway. Hopefully a later UID
+					 * will work properly.
+					 */
 				}
 			}
-			if (user_id > uid_max) {
-				fprintf (stderr,
-				         _("%s: Can't get unique UID (no more available UIDs)\n"),
-				         Prog);
-				SYSLOG ((LOG_WARN, "no more available UID on the system"));
-				free (used_uids);
-				return -1;
-			}
 		}
 	}
 
-	free (used_uids);
-	*uid = user_id;
-	return 0;
+	/* The code reached here and found no available IDs in the range */
+	fprintf(stderr,
+			_("%s: Can't get unique UID (no more available UIDs)\n"),
+			Prog);
+	SYSLOG((LOG_WARN, "no more available UIDs on the system"));
+	free(used_uids);
+	return -1;
 }