qaulau / modwsgi

Automatically exported from code.google.com/p/modwsgi
0 stars 0 forks source link

"Unable to change to uid=X" does not abort #295

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The code bit "setuid(daemon->group->uid)" (lines 10469-10473 in mod_wsgi.c 3.4) 
may fail. The failure is caught, but apart form an error in the log, nothing 
else happens. This allows the (Apache) process to continue with elevated (i.e. 
root) privileges on resource limited systems, resulting in *all* files on the 
mancine being readable/writeable.

There may be other similar parts of the code where the same principle applies, 
but this is a specific bug we've run into.

== What steps will reproduce the problem?
1. Resource-starve the system (e.g. use nproc=x in limits.conf with some low 
setting)
2. restart apache

One can also use dummy, sleeping processes to tip over the system.

== What is the expected output? What do you see instead?

What's *unexpected* is that the web server is now running as the user who 
started it, ie. root, most likely. That's been considered a security hazard 
since the last century.

Either the module should retry or abort. For systems that exercie failover / HA 
/ load balancing, this is probably the least bad option.

Original issue reported on code.google.com by kis...@gmail.com on 27 Mar 2013 at 7:00

GoogleCodeExporter commented 9 years ago
Seems to a be a Linux specific issue:

EAGAIN
The uid does not match the current uid and uid brings process over its 
RLIMIT_NPROC resource limit.
EPERM
The user is not privileged (Linux: does not have the CAP_SETUID capability) and 
uid does not match the real UID or saved set-user-ID of the calling process.

Under MacOS X EAGAIN is not a possible return value.

FWIW, mod_cgid in Apache 2.2, which process creation/management was partly 
modelled off, ultimately doesn't validate whether setuid() returns an error 
either. They do do checking in Apache 2.4 although I am not sure how they 
protect against a tight loop of process creation if they kill off the process.

I guess you could possibly flag the process in a bad state and somehow block it 
from doing anything but otherwise keep it around. Alternatively you sleep for a 
while and exit the process so it gets created again, which is what is done in 
some other places in the code. Thus add in the if check after logging:

                /* Don't die immediately to avoid a fork bomb. */

                sleep(20);

                exit(-1);

Original comment by Graham.Dumpleton@gmail.com on 28 Mar 2013 at 5:45

GoogleCodeExporter commented 9 years ago
Changed in 3.X (branch) and 4.X (master).

https://github.com/GrahamDumpleton/mod_wsgi/tree/mod_wsgi-3.X
https://github.com/GrahamDumpleton/mod_wsgi

Original comment by Graham.Dumpleton@gmail.com on 28 Mar 2013 at 6:08

GoogleCodeExporter commented 9 years ago

Original comment by Graham.Dumpleton@gmail.com on 28 Mar 2013 at 6:09

GoogleCodeExporter commented 9 years ago

Original comment by Graham.Dumpleton@gmail.com on 28 Mar 2013 at 6:24

GoogleCodeExporter commented 9 years ago

Original comment by Graham.Dumpleton@gmail.com on 28 Mar 2013 at 6:41

GoogleCodeExporter commented 9 years ago

Original comment by Graham.Dumpleton@gmail.com on 30 Mar 2013 at 11:30

GoogleCodeExporter commented 9 years ago
We've been seeing the same issue on a machine running Graphite; it was only 
noticed because Graphite-web's Django app has a rotating loghandler, and when 
the wsgi process has not dropped privileges, the files are rotated as root, and 
the app user then cannot write to the log files any more. The server is a RHEL 
6 box, running wsgi 3.2

Original comment by and...@scalefactory.com on 13 May 2014 at 6:52

GoogleCodeExporter commented 9 years ago
Sorry, due to a lot of personal issues and other problems, which is totally 
unacceptable and shouldn't have made a damn difference, this hasn't made its 
way into an official release. Although I patched code on the new repo where I 
had mod_wsgi, looking at it now, it looks like I also managed to loose that 
change as well when I had to recreate the repo back from old repo on GoogleCode 
when had issue with the new repo. A quick patch is as follows, but you will 
need to compile from source code. Time to crank that engine and get a release 
done pronto since have screwed this up so badly.

@@ -10368,6 +10368,16 @@ static void wsgi_setup_access(WSGIDaemonProcess 
*daemon)
          ap_log_error(APLOG_MARK, APLOG_ALERT, errno, wsgi_server,
                       "mod_wsgi (pid=%d): Unable to change to uid=%ld.",
                       getpid(), (long)daemon->group->uid);
 +
 +        /* On Linux we can get back EAGAIN where the target user
 +         * had reached their process limit. In that case will be
 +         * left running as wrong user. Just exit on all failures to be
 +         * safe. Don't die immediately to avoid a fork bomb.
 +         */
 +
 +        sleep(20);
 +
 +        exit(-1);
      }

      /*

Original comment by Graham.Dumpleton@gmail.com on 13 May 2014 at 7:11

GoogleCodeExporter commented 9 years ago
Hi Graham,

That's okay, these things happen.. I'll see if we can build and package this 
from source. Thanks for the quick response :)

Cheers,
Andrew

Original comment by and...@scalefactory.com on 13 May 2014 at 7:15

GoogleCodeExporter commented 9 years ago

Original comment by Graham.Dumpleton@gmail.com on 21 May 2014 at 8:20

GoogleCodeExporter commented 9 years ago
In the meantime, one can do workarounds. This is what we use in Django (add 
this to django.wsgi):

====
if os.getuid() != getpwnam("<expected username here>").pw_uid:
    raise Exception("Attempt to start WSGIHandler by uid=%d" % os.getuid())
====

Robert

Original comment by kis...@gmail.com on 21 May 2014 at 8:56