[PATCH] Re: [BUGFIXES] Wrong GID for PID file and UMASK for logs

[Dean Gaudet <dg...@arctic.org>]

On Mon, 24 Nov 1997, Dean Gaudet wrote:

> On Mon, 24 Nov 1997, Gregory A Lundberg wrote:
> 
> > Problem: If Apache must create its log files, the PID file is created
> > after changing to the new UID/GID, making the file group-owned by the
> > public web.  Does this create any security problems?  Unknown, but why
> > take the risk?
> 
> Yeah this should be fixed.
> 
> > Problem: The PID file and error log(s) are created using the default
> > umask.  For most systems, the default umask is good enough, but why depend
> > upon that?  Enforce a umask of at least 0133 on the PID and error logs
> > (you do already on the activity log) and save some poor admin a headache.
> 
> I'd rather not start using the umask() call... this is something similar
> to clearing the environment -- we should be doing this in a wrapper around
> apache.  Like in apachectl.

This is actually far more broken than we hinted at earlier.  Because
set_group_privs() is done in the parent, all the files created after a
restart are owned by the group of the previous config file read.  The only
way to fix this is to move the set_group_privs() into the child, which I'm
happy doing.  This means the parent never changes its identity. 

How's this?

fork()ing is expensive enough that adding in a couple extra syscalls to
play with gid will hardly cause much of a performance hit. 

Dean

Index: main/http_main.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_main.c,v
retrieving revision 1.256
diff -u -r1.256 http_main.c
--- http_main.c	1997/12/12 08:09:19	1.256
+++ http_main.c	1997/12/16 08:07:55
@@ -2216,12 +2216,7 @@
 #endif /* ndef WIN32 or __EMX__ */
 }
 
-/* Reset group privileges, after rereading the config files
- * (our uid may have changed, and if so, we want the new perms).
- *
- * Don't reset the uid yet --- we do that only in the child process,
- * so as not to lose any root privs.  But we can set the group stuff
- * now, once, as opposed to once per each new child.
+/* Set group privileges.
  *
  * Note that we use the username as set in the config files, rather than
  * the lookup of to uid --- the same uid may have multiple passwd entries,
@@ -2765,6 +2760,7 @@
     reopen_scoreboard(pchild);
     SAFE_ACCEPT(accept_mutex_child_init(pchild));
 
+    set_group_privs();
 #ifdef MPE
     /* Only try to switch if we're running as MANAGER.SYS */
     if (geteuid() == 1 && user_id > 1) {
@@ -3298,8 +3294,8 @@
 	server_conf = read_config(pconf, ptrans, server_confname);
 	setup_listeners(pconf);
 	open_logs(server_conf, pconf);
+	log_pid(pconf, pid_fname);
 	init_modules(pconf, server_conf);
-	set_group_privs();
 	SAFE_ACCEPT(accept_mutex_init(pconf));
 	if (!is_graceful) {
 	    reinit_scoreboard(pconf);
@@ -3312,7 +3308,6 @@
 #endif
 
 	set_signals();
-	log_pid(pconf, pid_fname);
 
 	if (daemons_max_free < daemons_min_free + 1)	/* Don't thrash... */
 	    daemons_max_free = daemons_min_free + 1;