Commit 7e731fba authored by Leigh Stoller's avatar Leigh Stoller

Small change to suexec code. This change has the potential for creating

unanticipated breakage. If that happens, just need to back out the
changes under the "suexec-stuff" tag. However, the better solution will
probably be to fix the PHP scripts that break by adding the proper
groups in the call to suexec (in the web page, see below) or by fixing
the backend Perl script that breaks.

This fix is primarily to address the problem of some users being in more
groups (cause of subgroups) then the max number of groups allowed
(NGROUPS).  The groups that really mattered (say, for creating an
experiment in a subgroup) could be left out cause they were at the end
of the list.

* suexec.c: Change how groups are handled. Instead of taking a single
  gid argument (the gid to setgid as), now takes a comma separated list
  of groups. Further, instead of doing a setgroups to the user's entire
  group list as specified in the groups file (getgroups), setgroups to
  just the groups listed on the command line, plus the user's primary
  group from the password file (this is to prevent potential breakage
  with accessing files from the users homedir, although might not really
  be necessary).

  This change is somewhat rational in the sense that in our case, suexec
  is not being used to run arbitrary user code (CGIs), but only to run
  specific scripts that we say should be run. The environment for
  running those scripts can be more tightly controlled then it would
  otherwise need to be if running some random CGI the user has in his
  public html directory.

* www: Change the gid argument to SUEXEC() in a number of scripts so
  that the project and subgroup are explicitly given to suexec, as
  described above. For example, in beginexp:

	SUEXEC(gid, "$pid,$unix_gid", ....);

  Aside: note that project names (pid) are always one to one with their
  unix group name, but subgroup names are not, and *always* have to be
  looked up in the DB, hence the "unix_gid" argument.

  Script breakage should require nothing more then adding the proper
  group to the list as above.
parent 5284665a
...@@ -266,6 +266,7 @@ int main(int argc, char *argv[]) ...@@ -266,6 +266,7 @@ int main(int argc, char *argv[])
char dwd[AP_MAXPATH]; /* docroot working directory */ char dwd[AP_MAXPATH]; /* docroot working directory */
#ifdef TESTBED #ifdef TESTBED
char cmdpath[AP_MAXPATH]; /* full path to command */ char cmdpath[AP_MAXPATH]; /* full path to command */
int grouplist[NGROUPS], ngroups = 0;
#endif #endif
struct passwd *pw; /* password entry holder */ struct passwd *pw; /* password entry holder */
struct group *gr; /* group entry holder */ struct group *gr; /* group entry holder */
...@@ -367,6 +368,72 @@ int main(int argc, char *argv[]) ...@@ -367,6 +368,72 @@ int main(int argc, char *argv[])
exit(105); exit(105);
} }
#ifdef TESTBED
{
char *cp, *bp, *rname, *temp_gname = strdup(target_gname);
gid_t rgid;
int i;
actual_gname = NULL;
bp = temp_gname;
while ((bp = strsep(&temp_gname, ",")) != NULL) {
if (!*bp)
continue;
/*
* Error out if the target group name is invalid.
*/
if (strspn(bp, "1234567890") != strlen(bp)) {
if ((gr = getgrnam(bp)) == NULL) {
log_err("crit: invalid target group name: (%s)\n", bp);
exit(106);
}
rgid = gr->gr_gid;
rname = gr->gr_name;
}
else {
rgid = atoi(bp);
rname = bp;
}
/* Watch for duplicates */
for (i = 0; i < ngroups; i++) {
if (grouplist[i] == rgid)
goto skip;
}
grouplist[ngroups++] = rgid;
/*
* Error out if attempt is made to execute as root group
* or as a GID less than GID_MIN. Tsk tsk.
*/
if ((rgid == 0) || (rgid < GID_MIN)) {
log_err("crit: cannot run as forbidden gid (%d/%s)\n",
gid, cmd);
exit(108);
}
/* see below; need room for primary group in first two slots */
if (ngroups >= (NGROUPS - 2)) {
log_err("crit: Too many groups: (%s)\n", bp);
exit(106);
}
if (actual_gname) {
cp = (char *) malloc(strlen(actual_gname) + strlen(rname) + 2);
strcpy(cp, actual_gname);
strcat(cp, ",");
strcat(cp, rname);
free(actual_gname);
actual_gname = cp;
}
else {
actual_gname = strdup(rname);
}
skip:
}
}
#else
/* /*
* Error out if the target group name is invalid. * Error out if the target group name is invalid.
*/ */
...@@ -382,6 +449,7 @@ int main(int argc, char *argv[]) ...@@ -382,6 +449,7 @@ int main(int argc, char *argv[])
gid = atoi(target_gname); gid = atoi(target_gname);
actual_gname = strdup(target_gname); actual_gname = strdup(target_gname);
} }
#endif
#ifdef _OSD_POSIX #ifdef _OSD_POSIX
/* /*
...@@ -438,42 +506,39 @@ int main(int argc, char *argv[]) ...@@ -438,42 +506,39 @@ int main(int argc, char *argv[])
exit(107); exit(107);
} }
/*
* Error out if attempt is made to execute as root group
* or as a GID less than GID_MIN. Tsk tsk.
*/
if ((gid == 0) || (gid < GID_MIN)) {
log_err("crit: cannot run as forbidden gid (%d/%s)\n", gid, cmd);
exit(108);
}
#ifdef TESTBED #ifdef TESTBED
{ {
int groups[NGROUPS], ngroups, i; int groups[NGROUPS], i, idx = 0;
ngroups = NGROUPS; groups[idx++] = primary_gid;
getgrouplist(actual_uname, primary_gid, groups, &ngroups); /* duplicate has something to do with effective gid */
if (ngroups == NGROUPS) { groups[idx++] = primary_gid;
log_err("crit: Too many groups (%ld: %s)\n", gid, cmd);
} /* Move over the grouplist from above. */
for (i = 0; i < ngroups; i++) { for (i = 0; i < ngroups; i++) {
if (groups[i] == gid) if (grouplist[i] != primary_gid)
break; groups[idx++] = grouplist[i];
}
if (i == ngroups) {
/* Nasty! */
if (i == NGROUPS)
ngroups--;
groups[ngroups++] = gid;
} }
if (((setgid(primary_gid)) != 0) || if (setgid(primary_gid) != 0) {
(setgroups(ngroups, groups) != 0)) { log_err("emerg: failed to setgid (%ld: %s)\n", primary_gid, cmd);
log_err("emerg: failed to setgid (%ld: %s)\n", gid, cmd); exit(109);
exit(109); }
if (setgroups(idx, groups) != 0) {
log_err("emerg: failed to setgroups (%s: %s)\n", actual_gname,cmd);
exit(109);
} }
} }
#else #else
/*
* Error out if attempt is made to execute as root group
* or as a GID less than GID_MIN. Tsk tsk.
*/
if ((gid == 0) || (gid < GID_MIN)) {
log_err("crit: cannot run as forbidden gid (%d/%s)\n", gid, cmd);
exit(108);
}
/* /*
* Change UID/GID here so that the following tests work over NFS. * Change UID/GID here so that the following tests work over NFS.
* *
......
...@@ -407,7 +407,7 @@ TBGroupUnixInfo($exp_pid, $exp_gid, $unix_gid, $unix_name); ...@@ -407,7 +407,7 @@ TBGroupUnixInfo($exp_pid, $exp_gid, $unix_gid, $unix_name);
# #
set_time_limit(0); set_time_limit(0);
$retval = SUEXEC($uid, $unix_gid, $retval = SUEXEC($uid, "$exp_pid,$unix_gid",
"webbatchexp $batcharg -E $exp_desc $exp_swappable ". "webbatchexp $batcharg -E $exp_desc $exp_swappable ".
"$linktestarg -p $exp_pid -g $exp_gid -e $exp_id ". "$linktestarg -p $exp_pid -g $exp_gid -e $exp_id ".
($nonsfile ? "" : "$thensfile"), ($nonsfile ? "" : "$thensfile"),
......
<?php <?php
# #
# EMULAB-COPYRIGHT # EMULAB-COPYRIGHT
# Copyright (c) 2000-2003 University of Utah and the Flux Group. # Copyright (c) 2000-2004 University of Utah and the Flux Group.
# All rights reserved. # All rights reserved.
# #
include("defs.php3"); include("defs.php3");
...@@ -157,7 +157,8 @@ if ($dochange == "1") { ...@@ -157,7 +157,8 @@ if ($dochange == "1") {
# if something breaks we get the mail from the web interface. # if something breaks we get the mail from the web interface.
# This might change depending on how often we get email! # This might change depending on how often we get email!
# #
$retval = SUEXEC($uid, $unix_gid, $cmd, SUEXEC_ACTION_IGNORE); $retval = SUEXEC($uid, "$pid,$unix_gid", $cmd,
SUEXEC_ACTION_IGNORE);
if ($retval) { if ($retval) {
# Ug, I know this hardwired return value is bad! # Ug, I know this hardwired return value is bad!
if ($retval == 2) { if ($retval == 2) {
......
<?php <?php
# #
# EMULAB-COPYRIGHT # EMULAB-COPYRIGHT
# Copyright (c) 2000-2003 University of Utah and the Flux Group. # Copyright (c) 2000-2004 University of Utah and the Flux Group.
# All rights reserved. # All rights reserved.
# #
include("defs.php3"); include("defs.php3");
...@@ -107,7 +107,7 @@ flush(); ...@@ -107,7 +107,7 @@ flush();
# #
# Run the backend script. # Run the backend script.
# #
$retval = SUEXEC($uid, $unix_gid, "webendexp $exp_pid $exp_eid", $retval = SUEXEC($uid, "$exp_pid,$unix_gid", "webendexp $exp_pid $exp_eid",
SUEXEC_ACTION_IGNORE); SUEXEC_ACTION_IGNORE);
# #
......
...@@ -166,8 +166,8 @@ if (strcmp($mode, "record") == 0) { ...@@ -166,8 +166,8 @@ if (strcmp($mode, "record") == 0) {
ignore_user_abort(1); ignore_user_abort(1);
# Start the script and pipe its output to the user. # Start the script and pipe its output to the user.
$fp = popen("$TBSUEXEC_PATH $uid $unix_gid webfeedback -d $duration $pid $gid $eid", $fp = popen("$TBSUEXEC_PATH $uid $pid,$unix_gid webfeedback ".
"r"); "-d $duration $pid $gid $eid", "r");
if (! $fp) { if (! $fp) {
USERERROR("Feedback failed!", 1); USERERROR("Feedback failed!", 1);
} }
...@@ -204,7 +204,7 @@ else if (strcmp($mode, "clear") == 0) { ...@@ -204,7 +204,7 @@ else if (strcmp($mode, "clear") == 0) {
if (isset($clear_bootstrap) && !strcmp($clear_bootstrap, "1")) if (isset($clear_bootstrap) && !strcmp($clear_bootstrap, "1"))
$options .= " -b"; $options .= " -b";
if ($options != "") { if ($options != "") {
$retval = SUEXEC($uid, $unix_gid, $retval = SUEXEC($uid, "$pid,$unix_gid",
"webfeedback $options $pid $gid $eid", "webfeedback $options $pid $gid $eid",
SUEXEC_ACTION_USERERROR); SUEXEC_ACTION_USERERROR);
} }
......
...@@ -68,7 +68,7 @@ if ($linktest_pid && !isset($frame)) { ...@@ -68,7 +68,7 @@ if ($linktest_pid && !isset($frame)) {
# Form submitted. Kill running linktest and zap back to the initial # Form submitted. Kill running linktest and zap back to the initial
# page to redisplay the menu. # page to redisplay the menu.
# #
SUEXEC($uid, $unix_gid, "weblinktest -k $pid $eid", SUEXEC($uid, "$pid,$unix_gid", "weblinktest -k $pid $eid",
SUEXEC_ACTION_DIE); SUEXEC_ACTION_DIE);
header("Location: linktest.php3?pid=$pid&eid=$eid"); header("Location: linktest.php3?pid=$pid&eid=$eid");
return; return;
...@@ -116,7 +116,7 @@ function SPEWCLEANUP() ...@@ -116,7 +116,7 @@ function SPEWCLEANUP()
global $fp; global $fp;
if (connection_aborted() && $fp) { if (connection_aborted() && $fp) {
SUEXEC($uid, $unix_gid, "weblinktest -k $pid $eid", SUEXEC($uid, "$pid,$unix_gid", "weblinktest -k $pid $eid",
SUEXEC_ACTION_IGNORE); SUEXEC_ACTION_IGNORE);
pclose($fp); pclose($fp);
exit(); exit();
...@@ -129,7 +129,7 @@ function SPEWCLEANUP() ...@@ -129,7 +129,7 @@ function SPEWCLEANUP()
if (isset($frame)) { if (isset($frame)) {
if ($frame == "stopbutton") { if ($frame == "stopbutton") {
if (isset($submit) && $submit == "Stop") { if (isset($submit) && $submit == "Stop") {
SUEXEC($uid, $unix_gid, "weblinktest -k $pid $eid", SUEXEC($uid, "$pid,$unix_gid", "weblinktest -k $pid $eid",
SUEXEC_ACTION_IGNORE); SUEXEC_ACTION_IGNORE);
echo "<html> echo "<html>
...@@ -164,7 +164,7 @@ if (isset($frame)) { ...@@ -164,7 +164,7 @@ if (isset($frame)) {
set_time_limit(0); set_time_limit(0);
$fp = popen("$TBSUEXEC_PATH ". $fp = popen("$TBSUEXEC_PATH ".
"$uid $unix_gid weblinktest -l $level $pid $eid", "$uid $pid,$unix_gid weblinktest -l $level $pid $eid",
"r"); "r");
if (! $fp) { if (! $fp) {
USERERROR("Linktest failed!", 1); USERERROR("Linktest failed!", 1);
......
...@@ -107,7 +107,8 @@ echo "<br> ...@@ -107,7 +107,8 @@ echo "<br>
<br><br>\n"; <br><br>\n";
flush(); flush();
SUEXEC($uid, $unix_gid, "webcreateimage -p $image_pid $image_name $node", SUEXEC($uid, "$image_pid,$unix_gid",
"webcreateimage -p $image_pid $image_name $node",
SUEXEC_ACTION_DUPDIE); SUEXEC_ACTION_DUPDIE);
echo "This will take 10 minutes or more; you will receive email echo "This will take 10 minutes or more; you will receive email
......
...@@ -188,7 +188,7 @@ TBGroupUnixInfo($pid, $gid, $unix_gid, $unix_name); ...@@ -188,7 +188,7 @@ TBGroupUnixInfo($pid, $gid, $unix_gid, $unix_name);
# #
# Do an initial parse test. # Do an initial parse test.
# #
$retval = SUEXEC($uid, $unix_gid, "webnscheck $nsfile", $retval = SUEXEC($uid, "$pid,$unix_gid", "webnscheck $nsfile",
SUEXEC_ACTION_IGNORE); SUEXEC_ACTION_IGNORE);
if ($retval != 0) { if ($retval != 0) {
...@@ -221,7 +221,7 @@ flush(); ...@@ -221,7 +221,7 @@ flush();
set_time_limit(0); set_time_limit(0);
# Run the script. # Run the script.
$retval = SUEXEC($uid, $unix_gid, $retval = SUEXEC($uid, "$pid,$unix_gid",
"webswapexp $rebootswitch " . ($reboot ? "-r " : "") . "webswapexp $rebootswitch " . ($reboot ? "-r " : "") .
($eventrestart ? "-e " : "") . ($eventrestart ? "-e " : "") .
"-s modify $pid $eid $nsfile", "-s modify $pid $eid $nsfile",
......
...@@ -896,7 +896,7 @@ if (isset($node)) { ...@@ -896,7 +896,7 @@ if (isset($node)) {
<br><br>\n"; <br><br>\n";
flush(); flush();
SUEXEC($uid, $unix_gid, "webcreateimage -p $pid $imagename $node", SUEXEC($uid, "$pid,$unix_gid", "webcreateimage -p $pid $imagename $node",
SUEXEC_ACTION_DUPDIE); SUEXEC_ACTION_DUPDIE);
echo "This will take 10 minutes or more; you will receive email echo "This will take 10 minutes or more; you will receive email
......
...@@ -1041,7 +1041,7 @@ if (isset($node)) { ...@@ -1041,7 +1041,7 @@ if (isset($node)) {
<br><br>\n"; <br><br>\n";
flush(); flush();
SUEXEC($uid, $unix_gid, "webcreateimage -p $pid $imagename $node", SUEXEC($uid, "$pid,$unix_gid", "webcreateimage -p $pid $imagename $node",
SUEXEC_ACTION_DUPDIE); SUEXEC_ACTION_DUPDIE);
echo "This will take 10 minutes or more; you will receive email echo "This will take 10 minutes or more; you will receive email
......
...@@ -126,7 +126,7 @@ fwrite($fp, $nsdata); ...@@ -126,7 +126,7 @@ fwrite($fp, $nsdata);
fclose($fp); fclose($fp);
chmod($nsfile, 0666); chmod($nsfile, 0666);
$retval = SUEXEC($uid, $unix_gid, $retval = SUEXEC($uid, "$pid,$unix_gid",
"webswapexp " . ($reboot ? "-r " : "") . "webswapexp " . ($reboot ? "-r " : "") .
($eventrestart ? "-e " : "") . ($eventrestart ? "-e " : "") .
"-s modify $pid $eid $nsfile", "-s modify $pid $eid $nsfile",
......
<?php <?php
# #
# EMULAB-COPYRIGHT # EMULAB-COPYRIGHT
# Copyright (c) 2003 University of Utah and the Flux Group. # Copyright (c) 2003, 2004 University of Utah and the Flux Group.
# All rights reserved. # All rights reserved.
# #
include("defs.php3"); include("defs.php3");
...@@ -50,6 +50,7 @@ if (! TBNodeIDtoExpt($nodeid, $pid, $eid, $gid)) { ...@@ -50,6 +50,7 @@ if (! TBNodeIDtoExpt($nodeid, $pid, $eid, $gid)) {
SPITERROR(400, "$nodeid is not reserved to an experiment!"); SPITERROR(400, "$nodeid is not reserved to an experiment!");
} }
TBExpLeader($pid, $eid, $creator); TBExpLeader($pid, $eid, $creator);
TBGroupUnixInfo($pid, $gid, $unix_gid, $unix_name);
# #
# We need the secret key. # We need the secret key.
...@@ -101,7 +102,8 @@ $arg = (isset($stamp) ? "-t " . escapeshellarg($stamp) : ""); ...@@ -101,7 +102,8 @@ $arg = (isset($stamp) ? "-t " . escapeshellarg($stamp) : "");
# Then do it for real, spitting out the data. Sure, the user could # Then do it for real, spitting out the data. Sure, the user could
# delete the file in the meantime, but thats his problem. # delete the file in the meantime, but thats his problem.
# #
$retval = SUEXEC($creator, $gid, "spewrpmtar -v $arg $nodeid $file", $retval = SUEXEC($creator, "$pid,$unix_gid",
"spewrpmtar -v $arg $nodeid $file",
SUEXEC_ACTION_IGNORE); SUEXEC_ACTION_IGNORE);
if ($retval < 0) { if ($retval < 0) {
...@@ -122,7 +124,7 @@ if ($retval) { ...@@ -122,7 +124,7 @@ if ($retval) {
# #
# Okay, now do it for real. # Okay, now do it for real.
# #
if ($fp = popen("$TBSUEXEC_PATH $creator $gid ". if ($fp = popen("$TBSUEXEC_PATH $creator $pid,$unix_gid ".
"spewrpmtar $nodeid $file", "r")) { "spewrpmtar $nodeid $file", "r")) {
header("Content-Type: application/octet-stream"); header("Content-Type: application/octet-stream");
header("Expires: Mon, 26 Jul 1997 05:00:00 GMT"); header("Expires: Mon, 26 Jul 1997 05:00:00 GMT");
......
...@@ -220,7 +220,7 @@ set_time_limit(0); ...@@ -220,7 +220,7 @@ set_time_limit(0);
# plain force swap, it passes -f for us. # plain force swap, it passes -f for us.
$args = ($idleswap ? "-i" : ($autoswap ? "-a" : "")); $args = ($idleswap ? "-i" : ($autoswap ? "-a" : ""));
$retval = SUEXEC($uid, $unix_gid, $retval = SUEXEC($uid, "$exp_pid,$unix_gid",
($force ? ($force ?
"webidleswap $args $exp_pid $exp_eid" : "webidleswap $args $exp_pid $exp_eid" :
"webswapexp -s $inout $exp_pid $exp_eid"), "webswapexp -s $inout $exp_pid $exp_eid"),
......
...@@ -138,7 +138,7 @@ echo "<h2>Starting node update. Please wait a moment ... ...@@ -138,7 +138,7 @@ echo "<h2>Starting node update. Please wait a moment ...
flush(); flush();
$retval = SUEXEC($uid, $unix_gid, $retval = SUEXEC($uid, "$pid,$unix_gid",
"webnodeupdate -b $pid $eid" . "webnodeupdate -b $pid $eid" .
(isset($nodeid) ? " $nodeid" : ""), (isset($nodeid) ? " $nodeid" : ""),
SUEXEC_ACTION_IGNORE); SUEXEC_ACTION_IGNORE);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment