rpms
/
shadow-utils
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

improve group allocation algorithm - patch by Stephen Gallager (#1089738)

This commit is contained in:
Tomas Mraz 2014-06-30 15:22:33 +02:00
parent c509d20844
commit dad42cc2f5
2 changed files with 648 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

642
shadow-4.1.5.1-group-alloc.patch Normal file
View File

@ -0,0 +1,642 @@
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,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

7
shadow-utils.spec
View File

@ -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 <tmraz@redhat.com> - 2:4.1.5.1-12
- improve group allocation algorithm - patch by Stephen Gallager (#1089738)
* Sun Jun 08 2014 Fedora Release Engineering <rel-eng@lists.fedoraproject.org> - 2:4.1.5.1-11
- Rebuilt for https://fedoraproject.org/wiki/Fedora_21_Mass_Rebuild