openstack/oslo.concurrency
Code Issues Proposed changes

Address some potential security issues in lockutils

Browse Source 
Adds a note to the lock_path help text explaining how to secure the
target directory.  Also opens lock files in append mode so there is
no possibility of overwriting a file due to a malicious symlink.

Change-Id: I77b72b20088fe66b573c23bd1fd98376c2b0f168
This commit is contained in:
Ben Nemec 2014-08-21 22:20:50 +00:00
parent f1688a70f2
commit 60167d0b37
2 changed files with 16 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
oslo/concurrency/lockutils.py
View File

@ -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:

9
tests/unit/test_lockutils.py
View File

@ -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):