diff --git a/oslo/concurrency/lockutils.py b/oslo/concurrency/lockutils.py index 2de7357..0aea6bf 100644 --- a/oslo/concurrency/lockutils.py +++ b/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: diff --git a/tests/unit/test_lockutils.py b/tests/unit/test_lockutils.py index 86593b1..1715050 100644 --- a/tests/unit/test_lockutils.py +++ b/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):