From dad42cc2f595d9a4d02d832e9f1c70be64b97393 Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Mon, 30 Jun 2014 15:22:33 +0200 Subject: [PATCH] improve group allocation algorithm - patch by Stephen Gallager (#1089738) --- shadow-4.1.5.1-group-alloc.patch | 642 +++++++++++++++++++++++++++++++ shadow-utils.spec | 7 +- 2 files changed, 648 insertions(+), 1 deletion(-) create mode 100644 shadow-4.1.5.1-group-alloc.patch diff --git a/shadow-4.1.5.1-group-alloc.patch b/shadow-4.1.5.1-group-alloc.patch new file mode 100644 index 0000000..892ae7d --- /dev/null +++ b/shadow-4.1.5.1-group-alloc.patch @@ -0,0 +1,642 @@ +From e551be23be24508ecf5c8afdf74fd69b88832ecd Mon Sep 17 00:00:00 2001 +From: Stephen Gallagher +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,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; +- } ++ /* 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))); ++ 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; + } + +-- +1.9.3 + diff --git a/shadow-utils.spec b/shadow-utils.spec index 1585a51..138872f 100644 --- a/shadow-utils.spec +++ b/shadow-utils.spec @@ -1,7 +1,7 @@ Summary: Utilities for managing accounts and shadow password files Name: shadow-utils Version: 4.1.5.1 -Release: 11%{?dist} +Release: 12%{?dist} Epoch: 2 URL: http://pkg-shadow.alioth.debian.org/ Source0: http://pkg-shadow.alioth.debian.org/releases/shadow-%{version}.tar.bz2 @@ -24,6 +24,7 @@ Patch14: shadow-4.1.5.1-default-range.patch Patch15: shadow-4.1.5.1-manfix.patch Patch16: shadow-4.1.5.1-crypt-null.patch Patch17: shadow-4.1.5.1-userdel-helpfix.patch +Patch18: shadow-4.1.5.1-group-alloc.patch License: BSD and GPLv2+ Group: System Environment/Base @@ -69,6 +70,7 @@ are used for managing group accounts. %patch15 -p1 -b .manfix %patch16 -p1 -b .crypt-null %patch17 -p1 -b .userdel +%patch18 -p1 -b .group-alloc iconv -f ISO88591 -t utf-8 doc/HOWTO > doc/HOWTO.utf8 cp -f doc/HOWTO.utf8 doc/HOWTO @@ -222,6 +224,9 @@ rm -rf $RPM_BUILD_ROOT %{_mandir}/man8/vigr.8* %changelog +* Mon Jun 30 2014 Tomas Mraz - 2:4.1.5.1-12 +- improve group allocation algorithm - patch by Stephen Gallager (#1089738) + * Sun Jun 08 2014 Fedora Release Engineering - 2:4.1.5.1-11 - Rebuilt for https://fedoraproject.org/wiki/Fedora_21_Mass_Rebuild