Bug#983427: libpam-runtime: please add support for DPKG_ROOT
Hi sam, Quoting Sam Hartman (2021-08-26 22:23:18) > > "Johannes" == Johannes 'josch' Schauer writes: > > Johannes> diff --git a/debian/libpam-runtime.postinst > > I think that your patch ends up with things like $confdir set to > "//etc/pam.d" when $DPKG_ROOT is empty. You get one slash from the > initialization of the variable and one slash from the separator in the > code you added. > > > Please review the following alternate patch hopefully before it makes > its way into testing: you are right and your changed patch looks good to me. I just tested your patch in our salsaci setup and there does not seem to be a regression. Thanks! cheers, josch signature.asc Description: signature
Bug#983427: libpam-runtime: please add support for DPKG_ROOT
> "Johannes" == Johannes 'josch' Schauer writes:
Johannes> diff --git a/debian/libpam-runtime.postinst
I think that your patch ends up with things like $confdir set to
"//etc/pam.d" when $DPKG_ROOT is empty. You get one slash from the
initialization of the variable and one slash from the separator in the
code you added.
Please review the following alternate patch hopefully before it makes
its way into testing:
commit b296f47cab5c8d97e2d57ef35694ba8328a9477f
Author: Sam Hartman
Date: Thu Aug 26 14:17:22 2021 -0600
pam-auth-update: support DPKG_ROOT
Patch from Johannes 'josch' Schauer to implement a --root argument to
pam-auth-update and to use it in the call in libpam-runtime.
* debian/local/pam-auth-update: support --root
* debian/libpam-runtime.postinst: call with --root $DPKG_ROOT
diff --git a/debian/changelog b/debian/changelog
index 848f13c0..96dd28fc 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -5,6 +5,8 @@ pam (1.4.0-10) UNRELEASED; urgency=medium
* Include upstream patch not to use crypt_checksalt; without this
passwords set prior to bullseye were considered expired, Closes:
#992848
+ * Support DPKG_ROOT for pam-auth-update, thanks Johannes 'josch' Schauer
+Closes: #983427
-- Sam Hartman Thu, 26 Aug 2021 13:43:23 -0600
diff --git a/debian/libpam-runtime.postinst b/debian/libpam-runtime.postinst
index 518e8d24..053fdae2 100644
--- a/debian/libpam-runtime.postinst
+++ b/debian/libpam-runtime.postinst
@@ -29,7 +29,7 @@ then
done
fi
-pam-auth-update --package $force
+pam-auth-update --root "$DPKG_ROOT" --package $force
if [ -n "$force" ]; then
rm -f /etc/pam.d/common-auth.pam-old \
diff --git a/debian/local/pam-auth-update b/debian/local/pam-auth-update
index 5b3c8a07..6c4134bb 100644
--- a/debian/local/pam-auth-update
+++ b/debian/local/pam-auth-update
@@ -88,6 +88,11 @@ while ($#ARGV >= 0) {
$force = 1;
} elsif ($opt eq '--package') {
$package = 1;
+} elsif ($opt eq '--root') {
+my $rootdir = shift @ARGV;
+$savedir = "${rootdir}$savedir";
+$confdir = "${rootdir}$confdir";
+ $inputdir = "${rootdir}$inputdir";
} elsif ($opt eq '--remove') {
while ($#ARGV >= 0) {
last if ($ARGV[0] =~ /^--/);
signature.asc
Description: PGP signature
Bug#983427: libpam-runtime: please add support for DPKG_ROOT
Hi, Quoting Sam Hartman (2021-06-22 20:48:43) > So, the feature seems achievable for a significant important use case > and so I think we should take the patch. for your convenience, I created a MR on salsa with the proposed changes: https://salsa.debian.org/vorlon/pam/-/merge_requests/6 Thanks! cheers, josch signature.asc Description: signature
Bug#983427: libpam-runtime: please add support for DPKG_ROOT
control: tags -1 confirmed > "Johannes" == Johannes 'josch' Schauer writes: Johannes> Hi, Johannes> since dpkg 1.18.5, dpkg sets the variable DPKG_ROOT when Johannes> invoking maintainer scripts. Usually that variable is Johannes> empty but when calling dpkg with --root and Johannes> --force-script-chrootless, dpkg will set DPKG_ROOT to the Johannes> new root directory. In that mode, maintainer scripts are Johannes> called without chroot(1) around them, and thus have to be Johannes> able to possibly operate on the path from DPKG_ROOT Johannes> instead of working on /. This is useful for bootstrapping, Johannes> creating chroots for foreign architectures where utilities I spoke to Johannes and Helmut today. Steve, I think we should take this patch and will do so unless you object if I'm the next to upload. I found the foreign architecture use case compelling. There are cases where qemu has not been available or where it's been horribly slow in the early architecture bootstrap phase, even for architectures that Debian clearly cares about. So even using a conservative definition of what architectures we care about , we cannot always usefully rely on qemu for bootstrap. According to Helmut roughly build-essential needs to support this for the bootstrap case (and some of those packages do not have maintainer scripts). According to Johannes there are roughly 11 packages including pam that still need to be patched. So, the feature seems achievable for a significant important use case and so I think we should take the patch. I'm less convinced by some of the other arguments. Helmut, Johannes and I are continuing to chat about that offline, but I think it has no impact on PAM. signature.asc Description: PGP signature
Bug#983427: libpam-runtime: please add support for DPKG_ROOT
> "Johannes" == Johannes 'josch' Schauer writes: I took a look at the references in the bug. In particular, https://wiki.debian.org/Teams/Dpkg/Spec/InstallBootstrap My goal was to try and do homework prior to a call. Here's where I got: That page actually seems to talk about a different problem than is discussed in the bug. In particular it's talking about complexities around having the order of dependencies hard-coded in debootstrap. The assertion there is that pseudo-essential packages should not have maintainer scripts in the install case. Assuming that's the direction we take, the patch proposed in this bug would not be needed. However, the dpkg team page is long on assertions, and fairly short on reasoning behind them. It's absolutely true that boostrapping information is embedded in debootstrap and cdebootstrap. And there are some downsides to that. But it also appears to be some upsides. Namely that we can use our existing mechanisms for package maintenance, and that we stick the boostrapping information all in one place. The dpkg page points to several "recent examples" of the problem. I read through https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=767999 I'll admit that the reasoning in that bug report makes a lot more sense to me than what's written on the dpkg page. However, there wasn't really a consensus in the bug description. Some parties were arguing that it was desirable to embed the bootstrapping information in debootstrap. Some parties were arguing that this was undesirable. It was generally agreed that base-passwd was buggy for not having its functionality available by unpack time, although it looks like that got fixed. So, I don't think the cited examples support the conclusions drawn from them, and even if the conclusions are correct, I don't see the dpkg bootstrapping page as support for this patch. If this patch is desirable, the motivation needs to come from elsewhere. I think we're all agreed that the patch does make rootless bootstrapping more possible. I think we're all agreed that we don't want that to extend across the entire archive. The question in my mind is whether rootless bootstrapping is desirable at all when all the ttrade offs are considered, and if so, what scope it should have. I have a strong desire to respect actual informed consensus where it exists. It's harmful if each individual maintainer makes their own call. And yet, if there isn't an informed consensus, I'm not sure pam is the right place to start introducing complexity. signature.asc Description: PGP signature
Bug#983427: libpam-runtime: please add support for DPKG_ROOT
Hi Sam,
Quoting Sam Hartman (2021-02-25 17:56:07)
> I'm setting a calendar note to come back tho this in May.
> Apologies for not having time sooner; I'm in the middle of planning for
> a move and trying to deal with bullseye issues.
I hope that everything went okay with your move and we can schedule a voice
call if you like. For me, evenings after 20:00 CEST would fit best.
We now have a set of scripts that patches src:pam (and others) to test the
DPKG_ROOT approach. We can now verify that creating a chroot that way results
in a bit-by-bit identical chroot compared to a chroot created the normal way:
https://salsa.debian.org/helmutg/dpkg-root-demo
In the process of getting that far we also extended the patch to src:pam.
Please find the patch attached.
Thanks!
cheers, joschdiff -Nru pam-1.4.0/debian/libpam-modules.postinst pam-1.4.0/debian/libpam-modules.postinst
--- pam-1.4.0/debian/libpam-modules.postinst 2021-01-30 23:09:52.0 +0100
+++ pam-1.4.0/debian/libpam-modules.postinst 2021-06-17 00:37:49.0 +0200
@@ -5,16 +5,16 @@
if [ -z "$2" ] || dpkg --compare-versions "$2" lt 0.99.7.1-3
then
- if ! [ -f /etc/security/opasswd ]; then
+ if ! [ -f "$DPKG_ROOT/etc/security/opasswd" ]; then
umask 066
- touch /etc/security/opasswd
+ touch "$DPKG_ROOT/etc/security/opasswd"
umask 022
fi
fi
-if dpkg --compare-versions "$2" lt 0.99.9.0-1 && ! [ -f /etc/environment ]
+if dpkg --compare-versions "$2" lt 0.99.9.0-1 && ! [ -f "$DPKG_ROOT/etc/environment" ]
then
- touch /etc/environment
+ touch "$DPKG_ROOT/etc/environment"
fi
if dpkg --compare-versions "$2" lt-nl 1.1.2-1 \
diff -Nru pam-1.4.0/debian/libpam-runtime.postinst pam-1.4.0/debian/libpam-runtime.postinst
--- pam-1.4.0/debian/libpam-runtime.postinst 2021-01-30 23:09:52.0 +0100
+++ pam-1.4.0/debian/libpam-runtime.postinst 2021-06-17 00:37:49.0 +0200
@@ -8,7 +8,7 @@
sed -n -e'1,/# here are the per-package modules (the "Primary" block)/p;
/# here.s the fallback if no module succeeds/,/# and here are more per-package modules (the "Additional" block)/p;
/# end of pam-auth-update config/,$p' \
- /etc/pam.d/"$configfile" | md5sum | awk '{ print $1 }'
+ "$DPKG_ROOT/etc/pam.d/$configfile" | md5sum | awk '{ print $1 }'
}
# If the user has removed the config file, respect this sign of dementia
@@ -20,26 +20,26 @@
for configfile in common-auth common-account common-session \
common-password
do
- if [ -f /etc/pam.d/$configfile ] && \
+ if [ -f "$DPKG_ROOT/etc/pam.d/$configfile" ] && \
! fgrep -q $(calculate_md5sum $configfile) \
- /usr/share/pam/$configfile.md5sums 2>/dev/null
+ "$DPKG_ROOT/usr/share/pam/$configfile.md5sums" 2>/dev/null
then
force=
fi
done
fi
-pam-auth-update --package $force
+pam-auth-update --root "$DPKG_ROOT" --package $force
if [ -n "$force" ]; then
- rm -f /etc/pam.d/common-auth.pam-old \
- /etc/pam.d/common-account.pam-old \
- /etc/pam.d/common-password.pam-old \
- /etc/pam.d/common-session.pam-old
+ rm -f "$DPKG_ROOT/etc/pam.d/common-auth.pam-old" \
+ "$DPKG_ROOT/etc/pam.d/common-account.pam-old" \
+ "$DPKG_ROOT/etc/pam.d/common-password.pam-old" \
+ "$DPKG_ROOT/etc/pam.d/common-session.pam-old"
elif dpkg --compare-versions "$2" lt-nl 1.1.0-1 \
-&& [ ! -e /etc/pam.d/common-session-noninteractive ]
+&& [ ! -e "$DPKG_ROOT/etc/pam.d/common-session-noninteractive" ]
then
- cp -a /etc/pam.d/common-session /etc/pam.d/common-session-noninteractive
+ cp -a "$DPKG_ROOT/etc/pam.d/common-session" "$DPKG_ROOT/etc/pam.d/common-session-noninteractive"
fi
#DEBHELPER#
diff -Nru pam-1.4.0/debian/local/pam-auth-update pam-1.4.0/debian/local/pam-auth-update
--- pam-1.4.0/debian/local/pam-auth-update 2021-02-25 23:10:16.0 +0100
+++ pam-1.4.0/debian/local/pam-auth-update 2021-06-17 00:37:49.0 +0200
@@ -88,6 +88,11 @@
$force = 1;
} elsif ($opt eq '--package') {
$package = 1;
+ } elsif ($opt eq '--root') {
+ my $rootdir = shift @ARGV;
+ $savedir = "$rootdir/$savedir";
+ $confdir = "$rootdir/$confdir";
+ $inputdir = "$rootdir/$inputdir";
} elsif ($opt eq '--remove') {
while ($#ARGV >= 0) {
last if ($ARGV[0] =~ /^--/);
signature.asc
Description: signature
Bug#983427: libpam-runtime: please add support for DPKG_ROOT
I'm setting a calendar note to come back tho this in May. Apologies for not having time sooner; I'm in the middle of planning for a move and trying to deal with bullseye issues.
Bug#983427: libpam-runtime: please add support for DPKG_ROOT
Hi, I don't want to start a discussion. So no need to reply. I just wanted to clarify some things. Quoting Sam Hartman (2021-02-24 23:12:11) > I'm not at all convinced this is a good idea. We're replacing a great, > well-tested facility--namely running maintainer scripts in root directories > with a mechanism that requires support to be spread through each package. This is not meant to replace running maintainer scripts in root directories ever. This is an additional feature and is probably only going to be implemented for the packages in the pseudo essential set. Since maintainer scripts will retain their old behaviour if the DPKG_ROOT variable is empty, this should not add bugs for normal operation where maintainer scripts are run is the root directory. To reduce the perceived impact on "normal" maintainer script usage even more, the patch could be amended to say: if [ "$DPKG_ROOT" = "" ]; then pam-auth-update --package $force else pam-auth-update --root "$DPKG_ROOT" --package $force fi That way it is clear that the "non standard" (and less tested) codepath is only taken if dpkg was called with --root set. > It's certainly too late for bullseye to consider something like this. Yes, absolutely. Sorry for not having been clear on that. > I'd be happy to sit down with one of the proponents over a voice call in May > or June to understand the current state of this effort, what level of > consensus has been achieved and consider next steps. I think that initial > discussion would be too high bandwidth for email. Thank you! :) cheers, josch signature.asc Description: signature
Bug#983427: libpam-runtime: please add support for DPKG_ROOT
control: tags -1 wontfix I'm not at all convinced this is a good idea. We're replacing a great, well-tested facility--namely running maintainer scripts in root directories with a mechanism that requires support to be spread through each package. I appreciate that this approach has the support of a significant number of people in the community. And obviously, if the consensus of the project is this is the direction we're going, then PAM should go along. I'm wondering though whether we've reached the level of project consensus, or simply consensus of those who think it is a good idea. If this isn't ultimately the direction we're going, then I think adding this complexity spread throughout our system is harmful. It's certainly too late for bullseye to consider something like this. I'd be happy to sit down with one of the proponents over a voice call in May or June to understand the current state of this effort, what level of consensus has been achieved and consider next steps. I think that initial discussion would be too high bandwidth for email. Obviously, if vorlon believes this is the right direction he can remove the wontfix tag.
Bug#983427: libpam-runtime: please add support for DPKG_ROOT
Package: libpam-runtime
Version: 1.4.0-4.1
Severity: wishlist
Tags: patch
User: debian-d...@lists.debian.org
Usertags: dpkg-root-support
Hi,
since dpkg 1.18.5, dpkg sets the variable DPKG_ROOT when invoking
maintainer scripts. Usually that variable is empty but when calling dpkg
with --root and --force-script-chrootless, dpkg will set DPKG_ROOT to
the new root directory. In that mode, maintainer scripts are called
without chroot(1) around them, and thus have to be able to possibly
operate on the path from DPKG_ROOT instead of working on /. This is
useful for bootstrapping, creating chroots for foreign architectures
where utilities from inside the chroot cannot be executed, avoiding
dependency loops between postinst scripts, installation without
requiring superuser privileges and for creating installations that do
not even contain dpkg. See
https://wiki.debian.org/Teams/Dpkg/Spec/InstallBootstrap for more
information.
To see the problem, you can run:
$ mmdebstrap --mode=chrootless --variant=custom --include=libpam-runtime
unstable /dev/null
[...]
Setting up libpam-runtime (1.4.0-4.1) ...
open(/var/lib/pam/seen) failed: Permission denied at /usr/sbin/pam-auth-update
line 232, line 11.
dpkg: error processing package libpam-runtime (--configure):
installed libpam-runtime package post-installation script subprocess returned
error exit status 13
Errors were encountered while processing:
libpam-runtime
E: Sub-process /usr/bin/dpkg returned an error code (1)
The following patch fixes the problem:
diff --git a/debian/libpam-runtime.postinst b/debian/libpam-runtime.postinst
index 518e8d24..053fdae2 100644
--- a/debian/libpam-runtime.postinst
+++ b/debian/libpam-runtime.postinst
@@ -29,7 +29,7 @@ then
done
fi
-pam-auth-update --package $force
+pam-auth-update --root "$DPKG_ROOT" --package $force
if [ -n "$force" ]; then
rm -f /etc/pam.d/common-auth.pam-old \
diff --git a/debian/local/pam-auth-update b/debian/local/pam-auth-update
index 6d17ab72..18fd898c 100644
--- a/debian/local/pam-auth-update
+++ b/debian/local/pam-auth-update
@@ -82,6 +82,11 @@ while ($#ARGV >= 0) {
$force = 1;
} elsif ($opt eq '--package') {
$package = 1;
+ } elsif ($opt eq '--root') {
+ my $rootdir = shift @ARGV;
+ $savedir = "$rootdir/$savedir";
+ $confdir = "$rootdir/$confdir";
+ $inputdir = "$rootdir/$inputdir";
} elsif ($opt eq '--remove') {
while ($#ARGV >= 0) {
last if ($ARGV[0] =~ /^--/);
Possibly, more is needed to correctly implement DPKG_ROOT support, but
with above patch, it is possible to run above mmdebstrap command
successfully.
Thanks!
cheers, josch
