All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Gruenbacher <andreas.gruenbacher@gmail.com>
To: david@fromorbit.com, fstests@vger.kernel.org
Cc: Andreas Gruenbacher <agruenba@redhat.com>
Subject: [PATCH] src/runas: Fixes and cleanups
Date: Sun, 11 Oct 2015 19:24:21 +0200	[thread overview]
Message-ID: <1444584261-32410-1-git-send-email-andreas.gruenbacher@gmail.com> (raw)

The runas helper runs a command as another user and/or with different group
memberships.  Fix the following problems:

 * Use setgid instead of setegid and setuid instead of seteuid.
   Otherwise, the command will run with the original real UID
   and/or GID; those could be made the effective IDs again.

 * When only a GID is specified, remove all supplementary
   GIDs.  Otherwise, the command would remain in the same
   supplementary groups as runas -- which often is the root
   group.

 * Use execvp instead of execv which searches the PATH when
   necessary.  The runas helper is always called either with a
   '/' in the pathname or as "runas ... `which program`", so
   we obviously want PATH lookup, anyway.

 * There is no advantage in fork'ing and waiting for the child
   over directly exec'ing the command; the test cases already
   have to deal with commands which can be killed by signals.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 src/runas.c       | 50 +++++++++++---------------------------------------
 tests/generic/237 |  2 +-
 tests/shared/051  |  4 ++--
 3 files changed, 14 insertions(+), 42 deletions(-)

diff --git a/src/runas.c b/src/runas.c
index 37cae7e..1e7ea25 100644
--- a/src/runas.c
+++ b/src/runas.c
@@ -48,11 +48,9 @@ main(int argc, char **argv)
 	int c;
         uid_t uid = -1;
         gid_t gid = -1;
-        int pid;
         char **cmd;
         gid_t sgids[SUP_MAX];
         int sup_cnt = 0;
-	int status;
 	char *p;
 
 	prog = basename(argv[0]);
@@ -99,56 +97,30 @@ main(int argc, char **argv)
 	} 
 
         if (gid != -1) {
-	    if (setegid(gid) == -1) {
-		fprintf(stderr, "%s: setegid(%d) failed: %s\n",
+	    if (setgid(gid) == -1) {
+		fprintf(stderr, "%s: setgid(%d) failed: %s\n",
 			prog, (int)gid, strerror(errno));
 		exit(1);
-	    }	
+	    }
         }
 
-        if (sup_cnt > 0) {
+	if (gid != -1 || sup_cnt != 0) {
 	    if (setgroups(sup_cnt, sgids) == -1) {
 		fprintf(stderr, "%s: setgroups() failed: %s\n",
 			prog, strerror(errno));
 		exit(1);
-	    }	
+	    }
 	}
 
         if (uid != -1) {
-	    if (seteuid(uid) == -1) {
-		fprintf(stderr, "%s: seteuid(%d) failed: %s\n",
+	    if (setuid(uid) == -1) {
+		fprintf(stderr, "%s: setuid(%d) failed: %s\n",
 			prog, (int)uid, strerror(errno));
 		exit(1);
-	    }	
+	    }
         }
 
-	pid = fork();
-	if (pid == -1) {
-            fprintf(stderr, "%s: fork failed: %s\n",
-	            prog, strerror(errno));
-            exit(1);
-        }
-	if (pid == 0) {
-            execv(cmd[0], cmd);
-            fprintf(stderr, "%s: %s\n", cmd[0], strerror(errno));
-            exit(errno);
-	}
-
-        wait(&status);
-	if (WIFSIGNALED(status)) {
-	    fprintf(stderr, "%s: command terminated with signal %d\n", 
-                 prog, WTERMSIG(status));
-	    exit(1);
-	}
-	else if (WIFEXITED(status)) {
-	    exit(WEXITSTATUS(status));
-        }
-	else {
-	    fprintf(stderr, "%s: command bizarre wait status 0x%x\n", 
-               prog, status);
-	    exit(1);
-	}
-
-	exit(0);
-	/* NOTREACHED */
+	execvp(cmd[0], cmd);
+	fprintf(stderr, "%s: %s\n", cmd[0], strerror(errno));
+	exit(1);
 }
diff --git a/tests/generic/237 b/tests/generic/237
index ff11ed3..f2669cd 100755
--- a/tests/generic/237
+++ b/tests/generic/237
@@ -69,7 +69,7 @@ touch file1
 chown $acl1.$acl1 file1
 
 echo "Expect to FAIL"
-$runas -u $acl2 -g $acl2 -- `which setfacl` -m u::rwx file1 2>&1 | sed 's/^setfacl: \/.*file1: Operation not permitted$/setfacl: file1: Operation not permitted/'
+$runas -u $acl2 -g $acl2 -- setfacl -m u::rwx file1 2>&1 | sed 's/^setfacl: \/.*file1: Operation not permitted$/setfacl: file1: Operation not permitted/'
 
 echo "Test over."
 # success, all done
diff --git a/tests/shared/051 b/tests/shared/051
index 44dfd64..262cef1 100755
--- a/tests/shared/051
+++ b/tests/shared/051
@@ -294,10 +294,10 @@ done
 popd >/dev/null
 chown -R 12345.54321 root
 echo "Change #1..."
-$runas -u 12345 -g 54321 -- `which chacl` -r u::rwx,g::-w-,o::--x root
+$runas -u 12345 -g 54321 -- chacl -r u::rwx,g::-w-,o::--x root
 find root -print | xargs chacl -l
 echo "Change #2..."
-$runas -u 12345 -g 54321 -- `which chacl` -r u::---,g::---,o::--- root
+$runas -u 12345 -g 54321 -- chacl -r u::---,g::---,o::--- root
 find root -print | xargs chacl -l
 
 #-------------------------------------------------------
-- 
2.5.0


                 reply	other threads:[~2015-10-11 17:24 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1444584261-32410-1-git-send-email-andreas.gruenbacher@gmail.com \
    --to=andreas.gruenbacher@gmail.com \
    --cc=agruenba@redhat.com \
    --cc=david@fromorbit.com \
    --cc=fstests@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.