[Pkg-samba-maint] Bug#1055752: `groupadd --force --system sambashare` in samba.postinst is wrong

[Michael Tokarev]

Control: tag -1 + moreinfo

10.11.2023 18:04, Osamu Aoki:
> Source: samba
> Severity: normal
> 
> Problem: `groupadd --force --system sambashare` in samba.postinst is wrong
> 
> Versions:  2:4.17.12+dfsg-0+deb12u1, 2:4.19.2+dfsg-1
> Salsa:     0610d7670c6 ("update changelog; upload version 4.19.2+dfsg-1 to unstable", 2023-10-16)
> 
> groupadd is in essential but command syntax is not the same as addgroup
> from adduser package.  Simply replacing adduser is not the right fix.
> 
> I see you committed on this happened from:
>    1eb07efc2fb ("d/winbind.postinst: switch addgroup => groupadd and eliminate getent", 2022-11-03)
> 
> What happened was adduser is not essential.  So if you don't depend on
> it, piuparts fails.  (Yes, there may have been some transitional problem
> etc.  But this is the core of the issue.)  So please add depends to
> adduser and use the older good code.
> 
> If you insist on using groupadd from shadow package, you need to use
> something along (but this may still fail on some corner cases:
> 
> groupadd -f -K MIN_GID=100 -K MAX_GID=999 sambashare
> 
> I still think this use of groupadd is bad idea.
> 
> Use of getent in old code should be no problem since it is in libc-bin
> which is priority required.

Why are you saying it all?  I don't follow.  Sure thing, groupadd does not
have the same syntax as addgroup, but this is irrelevant.

 From groupadd manpage:

        --force
            This option causes the command to simply exit with success status
            if the specified group already exists

So this eliminates the need for getent, I can use just a single call to
groupadd, it will do nothing if the group is already exists.

        --system
            Create a system group.

            The numeric identifiers of new system groups are chosen in the
            SYS_GID_MIN-SYS_GID_MAX range, defined in login.defs, instead of
            GID_MIN-GID_MAX.

Why do you suggest to hard-code -K MIN_GID && MAX_GID instead of using
whatever values are configured in login.defs?  I'd say the opposite:
if addgroup always used 100 & 999 here, instead of values from login.defs,
it is a bug in addgroup, and I don't want to use buggy software.

I don't see the point. groupadd suits the task perfectly.

/mjt