This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Define secure_getenv (v2)


On 07/27/2012 02:58 PM, Carlos O'Donell wrote:
On 7/27/2012 8:53 AM, Florian Weimer wrote:
On 07/27/2012 02:52 PM, Carlos O'Donell wrote:

Please tweak the test not to fail and issue a warning when the supplementary
group is missing.

Should I print the warning to stderr or stdout?

Good question. Given that this is a warning about the test itself, not the thing that the test is testing, I would output this to stderr. That way a view of stderr would show the warning along with other warnings about the build.

What about this? Tested on x86_64-redhat-linux-gnu, also with a user without supplementary GIDs.


--
Florian Weimer / Red Hat Product Security Team

2012-07-30  Florian Weimer  <fweimer@redhat.com>

	* stdlib/tst-secure-getenv.c: Use printf for error reporting.
	Exit with zero in case no suitable GID is found, and write a
	message to standard error.

diff --git a/stdlib/tst-secure-getenv.c b/stdlib/tst-secure-getenv.c
index 76d8de6..10fe07a 100644
--- a/stdlib/tst-secure-getenv.c
+++ b/stdlib/tst-secure-getenv.c
@@ -45,7 +45,7 @@ choose_gid (void)
   int ret = getgroups (count, groups);
   if (ret < 0)
     {
-      perror ("getgroups");
+      printf ("getgroups: %m\n");
       exit (1);
     }
   gid_t current = getgid ();
@@ -72,29 +72,29 @@ run_executable_sgid (gid_t target)
   if (asprintf (&dirname, "%s/secure-getenv.%jd",
 		test_dir, (intmax_t) getpid ()) < 0)
     {
-      perror ("asprintf");
+      printf ("asprintf: %m\n");
       goto err;
     }
   if (mkdir (dirname, 0700) < 0)
     {
-      perror ("mkdir");
+      printf ("mkdir: %m\n");
       goto err;
     }
   if (asprintf (&execname, "%s/bin", dirname) < 0)
     {
-      perror ("asprintf");
+      printf ("asprintf: %m\n");
       goto err;
     }
   infd = open ("/proc/self/exe", O_RDONLY);
   if (infd < 0)
     {
-      perror ("open");
+      printf ("open (/proc/self/exe): %m\n");
       goto err;
     }
   outfd = open (execname, O_WRONLY | O_CREAT | O_EXCL, 0700);
   if (outfd < 0)
     {
-      perror ("open");
+      printf ("open (%s): %m\n", execname);
       goto err;
     }
   char buf[4096];
@@ -103,7 +103,7 @@ run_executable_sgid (gid_t target)
       ssize_t rdcount = read (infd, buf, sizeof (buf));
       if (rdcount < 0)
 	{
-	  perror ("read");
+	  printf ("read: %m\n");
 	  goto err;
 	}
       if (rdcount == 0)
@@ -117,7 +117,7 @@ run_executable_sgid (gid_t target)
 	    errno = ENOSPC;
 	  if (wrcount <= 0)
 	    {
-	      perror ("write");
+	      printf ("write: %m\n");
 	      goto err;
 	    }
 	  p += wrcount;
@@ -125,29 +125,29 @@ run_executable_sgid (gid_t target)
     }
   if (fchown (outfd, getuid (), target) < 0)
     {
-      perror ("fchown");
+      printf ("fchown (%s): %m\n", execname);
       goto err;
     }
   if (fchmod (outfd, 02750) < 0)
     {
-      perror ("fchmod");
+      printf ("fchmod (%s): %m\n", execname);
       goto err;
     }
   if (close (outfd) < 0)
     {
-      perror ("close");
+      printf ("close (outfd): %m\n");
       goto err;
     }
   if (close (infd) < 0)
     {
-      perror ("close");
+      printf ("close (infd): %m\n");
       goto err;
     }
 
   int kid = fork ();
   if (kid < 0)
     {
-      perror ("fork");
+      printf ("fork: %m\n");
       goto err;
     }
   if (kid == 0)
@@ -155,19 +155,19 @@ run_executable_sgid (gid_t target)
       /* Child process. */
       char *args[] = { execname, MAGIC_ARGUMENT, NULL };
       execve (execname, args, environ);
-      perror ("execve");
+      printf ("execve (%s): %m\n", execname);
       _exit (1);
     }
   int status;
   if (waitpid (kid, &status, 0) < 0)
     {
-      perror ("waitpid");
+      printf ("waitpid: %m\n");
       goto err;
     }
   if (!WIFEXITED (status) || WEXITSTATUS (status) != MAGIC_STATUS)
     {
-      fprintf (stderr, "Unexpected exit status %d from child process\n",
-	       status);
+      printf ("Unexpected exit status %d from child process\n",
+	      status);
       goto err;
     }
   ret = 0;
@@ -195,27 +195,28 @@ do_test (void)
 {
   if (getenv ("PATH") == NULL)
     {
-      fprintf (stderr, "PATH not set\n");
+      printf ("PATH not set\n");
       exit (1);
     }
   if (secure_getenv ("PATH") == NULL)
     {
-      fprintf (stderr, "PATH not set according to secure_getenv\n");
+      printf ("PATH not set according to secure_getenv\n");
       exit (1);
     }
   if (strcmp (getenv ("PATH"), secure_getenv ("PATH")) != 0)
     {
-      fprintf (stderr, "PATH mismatch (%s, %s)\n",
-	       getenv ("PATH"), secure_getenv ("PATH"));
+      printf ("PATH mismatch (%s, %s)\n",
+	      getenv ("PATH"), secure_getenv ("PATH"));
       exit (1);
     }
 
   gid_t target = choose_gid ();
   if (target == 0)
     {
-      fprintf (stderr, "Could not find a suitable GID user %jd\n",
+      fprintf (stderr, 
+	       "Could not find a suitable GID for user %jd, skipping test\n",
 	       (intmax_t) getuid ());
-      exit (1);
+      exit (0);
     }
   return run_executable_sgid (target);
 }
@@ -227,18 +228,18 @@ alternative_main (int argc, char **argv)
     {
       if (getgid () == getegid ())
 	{
-	  fprintf (stderr, "SGID failed: GID and EGID match (%jd)\n",
-		   (intmax_t) getgid ());
+	  printf ("SGID failed: GID and EGID match (%jd)\n",
+		  (intmax_t) getgid ());
 	  exit (2);
 	}
       if (getenv ("PATH") == NULL)
 	{
-	  fprintf (stderr, "PATH variable not present\n");
+	  printf ("PATH variable not present\n");
 	  exit (3);
 	}
       if (secure_getenv ("PATH") != NULL)
 	{
-	  fprintf (stderr, "PATH variable not filtered out\n");
+	  printf ("PATH variable not filtered out\n");
 	  exit (4);
 	}
       exit (MAGIC_STATUS);

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]