Bug 187189

Summary: [PATCH] pw(1): pw groupmod groupname -g ### creates duplicate groups
Product: Base System Reporter: Adam McDougall <ebay>
Component: binAssignee: Baptiste Daroussin <bapt>
Status: Closed FIXED    
Severity: Affects Some People CC: brd, vmiller
Priority: Normal Keywords: patch, regression
Version: 10.0-RELEASEFlags: bugmeister: mfc-stable10?
bugmeister: mfc-stable9?
bugmeister: mfc-stable8?
Hardware: Any   
OS: Any   
Attachments:
Description Flags
libutil patch mitigating duplicate groups when executing `pw group mod`
none
Reformatted gr_util.c patch (tested) none

Description Adam McDougall 2014-03-02 05:10:00 UTC
'pw groupmod www -g 12345' is expected to change the GID of www in
/etc/group from 80 to 12345 like it does in 9.2-RELEASE, but instead it
will create another duplicate of the www group with the new GID, causing
conflicting information.  It can happen more than once.

How-To-Repeat: # grep www /etc/group
www:*:80

# pw groupmod www -g 12345
# grep www /etc/group
www:*:80:
www:*:12345:
Comment 1 Rick Miller 2014-09-08 14:54:42 UTC
This behavior has been observed in 10.0-RELEASE-p7 as well while enforcing configuration via Puppet as evidenced by the following error message from Puppet:

Error: Could not set gid on group[nagios]: Execution of '/usr/sbin/pw groupmod nagios -g 1234' returned 1: pw:   entry inconsistent
pw: gr_copy(): Invalid argument
Error: /Stage[main]/Users::Userdata/Users::Virtual[nagios]/Group[nagios]/gid: change from 181 to 1234 failed: Could not set gid on group[nagios]: Execution of '/usr/sbin/pw groupmod nagios -g 1234' returned 1: pw: entry   inconsistent
pw: gr_copy(): Invalid argument
Comment 2 Rick Miller 2014-09-16 18:12:30 UTC
Created attachment 147383 [details]
libutil patch mitigating duplicate groups when executing `pw group mod`

A patch authored by Marc de la Gueronniere that mitigates duplicate groups when executing `pw groupmod`
Comment 3 Rick Miller 2014-09-16 18:15:12 UTC
The file was not specified in the patch, but is lib/libutil/gr_util.c
Comment 4 Adam McDougall 2014-09-27 20:16:10 UTC
Created attachment 147735 [details]
Reformatted gr_util.c patch (tested)

I had formatting difficulty with the supplied patch so I applied it by hand and I can confirm it solves my issue.  I made a new diff with tab indentation and the filename to patch inside.  Hopefully ready to commit?  Thanks.
Comment 5 Baptiste Daroussin freebsd_committer freebsd_triage 2014-10-28 16:27:51 UTC
Committed thanks
Comment 6 commit-hook freebsd_committer freebsd_triage 2014-10-28 16:28:23 UTC
A commit references this bug:

Author: bapt
Date: Tue Oct 28 16:27:30 UTC 2014
New revision: 273791
URL: https://svnweb.freebsd.org/changeset/base/273791

Log:
  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:		187189
  Reported by:	mcdouga9@egr.msu.edu
  Tested by:	mcdouga9@egr.msu.edu
  Patch by:	Marc de la Gueronniere

Changes:
  head/lib/libutil/gr_util.c
  head/usr.sbin/pw/tests/pw_modify.sh
Comment 7 commit-hook freebsd_committer freebsd_triage 2014-11-04 07:51:49 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