Merge "Address some potential security issues in lockutils"

oslo/concurrency/lockutils.py

@@ -40,7 +40,9 @@ util_opts = [
                 help='Enables or disables inter-process locks.'),
     cfg.StrOpt('lock_path',
                default=os.environ.get("OSLO_LOCK_PATH"),
-               help='Directory to use for lock files.')
+               help='Directory to use for lock files.  For security, the '
+                    'specified directory should only be writable by the user '
+                    'running the processes that need locking.')
 ]
 
 
@@ -81,7 +83,10 @@ class _FileLock(object):
             fileutils.ensure_tree(basedir)
             LOG.info(_LI('Created lock path: %s'), basedir)
 
-        self.lockfile = open(self.fname, 'w')
+        # Open in append mode so we don't overwrite any potential contents of
+        # the target file.  This eliminates the possibility of an attacker
+        # creating a symlink to an important file in our lock_path.
+        self.lockfile = open(self.fname, 'a')
 
         while True:
             try:

tests/unit/test_lockutils.py

@@ -427,6 +427,15 @@ class FileBasedLockingTestCase(test_base.BaseTestCase):
         thread.join()
         self.assertEqual(call_list, ['other', 'other', 'main', 'main'])
 
+    def test_non_destructive(self):
+        lock_file = os.path.join(self.lock_dir, 'not-destroyed')
+        with open(lock_file, 'w') as f:
+            f.write('test')
+        with lockutils.lock('not-destroyed', external=True,
+                            lock_path=self.lock_dir):
+            with open(lock_file) as f:
+                self.assertEqual(f.read(), 'test')
+
 
 class LockutilsModuleTestCase(test_base.BaseTestCase):