From e481437ab9ebe9a8bf8fbaabe986d42b2f765991 Mon Sep 17 00:00:00 2001
From: Iker Pedrosa <ipedrosa@redhat.com>
Date: Tue, 3 Aug 2021 08:57:20 +0200
Subject: [PATCH] usermod: allow all group types with -G option

The only way of removing a group from the supplementary list is to use
-G option, and list all groups that the user is a member of except for
the one that wants to be removed. The problem lies when there's a user
that contains both local and remote groups, and the group to be removed
is a local one. As we need to include the remote group with -G option
the command will fail.

This reverts commit 140510de9de4771feb3af1d859c09604043a4c9b. This way,
it would be possible to remove the remote groups from the supplementary
list.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1967641
Resolves: https://github.com/shadow-maint/shadow/issues/338

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
---
 src/usermod.c | 220 ++++++++++++++++++++--------------------------------------
 1 file changed, 77 insertions(+), 143 deletions(-)

diff --git a/src/usermod.c b/src/usermod.c
index 03bb9b9..a0c03af 100644
--- a/src/usermod.c
+++ b/src/usermod.c
@@ -187,7 +187,6 @@ 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 *);
@@ -201,9 +200,7 @@ 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);
@@ -261,11 +258,6 @@ static int get_groups (char *list)
 	}
 
 	/*
-	 * 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
 	 * group identifiers is permitted.
@@ -284,7 +276,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 = get_local_group (list);
+		grp = prefix_getgr_nam_gid (list);
 
 		/*
 		 * There must be a match, either by GID value or by
@@ -334,8 +326,6 @@ static int get_groups (char *list)
 		gr_free ((struct group *)grp);
 	} while (NULL != list);
 
-	close_group_files ();
-
 	user_groups[ngroups] = (char *) 0;
 
 	/*
@@ -348,44 +338,6 @@ 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
 {
@@ -1523,7 +1475,50 @@ static void close_files (void)
 	}
 
 	if (Gflg || lflg) {
-		close_group_files ();
+		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 */
+		}
 	}
 
 	if (is_shadow_pwd) {
@@ -1593,60 +1588,6 @@ static void close_files (void)
 }
 
 /*
- * 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
  *
  *	open_files() opens the two password files.
@@ -1681,7 +1622,38 @@ static void open_files (void)
 	}
 
 	if (Gflg || lflg) {
-		open_group_files ();
+		/*
+		 * 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
 	}
 #ifdef ENABLE_SUBIDS
 	if (vflg || Vflg) {
@@ -1718,44 +1690,6 @@ static void open_files (void)
 }
 
 /*
- * 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
  *
  *	usr_update() creates the password file entries for this user and
-- 
1.8.3.1