You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Gregory A Lundberg <lu...@vr.net> on 1997/11/24 16:16:14 UTC
[BUGFIXES] Wrong GID for PID file and UMASK for logs
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?
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.
Porting note: The man pages for umask() on my Linux system are, to say
the best, sparce. I found the function defined in <sys/stat.h> but it may
or may not exist there or anywhere on other systems; check the patch for
http_log.c compiles on your flavor.
These problems exist in both 1.2.4 and 1.3b3. The 1.3b3 patch is below,
I've attached the 1.2.4 patch.
*** http_log.c.orig Mon Nov 24 09:45:52 1997
--- http_log.c Mon Nov 24 09:57:48 1997
***************
*** 67,72 ****
--- 67,73 ----
#include "http_main.h"
#include <stdarg.h>
+ #include <sys/stat.h>
typedef struct {
char *t_name;
***************
*** 179,184 ****
--- 180,189 ----
void open_error_log (server_rec *s, pool *p)
{
char *fname;
+ mode_t oldumask;
+
+ oldumask = umask (0133);
+ umask (oldumask | 0133);
if (*s->error_fname == '|') {
FILE *dummy;
***************
*** 221,226 ****
--- 226,233 ----
exit(1);
}
}
+
+ umask (oldumask);
}
void open_logs (server_rec *s_main, pool *p)
***************
*** 341,354 ****
--- 348,365 ----
void log_pid (pool *p, char *pid_fname)
{
FILE *pid_file;
+ mode_t oldumask;
if (!pid_fname) return;
pid_fname = server_root_relative (p, pid_fname);
+ oldumask = umask (0133);
+ umask (oldumask | 0133);
if(!(pid_file = fopen(pid_fname,"w"))) {
perror("fopen");
fprintf(stderr,"httpd: could not log pid to file %s\n", pid_fname);
exit(1);
}
+ umask (oldumask);
fprintf(pid_file,"%ld\n",(long)getpid());
fclose(pid_file);
}
*** http_main.c.orig Mon Nov 24 09:39:55 1997
--- http_main.c Mon Nov 24 09:40:20 1997
***************
*** 3280,3285 ****
--- 3280,3286 ----
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));
***************
*** 3294,3300 ****
#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;
--- 3295,3300 ----
Gregory A Lundberg Senior Partner, VRnet Company
1441 Elmdale Drive email: lundberg@vr.net [205.133.13.8]
Kettering, OH 45409-1615 USA voice: +1 (937) 299-7653
[PATCH] Re: [BUGFIXES] Wrong GID for PID file and UMASK for logs
Posted by 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;
Re: [BUGFIXES] Wrong GID for PID file and UMASK for logs
Posted by Dean Gaudet <dg...@arctic.org>.
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.
> *** http_main.c.orig Mon Nov 24 09:39:55 1997
> --- http_main.c Mon Nov 24 09:40:20 1997
> ***************
> *** 3280,3285 ****
> --- 3280,3286 ----
> 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));
> ***************
> *** 3294,3300 ****
> #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;
> --- 3295,3300 ----
This is truncated... but +1 on this part of your patch.
Dean