Clean up file permissions and umask settings. Now we set the umask to 007

during early initialization so that we're guaranteed to get the right value
regardless of the shell umask used to invoke the command line script.  While
we're at it, we can remove almost all individual umask settings previously in
the code, and make file permissions consistently -rw-rw---- (IOW, files are no
longer other readable).

The only subsystem that wasn't changed was the archiver, because it uses its
own umask settings to ensure that private archives have the proper
permissions.  Eventually we'll mess with this, but if it ain't broken...

Note that check_perms complains about directory permissions, but I think
check_perms can be fixed (or perhaps, even removed?!).  If we decide to use
LMTPRunner and HTTPRunner exclusively then no outside process will be touching
our files potentially with the incorrect permissions, umask, owner, or group.
If we control all of our own touch points then I think we can lock out
'other'.

Another open question is whether Utils.set_global_password() can have its
umask setting removed.  It locks permissions down so even the group can't
write to the site password file, but the default umask of 007 might be good
enough even for this file.

Utils.makedirs() now takes an optional mode argument, which defaults to 02775
for backward compatibility.  First, the default mode can probably be changed
to 02770 (see above).  Second, all code that was tweaking the umask in order
to do a platform compatible os.mkdir() has now been refactored to use
Utils.makedirs().

Another tricky thing was getting SQLite via SQLAlchemy to create its
data/mailman.db file with the proper permissions.  From the comment in
dbcontext.py:

        # XXX By design of SQLite, database file creation does not honor
        # umask.  See their ticket #1193:
        # http://www.sqlite.org/cvstrac/tktview?tn=1193,31

More details in that file, but the work around is to essentially 'touch' the
database file if 'sqlite' is the scheme of the SQLAlchemy URL.  This little
pre-touch sets the right umask honoring permission and won't hurt if the file
already exists.  SQLite will happily keep the existing permissions, and in
fact that ticket referenced above recommends doing things this way.

In the Mailman.database.initialize(), create a global lock that prevents more
than one process from entering this init function at the same time.  It's
probably not strictly necessary given that I believe all the operations in
dbcontext.connect() are multi-processing safe, but it also doesn't seem to
hurt and prevents race conditions regardless of the database's own
safeguards (or lack thereof).

Make sure nightly_gzip.py calls initialize().

1 files changed, 29 insertions, 1 deletions

diff --git a/Mailman/database/dbcontext.py b/Mailman/database/dbcontext.py
index a13f20498..b2b1fb826 100644
--- a/Mailman/database/dbcontext.py
+++ b/Mailman/database/dbcontext.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2006 by the Free Software Foundation, Inc.
+# Copyright (C) 2006-2007 by the Free Software Foundation, Inc.
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -15,10 +15,12 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301,
 # USA.
 
+import os
 import weakref
 
 from sqlalchemy import *
 from string import Template
+from urlparse import urlparse
 
 from Mailman import Version
 from Mailman.configuration import config
@@ -49,6 +51,21 @@ class DBContext(object):
         # Calculate the engine url
         url = Template(config.SQLALCHEMY_ENGINE_URL).safe_substitute(
             config.paths)
+        # XXX By design of SQLite, database file creation does not honor
+        # umask.  See their ticket #1193:
+        # http://www.sqlite.org/cvstrac/tktview?tn=1193,31
+        #
+        # This sucks for us because the mailman.db file /must/ be group
+        # writable, however even though we guarantee our umask is 002 here, it
+        # still gets created without the necessary g+w permission, due to
+        # SQLite's policy.  This should only affect SQLite engines because its
+        # the only one that creates a little file on the local file system.
+        # This kludges around their bug by "touch"ing the database file before
+        # SQLite has any chance to create it, thus honoring the umask and
+        # ensuring the right permissions.  We only try to do this for SQLite
+        # engines, and yes, we could have chmod'd the file after the fact, but
+        # half dozen and all...
+        self._touch(url)
         self.metadata = BoundMetaData(url)
         self.metadata.engine.echo = config.SQLALCHEMY_ECHO
         # Create all the table objects, and then let SA conditionally create
@@ -74,6 +91,17 @@ class DBContext(object):
             raise SchemaVersionMismatchError(row.version)
         self.session = create_session()
 
+    def _touch(self, url):
+        parts = urlparse(url)
+        # XXX Python 2.5; use parts.scheme and parts.path
+        if parts[0] <> 'sqlite':
+            return
+        path = os.path.normpath(parts[2])
+        fd = os.open(path, os.O_WRONLY |  os.O_NONBLOCK | os.O_CREAT, 0666)
+        # Ignore errors
+        if fd > 0:
+            os.close(fd)
+
     # Cooperative method for use with @txn decorator
     def _withtxn(self, meth, *args, **kws):
         try: