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.