search

shadow-maint / shadow

Upstream shadow tree
Other
287 stars 227 forks source link

[rfc] drop everything that conflicts with util-linux #999

Open dkwo opened 1 month ago

dkwo commented 1 month ago

There are commands (as well as their pam files and man pages) that conflict with either coreutils or util-linux. Right now, linux distros either remove them by hand after building, or patch the makefile. It would be far better to have these as a configure option, either per command or in larger groups. As a byproduct, one could also try to respect the configure options for /usr/bin etc.

cc @alejandro-colomar

alejandro-colomar commented 1 month ago

Hmmm. Can you please list the conflicts with each of those packages?

alejandro-colomar commented 1 month ago

These are the conflicts with coreutils, AFAICS:

$ ( \
        apt-file show coreutils \
        | grep /bin/ \
        | awk '{print $2}' \
        | sed s/.usr.bin.//;
        find src/ -type f \
        | grep '\.c$' \
        | sed s/src.// \
        | sed s/.c$//;
    ) \
    | sort \
    | uniq -d;
groups
id

And with util-linux, at least on my Debian system:

$ ( \
        apt-file show util-linux \
        | grep /bin/ \
        | awk '{print $2}' \
        | sed s/.usr.bin.//;
        find src/ -type f \
        | grep '\.c$' \
        | sed s/src.// \
        | sed s/.c$//;
    ) \
    | sort \
    | uniq -d;
su
alejandro-colomar commented 1 month ago

Right now, linux distros either remove them by hand after building, or patch the makefile. It would be far better to have these as a configure option, either per command or in larger groups.

Can you please show how much code it takes to remove these downstream? And how much it would take to do it upstream?

As a byproduct, one could also try to respect the configure options for /usr/bin etc.

Can you clarify that?

alejandro-colomar commented 1 month ago

Related: https://github.com/shadow-maint/shadow/issues/464

dkwo commented 1 month ago

Thanks for looking at this, and sorry for being slow.

Right. util-linux on Void linux also has /usr/bin/{newgrp,login,chsh,chfn,nologin,,vipw,vigr}, so if any of these is in shadow, it would confict. There's also a mention of logoutd, but it seems very ancient stuff, so I would not worry about it. Of course, their man pages and pam files as well.

There's e.g. a patch in Arch linux https://gitlab.archlinux.org/archlinux/packaging/packages/shadow/-/raw/main/0001-Disable-replaced-tools-their-man-pages-and-PAM-integ.patch?ref_type=heads that one could look at.

Lastly, I meant that (last time I checked) shadow does not respect the configure_args="--bindir=/usr/bin --sbindir=/usr/bin".

alejandro-colomar commented 1 month ago

Thanks for looking at this, and sorry for being slow.

Right. util-linux on Void linux also has /usr/bin/{newgrp,login,chsh,chfn,nologin,,vipw,vigr}, so if any of these is in shadow, it would confict. There's also a mention of logoutd, but it seems very ancient stuff, so I would not worry about it. Of course, their man pages and pam files as well.

Thanks!

There's e.g. a patch in Arch linux https://gitlab.archlinux.org/archlinux/packaging/packages/shadow/-/raw/main/0001-Disable-replaced-tools-their-man-pages-and-PAM-integ.patch?ref_type=heads that one could look at.

Thanks again!

Lastly, I meant that (last time I checked) shadow does not respect the configure_args="--bindir=/usr/bin --sbindir=/usr/bin".

Hmmm, I don't know much about autotools. Could you point to documentation or standards that document configure_args? I never heard of it.

dkwo commented 1 month ago

Sorry, for the last I just meant the following (standard configure):

export lt_cv_sys_lib_dlsearch_path_spec="/usr/lib64 /usr/lib32 /usr/lib /lib /usr/local/lib"
./configure --bindir=/usr/bin --sbindir=/usr/bin
alejandro-colomar commented 1 month ago

Sorry, for the last I just meant the following (standard configure):

export lt_cv_sys_lib_dlsearch_path_spec="/usr/lib64 /usr/lib32 /usr/lib /lib /usr/local/lib"
./configure --bindir=/usr/bin --sbindir=/usr/bin

Ahhh, now it makes sense. Yeah, I agree not respecting --bindir and --sbindir is bad.

Do you want to send a patch for that?

jubalh commented 1 month ago

Personally I don't see a reason for upstream shadow to maintain such a list.

Can you please show how much code it takes to remove these downstream?

Almost nothing. And I feel like distributions have to pick which tools they want. Some might want X from here and Y from there. I don't think its like "take all the tools from shadow" or "take all the tools from util-linux". And to have such a configure switch for 3 projects..and keep it updated..just doesn't seem worth it.

In case of openSUSE you can see the Remove binaries we don't use here. Others will have patches to not build it in the first place.

dkwo commented 1 month ago

Do you want to send a patch for that?

The linked patch from Arch also deals with it, but we may want to be more careful, as in the past this issue broke things, see e.g. https://github.com/shadow-maint/shadow/pull/197 Still respecting the configure option should be possible.

dkwo commented 1 month ago

Almost nothing.

I disagree with that. Patching or removing stuff by hand is not good, and requires extra work to maintain. I feel there should be configure options in shadow to turn off cleanly everything that (potentially) overlaps with coreutils and util-linux (used by most), ideally as you say fine-grained so people can choose what to include.

hallyn commented 1 month ago

No, I don't think we want to go this route.

If there are programs which we feel should be used from util-linux or another suite, then I'd prefer to mark those deprecated and eventually drop them here. Why would we want to continue duplicating effort maintaining the code?

Switching debian to util-linux/su however did come with some regressions (reportedly, I did not really watch those). So this isn't to be taken lightly.

dkwo commented 1 month ago

drop them here.

Even better.

jubalh commented 1 month ago

Even better.

@dkwo Did you do some research/comparison regarding:

Switching debian to util-linux/su however did come with some regressions (reportedly, I did not really watch those). So this isn't to be taken lightly.

?

alejandro-colomar commented 1 month ago

I expect some distributions to not like the idea:

Those that don't use PAM need shadow/su. I wonder if util-linux/su has any feature (or lack of) that some distros decided to switch to it (didn't check).

Regarding the coreutils stuff, I expect their stuff to be more widely used than ours, so that might be easier to drop. Busybox also has id(1), so that's a very good candidate for saying good bye. Busybox doesn't seem to have groups(1), though (edit: it does exist; the source I checked was wrong or old).

We could add an issue for discussing the removal of id(1) (if none of you disagree), and leave this one for the rest of the discussion. I think that should be already an improvement for @dkwo (and also IMO, FWIW; less stuff to maintain).

Edit: add some research about id(1) and groups(1)

coreutils/id has existed since back in 1992:

alx@debian:~/src/gnu/coreutils/master$ git show --stat ccbd1d7dc518 -- src/id.c
commit ccbd1d7dc5189f4637468a8136f672e60ee0e531
Author: Jim Meyering <jim@meyering.net>
Date:   Sun Nov 1 05:44:29 1992 +0000

    Initial revision

 src/id.c | 346 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 346 insertions(+)

busybox/id has existed since 2000:

alx@debian:~/src/busybox/busybox/master$ git show --stat 94f5e0ba7ca7 -- coreutils/id.c
commit 94f5e0ba7ca7af260f4bf2d8c77b8e6f6f528b18
Author: Erik Andersen <andersen@codepoet.org>
Date:   Mon May 1 19:10:52 2000 +0000

    Some accrued fixes/updates.
        * cp/mv now accepts (and ignores) the -f flag, since it always
            does force anyway
        * tail can now accept -<num> commands (e.g. -10) for better
            compatibility with the standard tail command
        * added a simple id implementation; doesn't support supp. groups yet

 coreutils/id.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

busybox/groups exists since 2011:

alx@debian:~/src/busybox/busybox/master$ git show --stat 33092f100398
commit 33092f1003982fc26339c0fda66283805cfbcfb1
Author: Tito Ragusa <farmatito@tiscali.it>
Date:   Tue Jun 21 17:11:40 2011 +0200

    groups: new applet

    Signed-off-by: Tito Ragusa <farmatito@tiscali.it>
    Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

 coreutils/Config.src  |  6 ------
 coreutils/Kbuild.src  |  1 -
 coreutils/id.c        | 40 ++++++++++++++++++++++++++++++++++++----
 include/applets.src.h |  1 -
 4 files changed, 36 insertions(+), 12 deletions(-)
dkwo commented 1 month ago

How about then dropping everything, except for su, which goes behind a configure option, if some distros really care about it? @alejandro-colomar Thanks, any improvement will be appreciated.

ikerexxe commented 1 month ago

How about then dropping everything, except for su, which goes behind a configure option, if some distros really care about it?

Can you indicate specifically which binaries you want to drop from shadow?

dkwo commented 1 month ago

Re su: if the configure option --without-su is actually respected, then nothing to be done?

I'd like to drop groups as well as chfn, chsh, login, logoutd, nologin, vigr, vipw, as well as their pam and man pages. Move newgrp to replace sg (instead of it being a symlink). At least on my systems, there's no id, so I'm not sure about it.

alejandro-colomar commented 1 month ago

How about then dropping everything, except for su, which goes behind a configure option, if some distros really care about it? @alejandro-colomar Thanks, any improvement will be appreciated.

I prefer to go step by step.

Let's discuss what to do with coreutils clashes, and then discuss util-linux.

With coreutils, we have the following clashes:

alx@debian:~$ ( \
    find ~/src/gnu/coreutils/master/src/ -type f \
    | grep '\.c$' \
    | sed s/.home.alx.src.gnu.coreutils.master.src.// \
    | sed s/.c$//; \
    find ~/src/shadow/shadow/master/src/ -type f \
    | grep '\.c$' \
    | sed s/.home.alx.src.shadow.shadow.master.src.// \
    | sed s/.c$//; \
  ) \
  | sort \
  | uniq -d;
groups
id

I will open an issue for these.

ikerexxe commented 1 month ago

How about then dropping everything, except for su, which goes behind a configure option, if some distros really care about it? @alejandro-colomar Thanks, any improvement will be appreciated.

I prefer to go step by step.

Thanks for opening the other PR and going one step at a time. I think this will speed up the process as we may face some challenges with some of the removals.

jubalh commented 1 month ago

JFYI:

openSUSE uses the binaries from following packages:

shadow

coreutils

util-linux

removed

alejandro-colomar commented 1 month ago

@jubalh

JFYI:

openSUSE uses the binaries from following packages:

Did you miss id(1)?

jubalh commented 1 month ago

Did you miss id(1)?

Indeed I did :) Added it now.

thalman commented 1 month ago

This RFC is connected with issue #985.

I do not mind if we have this API in linux-utils but we should have it somewhere. Lot of projects depend on libuser when changing full name or shell, but unfortunately libuser is not actively developed, only maintained. Apart from using libuser (compile time option), the linux-utils has also its own implementation of those functions, but not exported as public API.

zeha commented 2 weeks ago

When I last checked in Debian, the vigr, vipw programs from util-linux were not as nice as the shadow versions.

Having said that, it certainly is nicer if only one project would have these programs - for Debian, it doesn't matter if it will be src:shadow or src:util-linux.

hallyn commented 1 week ago

Thans. I suppose having them from shadow makes sense anyway, since it ships passwd etc. But if the util-linux versions are brought to feature parity (with no risk of regressions in someone's obscure closet), I'd be only too happy to drop the shadow ones.