Files created by FastCGI server are world-writable

[Antonis Christofides <anthony@…>]

When I run my site using ./manage.py runfcgi (with many more parameters), all files created by django are world-writable. This includes the PID file specified with the pidfile parameter, and any files uploaded through the Django admin in FileField's.

The reason for this appears to be the command "os.umask(0)" in line 16 of django/utils/daemonize.py (also in line 41 for non-posix systems). What is the purpose of that statement?

[Antonis Christofides <anthony@…>]

Patch for properly setting umask

[Antonis Christofides <anthony@…>]

I am not an expert on Unix daemons, so I made some research. I found out that setting the umask when daemonizing appears to be standard practice. However, there is some confusion concerning what you actually set it to. The Linux daemon-writing howto (​http://www.netzmafia.de/skripten/unix/linux-daemon-howto.html) says to set it to zero, with the following rationale:

"By setting the umask to 0, we will have full access to the files generated by the daemon. Even if you aren't planning on using any files, it is a good idea to set the umask here anyway, just in case you will be accessing files on the filesystem."

This doesn't make much sense to me. Other texts, such as the howto daemonize, ​http://www-theorie.physik.unizh.ch/~dpotter/howto/daemonize, also recommend to set the umask to 0, without rationale. Finally, other texts recommend a nonzero value for the umask. For example, the Unix Daemon Server Programming, ​http://www.enderunix.org/docs/eng/daemon.php, says:

"Most servers runs as super-user, for security reasons they should protect files that they create. Setting user mask will prevent unsecure file priviliges that may occur on file creation."

and it gives, as an example, a umask of 027.

My conclusion is that it is good practice, when daemonizing, to properly reset the umask to a sane value; otherwise you inherit the parent's umask, and this might be insecure. But the actual value to which you set the umask to should be sane, and definitely nonzero. I don't know why some people recommend setting it to zero, but I dare wildly guess that an error was once made in some howto and then it has been copied generously. I can't find any other explanation.

Since there is no single umask value that can fit all cases (for example, I think 027 is reasonable, but in my installation I happen to need 002), it must be possible to override. I therefore wrote a patch where you can specify the umask as a runfcgi option, and the default is a sane 027.

[Antonis Christofides <anthony@…>]

Updated patch, default umask now 022 instead of 027

[Antonis Christofides <anthony@…>]

I think that using a umask of 027 may break backwards compatibility and create unexpected results. Specifically, uploaded files would now be unreadable by "other", which usually includes the web server, which means that in some installations newly uploaded files might be unreadable until the umask is set. So I changed the default umask to 022 and uploaded an updated patch.

[Antonis Christofides]

Hi Simon,

when you check "needs documentation", is this something for me? I _have_ provided documentation (albeit only one line), the topmost modification in the patch. Is more than that needed?

[Marc Fargas]

Replying to aptiko:

when you check "needs documentation", is this something for me? I _have_ provided documentation (albeit only one line), the topmost modification in the patch. Is more than that needed?

Errr... documentation does not mean comments, ​the contributing doc says it all, patches should have (when applicable):

• The patch itself (code changes).
• Tests that reproduce the bug fixed (and hence, pass) or that test the new functionality
• Documentation, changes to the docs/ folder so users see those changes.

[Antonis Christofides]

Replying to telenieko:

Errr... documentation does not mean comments, ​the contributing doc says it all, patches should have (when applicable):

• Documentation, changes to the docs/ folder so users see those changes.

The docs/ directory does not document any of runfcgi's arguments: there is nothing about workdir, outlog or errlog in there. The only thing that is mentioned is (​http://www.djangoproject.com/documentation/fastcgi/):

"If you specify help as the only option after runfcgi, it’ll display a list of all the available options."

The modifications I've provided include documentation of "umask" in "runfcgi help". If a one-liner there is enough for the "workdir" parameter (and in my opinion it is), then a one-liner about "umask" should also suffice.

[Malcolm Tredinnick]

Good catch. I'm a bit embarrassed to have missed this when the initial idea was floated. 022 is a pretty normal umask value in these situations, so it's a good default. Thanks for the patch.

[Malcolm Tredinnick]

(In [7800]) Fixed #6994 -- For fastcgi, set a more sensible default umask.
Also allow the umask value to be customised. Thanks, Antonis Christofides.

[petteyg359@…]

The umask option is converted to an int, instead of taken as typed.

./manage.py runfcgi umask=0111 creates a socket with umask 0157.
umask=0022 creates a socket with umask 0026.
umask=0027 creares a socket with umask 0033.

A user on IRC suggested use of ast.literal_eval() instead.

[Antonis Christofides]

I am not absolutely certain of the trac workflow used in Django, but in most software projects we don't reopen fixed bugs when the fix turns out to be buggy; instead, we file new bugs. The original bug was that the files are world-writeable, whereas the new bug is that the new configuration option does not work properly.

Therefore, please file this as a new bug (and CC me since I'm the author of the buggy code).