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 03bb9b9d..a0c03afa 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);

@@ -260,11 +257,6 @@ 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

@@ -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) {

@@ -1592,60 +1587,6 @@ 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

  *

@@ -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) {

@@ -1717,44 +1689,6 @@ 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.31.1