Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

configure.ac, lib/, src/: Presume working shadow group support in libc #1111

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

alejandro-colomar
Copy link
Collaborator

@alejandro-colomar alejandro-colomar commented Nov 6, 2024

This check was testing a specific bug in a prehistoric libc version. Red Hat 3 is long dead, and it doesn't make sense to test for that specific bug.


Revisions:

v2
  • Keep lib/shadow.c for musl, which doesn't have these APIs.
  • Include libc's <gshadow.h> if available.
$ git range-diff master gh/shadowgrp shadowgrp 
1:  7abca5a1 < -:  -------- configure.ac, lib/, src/: Presume working shadow group support in libc
-:  -------- > 1:  992aedca lib/: Include <gshadow.h> if it's available
-:  -------- > 2:  545bd8de configure.ac, lib/gshadow.c: Presume working shadow group support in libc
v2b
  • Remove obsolete comment
$ git range-diff master gh/shadowgrp shadowgrp 
1:  992aedca = 1:  992aedca lib/: Include <gshadow.h> if it's available
2:  545bd8de ! 2:  f7071ba1 configure.ac, lib/gshadow.c: Presume working shadow group support in libc
    @@ configure.ac: AC_CHECK_FUNC(secure_getenv, [AC_DEFINE(HAS_SECURE_GETENV,
     
      ## lib/gshadow.c ##
     @@
    + 
      #include <config.h>
      
    - /* Newer versions of Linux libc already have shadow support.  */
    +-/* Newer versions of Linux libc already have shadow support.  */
     -#if defined(SHADOWGRP) && !defined(HAVE_SHADOWGRP)        /*{ */
     +#if defined(SHADOWGRP) && !defined(HAVE_GSHADOW_H)        /*{ */
      
v3
  • Fix compatibility with libc. Patch 1 revealed this incompatibility with libc.
$ git range-diff master gh/shadowgrp shadowgrp 
1:  992aedca = 1:  992aedca lib/: Include <gshadow.h> if it's available
2:  f7071ba1 = 2:  f7071ba1 configure.ac, lib/gshadow.c: Presume working shadow group support in libc
-:  -------- > 3:  8df6b06e lib/gshadow_.h: Fix compatibility with libc's struct sgrp
v3b
  • Silence CodeQL warning by reformatting comment
$ git range-diff master gh/shadowgrp shadowgrp 
1:  992aedca = 1:  992aedca lib/: Include <gshadow.h> if it's available
2:  f7071ba1 ! 2:  e0f15b7a configure.ac, lib/gshadow.c: Presume working shadow group support in libc
    @@ lib/gshadow.c
      
     -/* Newer versions of Linux libc already have shadow support.  */
     -#if defined(SHADOWGRP) && !defined(HAVE_SHADOWGRP)        /*{ */
    -+#if defined(SHADOWGRP) && !defined(HAVE_GSHADOW_H)        /*{ */
    ++#if defined(SHADOWGRP) && !defined(HAVE_GSHADOW_H)
      
      #ident "$Id$"
      
    +@@ lib/gshadow.c: int putsgent (const struct sgrp *sgrp, FILE * fp)
    + }
    + #else
    + extern int ISO_C_forbids_an_empty_translation_unit;
    +-#endif                            /*} SHADOWGRP */
    ++#endif  // !SHADOWGRP
3:  8df6b06e = 3:  d8090da9 lib/gshadow_.h: Fix compatibility with libc's struct sgrp
v3c
  • Rebase
$ git range-diff alx/master..gh/shadowgrp shadow/master..shadowgrp 
1:  992aedca = 1:  0fb36eda lib/: Include <gshadow.h> if it's available
2:  e0f15b7a = 2:  81334580 configure.ac, lib/gshadow.c: Presume working shadow group support in libc
3:  d8090da9 = 3:  679d7552 lib/gshadow_.h: Fix compatibility with libc's struct sgrp
v3d
  • Rebase
$ git range-diff gh/master..gh/shadowgrp shadow/master..shadowgrp 
1:  0fb36eda = 1:  5032a3eb lib/: Include <gshadow.h> if it's available
2:  81334580 = 2:  d8478463 configure.ac, lib/gshadow.c: Presume working shadow group support in libc
3:  679d7552 = 3:  4a78c9cf lib/gshadow_.h: Fix compatibility with libc's struct sgrp
v3e
  • Rebase
$ git range-diff master..gh/shadowgrp shadow/master..shadowgrp 
1:  5032a3eb ! 1:  fb73d816 lib/: Include <gshadow.h> if it's available
    @@ lib/gshadow_.h
      
      /*
       * Shadow group security file structure
    -@@ lib/gshadow_.h: int putsgent ();
    - #endif
    +@@ lib/gshadow_.h: void endsgent (void);
    + int putsgent (const struct sgrp *, FILE *);
      
      #define   GSHADOW "/etc/gshadow"
     -#endif                            /* ifndef _H_GSHADOW */
2:  d8478463 = 2:  9a672ed6 configure.ac, lib/gshadow.c: Presume working shadow group support in libc
3:  4a78c9cf ! 3:  f5db1b5c lib/gshadow_.h: Fix compatibility with libc's struct sgrp
    @@ lib/gshadow.c: void endsgent (void)
        setsgent ();
      
        while ((sgrp = getsgent ()) != NULL) {
    --          if (strcmp (name, sgrp->sg_name) == 0) {
    -+          if (strcmp (name, sgrp->sg_namp) == 0) {
    +-          if (streq(name, sgrp->sg_name)) {
    ++          if (streq(name, sgrp->sg_namp)) {
                        break;
                }
        }
    @@ src/grpck.c: static void check_sgr_file (int *errors, bool *changed)
                                continue;
                        }
      
    --                  if (strcmp (sgr->sg_name, ent->sg_name) != 0) {
    -+                  if (strcmp (sgr->sg_namp, ent->sg_namp) != 0) {
    +-                  if (!streq(sgr->sg_name, ent->sg_name)) {
    ++                  if (!streq(sgr->sg_name, ent->sg_namp)) {
                                continue;
                        }
      
v3f
  • Rebase
$ git range-diff master..gh/shadowgrp shadow/master..shadowgrp 
1:  fb73d816 = 1:  bbb45215 lib/: Include <gshadow.h> if it's available
2:  9a672ed6 = 2:  a61aead2 configure.ac, lib/gshadow.c: Presume working shadow group support in libc
3:  f5db1b5c = 3:  4f5b8c59 lib/gshadow_.h: Fix compatibility with libc's struct sgrp
v3g
  • Fix typo introduced in v3e.
$ git range-diff master gh/shadowgrp shadowgrp 
1:  bbb45215 = 1:  bbb45215 lib/: Include <gshadow.h> if it's available
2:  a61aead2 = 2:  a61aead2 configure.ac, lib/gshadow.c: Presume working shadow group support in libc
3:  4f5b8c59 ! 3:  c8a122a2 lib/gshadow_.h: Fix compatibility with libc's struct sgrp
    @@ src/grpck.c: static void check_sgr_file (int *errors, bool *changed)
                        }
      
     -                  if (!streq(sgr->sg_name, ent->sg_name)) {
    -+                  if (!streq(sgr->sg_name, ent->sg_namp)) {
    ++                  if (!streq(sgr->sg_namp, ent->sg_namp)) {
                                continue;
                        }
v3h
  • Rebase
$ git range-diff master..gh/shadowgrp shadow/master..shadowgrp 
1:  bbb45215 = 1:  d2a28602 lib/: Include <gshadow.h> if it's available
2:  a61aead2 = 2:  d9162817 configure.ac, lib/gshadow.c: Presume working shadow group support in libc
3:  c8a122a2 ! 3:  0f40b17b lib/gshadow_.h: Fix compatibility with libc's struct sgrp
    @@ src/groupadd.c: static void new_grent (struct group *grent)
        if (pflg) {
                sgent->sg_passwd = group_passwd;
        } else {
    -@@ src/groupadd.c: static void grp_update (void)
    +@@ src/groupadd.c: grp_update(void)
        if (is_shadow_grp && (sgr_update (&sgrp) == 0)) {
                fprintf (stderr,
                         _("%s: failed to prepare the new %s entry '%s'\n"),
    @@ src/groupmod.c: static void new_grent (struct group *grent)
        }
      
        /* Always update the shadowed password if there is a shadow entry
    -@@ src/groupmod.c: static void grp_update (void)
    +@@ src/groupmod.c: grp_update(void)
                         * gshadow entry when a new password is requested.
                         */
                        bzero(&sgrp, sizeof sgrp);
    @@ src/groupmod.c: static void grp_update (void)
                        sgrp.sg_passwd = xstrdup (grp.gr_passwd);
                        sgrp.sg_adm    = &empty;
                        sgrp.sg_mem    = dup_list (grp.gr_mem);
    -@@ src/groupmod.c: static void grp_update (void)
    +@@ src/groupmod.c: grp_update(void)
                if (sgr_update (&sgrp) == 0) {
                        fprintf (stderr,
                                 _("%s: failed to prepare the new %s entry '%s'\n"),
v3i
  • Rebase
$ git range-diff master..gh/shadowgrp shadow/master..shadowgrp 
1:  d2a28602 = 1:  833ddd00 lib/: Include <gshadow.h> if it's available
2:  d9162817 = 2:  5eb151a3 configure.ac, lib/gshadow.c: Presume working shadow group support in libc
3:  0f40b17b ! 3:  4d42143b lib/gshadow_.h: Fix compatibility with libc's struct sgrp
    @@ Commit message
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## lib/gshadow.c ##
    -@@ lib/gshadow.c: void endsgent (void)
    +@@ lib/gshadow.c: sgetsgent(const char *string)
        if (NULL != cp || i != FIELDS)
    -           return 0;
    +           return NULL;
      
     -  sgroup.sg_name = fields[0];
     +  sgroup.sg_namp = fields[0];
        sgroup.sg_passwd = fields[1];
    -   if (0 != nadmins) {
    -           nadmins = 0;
    -@@ lib/gshadow.c: void endsgent (void)
    + 
    +   free(sgroup.sg_adm);
    +@@ lib/gshadow.c: sgetsgent(const char *string)
        setsgent ();
      
        while ((sgrp = getsgent ()) != NULL) {
v3j
  • Rebase
$ git range-diff master..gh/shadowgrp shadow/master..shadowgrp 
1:  833ddd00 = 1:  6dbb2331 lib/: Include <gshadow.h> if it's available
2:  5eb151a3 = 2:  5157fed4 configure.ac, lib/gshadow.c: Presume working shadow group support in libc
3:  4d42143b = 3:  0f45349c lib/gshadow_.h: Fix compatibility with libc's struct sgrp

This was referenced Nov 6, 2024
@alejandro-colomar alejandro-colomar marked this pull request as draft November 6, 2024 12:49
@alejandro-colomar

This comment was marked as outdated.

lib/gshadow.c Fixed Show fixed Hide fixed
@alejandro-colomar
Copy link
Collaborator Author

Queued after the release of 4.17.0.

@zeha
Copy link
Contributor

zeha commented Dec 7, 2024

If the title stays "presume working shadow group support in libc", then the !defined(HAVE_GSHADOW_H) code should also go, no?

@alejandro-colomar
Copy link
Collaborator Author

If the title stays "presume working shadow group support in libc", then the !defined(HAVE_GSHADOW_H) code should also go, no?

I presume it works if it's there, but I consider the possibility that it doesn't exist.

musl libc for example doesn't have gshadow.h at all.

The existing code was assuming that libc's <shadow.h> includes
<gshadow.h>.  That's not true.

	alx@debian:~$ find /usr/include/shadow.h
	/usr/include/shadow.h
	alx@debian:~$ find /usr/include/gshadow.h
	/usr/include/gshadow.h
	alx@debian:~$ grep include.*gshadow /usr/include/shadow.h
	alx@debian:~$

As a result, we were unconditionally including our own "gshadow_.h".

Fix that incorrect assumption, and do the following instead:

-  Include unconditionally our own "gshadow_.h".
-  Make our "gshadow_.h" include <gshadow.h> if it exists,
   and only provide the declarations otherwise.

While at it, fix the include guard to be consistent with the project.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
…libc

This check was testing a specific bug in a prehistoric libc version.
Red Hat 3 is long dead, and it doesn't make sense to test for that
specific bug.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
The name of the first field was different.  Rename for compatiblity with
libc.

	$ diff -wU10 \
		<(grepc sgrp . | sed_rm_ccomments) \
		<(grepc sgrp /usr/include/ | sed_rm_ccomments);
	--- /dev/fd/63	2024-11-06 14:49:03.287204461 +0100
	+++ /dev/fd/62	2024-11-06 14:49:03.287204461 +0100
	@@ -1,6 +1,7 @@
	-./lib/gshadow_.h:struct sgrp {
	-	char *sg_name;
	+/usr/include/gshadow.h:struct sgrp
	+  {
	+    char *sg_namp;
		char *sg_passwd;
		char **sg_adm;
		char **sg_mem;
	 };

This originates from a typo in this project, which was later copied by
glibc, and so the typo was set in stone.  The typo was eventually fixed
in shadow, but glibc had already set the name in stone, so we should
just learn to live with it.

	$ grep -rn -C3 sg_name ChangeLog
	1607-
	1608-2011-07-30  Nicolas François  <nicolas.francois@centraliens.net>
	1609-
	1610:	* src/chgpasswd.c: Fix typo sp -> sg. sg_namp -> sg_name
	1611-	* src/chgpasswd.c: Always update the group file when SHADOWGRP is
	1612-	not enabled.
	1613-

This is a scripted change:

	$ find lib* src -type f \
	| xargs sed -i 's/\<sg_name\>/sg_namp/g';

Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar alejandro-colomar marked this pull request as ready for review December 22, 2024 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants