[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Proposed RC fix for acct



Hi Sven,

On Fri, Jun 06, 2025 at 12:38:40AM +0200, Sven Geuer wrote:
> On Thu, 2025-06-05 at 21:55 +0100, Andrew Bower wrote:
> > On Thu, Jun 05, 2025 at 10:22:46PM +0200, Sven Geuer wrote:
> > > On Thu, 2025-06-05 at 08:16 +0100, Andrew Bower wrote:
> > > > I have prepared a drive-by contribution at
> > > > https://salsa.debian.org/pkg-security-team/acct/-/merge_requests/6 to
> > > > fix RC bug https://bugs.debian.org/1074591 raised against acct.
> > > > 
> > > > If this fix is acceptable to the team I am willing to prepare an unblock
> > > > request for the package, explaining that the documentation changes are
> > > > an integral part of the fix to ensure that users understand the
> > > > package's limitations and whether it is suitable for their use case.
> > > 
> > > Thanks for your MR and your offer!
> > 
> > My pleasure!
> > 
> > > > The login analysis for a running system is no longer effective in acct
> > > > on trixie
> > > 
> > > May be I am wrong, but wouldn't it be effective with wtmpdb and libpam-
> > > wtmpdb installed? If the answer is yes it seems reasonable to me to
> > > have acct depend on wtmpdb and libpam-wtmpdb. It would also allow for a
> > > simplified change to the documentation.
> > 
> > This is a good question!
> > 
> > Perhaps I should transfer this comment to the bug but the reason for the
> > user-reported error (which I'm not convinced is really serious although
> > I think a documentation update is due at a minimum) is that the last
> > command is invoked with the '-f' option to refer specifically to
> > /var/log/wtmp, which either does not exist or is not in wtmpdb format.
> > 
> > So what this patch does to the cron job is to let 'last' use the default
> > live database location and indeed limits to just the last month which is
> > clearly what was originally intended with the cron job and obviates the
> > need for the current README.Debian contents, too.
> > 
> > Now that was just the cron job (which probably isn't that important
> > anymore.) So far as I can tell there are 4 main uses for this package:
> > 
> > 1. Live login analysis
> > 2. Live process accounting analysis
> > 3. Forensic login analysis of another target
> > 4. Forensic process accounting of another target
> > 
> > The live login analysis relies on /var/log/wtmp being written in utmp
> > format, which no longer happens (well, can't be guaranteed to happen
> > comprehensively) in Debian 13 and wouldn't be helped by the presence of
> > wtmpdb. So that makes use case 1 not very useful in trixie.
> > 
> > But the other three use cases are still valid and the forensic use of
> > login analysis is useful not just for older Debian installations but
> > of other distributions which have not dropped wtmp.
> > 
> > > If the functionality changed at least a 'Suggests' should be added to
> > > d/control, IMHO.
> > > 
> > > Does this make sense to you, Andrew?
> > 
> > Yes, I think 'Suggests wtmpdb' could be justified normally, although it
> > only helps the low importance cron job, but I thought changes to
> > dependencies weren't desirable at this freeze stage?
> 
> Assuming you refer to this line
> 
> 7. changing relations (depends, conflicts, ...) between packages
> 
> under https://release.debian.org/testing/freeze_policy.html#appropriate,
> I read this as changes to the Depends field are not desirable, while
> changes to the Recommmends and Suggests fields are not covered, though
> the '...' might include them.
> 
> I really would prefer to have the Suggests field included, so, asking for a
> pre-approval might be right measure to be on the save side.

You are right, I think this is warranted. I had missed the point that
the original bug poster did not have wtmpdb installed and focussed on
the issues you get when you *do* have it installed.

So on reflection I think 'Recommends' is actually reasonable because we
really do recommend that people who are interested in login accounting
install 'wtmpdb'.

I have updated the MR accordingly, improved the documentation changes
and split out the elements of my proposed update.

Thanks!

Andrew

Attachment: signature.asc
Description: PGP signature


Reply to: