Debian Bug report logs - #295416
userdel should not remove the group which is primary for someone else

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Paul Evans <pevans@zeus.com>

To: Debian Bug Tracking System <submit@bugs.debian.org>

Subject: adduser: deluser will silently delete a group of the same name

Date: Tue, 15 Feb 2005 18:05:47 +0000

Package: adduser
Version: 3.59
Severity: normal


Example:
  # addgroup monkey
  # adduser --ingroup monkey monkey
  # deluser monkey

This example will silently delete the "monkey" group, providing no
output message that it will be doing so. The manpage also gives no
indication that this will happen.

The "monkey" group is deleted even if other users exist for which
"monkey" is their primary group. This behaviour is exhibited for both
normal and --system accounts.

Suggested fixes:

  * Don't delete similarly named group.

or

  * Provide a warning when running deluser; e.g.:

    # deluser monkey
    Removing user `monkey'...
    Warning: Also removing group `monkey'...
    done.
    
    Also document this behaviour in the manpage.

-- System Information:
Debian Release: 3.1
  APT prefers testing
  APT policy: (990, 'testing'), (500, 'unstable')
Architecture: i386 (i686)
Kernel: Linux 2.4.28-leo
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8)

Versions of packages adduser depends on:
ii  debconf                     1.4.30.11    Debian configuration management sy 
ii  passwd                      1:4.0.3-30.8 Change and administer password and 
ii  perl-base                   5.8.4-6      The Pathologically Eclectic Rubbis

-- debconf information excluded

Message #10 received at 295416@bugs.debian.org (full text, mbox, reply):

From: Marc Haber <mh+debian-packages@zugschlus.de>

To: Paul Evans <pevans@zeus.com>, 295416@bugs.debian.org

Cc: Marc Haber <mh+debian-packages@zugschlus.de>

Subject: Re: adduser: deluser will silently delete a group of the same name

Date: Wed, 16 Feb 2005 21:25:18 +0100

reassign #295416 passwd
retitle #295416 userdel will silently delete a group of the same name
thanks

On Tue, Feb 15, 2005 at 06:05:47PM +0000, Paul Evans wrote:
> Example:
>   # addgroup monkey
>   # adduser --ingroup monkey monkey
>   # deluser monkey
> 
> This example will silently delete the "monkey" group, providing no
> output message that it will be doing so. The manpage also gives no
> indication that this will happen.

deluser is a front-end for userdel here, which is responsible for
deleting the group:

[25/25]mh@lefler[chroot sid]:~$ grep monkey /etc/group
monkey:x:1002:
[26/26]mh@lefler[chroot sid]:~$ sudo userdel monkey
[27/27]mh@lefler[chroot sid]:~$ grep monkey /etc/group
[28/28]mh@lefler[chroot sid]:~$ 

I am reassigning appropriately.

Greetings
Marc

-- 
-----------------------------------------------------------------------------
Marc Haber         | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany  |  lose things."    Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature |  How to make an American Quilt | Fax: *49 621 72739835

Message #19 received at 295416@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 295416@bugs.debian.org

Subject: Deleting a user group in userdel should only be done if the group is empty

Date: Mon, 18 Apr 2005 18:30:28 +0200

Bug #295416 seems to be a general design issue : should userdel try to
remove the group the user it deletes belonged to if this is a "user
group".

If it doesn't, then admins are likely to keep up with dozens of
remaining user groups. Indeed, we cannot change this silently.

However, if for some (good or bad) reason, the suer groups has other
members than the currently deleted user, I agree with the bug
submitter : userdel should at least issue a warning....or maybe even
*not* delete the group (and issue an "error" message).

Any thoughts ?


-- 

Message #24 received at 295416@bugs.debian.org (full text, mbox, reply):

From: Alexander Gattin <arg@online.com.ua>

To: 295416@bugs.debian.org

Subject: Re: [Pkg-shadow-devel] Bug#295416: Deleting a user group in userdel should only be done if the group is empty

Date: Mon, 18 Apr 2005 22:25:17 +0300

Hi!

On Mon, Apr 18, 2005 at 06:30:28PM +0200, Christian Perrier wrote:
> However, if for some (good or bad) reason, the suer groups has other
> members than the currently deleted user, I agree with the bug
> submitter : userdel should at least issue a warning....or maybe even
> *not* delete the group (and issue an "error" message).

IMHO it should be researched where/how is userdel
used/scripted.

Although generally I prefer early abort/failure,
it seems more appropriate to delete user anyway, just
keep the group when there are other members in it
(and complain to stderr/syslog).

-- 
WBR,
xrgtn

Message #29 received at 295416@bugs.debian.org (full text, mbox, reply):

From: Alexander Gattin <arg@online.com.ua>

To: 295416@bugs.debian.org, 295416-submitter@bugs.debian.org

Cc: control@bugs.debian.org

Subject: Re: [Pkg-shadow-devel] Bug#295416: Deleting a user group in userdel should only be done if the group is empty

Date: Tue, 19 Apr 2005 08:18:09 +0300

tags 295416 confirmed
thanks

Hi!

On Mon, Apr 18, 2005 at 06:30:28PM +0200, Christian Perrier wrote:
> However, if for some (good or bad) reason, the suer groups has other
> members than the currently deleted user, I agree with the bug
> submitter : userdel should at least issue a warning....or maybe even
> *not* delete the group (and issue an "error" message).
> 
> Any thoughts ?

Yes. :)

This is not about removing groups which have other
members in them. And userdel never did this anyway
(userdel tries to remove personal groups since r1.16
CVS version, but only if USERGROUPS_ENAB is set and
group has no other members):
> 	/*
> 	 * we've removed their name from all the groups above, so
> 	 * now if they have a group with the same name as their
> 	 * user name, with no members, we delete it.
> 	 */
> 
> 	grp = getgrnam (user_name);
> 	if (grp && getdef_bool ("USERGROUPS_ENAB")
> 	    && (grp->gr_mem[0] == NULL)) {
> 
> 		gr_remove (grp->gr_name);

IMHO the problem is in removing groups which are empty
but are refered in /etc/passwd as primary for someone
else!

_This_ is a bug.

And proposed verbose indication of group removal is of
very low priority/value, IMHO again.

-- 
WBR,
xrgtn

Message #39 received at 295416@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 295416@bugs.debian.org

Cc: adduser@packages.debian.org

Subject: Re: Bug#295416: [Pkg-shadow-devel] Bug#295416: Deleting a user group in userdel should only be done if the group is empty

Date: Tue, 19 Apr 2005 07:15:51 +0200

Quoting Alexander Gattin (arg@online.com.ua):

> IMHO it should be researched where/how is userdel
> used/scripted.


As the bug report mentions, most of the time in deluser. In Debian, at
least.

Marc, CC'ing you : what is your opinion about the "right" behaviour
from userdel (from deluser point of view) when a user group has an
extra member and not the user him/herself only?

-userdel should fail and not remove the user groups

-userdel should remove the user group but not fail, just issue a
 warning

Message #44 received at 295416@bugs.debian.org (full text, mbox, reply):

From: Marc Haber <mh+debian-packages@zugschlus.de>

To: Christian Perrier <bubulle@debian.org>, 295416@bugs.debian.org

Cc: adduser@packages.debian.org, Marc Haber <mh+debian-packages@zugschlus.de>

Subject: Re: Re: Bug#295416: [Pkg-shadow-devel] Bug#295416: Deleting a user group in userdel should only be done if the group is empty

Date: Sat, 30 Apr 2005 20:14:14 +0200

On Tue, Apr 19, 2005 at 07:15:51AM +0200, Christian Perrier wrote:
> Marc, CC'ing you : what is your opinion about the "right" behaviour
> from userdel (from deluser point of view) when a user group has an
> extra member and not the user him/herself only?
> 
> -userdel should fail and not remove the user groups
> 
> -userdel should remove the user group but not fail, just issue a
>  warning

I think that basically userdel should behave as it behaves everywhere,
to keep people who want (or need) to use the low-level tools are not
in for any surprises.

Deluser does things before calling userdel, so I think that userdel
should do what has been requested as good as possible without failing.
This is most likely the way that won't have the system end up in a
badly inconsistent state.

Greetings
Marc

-- 
-----------------------------------------------------------------------------
Marc Haber         | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany  |  lose things."    Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature |  How to make an American Quilt | Fax: *49 621 72739835

Message #49 received at 295416@bugs.debian.org (full text, mbox, reply):

From: Alexander Gattin <arg@online.com.ua>

To: 295416@bugs.debian.org

Cc: adduser@packages.debian.org

Subject: Re: Bug#295416: Deleting a user group in userdel should only be done if the group is empty

Date: Sun, 1 May 2005 00:59:34 +0300

Hi!

On Sat, Apr 30, 2005 at 08:14:14PM +0200, Marc Haber wrote:
> On Tue, Apr 19, 2005 at 07:15:51AM +0200, Christian Perrier wrote:
> > Marc, CC'ing you : what is your opinion about the "right" behaviour
> > from userdel (from deluser point of view) when a user group has an
> > extra member and not the user him/herself only?
> > 
> > -userdel should fail and not remove the user groups
> > 
> > -userdel should remove the user group but not fail, just issue a
> >  warning
> 
> I think that basically userdel should behave as it behaves everywhere,
> to keep people who want (or need) to use the low-level tools are not
> in for any surprises.

So it will delete user and leave group in place,
[when the group has other members in it], as far
as I remember the common behaviour [of userdel].

> Deluser does things before calling userdel, so I think that userdel
> should do what has been requested as good as possible without failing.
> This is most likely the way that won't have the system end up in a
> badly inconsistent state.

Probably we should consider the following functional
split between adduser/deluser (i.e. high-level) and
useradd/userdel (low-level) tools:

* low-level tools should care of and keep the minimal
  consistency and integrity of the data that affects
  correct operation of these low-level tools
  themselves (so that e.g. results of useradd are
  revertable with userdel).
  I mean that useradd/userdell should only preserve
  integrity of /etc/passwd, shadow, group, gshadow and
  other most necessary files.
  These tools shouldn't care much about home dirs,
  NIS/LDAP/whatever interoperability, umasks/perms,
  mail spool files, good/bad user/groupnames and other
  sort of high-level consistency checks.

* all policy and high-level consistency checks are
  "praerogativa" of high-level tools.

The same I state in bug #264879, although rather
implicitly.

P.S. please, keep in mind that bug #295416 is not about
deleting group which has other members in it.
The bug is about userdel removing group which is
_primary_ for someone else (not having other _members_).

-- 
WBR,
xrgtn

Message #54 received at 295416-forwarded@bugs.debian.org (full text, mbox, reply):

From: Alexander Gattin <arg@online.com.ua>

To: Tomasz Kłoczko <kloczek@zie.pg.gda.pl>

Cc: 295416-forwarded@bugs.debian.org, control@bugs.debian.org

Subject: Bug#295416: userdel should not remove the group which is primary for someone else

Date: Sun, 1 May 2005 02:50:42 +0300

retitle 295416 userdel should not remove the group which is primary for someone else
thanks

Hi, Tomasz!

Please, indicate what do you think about this bug.

/* Going through standard procedure for
"forwarded-upstream" ;) */

On Mon, Apr 18, 2005 at 06:30:28PM +0200, Christian Perrier wrote:
> However, if for some (good or bad) reason, the suer groups has other
> members than the currently deleted user, I agree with the bug
> submitter : userdel should at least issue a warning....or maybe even
> *not* delete the group (and issue an "error" message).
> 
> Any thoughts ?

Yes. :)

This is not about removing groups which have other
members in them. And userdel never did this anyway
(userdel tries to remove personal groups since r1.16
CVS version, but only if USERGROUPS_ENAB is set and
group has no other members):
> 	/*
> 	 * we've removed their name from all the groups above, so
> 	 * now if they have a group with the same name as their
> 	 * user name, with no members, we delete it.
> 	 */
> 
> 	grp = getgrnam (user_name);
> 	if (grp && getdef_bool ("USERGROUPS_ENAB")
> 	    && (grp->gr_mem[0] == NULL)) {
> 
> 		gr_remove (grp->gr_name);

IMHO the problem is in removing groups which are empty
but are refered in /etc/passwd as primary for someone
else!

_This_ is a bug.

And proposed verbose indication of group removal is of
very low priority/value, IMHO again.

-- 
WBR,
xrgtn

Message #59 received at 295416@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 295416@bugs.debian.org

Cc: adduser@packages.debian.org

Subject: Re: [Pkg-shadow-devel] Bug#295416: Deleting a user group in userdel should only be done if the group is empty

Date: Sun, 1 May 2005 08:10:01 +0200

tags 295416 upstream
retitle 295416 userdel should not remove the user's primary group is it has other members
thanks


> Probably we should consider the following functional
> split between adduser/deluser (i.e. high-level) and
> useradd/userdel (low-level) tools:

The most important split, imho, is that useradd/userdel and other
low-level utilities are potentially shared among several Unices and
Linux distros.

So, as Marc mentioned, care should be taken to keep them behave
consistently among these.

For me, this forbids us (Debian maintainers) to patch them in any way
to make their behaviour different in Debian.

But, we should also remember that we work with upstream, so we're
likely to influence a change in the design.

>   I mean that useradd/userdell should only preserve
>   integrity of /etc/passwd, shadow, group, gshadow and
>   other most necessary files.


I mostly agree here. 


> The same I state in bug #264879, although rather
> implicitly.
> 
> P.S. please, keep in mind that bug #295416 is not about
> deleting group which has other members in it.
> The bug is about userdel removing group which is
> _primary_ for someone else (not having other _members_).


Not exactly. This is about userdel *always* deleting the primary group
of the user it deletes, no matter whether this group is used by
another user.

From your statement above ("userdel should guarantee the integrity of
system files"), such behaviour should *not* happen. userdel should
only delete the primary group of the user *only* if it has no other
members.

So, in my opinion, this bug is still an upstream bug. An I tag it
accordingly (which I forgot to do, though it is marked as "forwarded")

I also give it a better title.

Message #64 received at 295416@bugs.debian.org (full text, mbox, reply):

From: Alexander Gattin <arg@online.com.ua>

To: 295416@bugs.debian.org

Cc: adduser@packages.debian.org

Subject: Re: Bug#295416: [Pkg-shadow-devel] Deleting a user group in userdel should only be done if the group is empty

Date: Sun, 1 May 2005 13:34:16 +0300

Hi!

On Sun, May 01, 2005 at 08:10:01AM +0200, Christian Perrier wrote:
> > Probably we should consider the following functional
> > split between adduser/deluser (i.e. high-level) and
> > useradd/userdel (low-level) tools:
> The most important split, imho, is that useradd/userdel and other
> low-level utilities are potentially shared among several Unices and
> Linux distros.
> 
> So, as Marc mentioned, care should be taken to keep them behave
> consistently among these.

Of course, this should be taken into account as long as
it doesn't create problem for Debian.

But, problems are inavoidable, considering the
direction taken by Tomasz. He tends to extend
functionality of usedadd/userdel thus shifting the
tools into high-level.

This interferes with my proposed policy (split
between high/lowlevel tools).

> For me, this forbids us (Debian maintainers) to patch them in any way
> to make their behaviour different in Debian.
> 
> But, we should also remember that we work with upstream, so we're
> likely to influence a change in the design.

Yes.

IMHO, the high/lowlevel conflict can be solved in
all-satisfactory way by introducing switches to
usedadd/userdel that make them skip "high-level
consistency checks" [thus switching the tools into
low-level explicitly].

For example, usedadd could use smth. like
"--force-badname" cmdline option.

I think this is what we need here and which will not
break compatibility.

> > P.S. please, keep in mind that bug #295416 is not about
> > deleting group which has other members in it.
> > The bug is about userdel removing group which is
> > _primary_ for someone else (not having other _members_).
> Not exactly. This is about userdel *always* deleting the primary group
> of the user it deletes, no matter whether this group is used by
> another user.

No, userdel really always _tries_ to delete a primary
group for user being deleted (and this is not
documented, unfortunately).

But of course userdel doesn't delete a group which has
other members in it:
> cherokee:~# groupadd bug295416
> cherokee:~# useradd -g bug295416 bug295416
> cherokee:~# useradd -G bug295416 other295416
// created with primary group "users" (by default)
> cherokee:~# getent group bug295416
> bug295416:x:1005:other295416

Let's delete "bug295416" user now:
> cherokee:~# userdel bug295416
> cherokee:~# getent group bug295416
> bug295416:x:1005:other295416

Here you see that the "bug295416" group is left in
place.

But in the next scenario we can clearly see a problem:
> cherokee:~# groupadd bug295416
> cherokee:~# useradd -g bug295416 bug295416
> cherokee:~# useradd -g bug295416 other295416
> cherokee:~# userdel bug295416
> cherokee:~# getent group bug295416
> cherokee:~# 
Here you see that "bug295416" group is deleted. But
it's still the primary group for other295416:
> cherokee:~# getent passwd other295416
> other295416:x:1007:1005::/home/other295416:

Here the integrity of /etc/passwd+/etc/group is broken,
because an entry in former refers to gid 1005 which is
absent from latter.

> >From your statement above ("userdel should guarantee the integrity of
> system files"), such behaviour should *not* happen.

Yes, thus I forwarded a message to Tomasz. There is a
bug in upstream, IMHO.

-- 
My best regards,
xrgtn

Message #69 received at 295416@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 295416@bugs.debian.org

Subject: Could we propose a patch for the "userdel should not remove the group which is primary for someone else" bug ?

Date: Mon, 12 Sep 2005 07:33:17 +0200

http://bugs.debian.org/295416 is a long bug long, but it can be
resumed as:

root@mykerinos:~# groupadd bug295416
root@mykerinos:~# useradd -g bug295416 bug295416
root@mykerinos:~# useradd -g bug295416 other295416
root@mykerinos:~# userdel bug295416
root@mykerinos:~# getent group bug295416
root@mykerinos:~# 

Das ist nich gut....

I'm pretty sure that finding a patch for this is easy, if we assume
that Tomasz is currently focused on other tasks and can't handle it.

Who tries?



-- 

Message #74 received at 295416@bugs.debian.org (full text, mbox, reply):

From: Nicolas François <nicolas.francois@centraliens.net>

To: 295416@bugs.debian.org

Subject: Re: Bug#295416: Could we propose a patch for the "userdel should not remove the group which is primary for someone else" bug ?

Date: Sat, 1 Oct 2005 18:33:49 +0200

[Message part 1 (text/plain, inline)]

Hi!

On Mon, Sep 12, 2005 at 07:33:17AM +0200, bubulle@debian.org wrote:
> http://bugs.debian.org/295416 is a long bug long, but it can be
> resumed as:
> 
> root@mykerinos:~# groupadd bug295416
> root@mykerinos:~# useradd -g bug295416 bug295416
> root@mykerinos:~# useradd -g bug295416 other295416
> root@mykerinos:~# userdel bug295416
> root@mykerinos:~# getent group bug295416
> root@mykerinos:~# 
> 
> Das ist nich gut....
> 
> I'm pretty sure that finding a patch for this is easy, if we assume
> that Tomasz is currently focused on other tasks and can't handle it.

The attached patch do not really fix this, but can be used as a basis.

I think we all agree that the user must be warned, don't we?

The problem is shall we remove the group or not? Do we need a special
return value?
What should the warning tell? Currently:
    Warning: The primary group of bug295416 (bug295416, GID=1003)
    will be removed, but is also the primary group of other295416
    (ID=1002).

I tend to agree with Alexander: if we delete the group (with a warning),
the system databases won't be consistent.
I would prefer to let the group, and warn the administrator, so that she
can delete the group with groupdel if needed.

Kind Regards,
-- 
Nekral

[userdel.c.diff (text/plain, attachment)]

Message #79 received at 295416@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 295416@bugs.debian.org

Subject: Re: [Pkg-shadow-devel] Bug#295416: Could we propose a patch for the "userdel should not remove the group which is primary for someone else" bug ?

Date: Sat, 1 Oct 2005 19:12:09 +0200

> The attached patch do not really fix this, but can be used as a basis.
> 
> I think we all agree that the user must be warned, don't we?

Sure

> 
> The problem is shall we remove the group or not? Do we need a special
> return value?

We shouldn't remove the group because this would induce
inconsistencies in users databases.


> What should the warning tell? Currently:
>     Warning: The primary group of bug295416 (bug295416, GID=1003)
>     will be removed, but is also the primary group of other295416
>     (ID=1002).

"Cannot remove group %s which is a primary group for another user"

And a dedicated exit code (not sure about this as this could break
some utilities relying on non zero exit code to check that the user
has indeed been removed)

Message #84 received at 295416@bugs.debian.org (full text, mbox, reply):

From: Nicolas François <nicolas.francois@centraliens.net>

To: 295416@bugs.debian.org

Subject: Re: Bug#295416: Could we propose a patch for the "userdel should not remove the group which is primary for someone else" bug ?

Date: Sun, 2 Oct 2005 02:35:47 +0200

[Message part 1 (text/plain, inline)]

Hello Tomasz,

Can you have a look at these patches.

The second patch (userdel_2.patch) gives a special return value in this case.

Kind Regards,
-- 
Nekral

[userdel_1.patch (text/plain, attachment)]

[userdel_2.patch (text/plain, attachment)]

Message #89 received at 295416@bugs.debian.org (full text, mbox, reply):

From: Tomasz Kłoczko <kloczek@zie.pg.gda.pl>

To: Nicolas François <nicolas.francois@centraliens.net>, 295416@bugs.debian.org

Subject: Re: [Pkg-shadow-devel] Bug#295416: Could we propose a patch for the "userdel should not remove the group which is primary for someone else" bug ?

Date: Sun, 2 Oct 2005 17:09:45 +0200 (CEST)

[Message part 1 (text/plain, inline)]

On Sun, 2 Oct 2005, Nicolas François wrote:

> Hello Tomasz,
> 
> Can you have a look at these patches.
> 
> The second patch (userdel_2.patch) gives a special return value in this case.

userdel_1.patch can be commited (+- indentation) but userdel_2.patch not 
because it uses bit operation on exit code value and changes now used 
E_HOMEDIR value.

Also current variant of handle exit code by errors++ in two points in 
userdel also looks strange/incorrect because this not uses semantics like 
"exit immediately on first found error".
Best will be use fprintf() for output error message and immediately stop
program using fail_exit().

I'm not shure now why was used collecting some class of error cases and 
exit in one point of userdel program (I'm still reviewing code).

kloczek
-- 
-----------------------------------------------------------
*Ludzie nie mają problemów, tylko sobie sami je stwarzają*
-----------------------------------------------------------
Tomasz Kłoczko, sys adm @zie.pg.gda.pl|*e-mail: kloczek@rudy.mif.pg.gda.pl*

Message #94 received at 295416@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 295416@bugs.debian.org, Tomasz Kłoczko <kloczek@zie.pg.gda.pl>

Subject: Patches "half"-commited

Date: Sat, 8 Oct 2005 14:19:03 +0200

Tomasz, from what I see, you applied one of the two patches provided
by Nicolas to deal with userdel not deleting a group which is primary
for another user.

This seems enough to close down #295416, though. Agreed?




-- 

Message #101 received at 295416-done@bugs.debian.org (full text, mbox, reply):

From: Nicolas François <nicolas.francois@centraliens.net>

To: 295416-done@bugs.debian.org

Subject: userdel now checks if the group being deleted is primary for someone else

Date: Wed, 9 Nov 2005 23:17:16 +0100

Package: passwd
Version: 4.0.13-1

The first patch (see the BTS - Oct 2, 2005) was applied upstream in the
4.0.13 tar ball.  The second patch (which changes the userdel return value
in this case) could not be applied.

I'm hence closing this bug.

Kind Regards,
-- 
Nekral

Send a report that this bug log contains spam.