Bug 185666 - pw(8): Regression for 'pw usermod <user> -G <grouplist>' [regression]
Summary: pw(8): Regression for 'pw usermod <user> -G <grouplist>' [regression]
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: Baptiste Daroussin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-11 20:00 UTC by sub.mesa
Modified: 2015-07-08 18:17 UTC (History)
0 users

See Also:
bugmeister: mfc-stable10?
bugmeister: mfc-stable9?
bugmeister: mfc-stable8?


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description sub.mesa 2014-01-11 20:00:00 UTC
The pw(8) manpage states:

     -G grouplist  Set additional group memberships for an account.  grouplist
                   is a comma, space or tab-separated list of group names or
                   group numbers.  The user's name is added to the group lists
                   in /etc/group, and removed from any groups not specified in
                   grouplist.

This works as expected on FreeBSD 9.2-RELEASE where the user is removed from any groups not specified explicitly in the -G grouplist. However, on 10.0-RC1 and 10.0-RC3, I can reproduce 100% a regression where the username is *NOT* removed from the 'old group'. It is only added to the new group.

Fix: 

Workaround is to use 'groupmod' instead, to achieve a rough equivalent:

# remove <username> from the group <oldgroup>
pw groupmod <oldgroup> -d <username>
# add <username> to the group <newgroup>
pw groupmod <newgroup> -m <username>

Of course, the usermod -G command is superior, because it removes the user from all groups not specified explicitly.
How-To-Repeat: It works as expected on 9.2:

pw useradd testuser
pw groupadd testgroup
pw groupadd testgroup2
pw usermod testuser -G testgroup

# /etc/group section before:
# testuser:*:1001:
# testgroup:*:1002:testuser
# testgroup2:*:1003:

pw usermod testuser -G testgroup2

# /etc/group section after:
# testuser:*:1001:
# testgroup:*:1002:
# testgroup2:*:1003:testuser

However, on 10.0-RC1 and 10.0-RC3 amd64 the behavior is very different; it does not remove the 'old group':

pw useradd testuser
pw groupadd testgroup
pw groupadd testgroup2
pw usermod testuser -G testgroup

# /etc/group section before:
# testuser:*:1004:
# testgroup:*:1005:testuser
# testgroup2:*:1006:

pw usermod testuser -G testgroup2

# /etc/group section after:
# testuser:*:1004:
# testgroup:*:1005:testuser
# testgroup2:*:1006:testuser
Comment 1 commit-hook freebsd_committer freebsd_triage 2014-10-28 14:20:14 UTC
A commit references this bug:

Author: bapt
Date: Tue Oct 28 14:19:18 UTC 2014
New revision: 273779
URL: https://svnweb.freebsd.org/changeset/base/273779

Log:
  Fix a regression in pw usermod -G list

  The user was perperly adding the to different groups from "list" but was not
  removed from the other groups it could have belong to.
  While here add a regression test about this bug

  PR:		185666
  Reported by:	sub.mesa@gmail.com
  MFC after:	1 week

Changes:
  head/usr.sbin/pw/pw_user.c
  head/usr.sbin/pw/tests/pw_modify.sh
Comment 2 commit-hook freebsd_committer freebsd_triage 2014-11-04 07:51:46 UTC
A commit references this bug:

Author: bapt
Date: Tue Nov  4 07:50:51 UTC 2014
New revision: 274082
URL: https://svnweb.freebsd.org/changeset/base/274082

Log:
  MFC: 272445,272578,273772,273779,273782,273786,273787,273791

  Add a test for bug 191427 where pw(8) will go into an infinite loop
  Add some tests for modifying groups
  When a group is renamed then the group has been invalidated for sure.
  In that case get the group information using the new name.

  Fix a regression in pw usermod -G list

  The user was perperly adding the to different groups from "list" but was not
  removed from the other groups it could have belong to.

  Do not delete the group wheel when bad argument is passed to pw groupdel -g

  Check that the -g argument is actually a number, if not report an error.
  This argument is converted without checking with atoi(3) later so without this
  check it converts any alpha entries into 0 meaning it deletes the group wheel

  Ensure pw userdel -u <invalid> do not try to remove root

  Check the uid passed is actually a number as early as possible

  Fix renaming a group via the gr_copy function

  Add a regression test to pw(8) because the bug was discovered via using:
  pw groupmod

  PR:		193704 [1], 185666 [2], 90114 [3], 187189 [4]
  Submitted by:	Marc de la Gueronniere [4]
  Reported by:	az [1], sub.mesa@gmail.com [2], bkoenig@cs.tu-berlin.de [3],
  		mcdouga9@egr.msu.edu [4]

Changes:
_U  stable/10/
  stable/10/etc/mtree/BSD.tests.dist
  stable/10/lib/libutil/gr_util.c
  stable/10/usr.sbin/pw/Makefile
  stable/10/usr.sbin/pw/pw_group.c
  stable/10/usr.sbin/pw/pw_user.c
  stable/10/usr.sbin/pw/tests/
  stable/10/usr.sbin/pw/tests/Makefile
  stable/10/usr.sbin/pw/tests/pw_delete.sh
  stable/10/usr.sbin/pw/tests/pw_modify.sh
Comment 3 Glen Barber freebsd_committer freebsd_triage 2015-07-08 18:17:35 UTC
Close PRs that have had a corresponding fix committed.