Merge ~andersson123/ubuntu/+source/shadow:ubuntu/noble-proposed into ubuntu/+source/shadow:ubuntu/noble-devel

[Tim Andersson]

    * Don't include pam_lastlog.so as optional (LP: #2060676)
      - d/p/remove-lastlog-from-login-config.patch: Remove pam_lastlog.so as optional login dependency

[Lukas Märdian (slyon)]

Hi Tim! I left some inline comments below.

Overall, I'm not sure if the rationale is strong enough for this to be SRUed... As you mentioned in the bug report yourself, it's mostly a cosmetic issue, to avoid a warning log message. I'm not a SRU reviewer, but I'd suggest improving your [Impact] section, if you still want to get this landed.

IMO the risk & impact might be too high for just muting a log message. If you still want to go for it, I'd suggest getting the package into noble-proposed, but then adding the "block-proposed-noble" tag to the SRU bug. That way you could still do the full SRU verification, but keep the package update pending, until some other shadow SRU needs to be done and the two fixes could be deployed together as a bundle to noble-updates.

Another thought: When and why exactly did the pam_lastlog.so disappear? Might it be a better option to get that fixed instead? As that might actually affect users relying on the "last" command. Fixing that root cause (either by bringing back old pam_lastlog.so or some new lastlog2 binary), seems more valuable to me.

review: Needs Fixing

[Steve Langasek (vorlon)]

Is this change already present in oracular and plucky? By SRU policy it must be before uploading to Bible.

review: Needs Fixing

[Tim Andersson (andersson123)]

Thanks for your feedback guys! I spoke with juliank last week, and I'm going to do this SRU as a staged upload. I'm going to add information to the bug report about:
- When pam_lastlog.so disappeared
- how we could potentially create an SRU which removes pam_lastlog.so and instead relies on the lastlog2 binary - though this'll involve an SRU of shadow and an SRU of util-linux (lastlog2 is introduced in util-linux=2.40)
- How we could potentially create an SRU of shadow which re-introduces the old pam_lastlog.so binary
- Information on why this issue isn't present in oracular and plucky

[Tim Andersson (andersson123)]

Aside from that, I've pushed changes based on inline comments

[Tim Andersson (andersson123)]

I think this is perhaps ready for re-review :) :D

[Steve Langasek (vorlon)]

review: Needs Fixing

[Tim Andersson (andersson123)]

Hey Steve! Sorry, I added info to the bug regarding it being fixed in oracular and plucky. Should I also add that info to this MP or is it fine as is?

[Tim Andersson (andersson123)]

amended and ready for re-review

[Steve Langasek (vorlon)]

review: Needs Fixing

[Tim Andersson (andersson123)]

Oh, so sorry, I rushed that last changelog entry and missed that. amended

[Steve Langasek (vorlon)]

review: Approve

[Steve Langasek (vorlon)]

And uploaded

diff --git a/debian/changelog b/debian/changelog
index 0f1c0e2..1ae717b 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+shadow (1:4.13+dfsg1-4ubuntu3.3) noble; urgency=medium
+
+  * Don't include pam_lastlog.so as optional (LP: #2060676)
+  * pam_lastlog.so was deprecated prior to noble release, due to it
+    not being Y2038 compatible. The shared object file doesn't exist
+    in noble systems, so it should not exist in any pam config.
+
+ -- Tim 'andersson123' Andersson <[e-mail]>  Wed, 13 Nov 2024 12:55:16 +0000
+
 shadow (1:4.13+dfsg1-4ubuntu3.2) noble; urgency=medium
 
   * d/p/lp2063200/*: amend the patch to fix `useradd -D` breakage
diff --git a/debian/login.pam b/debian/login.pam
index aaadc64..f79f840 100644
--- a/debian/login.pam
+++ b/debian/login.pam
@@ -77,10 +77,6 @@ auth       optional   pam_group.so
 # (Replaces the use of /etc/limits in old login)
 session    required   pam_limits.so
 
-# Prints the last login info upon successful login
-# (Replaces the `LASTLOG_ENAB' option from login.defs)
-session    optional   pam_lastlog.so
-
 # Prints the status of the user's mailbox upon successful login
 # (Replaces the `MAIL_CHECK_ENAB' option from login.defs). 
 #