Mailing List Archive

Formal geteuid patch...
Hi,

been chasing round the bug:

Subject: WWW Form Bug Report: "Seems to ignore file modes even when not running as root" on FreeBSD

...which was submitted about a week ago. Archie@tribe.com sez:

> Symptoms:
> --
> The httpd binary must be setuid-root in order to bind to port 80. In
> conf/httpd.conf I have the lines User www Group www User "www" is a
> normal user on my system. However, I can still access this file via the
> server: -rw------- 1 root bin 3828 Oct 3 19:07 doc.shtml
> Shouldn't I not be allowed to read this since the httpd server is
> running as user "www" ? A "ps" listing shows two httpd processes both
> running as root. The Apache version is 0.8.14. Thanks for a great
> server! -Archie

We determined that Apache 0.8.14 wasn't setuid'ing properly because
the real getuid wasn't root, and the code to date assumes that root is
the only person who'll be running the server. Archie had some interesting
points to mention about how he wants to be able to run the webserver:

> Yeah, I guess it boils down to how you define the semantics of the User
> and Group directives in conf/httpd.conf ... my understanding was that
> setting
>
> User xxx
> Group yyy
>
> means "I want the server to run having no more privileges for doing stuff
> than if it were user xxx in group yyy, no matter of how it got started."
>
> Now the mere existence of this directive implies that the server can
> change it's euid and egid, so therefore it either is getting executed
> by root or is setuid-root. However, the meaning of these directives
> should be orthogonal to the details of the startup procedure. At least,
> that's my half-educated take on it.

Fair enough. Archie wants a non-root user to be able to invoke a program
that needs root privileges, so he did a 'chmod 4000 ./httpd', meaning that
the program will run with an effective userid of the program's owner (root),
rather than the uid of the person invoking the program. This doesn't seem
like an unreasonable way to use apache.

1) Royston Shufflebotham's patch sent to apache-bugs:
Subject: Apache Bugfix: v0.8.14 was not running setuid properly (fwd)
is a workable fix for this problem, probably coming from exactly the
same setup.

Q) Has it been formally wrapped up into a for_Apache_0.8.14? I can't
seem to find it anywhere.

I offered a version of this patch to archie@tribe.com to fix things
temporarily, and I've uploaded it

to: hyperreal.com/patches/incoming
called: nn_geteuid.apache_0.8.14.patch

can someone move this to for_Apache_0.8.14 for me? I think this is
a genuine bug-fix rather than a feature enhancement, because of
the clear security issues.

2) Felix von Leitner's SETUID patch, originally offered against 0.8.11
but not yet accepted, also effects the same block of code but in a
different way.

Cheers,
Ay.
Re: Formal geteuid patch... [ In reply to ]
Subject: Remove some -Wwrite-strings warnings
Affects: mod_include.c, http_log.h, http_log.c, alloc.h, alloc.c,
http_main.h, http_main.c, httpd.h, util.c, http_request.c,
http_protocol.h, http_protocol.c
ChangeLog: Add const to prototypes to remove warnings about passing
static strings
Comment: There's more to be done after this patch, but this is enough
to be going on with.

I've said before and I'll say again that I don't think this is the time
to be doing large-scale cleanups. This is worthwhile, but I strongly feel
it is best put off until after 1.0.

rst
Re: Formal geteuid patch... [ In reply to ]
> I offered a version of this patch to archie@tribe.com to fix things
> temporarily, and I've uploaded it
>
> to: hyperreal.com/patches/incoming
> called: nn_geteuid.apache_0.8.14.patch

I've copied this to 18_geteuid.apache_0.8.14.patch

Note that I've also uploaded 17_const.0.8.14.patch
Subject: Remove some -Wwrite-strings warnings
Affects: mod_include.c, http_log.h, http_log.c, alloc.h, alloc.c,
http_main.h, http_main.c, httpd.h, util.c, http_request.c,
http_protocol.h, http_protocol.c
ChangeLog: Add const to prototypes to remove warnings about passing
static strings
Comment: There's more to be done after this patch, but this is enough
to be going on with.

David.
Re: Formal geteuid patch... [ In reply to ]
> I've said before and I'll say again that I don't think this is the time
> to be doing large-scale cleanups. This is worthwhile, but I strongly feel
> it is best put off until after 1.0.

You are right to question this, but my motivation was to find bugs;
(I didn't find any) and now that I've made the patch, I can see no reason
for not including it. The _only_ change it makes is to add const to
some prototypes; this cannot introduce any problems and can only help in
spotting errors.

David.