etc/pam.d/Makefile.am: Fix typo #949

alejandro-colomar · 2024-02-13T12:24:10Z

Fixes: 341d80c ("Makefile: move chpasswd and newusers to pamd target")
Link: #928 (comment)
Link: #926 (comment)
Reported-by: @DimStar77
Reported-by: @jubalh
Cc: @loqs
Cc: @dvzrv
Cc: @ikerexxe

To be cherry-picked to 4.14.x.

DimStar77

It depends - the original commit message mentioned wanting to move chpasswd up - now it's chgpasswd..

alejandro-colomar · 2024-02-13T12:27:40Z

@DimStar77

Do you mean this?

diff --git a/etc/pam.d/Makefile.am b/etc/pam.d/Makefile.am
index fe676b44..6a76551f 100644
--- a/etc/pam.d/Makefile.am
+++ b/etc/pam.d/Makefile.am
@@ -2,6 +2,7 @@
 # and also cooperate to make a distribution for `make dist'
 
 pamd_files = \
+       chpasswd \
        chfn \
        chgpasswd \
        chsh \
@@ -12,7 +13,6 @@ pamd_files = \
 
 pamd_acct_tools_files = \
        chage \
-       chpasswd \
        groupadd \
        groupdel \
        groupmod \

or this?

diff --git a/etc/pam.d/Makefile.am b/etc/pam.d/Makefile.am
index fe676b44..b8e4321f 100644
--- a/etc/pam.d/Makefile.am
+++ b/etc/pam.d/Makefile.am
@@ -2,8 +2,8 @@
 # and also cooperate to make a distribution for `make dist'
 
 pamd_files = \
+       chpasswd \
        chfn \
-       chgpasswd \
        chsh \
        groupmems \
        login \
@@ -12,7 +12,7 @@ pamd_files = \
 
 pamd_acct_tools_files = \
        chage \
-       chpasswd \
+       chgpasswd \
        groupadd \
        groupdel \
        groupmod \

Or none?

DimStar77 · 2024-02-13T12:28:58Z

@DimStar77

2nd variant is more in line with the commit message of the original PR;

alejandro-colomar · 2024-02-13T12:29:58Z

It depends - the original commit message mentioned wanting to move chpasswd up - now it's chgpasswd..

Yeah, that's why I want someone with the original problem to test this thing. :)
The problem was reproducible on Arch Linux, so if any of the packagers from Arch can check this, it would help. Did Suse or Gentoo have the original problem too maybe?

alejandro-colomar · 2024-02-13T12:30:24Z

@DimStar77

2nd variant is more in line with the commit message of the original PR;

Yep. Thanks!

DimStar77 · 2024-02-13T12:31:40Z

It depends - the original commit message mentioned wanting to move chpasswd up - now it's chgpasswd..

Yeah, that's why I want someone with the original problem to test this thing. :) The problem was reproducible on Arch Linux, so if any of the packagers from Arch can check this, it would help. Did Suse or Gentoo have the original problem too maybe?

At least openSUSE/SUSE did not - as we build shadow with --enable-account-tools-setuid

alejandro-colomar · 2024-02-13T12:35:43Z

v2 changes:

Respect the original commit message, so the typo seems somewhere else. Document it in the commit message. [@DimStar77 ]
Co-developed-by: @DimStar77

$ git range-diff shadow/master gh/tfix tfix 
1:  52af3050 < -:  -------- etc/pam.d/Makefile.am: Fix typo
-:  -------- > 1:  3ad184cc etc/pam.d/Makefile.am: Fix typo

alejandro-colomar · 2024-02-13T12:44:09Z

I've marked 4.14.4 as broken (https://github.com/shadow-maint/shadow/releases/tag/4.14.4), and set 4.14.3 as the latest release.

alejandro-colomar · 2024-02-13T13:10:12Z

I'll take your approval as a Signed-off-by ;)

alejandro-colomar · 2024-02-13T13:11:11Z

v2b changes:

Tags [@DimStar77 ]

$ git range-diff shadow/master gh/tfix tfix 
1:  3ad184cc ! 1:  8e8a817a etc/pam.d/Makefile.am: Fix typo
    @@ Commit message
         Cc: David Runge <[email protected]>
         Cc: Iker Pedrosa <[email protected]>
         Co-developed-by: Dominique Leuenberger <[email protected]>
    +    Signed-off-by: Dominique Leuenberger <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## etc/pam.d/Makefile.am ##

alejandro-colomar · 2024-02-13T14:10:06Z

@DimStar77

Can you please confirm that this fixes the build on Suse?

ikerexxe

Ups! Big mistake on my side for not realizing this. Thanks @alejandro-colomar for providing the fix.

jubalh · 2024-02-13T14:55:25Z

I pulled this PR into a patch and get:

Complete spec file

  --enable-shadowgrp \
  --enable-account-tools-setuid \
  --with-audit \
  --with-libpam \
  --with-sha-crypt \
  --with-acl \
  --with-attr \
  --with-nscd \
  --with-selinux \
  --without-libcrack \
  --without-libbsd \
  --with-group-name-max-length=32 \

Complete build log

[   71s] Making all in pam.d
[   71s] make[3]: *** No rule to make target 'chgpasswd', needed by 'all-am'.  Stop.

DimStar77 · 2024-02-13T15:24:36Z

I pulled this PR into a patch and get:
[ 71s] Making all in pam.d
[ 71s] make[3]: *** No rule to make target 'chgpasswd', needed by 'all-am'. Stop.

Patching the released 4.14.4 tarball with only this patch won't work - as the file https://github.com/shadow-maint/shadow/blob/master/etc/pam.d/chgpasswd is missing in the tarball (make dist had incomplete data)

you will have to add the content of that file to your build

jubalh · 2024-02-13T15:28:47Z

is missing in the tarball (make dist had incomplete data)

Ah of course. I thought I'm missing something here.
Excuse me, I was doing this while being in a meeting and wanted to use the time "productive" ;) That obviously didn't work out :)

Everything builds fine now.

alejandro-colomar · 2024-02-13T15:40:32Z

v2c changes:

Tags [@jubalh , @ikerexxe ]

$ git range-diff shadow/master gh/tfix tfix 
1:  8e8a817a ! 1:  60d6b698 etc/pam.d/Makefile.am: Fix typo
    @@ Commit message
         Reported-by: Michael Vetter <[email protected]>
         Cc: loqs <https://github.com/loqs>
         Cc: David Runge <[email protected]>
    -    Cc: Iker Pedrosa <[email protected]>
    +    Reviewed-by: Iker Pedrosa <[email protected]>
    +    Tested-by: Michael Vetter <[email protected]>
    +    Reviewed-by: Michael Vetter <[email protected]>
         Co-developed-by: Dominique Leuenberger <[email protected]>
         Signed-off-by: Dominique Leuenberger <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>

While I'd like if someone from Arch could have a look at this,

Versions 4.14.3 and older have been broken for Arch for a long time.
4.14.4 has introduced a regression for Gentoo (and possibly others), and is marked as broken, so no recent good version has ever worked for Arch correctly.
This fixes the regression for Gentoo (and possibly others).
We believe it probably won't break Arch, but we don't know for sure.

So, @ikerexxe , I'll let you merge. I think we can merge already, but maybe you want to give them some time to have a look.

loqs · 2024-02-13T17:27:31Z

This does not cause any change on Arch due to its use of --disable-account-tools-setuid ships chgpasswd without pam support.

The commit we're fixing mentions that it wanted to move 'chpasswd', but it removed 'ch_g_passwd' from 'pamd_acct_tools_files' and added 'chpasswd' to 'pamd_files'. It seems it removed the wrong thing by accident. Fixes: 341d80c ("Makefile: move chpasswd and newusers to pamd target") Link: <shadow-maint#928 (comment)> Link: <shadow-maint#926 (comment)> Reported-by: Dominique Leuenberger <[email protected]> Reported-by: Michael Vetter <[email protected]> Cc: David Runge <[email protected]> Reviewed-by: Iker Pedrosa <[email protected]> Tested-by: Michael Vetter <[email protected]> Reviewed-by: Michael Vetter <[email protected]> Reviewed-by: loqs <https://github.com/loqs> Co-developed-by: Dominique Leuenberger <[email protected]> Signed-off-by: Dominique Leuenberger <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

alejandro-colomar · 2024-02-13T17:44:14Z

This does not cause any change on Arch due to its use of --disable-account-tools-setuid ships chgpasswd without pam support.

v2d changes:

Arch review by @loqs

$ git range-diff shadow/master gh/tfix tfix 
1:  60d6b698 ! 1:  96fc104a etc/pam.d/Makefile.am: Fix typo
    @@ Commit message
         Link: <https://github.com/shadow-maint/shadow/issues/926#issuecomment-1941324761>
         Reported-by: Dominique Leuenberger <[email protected]>
         Reported-by: Michael Vetter <[email protected]>
    -    Cc: loqs <https://github.com/loqs>
         Cc: David Runge <[email protected]>
         Reviewed-by: Iker Pedrosa <[email protected]>
         Tested-by: Michael Vetter <[email protected]>
         Reviewed-by: Michael Vetter <[email protected]>
    +    Reviewed-by: loqs <https://github.com/loqs>
         Co-developed-by: Dominique Leuenberger <[email protected]>
         Signed-off-by: Dominique Leuenberger <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>

Now I'll merge. :)

4.14.4 is under certain conditions broken. See shadow-maint/shadow#949. Signed-off-by: Michael Vetter <[email protected]>

4.14.4 is under certain conditions broken. See shadow-maint/shadow#949. Signed-off-by: Michael Vetter <[email protected]> Closes: #35309 Signed-off-by: Sam James <[email protected]>

DimStar77 reviewed Feb 13, 2024

View reviewed changes

alejandro-colomar force-pushed the tfix branch from 52af305 to 3ad184c Compare February 13, 2024 12:34

alejandro-colomar mentioned this pull request Feb 13, 2024

4.14.5, and 4.14.6 stable releases #947

Closed

DimStar77 approved these changes Feb 13, 2024

View reviewed changes

alejandro-colomar force-pushed the tfix branch from 3ad184c to 8e8a817 Compare February 13, 2024 13:10

ikerexxe approved these changes Feb 13, 2024

View reviewed changes

jubalh approved these changes Feb 13, 2024

View reviewed changes

alejandro-colomar force-pushed the tfix branch from 8e8a817 to 60d6b69 Compare February 13, 2024 15:34

alejandro-colomar mentioned this pull request Feb 13, 2024

Install chpasswd,newusers configs when built with pam but without setuid #928

Merged

alejandro-colomar force-pushed the tfix branch from 60d6b69 to 96fc104 Compare February 13, 2024 17:43

alejandro-colomar merged commit 7eb10e6 into shadow-maint:master Feb 13, 2024
8 checks passed

alejandro-colomar deleted the tfix branch February 13, 2024 17:45

jubalh added a commit to jubalh/gentoo that referenced this pull request Feb 13, 2024

sys-apps/shadow: add 4.14.5

701a0b4

4.14.4 is under certain conditions broken. See shadow-maint/shadow#949. Signed-off-by: Michael Vetter <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

etc/pam.d/Makefile.am: Fix typo #949

etc/pam.d/Makefile.am: Fix typo #949

alejandro-colomar commented Feb 13, 2024 •

edited

DimStar77 left a comment •

edited

alejandro-colomar commented Feb 13, 2024 •

edited

DimStar77 commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024

DimStar77 commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024 •

edited

alejandro-colomar commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024 •

edited

ikerexxe left a comment

jubalh commented Feb 13, 2024 •

edited

DimStar77 commented Feb 13, 2024

jubalh commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024

loqs commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024

etc/pam.d/Makefile.am: Fix typo #949

etc/pam.d/Makefile.am: Fix typo #949

Conversation

alejandro-colomar commented Feb 13, 2024 • edited

DimStar77 left a comment • edited

Choose a reason for hiding this comment

alejandro-colomar commented Feb 13, 2024 • edited

DimStar77 commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024

DimStar77 commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024 • edited

alejandro-colomar commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024 • edited

ikerexxe left a comment

Choose a reason for hiding this comment

jubalh commented Feb 13, 2024 • edited

DimStar77 commented Feb 13, 2024

jubalh commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024

loqs commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024

alejandro-colomar commented Feb 13, 2024 •

edited

DimStar77 left a comment •

edited

alejandro-colomar commented Feb 13, 2024 •

edited

alejandro-colomar commented Feb 13, 2024 •

edited

alejandro-colomar commented Feb 13, 2024 •

edited

jubalh commented Feb 13, 2024 •

edited