From 140510de9de4771feb3af1d859c09604043a4c9b Mon Sep 17 00:00:00 2001

From: ikerexxe <ipedrosa@redhat.com>

Date: Fri, 27 Mar 2020 14:23:02 +0100

Subject: [PATCH 1/2] usermod: check only local groups with -G option

Check only local groups when adding new supplementary groups to a user

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1727236

---

 src/usermod.c | 220 ++++++++++++++++++++++++++++++++------------------

 1 file changed, 143 insertions(+), 77 deletions(-)

diff --git a/src/usermod.c b/src/usermod.c

index 05b98715..ef430296 100644

--- a/src/usermod.c

+++ b/src/usermod.c

@@ -183,6 +183,7 @@ static bool sub_gid_locked = false;

 static void date_to_str (/*@unique@*//*@out@*/char *buf, size_t maxsize,

                          long int date);

 static int get_groups (char *);

+static struct group * get_local_group (char * grp_name);

 static /*@noreturn@*/void usage (int status);

 static void new_pwent (struct passwd *);

 static void new_spent (struct spwd *);

@@ -196,7 +197,9 @@ static void grp_update (void);

 static void process_flags (int, char **);

 static void close_files (void);

+static void close_group_files (void);

 static void open_files (void);

+static void open_group_files (void);

 static void usr_update (void);

 static void move_home (void);

 static void update_lastlog (void);

@@ -253,6 +256,11 @@ static int get_groups (char *list)

 		return 0;

 	}

+	/*

+	 * Open the group files

+	 */

+	open_group_files ();

+

 	/*

 	 * So long as there is some data to be converted, strip off each

 	 * name and look it up. A mix of numerical and string values for

@@ -272,7 +280,7 @@ static int get_groups (char *list)

 		 * Names starting with digits are treated as numerical GID

 		 * values, otherwise the string is looked up as is.

 		 */

-		grp = prefix_getgr_nam_gid (list);

+		grp = get_local_group (list);

 		/*

 		 * There must be a match, either by GID value or by

@@ -322,6 +330,8 @@ static int get_groups (char *list)

 		gr_free ((struct group *)grp);

 	} while (NULL != list);

+	close_group_files ();

+

 	user_groups[ngroups] = (char *) 0;

 	/*

@@ -334,6 +344,44 @@ static int get_groups (char *list)

 	return 0;

 }

+/*

+ * get_local_group - checks if a given group name exists locally

+ *

+ *	get_local_group() checks if a given group name exists locally.

+ *	If the name exists the group information is returned, otherwise NULL is

+ *	returned.

+ */

+static struct group * get_local_group(char * grp_name)

+{

+	const struct group *grp;

+	struct group *result_grp = NULL;

+	long long int gid;

+	char *endptr;

+

+	gid = strtoll (grp_name, &endptr, 10);

+	if (   ('\0' != *grp_name)

+		&& ('\0' == *endptr)

+		&& (ERANGE != errno)

+		&& (gid == (gid_t)gid)) {

+		grp = gr_locate_gid ((gid_t) gid);

+	}

+	else {

+		grp = gr_locate(grp_name);

+	}

+

+	if (grp != NULL) {

+		result_grp = __gr_dup (grp);

+		if (NULL == result_grp) {

+			fprintf (stderr,

+					_("%s: Out of memory. Cannot find group '%s'.\n"),

+					Prog, grp_name);

+			fail_exit (E_GRP_UPDATE);

+		}

+	}

+

+	return result_grp;

+}

+

 #ifdef ENABLE_SUBIDS

 struct ulong_range

 {

@@ -1447,50 +1495,7 @@ static void close_files (void)

 	}

 	if (Gflg || lflg) {

-		if (gr_close () == 0) {

-			fprintf (stderr,

-			         _("%s: failure while writing changes to %s\n"),

-			         Prog, gr_dbname ());

-			SYSLOG ((LOG_ERR,

-			         "failure while writing changes to %s",

-			         gr_dbname ()));

-			fail_exit (E_GRP_UPDATE);

-		}

-#ifdef SHADOWGRP

-		if (is_shadow_grp) {

-			if (sgr_close () == 0) {

-				fprintf (stderr,

-				         _("%s: failure while writing changes to %s\n"),

-				         Prog, sgr_dbname ());

-				SYSLOG ((LOG_ERR,

-				         "failure while writing changes to %s",

-				         sgr_dbname ()));

-				fail_exit (E_GRP_UPDATE);

-			}

-		}

-#endif

-#ifdef SHADOWGRP

-		if (is_shadow_grp) {

-			if (sgr_unlock () == 0) {

-				fprintf (stderr,

-				         _("%s: failed to unlock %s\n"),

-				         Prog, sgr_dbname ());

-				SYSLOG ((LOG_ERR,

-				         "failed to unlock %s",

-				         sgr_dbname ()));

-				/* continue */

-			}

-		}

-#endif

-		if (gr_unlock () == 0) {

-			fprintf (stderr,

-			         _("%s: failed to unlock %s\n"),

-			         Prog, gr_dbname ());

-			SYSLOG ((LOG_ERR,

-			         "failed to unlock %s",

-			         gr_dbname ()));

-			/* continue */

-		}

+		close_group_files ();

 	}

 	if (is_shadow_pwd) {

@@ -1559,6 +1564,60 @@ static void close_files (void)

 #endif

 }

+/*

+ * close_group_files - close all of the files that were opened

+ *

+ *	close_group_files() closes all of the files that were opened related

+ *  with groups. This causes any modified entries to be written out.

+ */

+static void close_group_files (void)

+{

+	if (gr_close () == 0) {

+		fprintf (stderr,

+					_("%s: failure while writing changes to %s\n"),

+					Prog, gr_dbname ());

+		SYSLOG ((LOG_ERR,

+					"failure while writing changes to %s",

+					gr_dbname ()));

+		fail_exit (E_GRP_UPDATE);

+	}

+#ifdef SHADOWGRP

+	if (is_shadow_grp) {

+		if (sgr_close () == 0) {

+			fprintf (stderr,

+						_("%s: failure while writing changes to %s\n"),

+						Prog, sgr_dbname ());

+			SYSLOG ((LOG_ERR,

+						"failure while writing changes to %s",

+						sgr_dbname ()));

+			fail_exit (E_GRP_UPDATE);

+		}

+	}

+#endif

+#ifdef SHADOWGRP

+	if (is_shadow_grp) {

+		if (sgr_unlock () == 0) {

+			fprintf (stderr,

+						_("%s: failed to unlock %s\n"),

+						Prog, sgr_dbname ());

+			SYSLOG ((LOG_ERR,

+						"failed to unlock %s",

+						sgr_dbname ()));

+			/* continue */

+		}

+	}

+#endif

+	if (gr_unlock () == 0) {

+		fprintf (stderr,

+					_("%s: failed to unlock %s\n"),

+					Prog, gr_dbname ());

+		SYSLOG ((LOG_ERR,

+					"failed to unlock %s",

+					gr_dbname ()));

+		/* continue */

+	}

+}

+

 /*

  * open_files - lock and open the password files

  *

@@ -1594,38 +1653,7 @@ static void open_files (void)

 	}

 	if (Gflg || lflg) {

-		/*

-		 * Lock and open the group file. This will load all of the

-		 * group entries.

-		 */

-		if (gr_lock () == 0) {

-			fprintf (stderr,

-			         _("%s: cannot lock %s; try again later.\n"),

-			         Prog, gr_dbname ());

-			fail_exit (E_GRP_UPDATE);

-		}

-		gr_locked = true;

-		if (gr_open (O_CREAT | O_RDWR) == 0) {

-			fprintf (stderr,

-			         _("%s: cannot open %s\n"),

-			         Prog, gr_dbname ());

-			fail_exit (E_GRP_UPDATE);

-		}

-#ifdef SHADOWGRP

-		if (is_shadow_grp && (sgr_lock () == 0)) {

-			fprintf (stderr,

-			         _("%s: cannot lock %s; try again later.\n"),

-			         Prog, sgr_dbname ());

-			fail_exit (E_GRP_UPDATE);

-		}

-		sgr_locked = true;

-		if (is_shadow_grp && (sgr_open (O_CREAT | O_RDWR) == 0)) {

-			fprintf (stderr,

-			         _("%s: cannot open %s\n"),

-			         Prog, sgr_dbname ());

-			fail_exit (E_GRP_UPDATE);

-		}

-#endif

+		open_group_files ();

 	}

 #ifdef ENABLE_SUBIDS

 	if (vflg || Vflg) {

@@ -1661,6 +1689,44 @@ static void open_files (void)

 #endif				/* ENABLE_SUBIDS */

 }

+/*

+ * open_group_files - lock and open the group files

+ *

+ *	open_group_files() loads all of the group entries.

+ */

+static void open_group_files (void)

+{

+	if (gr_lock () == 0) {

+		fprintf (stderr,

+					_("%s: cannot lock %s; try again later.\n"),

+					Prog, gr_dbname ());

+		fail_exit (E_GRP_UPDATE);

+	}

+	gr_locked = true;

+	if (gr_open (O_CREAT | O_RDWR) == 0) {

+		fprintf (stderr,

+					_("%s: cannot open %s\n"),

+					Prog, gr_dbname ());

+		fail_exit (E_GRP_UPDATE);

+	}

+

+#ifdef SHADOWGRP

+	if (is_shadow_grp && (sgr_lock () == 0)) {

+		fprintf (stderr,

+					_("%s: cannot lock %s; try again later.\n"),

+					Prog, sgr_dbname ());

+		fail_exit (E_GRP_UPDATE);

+	}

+	sgr_locked = true;

+	if (is_shadow_grp && (sgr_open (O_CREAT | O_RDWR) == 0)) {

+		fprintf (stderr,

+					_("%s: cannot open %s\n"),

+					Prog, sgr_dbname ());

+		fail_exit (E_GRP_UPDATE);

+	}

+#endif

+}

+

 /*

  * usr_update - create the user entries

  *

-- 

2.25.4

From 8762f465d487a52bf68f9c0b7c3c1eb3caea7bc9 Mon Sep 17 00:00:00 2001

From: ikerexxe <ipedrosa@redhat.com>

Date: Mon, 30 Mar 2020 09:08:23 +0200

Subject: [PATCH 2/2] useradd: check only local groups with -G option

Check only local groups when adding new supplementary groups to a user

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1727236

---

 src/useradd.c | 234 +++++++++++++++++++++++++++++++++-----------------

 1 file changed, 157 insertions(+), 77 deletions(-)

diff --git a/src/useradd.c b/src/useradd.c

index 645d4a40..90210233 100644

--- a/src/useradd.c

+++ b/src/useradd.c

@@ -211,6 +211,7 @@ static void get_defaults (void);

 static void show_defaults (void);

 static int set_defaults (void);

 static int get_groups (char *);

+static struct group * get_local_group (char * grp_name);

 static void usage (int status);

 static void new_pwent (struct passwd *);

@@ -220,7 +221,10 @@ static void grp_update (void);

 static void process_flags (int argc, char **argv);

 static void close_files (void);

+static void close_group_files (void);

+static void unlock_group_files (void);

 static void open_files (void);

+static void open_group_files (void);

 static void open_shadow (void);

 static void faillog_reset (uid_t);

 static void lastlog_reset (uid_t);

@@ -731,6 +735,11 @@ static int get_groups (char *list)

 		return 0;

 	}

+	/*

+	 * Open the group files

+	 */

+	open_group_files ();

+

 	/*

 	 * So long as there is some data to be converted, strip off

 	 * each name and look it up. A mix of numerical and string

@@ -749,7 +758,7 @@ static int get_groups (char *list)

 		 * Names starting with digits are treated as numerical

 		 * GID values, otherwise the string is looked up as is.

 		 */

-		grp = prefix_getgr_nam_gid (list);

+		grp = get_local_group (list);

 		/*

 		 * There must be a match, either by GID value or by

@@ -799,6 +808,9 @@ static int get_groups (char *list)

 		user_groups[ngroups++] = xstrdup (grp->gr_name);

 	} while (NULL != list);

+	close_group_files ();

+	unlock_group_files ();

+

 	user_groups[ngroups] = (char *) 0;

 	/*

@@ -811,6 +823,44 @@ static int get_groups (char *list)

 	return 0;

 }

+/*

+ * get_local_group - checks if a given group name exists locally

+ *

+ *	get_local_group() checks if a given group name exists locally.

+ *	If the name exists the group information is returned, otherwise NULL is

+ *	returned.

+ */

+static struct group * get_local_group(char * grp_name)

+{

+	const struct group *grp;

+	struct group *result_grp = NULL;

+	long long int gid;

+	char *endptr;

+

+	gid = strtoll (grp_name, &endptr, 10);

+	if (   ('\0' != *grp_name)

+		&& ('\0' == *endptr)

+		&& (ERANGE != errno)

+		&& (gid == (gid_t)gid)) {

+		grp = gr_locate_gid ((gid_t) gid);

+	}

+	else {

+		grp = gr_locate(grp_name);

+	}

+

+	if (grp != NULL) {

+		result_grp = __gr_dup (grp);

+		if (NULL == result_grp) {

+			fprintf (stderr,

+					_("%s: Out of memory. Cannot find group '%s'.\n"),

+					Prog, grp_name);

+			fail_exit (E_GRP_UPDATE);

+		}

+	}

+

+	return result_grp;

+}

+

 /*

  * usage - display usage message and exit

  */

@@ -1530,23 +1580,9 @@ static void close_files (void)

 		SYSLOG ((LOG_ERR, "failure while writing changes to %s", spw_dbname ()));

 		fail_exit (E_PW_UPDATE);

 	}

-	if (do_grp_update) {

-		if (gr_close () == 0) {

-			fprintf (stderr,

-			         _("%s: failure while writing changes to %s\n"), Prog, gr_dbname ());

-			SYSLOG ((LOG_ERR, "failure while writing changes to %s", gr_dbname ()));

-			fail_exit (E_GRP_UPDATE);

-		}

-#ifdef	SHADOWGRP

-		if (is_shadow_grp && (sgr_close () == 0)) {

-			fprintf (stderr,

-			         _("%s: failure while writing changes to %s\n"),

-			         Prog, sgr_dbname ());

-			SYSLOG ((LOG_ERR, "failure while writing changes to %s", sgr_dbname ()));

-			fail_exit (E_GRP_UPDATE);

-		}

-#endif

-	}

+

+	close_group_files ();

+

 #ifdef ENABLE_SUBIDS

 	if (is_sub_uid  && (sub_uid_close () == 0)) {

 		fprintf (stderr,

@@ -1587,34 +1623,9 @@ static void close_files (void)

 		/* continue */

 	}

 	pw_locked = false;

-	if (gr_unlock () == 0) {

-		fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, gr_dbname ());

-		SYSLOG ((LOG_ERR, "failed to unlock %s", gr_dbname ()));

-#ifdef WITH_AUDIT

-		audit_logger (AUDIT_ADD_USER, Prog,

-		              "unlocking-group-file",

-		              user_name, AUDIT_NO_ID,

-		              SHADOW_AUDIT_FAILURE);

-#endif

-		/* continue */

-	}

-	gr_locked = false;

-#ifdef	SHADOWGRP

-	if (is_shadow_grp) {

-		if (sgr_unlock () == 0) {

-			fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sgr_dbname ());

-			SYSLOG ((LOG_ERR, "failed to unlock %s", sgr_dbname ()));

-#ifdef WITH_AUDIT

-			audit_logger (AUDIT_ADD_USER, Prog,

-			              "unlocking-gshadow-file",

-			              user_name, AUDIT_NO_ID,

-			              SHADOW_AUDIT_FAILURE);

-#endif

-			/* continue */

-		}

-		sgr_locked = false;

-	}

-#endif

+

+	unlock_group_files ();

+

 #ifdef ENABLE_SUBIDS

 	if (is_sub_uid) {

 		if (sub_uid_unlock () == 0) {

@@ -1647,6 +1658,71 @@ static void close_files (void)

 #endif				/* ENABLE_SUBIDS */

 }

+/*

+ * close_group_files - close all of the files that were opened

+ *

+ *	close_group_files() closes all of the files that were opened related

+ *  with groups. This causes any modified entries to be written out.

+ */

+static void close_group_files (void)

+{

+	if (do_grp_update) {

+		if (gr_close () == 0) {

+			fprintf (stderr,

+			         _("%s: failure while writing changes to %s\n"), Prog, gr_dbname ());

+			SYSLOG ((LOG_ERR, "failure while writing changes to %s", gr_dbname ()));

+			fail_exit (E_GRP_UPDATE);

+		}

+#ifdef	SHADOWGRP

+		if (is_shadow_grp && (sgr_close () == 0)) {

+			fprintf (stderr,

+			         _("%s: failure while writing changes to %s\n"),

+			         Prog, sgr_dbname ());

+			SYSLOG ((LOG_ERR, "failure while writing changes to %s", sgr_dbname ()));

+			fail_exit (E_GRP_UPDATE);

+		}

+#endif /* SHADOWGRP */

+	}

+}

+

+/*

+ * unlock_group_files - unlock all of the files that were locked

+ *

+ *	unlock_group_files() unlocks all of the files that were locked related

+ *  with groups. This causes any modified entries to be written out.

+ */

+static void unlock_group_files (void)

+{

+	if (gr_unlock () == 0) {

+		fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, gr_dbname ());

+		SYSLOG ((LOG_ERR, "failed to unlock %s", gr_dbname ()));

+#ifdef WITH_AUDIT

+		audit_logger (AUDIT_ADD_USER, Prog,

+		              "unlocking-group-file",

+		              user_name, AUDIT_NO_ID,

+		              SHADOW_AUDIT_FAILURE);

+#endif /* WITH_AUDIT */

+		/* continue */

+	}

+	gr_locked = false;

+#ifdef	SHADOWGRP

+	if (is_shadow_grp) {

+		if (sgr_unlock () == 0) {

+			fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sgr_dbname ());

+			SYSLOG ((LOG_ERR, "failed to unlock %s", sgr_dbname ()));

+#ifdef WITH_AUDIT

+			audit_logger (AUDIT_ADD_USER, Prog,

+			              "unlocking-gshadow-file",

+			              user_name, AUDIT_NO_ID,

+			              SHADOW_AUDIT_FAILURE);

+#endif /* WITH_AUDIT */

+			/* continue */

+		}

+		sgr_locked = false;

+	}

+#endif /* SHADOWGRP */

+}

+

 /*

  * open_files - lock and open the password files

  *

@@ -1668,37 +1744,8 @@ static void open_files (void)

 	/* shadow file will be opened by open_shadow(); */

-	/*

-	 * Lock and open the group file.

-	 */

-	if (gr_lock () == 0) {

-		fprintf (stderr,

-		         _("%s: cannot lock %s; try again later.\n"),

-		         Prog, gr_dbname ());

-		fail_exit (E_GRP_UPDATE);

-	}

-	gr_locked = true;

-	if (gr_open (O_CREAT | O_RDWR) == 0) {

-		fprintf (stderr, _("%s: cannot open %s\n"), Prog, gr_dbname ());

-		fail_exit (E_GRP_UPDATE);

-	}

-#ifdef  SHADOWGRP

-	if (is_shadow_grp) {

-		if (sgr_lock () == 0) {

-			fprintf (stderr,

-			         _("%s: cannot lock %s; try again later.\n"),

-			         Prog, sgr_dbname ());

-			fail_exit (E_GRP_UPDATE);

-		}

-		sgr_locked = true;

-		if (sgr_open (O_CREAT | O_RDWR) == 0) {

-			fprintf (stderr,

-			         _("%s: cannot open %s\n"),

-			         Prog, sgr_dbname ());

-			fail_exit (E_GRP_UPDATE);

-		}

-	}

-#endif

+	open_group_files ();

+

 #ifdef ENABLE_SUBIDS

 	if (is_sub_uid) {

 		if (sub_uid_lock () == 0) {

@@ -1733,6 +1780,39 @@ static void open_files (void)

 #endif				/* ENABLE_SUBIDS */

 }

+static void open_group_files (void)

+{

+	if (gr_lock () == 0) {

+		fprintf (stderr,

+		         _("%s: cannot lock %s; try again later.\n"),

+		         Prog, gr_dbname ());

+		fail_exit (E_GRP_UPDATE);

+	}

+	gr_locked = true;

+	if (gr_open (O_CREAT | O_RDWR) == 0) {

+		fprintf (stderr, _("%s: cannot open %s\n"), Prog, gr_dbname ());

+		fail_exit (E_GRP_UPDATE);

+	}

+

+#ifdef  SHADOWGRP

+	if (is_shadow_grp) {

+		if (sgr_lock () == 0) {

+			fprintf (stderr,

+			         _("%s: cannot lock %s; try again later.\n"),

+			         Prog, sgr_dbname ());

+			fail_exit (E_GRP_UPDATE);

+		}

+		sgr_locked = true;

+		if (sgr_open (O_CREAT | O_RDWR) == 0) {

+			fprintf (stderr,

+			         _("%s: cannot open %s\n"),

+			         Prog, sgr_dbname ());

+			fail_exit (E_GRP_UPDATE);

+		}

+	}

+#endif /* SHADOWGRP */

+}

+

 static void open_shadow (void)

 {

 	if (!is_shadow_pwd) {

-- 

2.25.4