From e481437ab9ebe9a8bf8fbaabe986d42b2f765991 Mon Sep 17 00:00:00 2001 From: Iker Pedrosa 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 --- 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