You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Rodent of Unusual Size <Ke...@Golux.Com> on 2000/01/10 16:32:59 UTC
[PATCH] addition of suexec umask setting
PR#4178 suggests that suexec should set a umask before
invoking a script. That seems like quite a good idea,
so here's a patch to do it. If --suexec-umask=nnn is
set on the ./configure command line, the wrapper will
be compiled to umask() to that value before script
invocation. If the option isn't given to ./configure,
the behaviour is unchanged from the way it is now -- the
script is invoked with the same umask as the server process.
The only thing I'm a little unsure about is the log
message I added if the umask allows g or o write access.
It's non-fatal, so I figured more info is better, and
someone installing a canned suexec ought to know something
like that.
Jim's RM, so I'm letting him decide whether this should
be committed.
--
#ken P-)}
Ken Coar <http://Golux.Com/coar/>
Apache Software Foundation <http://www.apache.org/>
"Apache Server for Dummies" <http://Apache-Server.Com/>
Come to the first official Apache Software Foundation
Conference! <http://ApacheCon.Com/>
Index: Makefile.tmpl
===================================================================
RCS file: /home/cvs/apache-1.3/Makefile.tmpl,v
retrieving revision 1.94
diff -u -r1.94 Makefile.tmpl
--- Makefile.tmpl 1999/12/09 17:19:35 1.94
+++ Makefile.tmpl 2000/01/10 15:12:08
@@ -135,6 +135,7 @@
suexec_uidmin = @suexec_uidmin@
suexec_gidmin = @suexec_gidmin@
suexec_safepath = @suexec_safepath@
+suexec_umask = @suexec_umask@
# some substituted configuration parameters
conf_user = @conf_user@
@@ -200,7 +201,8 @@
-DUSERDIR_SUFFIX=\"$(suexec_userdir)\" \
-DLOG_EXEC=\"$(suexec_logexec)\" \
-DDOC_ROOT=\"$(suexec_docroot)\" \
- -DSAFE_PATH=\"$(suexec_safepath)\" ' \
+ -DSAFE_PATH=\"$(suexec_safepath)\" \
+ $(suexec_umask)' \
suexec; \
fi
@echo "<=== $(SRC)/support"
Index: configure
===================================================================
RCS file: /home/cvs/apache-1.3/configure,v
retrieving revision 1.114
diff -u -r1.114 configure
--- configure 1999/12/10 11:03:00 1.114
+++ configure 2000/01/10 15:12:08
@@ -240,6 +240,8 @@
suexec_uidmin=100
suexec_gidmin=100
suexec_safepath="/usr/local/bin:/usr/bin:/bin"
+# if the umask is undefined, we don't change it
+#suexec_umask=0755
# the installation flags
iflags_program="-m 755 -s"
@@ -449,6 +451,7 @@
echo " --suexec-uidmin=UID set the suEXEC minimal allowed UID [$suexec_uidmin]"
echo " --suexec-gidmin=GID set the suEXEC minimal allowed GID [$suexec_gidmin]"
echo " --suexec-safepath=PATH set the suEXEC safe PATH [$suexec_safepath]"
+ echo " --suexec-umask=UMASK set the umask for the suEXEC'd script [server's umask]"
echo ""
echo "Deprecated options:"
echo " --layout backward compat only: use --show-layout"
@@ -976,6 +979,11 @@
suexec_safepath="$apc_optarg"
suexec_ok=1
;;
+ --suexec-umask=*)
+ suexec_umask_val="$apc_optarg"
+ suexec_umask="-DSUEXEC_UMASK=$apc_optarg"
+ suexec_ok=1
+ ;;
--server-uid=*)
conf_user="$apc_optarg"
# protect the '#' against interpretation as comment
@@ -1085,7 +1093,7 @@
for var in prefix exec_prefix bindir sbindir libexecdir mandir \
sysconfdir datadir iconsdir htdocsdir cgidir includedir \
localstatedir runtimedir logfiledir proxycachedir \
- suexec_docroot suexec_logexec; do
+ suexec_docroot suexec_logexec ; do
eval "val=\"\$$var\"";
val=`echo $val | sed -e 's:\(.\)/*$:\1:'`
eval "$var=\"$val\""
@@ -1240,6 +1248,11 @@
echo " caller ID: $suexec_caller"
echo " minimum user ID: $suexec_uidmin"
echo " minimum group ID: $suexec_gidmin"
+ if [ "x$suexec_umask" != "x" ]; then
+ echo " umask: $suexec_umask_val"
+ else
+ echo " umask: running server's"
+ fi
echo ""
fi
exit 0
@@ -1290,6 +1303,7 @@
-e "s%@suexec_uidmin@%$suexec_uidmin%g" \
-e "s%@suexec_gidmin@%$suexec_gidmin%g" \
-e "s%@suexec_safepath@%$suexec_safepath%g" \
+-e "s%@suexec_umask@%$suexec_umask%g" \
-e "s%@conf_user@%$conf_user%g" \
-e "s%@conf_group@%$conf_group%g" \
-e "s%@conf_port@%$conf_port%g" \
Index: src/CHANGES
===================================================================
RCS file: /home/cvs/apache-1.3/src/CHANGES,v
retrieving revision 1.1489
diff -u -r1.1489 CHANGES
--- CHANGES 2000/01/08 14:50:05 1.1489
+++ CHANGES 2000/01/10 15:12:25
@@ -1,5 +1,9 @@
Changes with Apache 1.3.10
+ *) Add --suexec-umask option to configure, and severity levels
+ to suexec log messages. Also clarify a couple of those messages,
+ which were perhaps a bit too cryptic. [Ken Coar] PR#4178
+
*) Added the mod_rewrite `URL Rewriting Guide' to the online
documentation (htdocs/manual/misc/rewriteguide.html). This paper
provides a large collection of practical solutions to URL based
Index: src/support/suexec.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/support/suexec.c,v
retrieving revision 1.51
diff -u -r1.51 suexec.c
--- suexec.c 1999/06/22 00:51:41 1.51
+++ suexec.c 2000/01/10 15:12:39
@@ -69,6 +69,18 @@
***********************************************************************
*
*
+ * Error messages in the suexec logfile are prefixed with severity values
+ * similar to those used by the main server:
+ *
+ * Sev Meaning
+ * emerg: Failure of some basic system function
+ * alert: Bug in the way Apache is communicating with suexec
+ * crit: Basic information is missing, invalid, or incorrect
+ * error: Script permission/configuration error
+ * warn:
+ * notice: Some issue of which the sysadmin/webmaster ought to be aware
+ * info: Normal activity message
+ * debug: Self-explanatory
*/
#include "ap_config.h"
@@ -206,7 +218,7 @@
if ((cleanenv = (char **) calloc(AP_ENVBUF, sizeof(char *))) == NULL) {
- log_err("failed to malloc memory for environment\n");
+ log_err("emerg: failed to malloc memory for environment\n");
exit(120);
}
@@ -261,7 +273,7 @@
*/
prog = argv[0];
if (argc < 4) {
- log_err("too few arguments\n");
+ log_err("alert: too few arguments\n");
exit(101);
}
target_uname = argv[1];
@@ -274,7 +286,7 @@
*/
uid = getuid();
if ((pw = getpwuid(uid)) == NULL) {
- log_err("invalid uid: (%ld)\n", uid);
+ log_err("crit: invalid uid: (%ld)\n", uid);
exit(102);
}
@@ -286,15 +298,17 @@
#ifdef _OSD_POSIX
/* User name comparisons are case insensitive on BS2000/OSD */
if (strcasecmp(HTTPD_USER, pw->pw_name)) {
- log_err("user mismatch (%s instead of %s)\n", pw->pw_name, HTTPD_USER);
+ log_err("crit: calling user mismatch (%s instead of %s)\n",
+ pw->pw_name, HTTPD_USER);
exit(103);
}
-#else /*_OSD_POSIX*/
+#else /* _OSD_POSIX */
if (strcmp(HTTPD_USER, pw->pw_name)) {
- log_err("user mismatch (%s instead of %s)\n", pw->pw_name, HTTPD_USER);
+ log_err("crit: calling user mismatch (%s instead of %s)\n",
+ pw->pw_name, HTTPD_USER);
exit(103);
}
-#endif /*_OSD_POSIX*/
+#endif /* _OSD_POSIX */
/*
* Check for a leading '/' (absolute path) in the command to be executed,
@@ -304,7 +318,7 @@
*/
if ((cmd[0] == '/') || (!strncmp(cmd, "../", 3))
|| (strstr(cmd, "/../") != NULL)) {
- log_err("invalid command (%s)\n", cmd);
+ log_err("error: invalid command (%s)\n", cmd);
exit(104);
}
@@ -322,7 +336,7 @@
* Error out if the target username is invalid.
*/
if ((pw = getpwnam(target_uname)) == NULL) {
- log_err("invalid target user name: (%s)\n", target_uname);
+ log_err("crit: invalid target user name: (%s)\n", target_uname);
exit(105);
}
@@ -331,7 +345,7 @@
*/
if (strspn(target_gname, "1234567890") != strlen(target_gname)) {
if ((gr = getgrnam(target_gname)) == NULL) {
- log_err("invalid target group name: (%s)\n", target_gname);
+ log_err("crit: invalid target group name: (%s)\n", target_gname);
exit(106);
}
gid = gr->gr_gid;
@@ -353,7 +367,8 @@
switch (pid = ufork(target_uname))
{
case -1: /* Error */
- log_err("failed to setup bs2000 environment for user %s: %s\n",
+ log_err("emerg: failed to setup bs2000 environment for user "
+ "%s: %s\n",
target_uname, strerror(errno));
exit(150);
case 0: /* Child */
@@ -362,12 +377,13 @@
while (pid != waitpid(pid, &status, 0))
;
/* @@@ FIXME: should we deal with STOP signals as well? */
- if (WIFSIGNALED(status))
+ if (WIFSIGNALED(status)) {
kill (getpid(), WTERMSIG(status));
+ }
exit(WEXITSTATUS(status));
}
}
-#endif /*_OSD_POSIX*/
+#endif /* _OSD_POSIX */
/*
* Save these for later since initgroups will hose the struct
@@ -380,7 +396,7 @@
* Log the transaction here to be sure we have an open log
* before we setuid().
*/
- log_err("uid: (%s/%s) gid: (%s/%s) cmd: %s\n",
+ log_err("info: (target/actual) uid: (%s/%s) gid: (%s/%s) cmd: %s\n",
target_uname, actual_uname,
target_gname, actual_gname,
cmd);
@@ -390,7 +406,7 @@
* a UID less than UID_MIN. Tsk tsk.
*/
if ((uid == 0) || (uid < UID_MIN)) {
- log_err("cannot run as forbidden uid (%d/%s)\n", uid, cmd);
+ log_err("crit: cannot run as forbidden uid (%d/%s)\n", uid, cmd);
exit(107);
}
@@ -399,7 +415,7 @@
* or as a GID less than GID_MIN. Tsk tsk.
*/
if ((gid == 0) || (gid < GID_MIN)) {
- log_err("cannot run as forbidden gid (%d/%s)\n", gid, cmd);
+ log_err("crit: cannot run as forbidden gid (%d/%s)\n", gid, cmd);
exit(108);
}
@@ -410,7 +426,7 @@
* and setgid() to the target group. If unsuccessful, error out.
*/
if (((setgid(gid)) != 0) || (initgroups(actual_uname, gid) != 0)) {
- log_err("failed to setgid (%ld: %s)\n", gid, cmd);
+ log_err("emerg: failed to setgid (%ld: %s)\n", gid, cmd);
exit(109);
}
@@ -418,7 +434,7 @@
* setuid() to the target user. Error out on fail.
*/
if ((setuid(uid)) != 0) {
- log_err("failed to setuid (%ld: %s)\n", uid, cmd);
+ log_err("emerg: failed to setuid (%ld: %s)\n", uid, cmd);
exit(110);
}
@@ -431,7 +447,7 @@
* directories. Yuck.
*/
if (getcwd(cwd, AP_MAXPATH) == NULL) {
- log_err("cannot get current working directory\n");
+ log_err("emerg: cannot get current working directory\n");
exit(111);
}
@@ -440,7 +456,8 @@
((chdir(USERDIR_SUFFIX)) != 0) ||
((getcwd(dwd, AP_MAXPATH)) == NULL) ||
((chdir(cwd)) != 0)) {
- log_err("cannot get docroot information (%s)\n", target_homedir);
+ log_err("emerg: cannot get docroot information (%s)\n",
+ target_homedir);
exit(112);
}
}
@@ -448,13 +465,13 @@
if (((chdir(DOC_ROOT)) != 0) ||
((getcwd(dwd, AP_MAXPATH)) == NULL) ||
((chdir(cwd)) != 0)) {
- log_err("cannot get docroot information (%s)\n", DOC_ROOT);
+ log_err("emerg: cannot get docroot information (%s)\n", DOC_ROOT);
exit(113);
}
}
if ((strncmp(cwd, dwd, strlen(dwd))) != 0) {
- log_err("command not in docroot (%s/%s)\n", cwd, cmd);
+ log_err("error: command not in docroot (%s/%s)\n", cwd, cmd);
exit(114);
}
@@ -462,7 +479,7 @@
* Stat the cwd and verify it is a directory, or error out.
*/
if (((lstat(cwd, &dir_info)) != 0) || !(S_ISDIR(dir_info.st_mode))) {
- log_err("cannot stat directory: (%s)\n", cwd);
+ log_err("error: cannot stat directory: (%s)\n", cwd);
exit(115);
}
@@ -470,7 +487,7 @@
* Error out if cwd is writable by others.
*/
if ((dir_info.st_mode & S_IWOTH) || (dir_info.st_mode & S_IWGRP)) {
- log_err("directory is writable by others: (%s)\n", cwd);
+ log_err("error: directory is writable by others: (%s)\n", cwd);
exit(116);
}
@@ -478,7 +495,7 @@
* Error out if we cannot stat the program.
*/
if (((lstat(cmd, &prg_info)) != 0) || (S_ISLNK(prg_info.st_mode))) {
- log_err("cannot stat program: (%s)\n", cmd);
+ log_err("error: cannot stat program: (%s)\n", cmd);
exit(117);
}
@@ -486,7 +503,7 @@
* Error out if the program is writable by others.
*/
if ((prg_info.st_mode & S_IWOTH) || (prg_info.st_mode & S_IWGRP)) {
- log_err("file is writable by others: (%s/%s)\n", cwd, cmd);
+ log_err("error: file is writable by others: (%s/%s)\n", cwd, cmd);
exit(118);
}
@@ -494,7 +511,7 @@
* Error out if the file is setuid or setgid.
*/
if ((prg_info.st_mode & S_ISUID) || (prg_info.st_mode & S_ISGID)) {
- log_err("file is either setuid or setgid: (%s/%s)\n", cwd, cmd);
+ log_err("error: file is either setuid or setgid: (%s/%s)\n", cwd, cmd);
exit(119);
}
@@ -506,7 +523,7 @@
(gid != dir_info.st_gid) ||
(uid != prg_info.st_uid) ||
(gid != prg_info.st_gid)) {
- log_err("target uid/gid (%ld/%ld) mismatch "
+ log_err("error: target uid/gid (%ld/%ld) mismatch "
"with directory (%ld/%ld) or program (%ld/%ld)\n",
uid, gid,
dir_info.st_uid, dir_info.st_gid,
@@ -519,10 +536,17 @@
* "[error] Premature end of script headers: ..."
*/
if (!(prg_info.st_mode & S_IXUSR)) {
- log_err("file has no execute permission: (%s/%s)\n", cwd, cmd);
+ log_err("error: file has no execute permission: (%s/%s)\n", cwd, cmd);
exit(121);
}
+#ifdef SUEXEC_UMASK
+ if ((SUEXEC_UMASK & 0022) == 0) {
+ log_err("notice: SUEXEC_UMASK of %03o allows "
+ "write permission to group and/or other\n", SUEXEC_UMASK);
+ }
+ umask(SUEXEC_UMASK);
+#endif /* SUEXEC_UMASK */
clean_env();
/*
@@ -561,6 +585,6 @@
*
* Oh well, log the failure and error out.
*/
- log_err("(%d)%s: exec failed (%s)\n", errno, strerror(errno), cmd);
+ log_err("emerg: (%d)%s: exec failed (%s)\n", errno, strerror(errno), cmd);
exit(255);
}
Re: [PATCH] addition of suexec umask setting
Posted by Greg Stein <gs...@lyra.org>.
On Tue, 11 Jan 2000, Rodent of Unusual Size wrote:
> Greg Stein wrote:
> >
> > if ((SUEXEC_UMASK & 0022) != 0022) {
> >
> > That should work better...
>
> Argh..
>
> How about
>
> if ((~SUEXEC_UMASK) & 0022) {
>
> I'd rather invert the logic before doing the test, rather than
> testing with inverted logic. :-)
hehe... works for me! :-)
+1
--
Greg Stein, http://www.lyra.org/
Re: [PATCH] addition of suexec umask setting
Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Greg Stein wrote:
>
> if ((SUEXEC_UMASK & 0022) != 0022) {
>
> That should work better...
Argh..
How about
if ((~SUEXEC_UMASK) & 0022) {
I'd rather invert the logic before doing the test, rather than
testing with inverted logic. :-)
--
#ken P-)}
Ken Coar <http://Golux.Com/coar/>
Apache Software Foundation <http://www.apache.org/>
"Apache Server for Dummies" <http://Apache-Server.Com/>
Come to the first official Apache Software Foundation
Conference! <http://ApacheCon.Com/>
Re: [PATCH] addition of suexec umask setting
Posted by Greg Stein <gs...@lyra.org>.
On Mon, 10 Jan 2000, Rodent of Unusual Size wrote:
>...
> +#ifdef SUEXEC_UMASK
> + if ((SUEXEC_UMASK & 0022) == 0) {
> + log_err("notice: SUEXEC_UMASK of %03o allows "
> + "write permission to group and/or other\n", SUEXEC_UMASK);
> + }
> + umask(SUEXEC_UMASK);
> +#endif /* SUEXEC_UMASK */
If the umask is 0020, then the above notice is not issued. The notice only
shows up if group AND other permission is enabled. I think the test should
be:
if ((SUEXEC_UMASK & 0022) != 0022) {
That should work better...
Cheers,
-g
--
Greg Stein, http://www.lyra.org/