[Pkg-shadow-devel] Bug#295416: Could we propose a patch for the "userdel should not remove the group which is primary for someone else" bug ?

[Nicolas François]

Hello Tomasz,

Can you have a look at these patches.

The second patch (userdel_2.patch) gives a special return value in this case.

Kind Regards,
-- 
Nekral
-------------- next part --------------
Index: userdel.c
===================================================================
RCS file: /cvsroot/shadow/src/userdel.c,v
retrieving revision 1.50
diff -u -r1.50 userdel.c
--- userdel.c	7 Sep 2005 15:00:45 -0000	1.50
+++ userdel.c	2 Oct 2005 00:16:10 -0000
@@ -116,6 +116,7 @@
 {
 	const struct group *grp;
 	struct group *ngrp;
+	struct passwd *pwd;
 
 #ifdef	SHADOWGRP
 	int deleted_user_group = 0;
@@ -170,18 +171,43 @@
 	if (grp && getdef_bool ("USERGROUPS_ENAB")
 	    && (grp->gr_mem[0] == NULL)) {
 
-		gr_remove (grp->gr_name);
+		/*
+		 * Scan the passwd file to check if this group is still
+		 * used as a primary group.
+		 */
+		setpwent ();
+		while ((pwd = getpwent ())) {
+			if (strcmp (pwd->pw_name, user_name) == 0)
+				continue;
+			if (pwd->pw_gid == grp->gr_gid) {
+				fprintf (stderr,
+					 _("%s: Cannot remove group %s which is a primary group for another user.\n"),
+					 Prog, grp->gr_name);
+				break;
+			}
+		}
+		endpwent();
+
+		if (pwd == NULL) {
+			/*
+			 * We can remove this group, it is not the primary
+			 * group of any remaining user.
+			 */
+			gr_remove (grp->gr_name);
 
 #ifdef SHADOWGRP
-		deleted_user_group = 1;
+			deleted_user_group = 1;
 #endif
 
 #ifdef WITH_AUDIT
-		audit_logger (AUDIT_USER_CHAUTHTOK, Prog, "deleting group",
-			      user_name, user_id, 0);
+			audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
+			              "deleting group",
+			              user_name, user_id, 0);
 #endif
-		SYSLOG ((LOG_INFO, "removed group `%s' owned by `%s'\n",
-			 grp->gr_name, user_name));
+			SYSLOG ((LOG_INFO,
+			        "removed group `%s' owned by `%s'\n",
+			        grp->gr_name, user_name));
+		}
 	}
 #ifdef	SHADOWGRP
 	if (!is_shadow_grp)
@@ -730,6 +756,7 @@
 				break;
 			}
 		}
+		endpwent();
 	}
 #endif
 
-------------- next part --------------
--- src/userdel.c.orig	2005-10-02 02:17:56.000000000 +0200
+++ src/userdel.c	2005-10-02 02:25:45.000000000 +0200
@@ -63,7 +63,10 @@
 #define E_NOTFOUND	6	/* specified user doesn't exist */
 #define E_USER_BUSY	8	/* user currently logged in */
 #define E_GRP_UPDATE	10	/* can't update group file */
-#define E_HOMEDIR	12	/* can't remove home directory */
+#define E_HOMEDIR	16	/* can't remove home directory */
+#define E_GRP_USED	32	/* group used by another user, not removed */
+/* The return value in case of a recoverable error */
+static int return_value = E_SUCCESS;
 static char *user_name;
 static uid_t user_id;
 static char *user_home;
@@ -207,6 +210,8 @@
 			SYSLOG ((LOG_INFO,
 			        "removed group `%s' owned by `%s'\n",
 			        grp->gr_name, user_name));
+		} else {
+			return_value |= E_GRP_USED;
 		}
 	}
 #ifdef	SHADOWGRP
@@ -601,7 +606,6 @@
 {
 	struct passwd *pwd;
 	int arg;
-	int errors = 0;
 
 #ifdef USE_PAM
 	pam_handle_t *pamh = NULL;
@@ -730,7 +734,7 @@
 			 _("%s: %s not owned by %s, not removing\n"),
 			 Prog, user_home, user_name);
 		rflg = 0;
-		errors++;
+		return_value |= E_HOMEDIR;
 	}
 #ifdef EXTRA_CHECK_HOME_DIR
 	/* This may be slow, the above should be good enough. */
@@ -752,7 +756,7 @@
 					 ("%s: not removing directory %s (would remove home of user %s)\n"),
 					 Prog, user_home, pwd->pw_name);
 				rflg = 0;
-				errors++;
+				return_value |= E_HOMEDIR;
 				break;
 			}
 		}
@@ -771,7 +775,7 @@
 				      "deleting home directory", user_name,
 				      user_id, 1);
 #endif
-			errors++;
+			return_value |= E_HOMEDIR;
 		}
 #ifdef WITH_AUDIT
 		audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
@@ -801,10 +805,10 @@
 		pam_end (pamh, PAM_SUCCESS);
 #endif				/* USE_PAM */
 #ifdef WITH_AUDIT
-	if (errors)
+	if (return_value & E_HOMEDIR)
 		audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
 			      "deleting home directory", user_name, -1, 0);
 #endif
-	exit (errors ? E_HOMEDIR : E_SUCCESS);
+	exit (return_value);
 	/* NOT REACHED */
 }