Bug 238995 - adduser does not check for pre-existing user field entries in /etc/group
Summary: adduser does not check for pre-existing user field entries in /etc/group
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-04 19:29 UTC by Dennis Clarke
Modified: 2024-05-09 13:17 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dennis Clarke 2019-07-04 19:29:22 UTC
Seems minor but merely annoying. One may edit /etc/group before running
adduser and put in the future username for some group.  Seems trivial.
 
vesta# uname -a 
FreeBSD vesta 12.0-RELEASE-p4 FreeBSD 12.0-RELEASE-p4 GENERIC  amd64

Toss a group into /etc/group :

vesta# /usr/bin/printf "foo:*:12345:someuser\n" >> /etc/group
vesta# grep 'foo' /etc/group
foo:*:12345:someuser
vesta# 

Now run adduser and toss a new user into the system with that group
as a part of the creation process : 

vesta# adduser
Username: someuser
Full name: Some Test User
Uid (Leave empty for default): 54321
Login group [someuser]: 
Login group is someuser. Invite someuser into other groups? []: foo
Login class [default]: 
Shell (sh csh tcsh bash rbash git-shell nologin) [sh]: 
Home directory [/home/someuser]: 
Home directory permissions (Leave empty for default): 
Use password-based authentication? [yes]: 
Use an empty password? (yes/no) [no]: 
Use a random password? (yes/no) [no]: 
Enter password: 
Enter password again: 
Lock out the account after creation? [no]: 
Username   : someuser
Password   : *****
Full Name  : Some Test User
Uid        : 54321
Class      : 
Groups     : someuser foo
Home       : /home/someuser
Home Mode  : 
Shell      : /bin/sh
Locked     : no
OK? (yes/no): yes
adduser: INFO: Successfully added (someuser) to the user database.
Add another user? (yes/no): no
Goodbye!
vesta# 

Here adduser duplicates the entry for the group 'foo' : 

vesta# grep 'foo' /etc/group
foo:*:12345:someuser,someuser
vesta# 
 
Seems trivial and slightly annoying.

-- 
Dennis Clarke
RISC-V/SPARC/PPC/ARM/CISC
UNIX and Linux spoken
GreyBeard and suspenders optional
Comment 1 Dennis Clarke 2021-11-23 05:02:15 UTC
Here we are at the end of 2021 and heading into 2022 and we still
have this situation : 

We have the following group in /etc/group : 

aarch64:*:31415:aarch64

We then create the user aarch64 with the slightly borked adduser : 

europa# adduser 
Username: aarch64
Full name: ARM64 QEMU
Uid (Leave empty for default): 31415
Login group [aarch64]: 
Login group is aarch64. Invite aarch64 into other groups? []: devl
Login class [default]: 
Shell (sh csh tcsh git-shell bash rbash nologin) [sh]: 
Home directory [/home/aarch64]: 
Home directory permissions (Leave empty for default): 
Use password-based authentication? [yes]: 
Use an empty password? (yes/no) [no]: 
Use a random password? (yes/no) [no]: 
Enter password: 
Enter password again: 
Lock out the account after creation? [no]: 
Username   : aarch64
Password   : *****
Full Name  : ARM64 QEMU
Uid        : 31415
Class      : 
Groups     : aarch64 devl
Home       : /home/aarch64
Home Mode  : 
Shell      : /bin/sh
Locked     : no
OK? (yes/no): yes
adduser: INFO: Successfully added (aarch64) to the user database.
Add another user? (yes/no): no
Goodbye!
europa# 

Sure enough the "adduser" creates yet another group with the exact same
name but a separate gid : 

europa# grep 'aarch' /etc/group
devl:*:20002:dclarke,riscv,aarch64
aarch64:*:31415:aarch64
aarch64:*:31416:
europa# 

This is just plain blunt trauma wrong.  So now I need to go manually fix
this mess and change the gid ownership in the new user home directory.

-- 
Dennis Clarke
RISC-V/SPARC/PPC/ARM/CISC
UNIX and Linux spoken
GreyBeard and suspenders optional
Comment 2 Dennis Clarke 2021-11-23 05:06:49 UTC
Minor followup here is that the ownership of the user home directory
was in fact correct ! 

europa# cd /usr/home/aarch64/
europa# ls -lapbin 
total 10
65852 drwxr-xr-x  2 31415  31415     9 Nov 23 04:56 ./
   34 drwxr-xr-x  6 0      0         6 Nov 23 04:56 ../
65857 -rw-r--r--  1 31415  31415   962 Nov 23 04:56 .cshrc
65855 -rw-r--r--  1 31415  31415   323 Nov 23 04:56 .login
65858 -rw-r--r--  1 31415  31415    91 Nov 23 04:56 .login_conf
65853 -rw-------  1 31415  31415   301 Nov 23 04:56 .mail_aliases
65856 -rw-r--r--  1 31415  31415   267 Nov 23 04:56 .mailrc
65854 -rw-r--r--  1 31415  31415   978 Nov 23 04:56 .profile
65859 -rw-r--r--  1 31415  31415  1015 Nov 23 04:56 .shrc
europa# 

That is correct given the group entry : 

aarch64:*:31415:aarch64


Regardless we did get adduser doing the dumb task of creating another
group entry with the same name and a different gid.

Dennis
Comment 3 Dennis Clarke 2023-07-06 12:39:52 UTC
Here in middle of 2023 and adduser and groupmod options to pw seem to be
just plain wrong.

pluto# uname -apKU
FreeBSD pluto 13.2-RELEASE-p1 FreeBSD 13.2-RELEASE-p1 GENERIC amd64 amd64 1302001 1302001
pluto# 

This is a perfectly stable server that does nothing much but serve NFSv3
as well as iSCSI over 10Gbit links. Runs great.

For the sake of experiment I am thinking of planting a QEMU emulation of
a RISC-V instance on that very idle server.

First I create a group :

pluto# pw groupadd -n riscv -g 14142 

Then add a user :

pluto# adduser
Username: riscv
Full name: qemu rv64imafdc
Uid (Leave empty for default): 14142
Login group [riscv]: 
Login group is riscv. Invite riscv into other groups? []: 
Login class [default]: 
Shell (sh csh tcsh nologin) [sh]: 
Home directory [/home/riscv]: 
Home directory permissions (Leave empty for default): 
Use password-based authentication? [yes]: 
Use an empty password? (yes/no) [no]: 
Use a random password? (yes/no) [no]: 
Enter password: 
Enter password again: 
Lock out the account after creation? [no]: 
Username   : riscv
Password   : *****
Full Name  : qemu rv64imafdc
Uid        : 14142
Class      : 
Groups     : riscv 
Home       : /home/riscv
Home Mode  : 
Shell      : /bin/sh
Locked     : no
OK? (yes/no): yes
adduser: INFO: Successfully added (riscv) to the user database.
Add another user? (yes/no): no
Goodbye!
pluto# 

If I look at that group I see that the user does not exist as a member :

pluto# pw groupshow -n riscv
riscv:*:14142:

I have to manually add in the member myself :

pluto# pw groupmod -n riscv -g 14142 -m riscv

Since I have no reason to trust the pw command I need to verify :

pluto# grep 'riscv' /etc/group
riscv:*:14142:riscv

Seems like broken behavior that is very counterintuitive. 

related to : 

    pw(8) usermod: numeric ID (uid) in lieu of name for option -n
    https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=269193

    pw accepts an "illegal option" but completes the command without error
    https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=263188

      Oh joy! bug 263188 seems to have been fixed!


-- 
Dennis Clarke
RISC-V/SPARC/PPC/ARM/CISC
UNIX and Linux spoken
GreyBeard and suspenders optional
Comment 4 commit-hook freebsd_committer freebsd_triage 2023-07-19 13:40:03 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=17839f45d86e79065a65ad3e2522dd69b29a652c

commit 17839f45d86e79065a65ad3e2522dd69b29a652c
Author:     Naman Sood <mail@nsood.in>
AuthorDate: 2023-07-19 12:44:21 +0000
Commit:     Joseph Mingrone <jrm@FreeBSD.org>
CommitDate: 2023-07-19 13:36:09 +0000

    pw: Ensure group membership is not duplicated

    Fix the following problem:

    1. A nonexistent user, someuser, is added to somegroup in /etc/group.
    2. someuser is then created with membership in somegroup.

    The entry for somegroup in /etc/group will then contain

        somegroup:*:12345:someuser,someuser

    With this fix, the entry will be

        somegroup:*:12345:someuser

    PR:             238995
    Reviewed by:    bapt, jrm
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D41076

 usr.sbin/pw/pw.h       | 2 ++
 usr.sbin/pw/pw_group.c | 2 +-
 usr.sbin/pw/pw_user.c  | 3 +++
 3 files changed, 6 insertions(+), 1 deletion(-)
Comment 5 commit-hook freebsd_committer freebsd_triage 2023-07-19 13:42:05 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=1a8d37b8cffc805626a3954496845b7a14a45bea

commit 1a8d37b8cffc805626a3954496845b7a14a45bea
Author:     Naman Sood <mail@nsood.in>
AuthorDate: 2023-07-19 13:06:06 +0000
Commit:     Joseph Mingrone <jrm@FreeBSD.org>
CommitDate: 2023-07-19 13:40:53 +0000

    pw: Use existing group entry, even if it already has members

    Fix the following problem:

    1. A nonexistent user, someuser, is added to /etc/group as
       someuser:*:12345:someuser.
    2. someuser is then created with the default login group.

    A second group entry for someuser will be created.

       someuser:*:12345:someuser
       someuser:*:12346:

    With this fix, the existing group entry will be used.

    PR:             238995
    Reviewed by:    bapt, jrm
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D41057

 usr.sbin/pw/pw_user.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
Comment 6 commit-hook freebsd_committer freebsd_triage 2023-07-19 13:47:06 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=181692ab0896637bc174ab3e1ef319991dfa371f

commit 181692ab0896637bc174ab3e1ef319991dfa371f
Author:     Naman Sood <mail@nsood.in>
AuthorDate: 2023-07-19 13:27:14 +0000
Commit:     Joseph Mingrone <jrm@FreeBSD.org>
CommitDate: 2023-07-19 13:43:12 +0000

    pw: Add regression tests for useradd bug fixes

    PR:             238995
    Reviewed by:    jrm
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D41080

 usr.sbin/pw/tests/pw_useradd_test.sh | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
Comment 7 Naman Sood 2023-07-19 13:57:17 UTC
Hello! I just finished looking into this, and a few things to note:

1. Your 2023 example is expected behavior - `pw groupshow` will list the groups as in /etc/group, but group(5) notes that "A user is automatically in a group if that group specified in their /etc/passwd entry and does not need to be added that group in the group file." Since in that example, the user riscv's login group was riscv, pw did not add it to the group file. If you'd like to get group membership that keeps such things in mind, `id riscv` shows that.

root@neon-testvm:~ # pw groupadd -n testuser -g 4242
root@neon-testvm:~ # adduser
Username: testuser
Full name: 
Uid (Leave empty for default): 4242
Login group [testuser]: 
Login group is testuser. Invite testuser into other groups? []: 
Login class [default]: 
Shell (sh csh tcsh nologin) [sh]: 
Home directory [/home/testuser]: 
Home directory permissions (Leave empty for default): 
Use password-based authentication? [yes]: no
Lock out the account after creation? [no]: 
Username   : testuser
Password   : <disabled>
Full Name  : 
Uid        : 4242
Class      : 
Groups     : testuser 
Home       : /home/testuser
Home Mode  : 
Shell      : /bin/sh
Locked     : no
OK? (yes/no): yes
adduser: INFO: Successfully added (testuser) to the user database.
Add another user? (yes/no): no
Goodbye!
root@neon-testvm:~ # cat /etc/group | grep testuser
testuser:*:4242:
root@neon-testvm:~ # pw groupshow testuser
testuser:*:4242:
root@neon-testvm:~ # id testuser
uid=4242(testuser) gid=4242(testuser) groups=4242(testuser)

The bugs exposed by your 2021 and 2019 examples have been fixed in the commits above.
Comment 8 Joseph Mingrone freebsd_committer freebsd_triage 2023-07-19 14:01:32 UTC
Dennis, thank you for describing the issues.  Please open a new report, if we missed something.  Naman, thanks for the fixes.
Comment 9 commit-hook freebsd_committer freebsd_triage 2024-05-09 13:17:21 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=3532d9c66ecefd356ec670f014e4647537b59ef1

commit 3532d9c66ecefd356ec670f014e4647537b59ef1
Author:     Naman Sood <mail@nsood.in>
AuthorDate: 2023-07-19 12:44:21 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2024-05-09 11:04:30 +0000

    pw: Ensure group membership is not duplicated

    Fix the following problem:

    1. A nonexistent user, someuser, is added to somegroup in /etc/group.
    2. someuser is then created with membership in somegroup.

    The entry for somegroup in /etc/group will then contain

        somegroup:*:12345:someuser,someuser

    With this fix, the entry will be

        somegroup:*:12345:someuser

    PR:             238995
    Reviewed by:    bapt, jrm
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D41076

    (cherry picked from commit 17839f45d86e79065a65ad3e2522dd69b29a652c)

 usr.sbin/pw/pw.h       | 2 ++
 usr.sbin/pw/pw_group.c | 2 +-
 usr.sbin/pw/pw_user.c  | 3 +++
 3 files changed, 6 insertions(+), 1 deletion(-)