diff mbox series

[2/4] tst-env-setuid: Use support_capture_subprogram_self_sgid

Message ID 20210316070755.330084-3-siddhesh@sourceware.org
State New
Headers show
Series tunables and setxid programs | expand

Commit Message

Siddhesh Poyarekar March 16, 2021, 7:07 a.m. UTC
Use the support_capture_subprogram_self_sgid to spawn an sgid child.
---
 elf/tst-env-setuid.c | 197 +++----------------------------------------
 1 file changed, 14 insertions(+), 183 deletions(-)

Comments

Carlos O'Donell April 6, 2021, 4:39 p.m. UTC | #1
On 3/16/21 3:07 AM, Siddhesh Poyarekar via Libc-alpha wrote:
> Use the support_capture_subprogram_self_sgid to spawn an sgid child.

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  elf/tst-env-setuid.c | 197 +++----------------------------------------
>  1 file changed, 14 insertions(+), 183 deletions(-)
> 
> diff --git a/elf/tst-env-setuid.c b/elf/tst-env-setuid.c
> index 60ae0ca380..49b5e319e2 100644
> --- a/elf/tst-env-setuid.c
> +++ b/elf/tst-env-setuid.c
> @@ -29,173 +29,12 @@
>  #include <sys/wait.h>
>  #include <unistd.h>
>  
> +#include <support/check.h>
>  #include <support/support.h>
>  #include <support/test-driver.h>
> +#include <support/capture_subprocess.h>

OK.

>  
>  static char SETGID_CHILD[] = "setgid-child";
> -#define CHILD_STATUS 42
> -
> -/* Return a GID which is not our current GID, but is present in the
> -   supplementary group list.  */
> -static gid_t
> -choose_gid (void)
> -{
> -  const int count = 64;
> -  gid_t groups[count];
> -  int ret = getgroups (count, groups);
> -  if (ret < 0)
> -    {
> -      printf ("getgroups: %m\n");
> -      exit (1);
> -    }
> -  gid_t current = getgid ();
> -  for (int i = 0; i < ret; ++i)
> -    {
> -      if (groups[i] != current)
> -	return groups[i];
> -    }
> -  return 0;
> -}
> -
> -/* Spawn and execute a program and verify that it returns the CHILD_STATUS.  */
> -static pid_t
> -do_execve (char **args)
> -{
> -  pid_t kid = vfork ();
> -
> -  if (kid < 0)
> -    {
> -      printf ("vfork: %m\n");
> -      return -1;
> -    }
> -
> -  if (kid == 0)
> -    {
> -      /* Child process.  */
> -      execve (args[0], args, environ);
> -      _exit (-errno);
> -    }
> -
> -  if (kid < 0)
> -    return 1;
> -
> -  int status;
> -
> -  if (waitpid (kid, &status, 0) < 0)
> -    {
> -      printf ("waitpid: %m\n");
> -      return 1;
> -    }
> -
> -  if (WEXITSTATUS (status) == EXIT_UNSUPPORTED)
> -    return EXIT_UNSUPPORTED;
> -
> -  if (!WIFEXITED (status) || WEXITSTATUS (status) != CHILD_STATUS)
> -    {
> -      printf ("Unexpected exit status %d from child process\n",
> -	      WEXITSTATUS (status));
> -      return 1;
> -    }
> -  return 0;
> -}
> -
> -/* Copies the executable into a restricted directory, so that we can
> -   safely make it SGID with the TARGET group ID.  Then runs the
> -   executable.  */
> -static int
> -run_executable_sgid (gid_t target)
> -{
> -  char *dirname = xasprintf ("%s/tst-tunables-setuid.%jd",
> -			     test_dir, (intmax_t) getpid ());
> -  char *execname = xasprintf ("%s/bin", dirname);
> -  int infd = -1;
> -  int outfd = -1;
> -  int ret = 0;
> -  if (mkdir (dirname, 0700) < 0)
> -    {
> -      printf ("mkdir: %m\n");
> -      goto err;
> -    }
> -  infd = open ("/proc/self/exe", O_RDONLY);
> -  if (infd < 0)
> -    {
> -      printf ("open (/proc/self/exe): %m\n");
> -      goto err;
> -    }
> -  outfd = open (execname, O_WRONLY | O_CREAT | O_EXCL, 0700);
> -  if (outfd < 0)
> -    {
> -      printf ("open (%s): %m\n", execname);
> -      goto err;
> -    }
> -  char buf[4096];
> -  for (;;)
> -    {
> -      ssize_t rdcount = read (infd, buf, sizeof (buf));
> -      if (rdcount < 0)
> -	{
> -	  printf ("read: %m\n");
> -	  goto err;
> -	}
> -      if (rdcount == 0)
> -	break;
> -      char *p = buf;
> -      char *end = buf + rdcount;
> -      while (p != end)
> -	{
> -	  ssize_t wrcount = write (outfd, buf, end - p);
> -	  if (wrcount == 0)
> -	    errno = ENOSPC;
> -	  if (wrcount <= 0)
> -	    {
> -	      printf ("write: %m\n");
> -	      goto err;
> -	    }
> -	  p += wrcount;
> -	}
> -    }
> -  if (fchown (outfd, getuid (), target) < 0)
> -    {
> -      printf ("fchown (%s): %m\n", execname);
> -      goto err;
> -    }
> -  if (fchmod (outfd, 02750) < 0)
> -    {
> -      printf ("fchmod (%s): %m\n", execname);
> -      goto err;
> -    }
> -  if (close (outfd) < 0)
> -    {
> -      printf ("close (outfd): %m\n");
> -      goto err;
> -    }
> -  if (close (infd) < 0)
> -    {
> -      printf ("close (infd): %m\n");
> -      goto err;
> -    }
> -
> -  char *args[] = {execname, SETGID_CHILD, NULL};
> -
> -  ret = do_execve (args);
> -
> -err:
> -  if (outfd >= 0)
> -    close (outfd);
> -  if (infd >= 0)
> -    close (infd);
> -  if (execname)
> -    {
> -      unlink (execname);
> -      free (execname);
> -    }
> -  if (dirname)
> -    {
> -      rmdir (dirname);
> -      free (dirname);
> -    }
> -  return ret;
> -}
>  
>  #ifndef test_child
>  static int
> @@ -256,40 +95,32 @@ do_test (int argc, char **argv)
>    if (argc == 2 && strcmp (argv[1], SETGID_CHILD) == 0)
>      {
>        if (getgid () == getegid ())
> -	{
> -	  /* This can happen if the file system is mounted nosuid.  */
> -	  fprintf (stderr, "SGID failed: GID and EGID match (%jd)\n",
> -		   (intmax_t) getgid ());
> -	  exit (EXIT_UNSUPPORTED);
> -	}
> +	/* This can happen if the file system is mounted nosuid.  */
> +	FAIL_UNSUPPORTED ("SGID failed: GID and EGID match (%jd)\n",
> +			  (intmax_t) getgid ());
>  
>        int ret = test_child ();
>  
>        if (ret != 0)
>  	exit (1);
>  
> -      exit (CHILD_STATUS);
> +      exit (EXIT_SUCCESS);
>      }
>    else
>      {
>        if (test_parent () != 0)
>  	exit (1);
>  
> -      /* Try running a setgid program.  */
> -      gid_t target = choose_gid ();
> -      if (target == 0)
> -	{
> -	  fprintf (stderr,
> -		   "Could not find a suitable GID for user %jd, skipping test\n",
> -		   (intmax_t) getuid ());
> -	  exit (0);
> -	}
> +      int status = support_capture_subprogram_self_sgid (SETGID_CHILD);
>  
> -      return run_executable_sgid (target);
> -    }
> +      if (WEXITSTATUS (status) == EXIT_UNSUPPORTED)
> +	return EXIT_UNSUPPORTED;
> +
> +      if (!WIFEXITED (status))
> +	FAIL_EXIT1 ("Unexpected exit status %d from child process\n", status);
>  
> -  /* Something went wrong and our argv was corrupted.  */
> -  _exit (1);
> +      return 0;
> +    }

OK. Refactor to use new support_capture_subprogram_self_sgid.

>  }
>  
>  #define TEST_FUNCTION_ARGV do_test
>
diff mbox series

Patch

diff --git a/elf/tst-env-setuid.c b/elf/tst-env-setuid.c
index 60ae0ca380..49b5e319e2 100644
--- a/elf/tst-env-setuid.c
+++ b/elf/tst-env-setuid.c
@@ -29,173 +29,12 @@ 
 #include <sys/wait.h>
 #include <unistd.h>
 
+#include <support/check.h>
 #include <support/support.h>
 #include <support/test-driver.h>
+#include <support/capture_subprocess.h>
 
 static char SETGID_CHILD[] = "setgid-child";
-#define CHILD_STATUS 42
-
-/* Return a GID which is not our current GID, but is present in the
-   supplementary group list.  */
-static gid_t
-choose_gid (void)
-{
-  const int count = 64;
-  gid_t groups[count];
-  int ret = getgroups (count, groups);
-  if (ret < 0)
-    {
-      printf ("getgroups: %m\n");
-      exit (1);
-    }
-  gid_t current = getgid ();
-  for (int i = 0; i < ret; ++i)
-    {
-      if (groups[i] != current)
-	return groups[i];
-    }
-  return 0;
-}
-
-/* Spawn and execute a program and verify that it returns the CHILD_STATUS.  */
-static pid_t
-do_execve (char **args)
-{
-  pid_t kid = vfork ();
-
-  if (kid < 0)
-    {
-      printf ("vfork: %m\n");
-      return -1;
-    }
-
-  if (kid == 0)
-    {
-      /* Child process.  */
-      execve (args[0], args, environ);
-      _exit (-errno);
-    }
-
-  if (kid < 0)
-    return 1;
-
-  int status;
-
-  if (waitpid (kid, &status, 0) < 0)
-    {
-      printf ("waitpid: %m\n");
-      return 1;
-    }
-
-  if (WEXITSTATUS (status) == EXIT_UNSUPPORTED)
-    return EXIT_UNSUPPORTED;
-
-  if (!WIFEXITED (status) || WEXITSTATUS (status) != CHILD_STATUS)
-    {
-      printf ("Unexpected exit status %d from child process\n",
-	      WEXITSTATUS (status));
-      return 1;
-    }
-  return 0;
-}
-
-/* Copies the executable into a restricted directory, so that we can
-   safely make it SGID with the TARGET group ID.  Then runs the
-   executable.  */
-static int
-run_executable_sgid (gid_t target)
-{
-  char *dirname = xasprintf ("%s/tst-tunables-setuid.%jd",
-			     test_dir, (intmax_t) getpid ());
-  char *execname = xasprintf ("%s/bin", dirname);
-  int infd = -1;
-  int outfd = -1;
-  int ret = 0;
-  if (mkdir (dirname, 0700) < 0)
-    {
-      printf ("mkdir: %m\n");
-      goto err;
-    }
-  infd = open ("/proc/self/exe", O_RDONLY);
-  if (infd < 0)
-    {
-      printf ("open (/proc/self/exe): %m\n");
-      goto err;
-    }
-  outfd = open (execname, O_WRONLY | O_CREAT | O_EXCL, 0700);
-  if (outfd < 0)
-    {
-      printf ("open (%s): %m\n", execname);
-      goto err;
-    }
-  char buf[4096];
-  for (;;)
-    {
-      ssize_t rdcount = read (infd, buf, sizeof (buf));
-      if (rdcount < 0)
-	{
-	  printf ("read: %m\n");
-	  goto err;
-	}
-      if (rdcount == 0)
-	break;
-      char *p = buf;
-      char *end = buf + rdcount;
-      while (p != end)
-	{
-	  ssize_t wrcount = write (outfd, buf, end - p);
-	  if (wrcount == 0)
-	    errno = ENOSPC;
-	  if (wrcount <= 0)
-	    {
-	      printf ("write: %m\n");
-	      goto err;
-	    }
-	  p += wrcount;
-	}
-    }
-  if (fchown (outfd, getuid (), target) < 0)
-    {
-      printf ("fchown (%s): %m\n", execname);
-      goto err;
-    }
-  if (fchmod (outfd, 02750) < 0)
-    {
-      printf ("fchmod (%s): %m\n", execname);
-      goto err;
-    }
-  if (close (outfd) < 0)
-    {
-      printf ("close (outfd): %m\n");
-      goto err;
-    }
-  if (close (infd) < 0)
-    {
-      printf ("close (infd): %m\n");
-      goto err;
-    }
-
-  char *args[] = {execname, SETGID_CHILD, NULL};
-
-  ret = do_execve (args);
-
-err:
-  if (outfd >= 0)
-    close (outfd);
-  if (infd >= 0)
-    close (infd);
-  if (execname)
-    {
-      unlink (execname);
-      free (execname);
-    }
-  if (dirname)
-    {
-      rmdir (dirname);
-      free (dirname);
-    }
-  return ret;
-}
 
 #ifndef test_child
 static int
@@ -256,40 +95,32 @@  do_test (int argc, char **argv)
   if (argc == 2 && strcmp (argv[1], SETGID_CHILD) == 0)
     {
       if (getgid () == getegid ())
-	{
-	  /* This can happen if the file system is mounted nosuid.  */
-	  fprintf (stderr, "SGID failed: GID and EGID match (%jd)\n",
-		   (intmax_t) getgid ());
-	  exit (EXIT_UNSUPPORTED);
-	}
+	/* This can happen if the file system is mounted nosuid.  */
+	FAIL_UNSUPPORTED ("SGID failed: GID and EGID match (%jd)\n",
+			  (intmax_t) getgid ());
 
       int ret = test_child ();
 
       if (ret != 0)
 	exit (1);
 
-      exit (CHILD_STATUS);
+      exit (EXIT_SUCCESS);
     }
   else
     {
       if (test_parent () != 0)
 	exit (1);
 
-      /* Try running a setgid program.  */
-      gid_t target = choose_gid ();
-      if (target == 0)
-	{
-	  fprintf (stderr,
-		   "Could not find a suitable GID for user %jd, skipping test\n",
-		   (intmax_t) getuid ());
-	  exit (0);
-	}
+      int status = support_capture_subprogram_self_sgid (SETGID_CHILD);
 
-      return run_executable_sgid (target);
-    }
+      if (WEXITSTATUS (status) == EXIT_UNSUPPORTED)
+	return EXIT_UNSUPPORTED;
+
+      if (!WIFEXITED (status))
+	FAIL_EXIT1 ("Unexpected exit status %d from child process\n", status);
 
-  /* Something went wrong and our argv was corrupted.  */
-  _exit (1);
+      return 0;
+    }
 }
 
 #define TEST_FUNCTION_ARGV do_test