Mailing List Archive

Urgle...this would break my site. Any patch that plays with setuid
must also be bulletproof. It's worth looking at again after 1.0.

.....Roy

Re the SETUID patch.

It seems to me that this violates rule 1 of security; "Thou shalt run as root
for no longer than is absolutely necessary".

On the other hand, if a sysadmin understands the issues, why not let them do
it?

--
Ben Laurie Phone: +44 (181) 994 6435
Freelance Consultant Fax: +44 (181) 994 6472
and Technical Director Email: ben@algroup.co.uk (preferred)
A.L. Digital Ltd, benl@fear.demon.co.uk (backup)
London, England.

[.Note for the paranoid: "fear" as in "Fear and Loathing
in Las Vegas", "demon" as in Demon Internet Services, a
commercial Internet access provider.]

Could the author please upload a clean patch to hyperreal.

Thank you.

<Aram>

At 02:30 PM 9/13/95 -0700, you wrote:
>
>monster patch for consideration...
>
>Forwarded message:
>> From leitner@prz.tu-berlin.de Wed Sep 13 13:30:23 1995
>> From: leitner@prz.tu-berlin.de
>> Message-Id: <199509132029.WAA00729@tubkom.prz.tu-berlin.de>
>> Subject: Apache secure CGI bin patch
>> To: apache-bugs@mail.apache.org
>> Date: Wed, 13 Sep 1995 22:29:21 +0100 (MESZ)
>> Mime-Version: 1.0
>> Content-Type: text/plain; charset=ISO-8859-1
>> Content-Transfer-Encoding: 8bit
>> Content-Length: 12694
>>
>> Hi folks,
>>
>> I wanted to patch some server for quite a while, and now I finally found
>> a worthy server, Apache ;)
>>
>> This is my patch. Go into the apache_0.8.11 directory and run "patch -s
>> -p1 < apache.diff" where apache.diff is included here. Please read the
>> file README.SETUID in the src/ directory to see what this patch does.
>>
>> I hope I'll get on the Apache credits list now ;))
>>
>> Felix
>>
>>
>> diff -u --recursive --new-file apache_0.8.11/src/Configuration
apache_0.8.11.setuid/src/Configuration
>> --- apache_0.8.11/src/Configuration Tue Aug 8 18:56:04 1995
>> +++ apache_0.8.11.setuid/src/Configuration Tue Sep 5 23:16:42 1995
>> @@ -34,11 +34,11 @@
>> # defaults in. Note that this config file does not include DBM auth by
>> # default --- configure it in below if you need it].
>>
>> -CFLAGS= -O2
>> +CFLAGS= -O2 -DSETUID
>>
>> # Place here any flags you may need upon linking, such as a flag to
>> # prevent dynamic linking (if desired)
>> -LFLAGS=
>> +LFLAGS= -s
>>
>> # Place here any extra libraries you may need to link to.
>> # -lndbm is commonly required for DBM auth, if that is configured in.
>> diff -u --recursive --new-file apache_0.8.11/src/README.SETUID
apache_0.8.11.setuid/src/README.SETUID
>> --- apache_0.8.11/src/README.SETUID
>> +++ apache_0.8.11.setuid/src/README.SETUID Wed Sep 13 21:55:52 1995
>> @@ -0,0 +1,80 @@
>> +This README describes my SETUID patch to the wonderful Apache HTTP
>> +server. It tries to fix Apache so that sysadmins have fewer reasons to
>> +disable CGI binaries in user's directories. Normally, a sysadmin with
>> +security in mind cannot allow CGI binaries in user's directories, since
>> +user can kill each other's CGI bins, and they run under another UID, so
>> +users can install backdoors to allow remote login under the UID of their
>> +CGI bins. Besides, if users want to do some logging with their CGI
>> +binaries, they have to chmod u+X them or make the log file world
>> +writable. Both is unsatisfactory.
>> +
>> +Another thing that causes much grief to me is the .htaccess file. Now
>> +users can password-protect their files, but every local user can just
>> +copy the files from their account, since they have to be world readable
>> +so that the HTTP daemon can read them.
>> +
>> +My patch changes the Apache daemon in the following ways :
>> +
>> + * When run standalone, the daemon does not setuid(wanted), but it
>> + does seteuid(wanted). This makes it possible that the daemon can
>> + later seteuid to some other uid. The daemon still does all the dirty
>> + work (like writing log files, etc) under the EUID you set up.
>> + * When run from inetd, it must be run as root and I changed the code
>> + that it changes UID just like the standalone server.
>> + * Files in home directories are now read under the EUID of the owner
>> + of the home directory. That means they no longer need to be world
>> + readable. Nor does the public_html directory need to be world
>> + executable !
>> + * CGI-bins in home directories are now run under the UID of the owner
>> + of the home. That means, if a user does a "kill -9 -1", it just his
>> + processes that die. And if you have a fascist Unix with process,
>> + CPU and memory quota, you can keep users from killing your HTTP
>> + server this way.
>> + Note that before CGI-bins are executed, the real UID is changed to
>> + the effective UID, so no CGI-bin can setuid(getuid) and be root !
>> + * Server Parsed HTML files can include both files and program outputs.
>> + Now this is a little tricky, because a malevolent user could use a
>> + server parsed HTML file to read any file in another user's
>> + public_html hierarchy. For this reason, no UID swapping takes place
>> + when including files. The same is true for foreign CGI-bins ! They
>> + have to be executable to the user to be includable.
>> + * CGI-bins can only be executed if they belong to the owner of
>> + the home directory. An exception are server parsed HTML included
>> + CGI-bins, otherwise one could not include foreign CGI bins.
>> + This is important, because otherwise people would compromise their
>> + accounts if they accidentally left one directory in their
>> + public_html hierarchy group or world writable by accident. Then any
>> + local malevolent user could create a CGI-bin that would modify that
>> + user's .rhosts file.
>> + * CGI-bins can not be executed if they are group or world
>> + writable. If someone had group or world writable CGI-bins, anyone
>> + could modify that CGI-bin and then compromise that user's account by
>> + modifying .rhosts.
>> +
>> +Please note that my code is ugly. It does not do error checking.
>> +If seteuid fails, you'll not get the error you'll expected, but you'll
>> +get an error that some files could not be read.
>> +
>> +To make the decision to integrate my patches easier, the changes are
>> +only effective if you #define SETUID.
>> +
>> +If you do not understand what the heck I am talking about, I suggest
>> +that you do not run a web server until you have consulted a security
>> +specialist.
>> +
>> +Since I am but a humble admin and C/C++/Perl hacker, I can't guarantee
>> +that this code works, does anything at all, and does not open new holes.
>> +I advise you to think before enabling SETUID blindly.
>> +
>> +If you find a bug, add error handling, find a security hole, or just
>> +want to tell me something, feel free to e-mail me as
>> +leitner@prz.tu-berlin.de. Alternatively, you could try to e-mail me as
>> +leitner@inf.fu-berlin.de. If that does not work, too, you can still
>> +send me snail mail (I just *love* postcards) :
>> +
>> + Felix von Leitner
>> + Gervinusstraße 22 [Foreigners without ISO umlauts do s/ß/ss/]
>> + 10629 Berlin
>> + Germany
>> +
>> +Felix
>> diff -u --recursive --new-file apache_0.8.11/src/http_core.c
apache_0.8.11.setuid/src/http_core.c
>> --- apache_0.8.11/src/http_core.c Wed Aug 23 03:16:07 1995
>> +++ apache_0.8.11.setuid/src/http_core.c Wed Sep 13 20:39:23 1995
>> @@ -670,7 +670,15 @@
>> || (errstatus = set_last_modified (r, r->finfo.st_mtime)))
>> return errstatus;
>>
>> +#ifdef SETUID
>> + seteuid(0); /* First become root again so we can */
>> + seteuid(destuid); /* seteuid again */
>> +#endif
>> f = fopen (r->filename, "r");
>> +#ifdef SETUID
>> + seteuid(0); /* First become root again so we can */
>> + seteuid(user_id); /* seteuid again */
>> +#endif
>>
>> if (f == NULL) {
>> log_reason("file permissions deny server access", r->filename, r);
>> diff -u --recursive --new-file apache_0.8.11/src/http_main.c
apache_0.8.11.setuid/src/http_main.c
>> --- apache_0.8.11/src/http_main.c Wed Aug 23 03:16:10 1995
>> +++ apache_0.8.11.setuid/src/http_main.c Wed Sep 13 20:38:53 1995
>> @@ -102,6 +102,10 @@
>>
>> int standalone;
>> uid_t user_id;
>> +#ifdef SETUID
>> +uid_t destuid;
>> +char ininclude=0;
>> +#endif
>> char *user_name;
>> gid_t group_id;
>> int max_requests_per_child;
>> @@ -711,7 +715,11 @@
>> reopen_scoreboard (pconf);
>>
>> /* Only try to switch if we're running as root */
>> +#ifdef SETUID
>> + if(!getuid() && seteuid(user_id) == -1) {
>> +#else
>> if(!getuid() && setuid(user_id) == -1) {
>> +#endif
>> log_error ("unable to change uid", server_conf);
>> exit (1);
>> }
>> @@ -987,8 +995,10 @@
>> if (!server_conf->server_hostname)
>> server_conf->server_hostname = get_local_host(pconf);
>>
>> +#ifndef SETUID
>> user_id = getuid();
>> group_id = getgid();
>> +#endif
>>
>> server_conf->port = get_portnum(fileno(stdout));
>> conn = new_connection (ptrans, server_conf, stdin, stdout);
>> diff -u --recursive --new-file apache_0.8.11/src/httpd.h
apache_0.8.11.setuid/src/httpd.h
>> --- apache_0.8.11/src/httpd.h Thu Aug 24 02:09:47 1995
>> +++ apache_0.8.11.setuid/src/httpd.h Wed Sep 13 20:45:06 1995
>> @@ -101,6 +101,11 @@
>> #define DEFAULT_USER "#-1"
>> #define DEFAULT_GROUP "#-1"
>>
>> +#ifdef SETUID
>> +extern uid_t destuid;
>> +extern char ininclude;
>> +#endif
>> +
>> /* The name of the log files */
>> #define DEFAULT_XFERLOG "logs/access_log"
>> #define DEFAULT_ERRORLOG "logs/error_log"
>> diff -u --recursive --new-file apache_0.8.11/src/mod_asis.c
apache_0.8.11.setuid/src/mod_asis.c
>> --- apache_0.8.11/src/mod_asis.c Wed Aug 23 03:16:18 1995
>> +++ apache_0.8.11.setuid/src/mod_asis.c Wed Sep 13 19:56:20 1995
>> @@ -64,6 +64,8 @@
>> #include "util_script.h"
>> #include "http_main.h"
>>
>> +extern uid_t user_id;
>> +
>> int asis_handler (request_rec *r)
>> {
>> FILE *f;
>> @@ -73,8 +75,15 @@
>> log_reason("File does not exist", r->filename, r);
>> return NOT_FOUND;
>> }
>> -
>> +#ifdef SETUID
>> + seteuid(0); /* First become root again so we can */
>> + seteuid(destuid); /* seteuid again */
>> +#endif
>> f = fopen (r->filename, "r");
>> +#ifdef SETUID
>> + seteuid(0); /* First become root again so we can */
>> + seteuid(user_id); /* seteuid again */
>> +#endif
>>
>> if (f == NULL) {
>> log_reason("file permissions deny server access", r->filename, r);
>> diff -u --recursive --new-file apache_0.8.11/src/mod_cgi.c
apache_0.8.11.setuid/src/mod_cgi.c
>> --- apache_0.8.11/src/mod_cgi.c Wed Aug 23 03:16:20 1995
>> +++ apache_0.8.11.setuid/src/mod_cgi.c Wed Sep 13 21:39:50 1995
>> @@ -195,6 +195,9 @@
>>
>> cleanup_for_exec();
>>
>> +#ifdef SETUID
>> + setuid(destuid);
>> +#endif
>> if((!r->args) || (!r->args[0]) || (ind(r->args,'=') >= 0))
>> execle(r->filename, argv0, NULL, env);
>> else
>> diff -u --recursive --new-file apache_0.8.11/src/mod_include.c
apache_0.8.11.setuid/src/mod_include.c
>> --- apache_0.8.11/src/mod_include.c Wed Aug 23 03:16:23 1995
>> +++ apache_0.8.11.setuid/src/mod_include.c Wed Sep 13 20:37:27 1995
>> @@ -687,6 +687,10 @@
>>
>> /* This is a stub which parses a file descriptor. */
>>
>> +#ifdef SETUID
>> +extern char ininclude;
>> +#endif
>> +
>> void send_parsed_content(FILE *f, request_rec *r)
>> {
>> char directive[MAX_STRING_LEN], error[MAX_STRING_LEN];
>> @@ -712,12 +716,25 @@
>> rprintf(r,"%s",error);
>> ret = find_string(f,ENDING_SEQUENCE,NULL);
>> } else
>> +#ifdef SETUID
>> + { ininclude++;
>> +#endif
>> ret=handle_exec(f, r, error);
>> +#ifdef SETUID
>> + ininclude--; }
>> +#endif
>> }
>> else if(!strcmp(directive,"config"))
>> ret=handle_config(f, r, error, timefmt, &sizefmt);
>> else if(!strcmp(directive,"include"))
>> +#ifdef SETUID
>> + { ininclude++;
>> +#endif
>> ret=handle_include(f, r, error, noexec);
>> +#ifdef SETUID
>> + ininclude--;
>> + }
>> +#endif
>> else if(!strcmp(directive,"echo"))
>> ret=handle_echo(f, r, error);
>> else if(!strcmp(directive,"fsize"))
>> diff -u --recursive --new-file apache_0.8.11/src/mod_userdir.c
apache_0.8.11.setuid/src/mod_userdir.c
>> --- apache_0.8.11/src/mod_userdir.c Wed Aug 23 03:16:28 1995
>> +++ apache_0.8.11.setuid/src/mod_userdir.c Wed Sep 13 20:31:22 1995
>> @@ -69,6 +69,7 @@
>>
>> #include "httpd.h"
>> #include "http_config.h"
>> +#include "http_conf_globals.h"
>>
>> module userdir_module;
>>
>> @@ -97,12 +98,19 @@
>> { NULL }
>> };
>>
>> +#ifdef SETUID
>> +extern char ininclude;
>> +#endif
>> +
>> int translate_userdir (request_rec *r)
>> {
>> void *server_conf = r->server->module_config;
>> char *userdir = (char *)get_module_config(server_conf, &userdir_module);
>> char *name = r->uri;
>>
>> +#ifdef SETUID
>> + destuid=user_id;
>> +#endif
>> if (userdir && (name[0] == '/') && (name[1] == '~'))
>> {
>> struct passwd *pw;
>> @@ -112,6 +120,10 @@
>> w = getword(r->pool, &dname, '/');
>> if(!(pw=getpwnam(w)))
>> return NOT_FOUND;
>> +#ifdef SETUID
>> + if (ininclude==0)
>> + destuid=pw->pw_uid;
>> +#endif
>>
>> /* The 'dname' funny business involves backing it up to capture
>> * the '/' delimiting the "/~user" part from the rest of the URL,
>> diff -u --recursive --new-file apache_0.8.11/src/util.c
apache_0.8.11.setuid/src/util.c
>> --- apache_0.8.11/src/util.c Wed Aug 23 03:16:31 1995
>> +++ apache_0.8.11.setuid/src/util.c Wed Sep 13 21:36:59 1995
>> @@ -532,6 +532,25 @@
>> }
>>
>> int can_exec(struct stat *finfo) {
>> +#ifdef SETUID /* hack by Felix von Leitner <leitner@prz.tu-berlin.de> */
>> + if (finfo->st_mode & (S_IWGRP+S_IWOTH)) {
>> + fprintf(stderr,"group or world writable\n");
>> + return 0; /* Never allow to execute group or world writable CGIs */
>> + }
>> + if (ininclude==0)
>> + if (finfo->st_uid != destuid) {
>> + fprintf(stderr,"CGI uid=%d, home uid=%d\n",finfo->st_uid,destuid);
>> + return 0; /* Do not allow to execute CGIs not belonging
>> + to the owner of the home */
>> + }
>> + if(destuid == finfo->st_uid)
>> + if(finfo->st_mode & S_IXUSR)
>> + return 1;
>> + if(group_id == finfo->st_gid)
>> + if(finfo->st_mode & S_IXGRP)
>> + return 1;
>> + return (finfo->st_mode & S_IXOTH);
>> +#else
>> if(user_id == finfo->st_uid)
>> if(finfo->st_mode & S_IXUSR)
>> return 1;
>> @@ -539,6 +558,7 @@
>> if(finfo->st_mode & S_IXGRP)
>> return 1;
>> return (finfo->st_mode & S_IXOTH);
>> +#endif
>> }
>>
>> #ifdef NEED_STRDUP
>>
>
>
--
Aram W. Mirzadeh, MIS Manager, Qosina Corporation
http://www.qosina.com/~awm/, awm@qosina.com
Apache httpd server team http://www.apache.org

> Could the author please upload a clean patch to hyperreal.

You have his email address. If you do, ask him to wait till 1.0

> >monster patch for consideration...
> >
> >Forwarded message:
> >> From leitner@prz.tu-berlin.de Wed Sep 13 13:30:23 1995
> >> From: leitner@prz.tu-berlin.de
> >> Message-Id: <199509132029.WAA00729@tubkom.prz.tu-berlin.de>
> >> Subject: Apache secure CGI bin patch

>
> Felix Leitner: (that name rings a bell)
Wasn't there a Felix L? in James Bond? CIA agent, or something.


--
Ben Laurie Phone: +44 (181) 994 6435
Freelance Consultant Fax: +44 (181) 994 6472
and Technical Director Email: ben@algroup.co.uk (preferred)
A.L. Digital Ltd, benl@fear.demon.co.uk (backup)
London, England.

[.Note for the paranoid: "fear" as in "Fear and Loathing
in Las Vegas", "demon" as in Demon Internet Services, a
commercial Internet access provider.]

> Re the SETUID patch.
>
> It seems to me that this violates rule 1 of security; "Thou shalt run as root
> for no longer than is absolutely necessary".
>
> On the other hand, if a sysadmin understands the issues, why not let them
^^
clue ;)

> do it?

> Ben Laurie Phone: +44 (181) 994 6435

SETUID is actually a very useful feature, and can allow different modes
of expression in your webbing environment. In environments with
a non-UNIX way of managing security (MOO, COLD) all programs run SETUID,
ie all scripts run with the sUID of the script's owner, rather then the
sUID of the person invoking the script. It's a completely different
mindset, but it's not any less secure once the 'differences' are understood.

A projected Apache system might not 'understand' a specific security or
resource access model at all. Ie it might not care if it's running on
a UNIX platform. So the questions of:

where do I get the data from
how do I run these programs
how do I enforce security

would be parceled off to a module:

mod_unix_environment.c

or one of:

mod_dos_environment.c
mod_mac_environment.c
mod_moo_environment.c

[.Ok, It's more than likely that it WILL be running on a UNIX platform,
because it makes sense to use a powerful base for all your software.
However just because Apache has a neat way of managing access to
a port, or managing resources under load, or recording access etc, etc,
*doesn't* mean that it's doing the right thing for people who want to be
creative.]


Ay, pipedreaming.

> > Felix Leitner: (that name rings a bell)
> Wasn't there a Felix L? in James Bond? CIA agent, or something.

...don't encourage me,

David Hedison was "Felix Leiter" in Licence to Kill (1989)
David Hedison was "Felix Leiter" in Live and Let Die (1973)
Cec Linder was "Felix Leiter" in Goldfinger (1964)
Jack Lord was "Felix Leiter" in Dr. No (1962)
John Terry was "Felix Leiter" in Living Daylights, The (1987)
Rik Von Nutter was "Felix Leiter" in Thunderball (1965)

On Thu, 14 Sep 1995, Andrew Wilson wrote:
> mod_dos_environment.c
> mod_mac_environment.c

You're a very sick person. Get help immediately.

:)

Brian

--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
brian@organic.com brian@hyperreal.com http://www.[hyperreal,organic].com/

Felix Leitner: (that name rings a bell)
> >> +Please note that my code is ugly. It does not do error checking.
> >> +If seteuid fails, you'll not get the error you'll expected, but you'll
> >> +get an error that some files could not be read.

Wheee, yes. Ideal candidate for immediate release, er... Please, can we make
error checking mandatory in all patches. Such that if you make a system
call that can potentially fail - you should run even the simplest check of
return values. Failure to check should make for -1 I think.

> >> +To make the decision to integrate my patches easier, the changes are
> >> +only effective if you #define SETUID.
> >> +
> >> +If you do not understand what the heck I am talking about, I suggest
> >> +that you do not run a web server until you have consulted a security
> >> +specialist.
> >> +
> >> +Since I am but a humble admin and C/C++/Perl hacker, I can't guarantee
> >> +that this code works, does anything at all, and does not open new holes.
> >> +I advise you to think before enabling SETUID blindly.

I suggest that you do not run a web server until you have consulted a security
specialist. ;)

> >> +#ifdef SETUID
> >> + seteuid(0); /* First become root again so we can */
> >> + seteuid(destuid); /* seteuid again */
> >> +#endif
> >> f =3D fopen (r->filename, "r");
> >> +#ifdef SETUID
> >> + seteuid(0); /* First become root again so we can */
> >> + seteuid(user_id); /* seteuid again */
> >> +#endif

Wheee, again.

Seriously, it's a nice idea but the patch is sucking and would immediately
require a refit if it ever got into the source. It's also one of those fabled
functional enhancements we hear about but are avoiding till 1.0 gets out.

I'm gunna play about with it anyhow. Just for kicks.

Ay (licenced to whine).