Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4.14.0: When built with --with-pam and --disable-account-tools-setuid some PAM configs are missing #810

Closed
dvzrv opened this issue Sep 18, 2023 · 14 comments · Fixed by #928

Comments

@dvzrv
Copy link

dvzrv commented Sep 18, 2023

Hi! Upon trying to package 4.14.0 for Arch Linux I ran into the PAM configurations for chpasswd and newusers not being installed if --with-pam and --disable-account-tools-setuid are provided.

This seems to be the case because they are both handled in pamd_acct_tools_files:

pamd_acct_tools_files = \
chage \
chgpasswd \
chpasswd \
groupadd \
groupdel \
groupmod \
newusers \
useradd \
userdel \
usermod

As both chpasswd and newusers link against libpam.so, I am wondering whether this behavior is correct/ expected.

I guess I'd have to install the relevant PAM configs manually for now?

@loqs
Copy link
Contributor

loqs commented Sep 18, 2023

The issue was introduced with support for --enable-account-tools-setuid 1 which classed chpasswd and newusers as both only supporting PAM with --enable-account-tools-setuid. The broken linking this caused was fixed in 2 and 3 while the missing PAM configs remain.

@alejandro-colomar
Copy link
Collaborator

alejandro-colomar commented Jan 23, 2024

Is this reproducible in 4.14.3 and/or master?

@loqs
Copy link
Contributor

loqs commented Jan 23, 2024

Yes for 4.14.3 and master (as of 0138819). If it helps with reproducing:

autoreconf -fiv
./configure --bindir=/usr/bin --disable-account-tools-setuid --enable-man --libdir=/usr/lib --mandir=/usr/share/man --prefix=/usr --sbindir=/usr/bin --sysconfdir=/etc --with-audit --with-fcaps --with-group-name-max-length=32 --with-libpam --without-libbsd --without-nscd --without-selinux --without-su
make
make install

@alejandro-colomar
Copy link
Collaborator

alejandro-colomar commented Jan 24, 2024

Can you please reduce those flags to a reproducible minimum? I'm having trouble configuring that.

...
checking whether PAM_ESTABLISH_CRED is declared... yes
checking whether PAM_DELETE_CRED is declared... yes
checking whether PAM_NEW_AUTHTOK_REQD is declared... yes
checking whether PAM_DATA_SILENT is declared... yes
checking for pam_fail_delay... yes
checking use login and su access checking if PAM not used... no
checking for setcap... no
configure: error: setcap command not available
$ sudo which setcap
/usr/sbin/setcap

(I hope I don't need to sudo ./configure ... or fake root.)

(Oh, well, I just needed to lie a little bit: PATH="/usr/sbin:$PATH") (No, I need to be root for installing. :(

@alejandro-colomar
Copy link
Collaborator

alejandro-colomar commented Jan 24, 2024

What are the PAM configs that are missing?

/etc/pam.d/{chage,...}?

@loqs
Copy link
Contributor

loqs commented Jan 24, 2024

What are the PAM configs that are missing?

/etc/pam.d/{chage,...}?

/etc/pam.d/{chage,newusers}

The following change appears to resolve the issue

--- a/etc/pam.d/Makefile.am
+++ b/etc/pam.d/Makefile.am
@@ -2,20 +2,20 @@
 # and also cooperate to make a distribution for `make dist'
 
 pamd_files = \
+	chage \
 	chfn \
 	chsh \
 	groupmems \
 	login \
+	newusers \
 	passwd
 
 pamd_acct_tools_files = \
-	chage \
 	chgpasswd \
 	chpasswd \
 	groupadd \
 	groupdel \
 	groupmod \
-	newusers \
 	useradd \
 	userdel \
 	usermod

@ikerexxe
Copy link
Collaborator

The issue was introduced with support for --enable-account-tools-setuid 1 which classed chpasswd and newusers as both only supporting PAM with --enable-account-tools-setuid. The broken linking this caused was fixed in 2 and 3 while the missing PAM configs remain.

This is kind of a complex problem, but those links helped me understand it. Out of curiosity, are you changing any configuration for Arch Linux? Those commits that you mention were introduced some time ago and your problem shouldn't happen unless you change the configuration.

@loqs
Copy link
Contributor

loqs commented Jan 25, 2024

This is kind of a complex problem, but those links helped me understand it. Out of curiosity, are you changing any configuration for Arch Linux? Those commits that you mention were introduced some time ago and your problem shouldn't happen unless you change the configuration.

I am not sure I understand what you are asking. I am able to reproduce /etc/pam.d/{chage,newusers} not being installed with the steps from #810 (comment) on the 4.14.3 tarball without any modifications.
If you are asking about how Arch integrates shadow I will defer to @dvzrv.

@dvzrv
Copy link
Author

dvzrv commented Jan 25, 2024

Can you please reduce those flags to a reproducible minimum? I'm having trouble configuring that.

...
checking whether PAM_ESTABLISH_CRED is declared... yes
checking whether PAM_DELETE_CRED is declared... yes
checking whether PAM_NEW_AUTHTOK_REQD is declared... yes
checking whether PAM_DATA_SILENT is declared... yes
checking for pam_fail_delay... yes
checking use login and su access checking if PAM not used... no
checking for setcap... no
configure: error: setcap command not available
$ sudo which setcap
/usr/sbin/setcap

You need setcap because that's what --disable-account-tools-setuid implies (see title of issue).
If you can't/ do not want to do this on your own machine, I think it should work in a container as well.

(I hope I don't need to sudo ./configure ... or fake root.)

(Oh, well, I just needed to lie a little bit: PATH="/usr/sbin:$PATH") (No, I need to be root for installing. :(

You can alternatively install to a local DESTDIR=foo and will be able to see the directory tree as well.

What are the PAM configs that are missing?

As mentioned in first sentence of the initial ticket description, those for chpasswd and newusers.

@ikerexxe
Copy link
Collaborator

If you are asking about how Arch integrates shadow I will defer to @dvzrv.

Yes, this is mostly a question for Arch maintainers. I don't understand why this is a problem for 4.14.0 but it wasn't before. I mean, those commits happened more than ten years ago so I'd have expected a complain at that time.

@dvzrv
Copy link
Author

dvzrv commented Jan 25, 2024

Out of curiosity, are you changing any configuration for Arch Linux? Those commits that you mention were introduced some time ago and your problem shouldn't happen unless you change the configuration.

I don't think this has something to do with our distribution specific modifications, since the reproducer provided by @loqs in #810 (comment) works with this repository directly (without any modification).

However, we do have distribution specific modifications, and you can have a look at the ones on top of 4.14.3 here: https://gitlab.archlinux.org/archlinux/packaging/upstream/shadow/-/commits/4.14.3.arch1
We mainly have to disable specific tooling due to them either no longer being in use or us replacing them with those in util-linux.
Additionally there are modifications to login.defs (removal of entries that are not supported because of PAM or util-linux and distribution specific overrides/defaults).

Yes, this is mostly a question for Arch maintainers. I don't understand why this is a problem for 4.14.0 but it wasn't before.

It is not uncommon for maintainers to not report issues upstream and just silently fix things. Before I took over the package a while ago, most of the changes were undocumented and inlined and it was really hard to figure out what changes we should actually be applying 😅

I mean, those commits happened more than ten years ago so I'd have expected a complain at that time.

Sure, but it might also just be confirmation bias or other distributions not using shadow in the same capacity as we do (I noticed huge discrepancies in how other distributions package it and which of the tooling they make use of).

@dvzrv
Copy link
Author

dvzrv commented Jan 25, 2024

@ikerexxe and here is the build script for 4.14.3 with which the sources are downloaded, verified, patched, built and installed: https://gitlab.archlinux.org/archlinux/packaging/packages/shadow/-/blob/a322022555c652550986e303cc9dc22fab194cd5/PKGBUILD

@alejandro-colomar
Copy link
Collaborator

alejandro-colomar commented Jan 25, 2024 via email

@ikerexxe
Copy link
Collaborator

Thanks for the explation @dvzrv, as I said it was more out of curiosity.

@loqs your proposal makes sense, as we don't want to run those two tools as normal users, but as superusers, and they don't need setuid bit. Do you mind opening a PR?

loqs added a commit to loqs/shadow that referenced this issue Jan 26, 2024
Install pam configs for chage and newusers when using ./configure --with-libpam --disable-account-tools-setuid.
Fixes shadow-maint#810.
loqs added a commit to loqs/shadow that referenced this issue Jan 30, 2024
Install pam configs for chage and newusers when using ./configure --with-libpam --disable-account-tools-setuid.
Fixes shadow-maint#810.
loqs added a commit to loqs/shadow that referenced this issue Jan 30, 2024
Install pam configs for chage and newusers when using ./configure --with-libpam --disable-account-tools-setuid.
Fixes shadow-maint#810.
loqs added a commit to loqs/shadow that referenced this issue Jan 30, 2024
Install pam configs for chpasswd and newusers when using ./configure --with-libpam --disable-account-tools-setuid.
Fixes shadow-maint#810.
loqs added a commit to loqs/shadow that referenced this issue Jan 30, 2024
Install pam configs for chpasswd and newusers when using ./configure --with-libpam --disable-account-tools-setuid.
Fixes shadow-maint#810.

Tested-by: David Runge <dvzrv@archlinux.org>
alejandro-colomar pushed a commit that referenced this issue Jan 30, 2024
Install pam configs for chpasswd and newusers when using ./configure --with-libpam --disable-account-tools-setuid.
Fixes #810.

Tested-by: David Runge <dvzrv@archlinux.org>
alejandro-colomar pushed a commit that referenced this issue Jan 30, 2024
Install pam configs for chpasswd and newusers when using:

	$ ./configure --with-libpam --disable-account-tools-setuid

Closes: <#810>
Link: <#928>
Tested-by: David Runge <dvzrv@archlinux.org>
Cherry-picked-from: 341d80c ("Makefile: move chpasswd and newusers to pamd target")
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants