Hi Sam,
I apologize for taking a while to come up to speed on this and for
a couple of false starts. It's been a while since pam has been a major
focus of mine, but I offered to help Steve out so I'm coming back up to
speed.
Sure, no problem there. I also get regular headaches when dealing with
PAM.
I think your second approach of looking into pam-configs, disabling
anything that uses pam_tally and then checking /etc/pam.d would be most
correct.
Unfortunately I'm uncomfortable with our (or at least my) ability to
get all the corner cases right this deep into the freeze.
I am also not entirely confident and I see your problems with the
freeze.
This kind of change should not happen that late and with these
consequences.
I'm currently thinking it would be good to just accept something like
your first patch, although I'm open to exploring your second approach.
Do you think it would be safe for me to change it to look in
/var/lib/pam rather than /usr/share/pam-configs for pam_tally?
I am not sure, what /var/lib/pam does in Debian, as I mentioned I am not
a native Debian user and I also need to think very hard about all the
stuff that is involved here.
I don't see a need to break the upgrade on a disabled module.
I agree the /etc/pam.d check is still important.
If we were going to do the second approach I think the steps involved
might look like:
* Check /var/lib/pam to see if any of the currently active profiles
include pam_tally; if so disable them
* modify pam-auth-update to never enable a profile including pam_tally
* Do the /etc/pam.d check.
Rationale here is that there's no reason to break the upgrade if a user
has a pam_tally module already disabled nor to display a warning if the
module is already disabled.
But unless we actually remove the file from /usr/share/pam-configs,
pam-auth-update will let a user enable it again.
That sounds great and I like that approach. I think we should implement
it. And I did some deeper digging into linux-pam. While doing this, I
found a nice loophole for us. linux-pam in version 1.4.0 has not
entirely
removed pam_tally and pam_tally2 but they have made it an extra option
to
their configure script. Only in version 1.5.0 these two modules are
completely removed.
see:
https://github.com/linux-pam/linux-pam/blob/9e5bea9e146dee574796259ca464ad2435be3590/configure.ac#L678-L694
So I created a patch, that enables pam_tally and pam_tally2 again for
our
1.4.0 package. I also output an error message to highlight this fact to
the user. This buys us some time without breaking stuff for our users.
We
can now take our time to come up with a good and tested solution without
having concerns about the freeze and without rushing things.
diff --git a/debian/libpam-modules.preinst b/debian/libpam-modules.preinst
index 3a86a8fb..91a2bf0c 100644
--- a/debian/libpam-modules.preinst
+++ b/debian/libpam-modules.preinst
@@ -4,6 +4,15 @@ set -e
. /usr/share/debconf/confmodule
+if dpkg --compare-versions "$2" lt-nl 1.4.0; then
+ db_version 2.0
+
+ if grep -rq pam_tally /etc/pam.d/ /usr/share/pam/ /usr/share/pam-configs/ >/dev/null; then
+ db_input critical libpam-modules/deprecate-tally || true
+ db_go || true
+ fi
+fi
+
if dpkg --compare-versions "$2" lt-nl 1.4.0-2; then
db_version 2.0
diff --git a/debian/libpam-modules.templates b/debian/libpam-modules.templates
index b928751e..2f5ad154 100644
--- a/debian/libpam-modules.templates
+++ b/debian/libpam-modules.templates
@@ -7,3 +7,11 @@ _Description: xscreensaver and xlockmore must be restarted before upgrading
authenticate to these programs. You should arrange for these programs
to be restarted or stopped before continuing this upgrade, to avoid
locking your users out of their current sessions.
+
+Template: libpam-modules/deprecate-tally
+Type: error
+_Description: you are using pam_tally or pam_tally2 in your configuration
+ these two modules have been deprecated from linux-pam and you need to
+ migrate to pam_faillock or something else. We still include these
+ deprecated modules to not break systems. The next stable release of
+ libpam-modules will no longer include pam_tally and pam_tally2.
diff --git a/debian/rules b/debian/rules
index f8ad3c18..92a4129b 100755
--- a/debian/rules
+++ b/debian/rules
@@ -31,6 +31,7 @@ override_dh_auto_configure:
--libdir=/lib/$(DEB_HOST_MULTIARCH) \
--enable-isadir=/lib/security \
--enable-cracklib \
+ --enable-tally --enable-tally2 \
$(CONFIGURE_OPTS)
# .install files don't have "except for" handling, so we need to exclude
