shadow-maint / shadow

Upstream shadow tree
Other
292 stars 228 forks source link

Install chpasswd,newusers configs when built with pam but without setuid #928

Closed loqs closed 7 months ago

loqs commented 7 months ago

Install pam configs for chage and newusers when using ./configure --with-libpam --disable-account-tools-setuid. Fixes https://github.com/shadow-maint/shadow/issues/810.

alejandro-colomar commented 7 months ago

Cc: @dvzrv

alejandro-colomar commented 7 months ago

@dvzrv Can you please confirm that this works for you?

dvzrv commented 7 months ago

@loqs I am wondering why you are adapting chage here. At least on Arch Linux we do not package a pam file for it currently and chage also does not link against libpam.so.

The ticket #810 is about chpasswd and newusers (not chage and newusers).

Here is a list of libs/executables and the relevant libraries they link against (from the Arch Linux package).

/usr/bin/chage
  NEEDED               libaudit.so.1
  NEEDED               libc.so.6
/usr/bin/chgpasswd
  NEEDED               libcrypt.so.2
  NEEDED               libc.so.6
/usr/bin/chpasswd
  NEEDED               libpam.so.0
  NEEDED               libcrypt.so.2
  NEEDED               libc.so.6
/usr/bin/expiry
  NEEDED               libc.so.6
/usr/bin/faillog
  NEEDED               libc.so.6
/usr/bin/getsubids
  NEEDED               libsubid.so.4
  NEEDED               libc.so.6
/usr/bin/gpasswd
  NEEDED               libaudit.so.1
  NEEDED               libcrypt.so.2
  NEEDED               libc.so.6
/usr/bin/groupadd
  NEEDED               libaudit.so.1
  NEEDED               libc.so.6
/usr/bin/groupdel
  NEEDED               libaudit.so.1
  NEEDED               libc.so.6
/usr/bin/groupmems
  NEEDED               libpam.so.0
  NEEDED               libpam_misc.so.0
  NEEDED               libc.so.6
/usr/bin/groupmod
  NEEDED               libaudit.so.1
  NEEDED               libc.so.6
/usr/bin/groups
  NEEDED               libc.so.6
/usr/bin/grpck
  NEEDED               libc.so.6
/usr/bin/grpconv
  NEEDED               libc.so.6
/usr/bin/grpunconv
  NEEDED               libc.so.6
/usr/bin/lastlog
  NEEDED               libaudit.so.1
  NEEDED               libc.so.6
/usr/bin/newgidmap
  NEEDED               libc.so.6
/usr/bin/newuidmap
  NEEDED               libc.so.6
/usr/bin/newusers
  NEEDED               libpam.so.0
  NEEDED               libc.so.6
/usr/bin/passwd
  NEEDED               libcrypt.so.2
  NEEDED               libpam.so.0
  NEEDED               libpam_misc.so.0
  NEEDED               libc.so.6
/usr/bin/pwck
  NEEDED               libc.so.6
/usr/bin/pwconv
  NEEDED               libc.so.6
/usr/bin/pwunconv
  NEEDED               libc.so.6
/usr/bin/sg
  NEEDED               libaudit.so.1
  NEEDED               libcrypt.so.2
  NEEDED               libc.so.6
/usr/bin/useradd
  NEEDED               libaudit.so.1
  NEEDED               libacl.so.1
  NEEDED               libattr.so.1
  NEEDED               libc.so.6
/usr/bin/userdel
  NEEDED               libaudit.so.1
  NEEDED               libc.so.6
/usr/bin/usermod
  NEEDED               libaudit.so.1
  NEEDED               libacl.so.1
  NEEDED               libattr.so.1
  NEEDED               libc.so.6
/usr/lib/libsubid.so.4.0.0
  NEEDED               libaudit.so.1
  NEEDED               libc.so.6

The following executables are setuid/setgid:

shadow W: File (usr/bin/chage) is setuid or setgid.
shadow W: File (usr/bin/expiry) is setuid or setgid.
shadow W: File (usr/bin/gpasswd) is setuid or setgid.
shadow W: File (usr/bin/groupmems) does not have the world readable bit set.
shadow W: File (usr/bin/passwd) is setuid or setgid.
shadow W: File (usr/bin/sg) is setuid or setgid.

As --disable-account-tools-setuid and --with-libpam lead to chage still being setuid, we do not want its PAM file.

@alejandro-colomar: The patch as such would only fix #810 halfway (only for newusers). If it would move chpasswd (instead of chage) from pamd_acct_tools_files to pamd_files it would fix it though.

(edit: fixed copy/paste mistake)

loqs commented 7 months ago

I renamed the branch to match the change which closed the pull request. Sorry about the noise.

alejandro-colomar commented 7 months ago

@alejandro-colomar: The patch as such would only fix #810 halfway (only for newusers). If it would move chpasswd (instead of chage) from pamd_acct_tools_files to pamd_files it would fix it though.

Now that he did the change, would you mind giving a Tested-by: tag? Thanks!

loqs commented 7 months ago

Do you mind changing the commit message to something like Makefile: move chpasswd and newusers to pamd target?

No problem. Do you want just the title replaced with that suggestion or the entire commit message?

dvzrv commented 7 months ago

@alejandro-colomar: The patch as such would only fix #810 halfway (only for newusers). If it would move chpasswd (instead of chage) from pamd_acct_tools_files to pamd_files it would fix it though.

Now that he did the change, would you mind giving a Tested-by: tag? Thanks!

I tried a rebased version of this patch on top of the changes we do on Arch Linux (as mentioned in https://github.com/shadow-maint/shadow/issues/810#issuecomment-1909835598) and that successfully installs the files with the configure flags in question.

In case @loqs amends the commit message he may add Tested-by: David Runge <dvzrv@archlinux.org>.

Thanks for your work on this all! :partying_face:

ikerexxe commented 7 months ago

No problem. Do you want just the title replaced with that suggestion or the entire commit message?

Just the first line. You can add whatever you want afterwards

alejandro-colomar commented 7 months ago

LGTM, thanks! I'll merge, and later cherry-pick to 4.14.x.