From 59585bb37022498ad26331f11ddb7088745dc2c9 Mon Sep 17 00:00:00 2001 From: ipedrosa Date: Thu, 14 May 2020 14:28:59 +0200 Subject: [PATCH] check only local groups when adding new supplementary groups to a user --- shadow-4.8.1-check-local-groups.patch | 642 ++++++++++++++++++++++++++ shadow-utils.spec | 8 +- 2 files changed, 649 insertions(+), 1 deletion(-) create mode 100644 shadow-4.8.1-check-local-groups.patch diff --git a/shadow-4.8.1-check-local-groups.patch b/shadow-4.8.1-check-local-groups.patch new file mode 100644 index 0000000..6e9d2bf --- /dev/null +++ b/shadow-4.8.1-check-local-groups.patch @@ -0,0 +1,642 @@ +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 + diff --git a/shadow-utils.spec b/shadow-utils.spec index b3e4575..8e5057f 100644 --- a/shadow-utils.spec +++ b/shadow-utils.spec @@ -1,7 +1,7 @@ Summary: Utilities for managing accounts and shadow password files Name: shadow-utils Version: 4.8.1 -Release: 2%{?dist} +Release: 3%{?dist} Epoch: 2 URL: http://pkg-shadow.alioth.debian.org/ Source0: https://github.com/shadow-maint/shadow/releases/download/%{version}/shadow-%{version}.tar.xz @@ -53,6 +53,8 @@ Patch40: shadow-4.8-ignore-login-prompt.patch Patch42: shadow-4.8-useradd-selinux-mail.patch # Clarify useradd man regarding "-d" parameter - already upstreamed Patch43: shadow-4.8.1-useradd-man-clarification.patch +# Upstreamed +Patch44: shadow-4.8.1-check-local-groups.patch License: BSD and GPLv2+ BuildRequires: gcc @@ -102,6 +104,7 @@ are used for managing group accounts. %patch40 -p1 -b .login-prompt %patch42 -p1 -b .useradd-selinux-mail %patch43 -p1 -b .useradd-man-clarification +%patch44 -p1 -b .check-local-groups iconv -f ISO88591 -t utf-8 doc/HOWTO > doc/HOWTO.utf8 cp -f doc/HOWTO.utf8 doc/HOWTO @@ -257,6 +260,9 @@ done %{_mandir}/man8/vigr.8* %changelog +* Thu May 14 2020 Iker Pedrosa - 2:4.8.1-3 +- check only local groups when adding new supplementary groups to a user (#1727236) + * Tue Mar 24 2020 Iker Pedrosa - 2:4.8.1-2 - useradd: clarify the useradd -d parameter behavior in man page