Re: Review of pam for bookworm
Hi Roberto,
On Sun, Aug 10, 2025 at 05:48:57PM -0400, Roberto C. Sánchez wrote:
> Hi Bastien
>
> On Thu, Jul 31, 2025 at 03:06:17PM +0200, Bastien Roucaries wrote:
> > Le dimanche 27 juillet 2025, 15:30:25 heure d’été d’Europe centrale Bastien
> > Roucaries a écrit :
> > > Le vendredi 25 juillet 2025, 20:13:13 heure d’été d’Europe centrale Bastien
> > >
> > > Roucaries a écrit :
> > > > Hi,
> > > >
> > > > Could you review my work for pam/bookworm
> > > > https://salsa.debian.org/rouca/pam/-/tree/bookworm/debian?ref_type=heads
> > > >
> > > > Can someone have an idea why autopkg fail ?
> > > >
> > > > I am trying to fix first bookworm then bullseye
> > >
> > > bullseye backported help welcome
> > Buster and stretch also are for review
> >
> > rouca
> > >
> > > > rouca
> >
> I have reviewed the branches for bookworm, buster, and stretch.
> Everything looks generally good, but I do have a few minor points and/or
> questions:
>
> - What is the source for the patch "Subject: pam_namespace from v1.7.1"?
> It appears to be a combination of more than one commit, with some
> added changes, but I would have expected to locate it in the
> vorlon/pam Salsa project and I could not.
>
> - On the buster and stretch branches, in str-skip-prefix.patch, there is
> a peculiar difference with the upstream commit (584c539798). The patch
> in the package adds this definition:
>
> +#define pam_str_skip_prefix(str_, prefix_) \
> + pam_str_skip_prefix_len((str_), (prefix_), sizeof(prefix_) - 1)
>
> The upstream commit has this:
>
> +#define pam_str_skip_prefix(str_, prefix_) \
> + pam_str_skip_prefix_len((str_), (prefix_), sizeof(prefix_) - 1 + PAM_MUST_BE_ARRAY(prefix_))
>
> My assumption here is that the PAM_MUST_BE_ARRAY() macro is not
> present/available in the buster version (most likely because it was
> introduced later). Is that right?
>
> Assuming that you are confident in the origin of the v1.7.1 namespace
> patch (and that it doesn't need a review) and that my understanding of the
> PAM_MUST_BE_ARRAY() is correct, then the changes look good to me.
Please note that for pam in bookworm we asked specifically the
maintainer if he can prepare an update. Cc'ed. Cc'ing as well the
security team alias.
As a tendency, and in particular for packages which have the
maintainer as the experts in the area, would would lean toward taking
an update made, or at least blessed/vetted by the maintainer still in
charge for the regular stable and oldstable releases.
The current approach was to make the update exposed in the proposed
updates suites once it's done. Maybe now we can sync up with Bastien's
work and see if they basically match.
For the pam_namespace issue: The best approach seems to indeed base
the pam_namespace code to pam's upstream 1.7.0+patches for the CVE
fixes or 1.7.1, this is how we approached it as well indeed.
Hope this helps,
Regards,
Salvatore
Reply to: