From 140510de9de4771feb3af1d859c09604043a4c9b Mon Sep 17 00:00:00 2001 From: ikerexxe 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 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