cmueller/ansible
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Solve race condition in password lookup (#42529)

Browse source 
NOTE:
1. use os.open() with os.O_CREAT|os.O_EXCL to check existence
and create a lock file if not exists, it's an atomic operation
2. the fastest process will create the lock file and others will
wait until the lock file is removed
3. after the writer finished writing to the password file, all the reading
operations use built-in open so processes can read the file parallel
This commit is contained in:
Zhikang Zhang 2018-08-15 15:10:52 -04:00 committed by GitHub
parent 5c1e620504
commit 0971a342d8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 84 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

46
lib/ansible/plugins/lookup/password.py
View file

@ -92,6 +92,9 @@ _raw:
import os
import string
import time
import shutil
import hashlib
from ansible.errors import AnsibleError, AnsibleAssertionError
from ansible.module_utils._text import to_bytes, to_native, to_text
@ -267,6 +270,40 @@ def _write_password_file(b_path, content):
f.write(b_content)
def _get_lock(b_path):
"""Get the lock for writing password file."""
first_process = False
b_pathdir = os.path.dirname(b_path)
lockfile_name = to_bytes("%s.ansible_lockfile" % hashlib.md5(b_path).hexdigest())
lockfile = os.path.join(b_pathdir, lockfile_name)
if not os.path.exists(lockfile) and b_path != to_bytes('/dev/null'):
try:
makedirs_safe(b_pathdir, mode=0o700)
fd = os.open(lockfile, os.O_CREAT | os.O_EXCL)
os.close(fd)
first_process = True
except OSError as e:
if e.strerror != 'File exists':
raise
counter = 0
# if the lock is got by other process, wait until it's released
while os.path.exists(lockfile) and not first_process:
time.sleep(2 ** counter)
if counter >= 2:
raise AnsibleError("Password lookup cannot get the lock in 7 seconds, abort..."
"This may caused by un-removed lockfile"
"you can manually remove it from controller machine at %s and try again" % lockfile)
counter += 1
return first_process, lockfile
def _release_lock(lockfile):
"""Release the lock so other processes can read the password file."""
if os.path.exists(lockfile):
os.remove(lockfile)
class LookupModule(LookupBase):
def run(self, terms, variables, **kwargs):
ret = []
@ -277,7 +314,10 @@ class LookupModule(LookupBase):
b_path = to_bytes(path, errors='surrogate_or_strict')
chars = _gen_candidate_chars(params['chars'])
changed = False
changed = None
# make sure only one process finishes all the job first
first_process, lockfile = _get_lock(b_path)
content = _read_password_file(b_path)
if content is None or b_path == to_bytes('/dev/null'):
@ -295,6 +335,10 @@ class LookupModule(LookupBase):
content = _format_content(plaintext_password, salt, encrypt=params['encrypt'])
_write_password_file(b_path, content)
if first_process:
# let other processes continue
_release_lock(lockfile)
if params['encrypt']:
password = do_encrypt(plaintext_password, params['encrypt'], salt=salt)
ret.append(password)

41
test/units/plugins/lookup/test_password.py
View file

@ -29,6 +29,7 @@ from ansible.compat.tests.mock import mock_open, patch
from ansible.errors import AnsibleError
from ansible.module_utils.six import text_type
from ansible.module_utils.six.moves import builtins
from ansible.module_utils._text import to_bytes
from ansible.plugins.loader import PluginLoader
from ansible.plugins.lookup import password
from ansible.utils import encrypt
@ -372,6 +373,14 @@ class TestLookupModule(unittest.TestCase):
self.fake_loader = DictDataLoader({'/path/to/somewhere': 'sdfsdf'})
self.password_lookup = password.LookupModule(loader=self.fake_loader)
self.os_path_exists = password.os.path.exists
self.os_open = password.os.open
password.os.open = lambda path, flag: None
self.os_close = password.os.close
password.os.close = lambda fd: None
self.os_remove = password.os.remove
password.os.remove = lambda path: None
self.makedirs_safe = password.makedirs_safe
password.makedirs_safe = lambda path, mode: None
# Different releases of passlib default to a different number of rounds
self.sha256 = passlib.registry.get_crypt_handler('pbkdf2_sha256')
@ -380,6 +389,10 @@ class TestLookupModule(unittest.TestCase):
def tearDown(self):
password.os.path.exists = self.os_path_exists
password.os.open = self.os_open
password.os.close = self.os_close
password.os.remove = self.os_remove
password.makedirs_safe = self.makedirs_safe
passlib.registry.register_crypt_handler(self.sha256, force=True)
@patch.object(PluginLoader, '_get_paths')
@ -425,7 +438,7 @@ class TestLookupModule(unittest.TestCase):
@patch('ansible.plugins.lookup.password._write_password_file')
def test_password_already_created_encrypt(self, mock_get_paths, mock_write_file):
mock_get_paths.return_value = ['/path/one', '/path/two', '/path/three']
password.os.path.exists = lambda x: True
password.os.path.exists = lambda x: x == to_bytes('/path/to/somewhere')
with patch.object(builtins, 'open', mock_open(read_data=b'hunter42 salt=87654321\n')) as m:
results = self.password_lookup.run([u'/path/to/somewhere chars=anything encrypt=pbkdf2_sha256'], None)
@ -436,7 +449,7 @@ class TestLookupModule(unittest.TestCase):
@patch('ansible.plugins.lookup.password._write_password_file')
def test_password_already_created_no_encrypt(self, mock_get_paths, mock_write_file):
mock_get_paths.return_value = ['/path/one', '/path/two', '/path/three']
password.os.path.exists = lambda x: True
password.os.path.exists = lambda x: x == to_bytes('/path/to/somewhere')
with patch.object(builtins, 'open', mock_open(read_data=b'hunter42 salt=87654321\n')) as m:
results = self.password_lookup.run([u'/path/to/somewhere chars=anything'], None)
@ -452,3 +465,27 @@ class TestLookupModule(unittest.TestCase):
results = self.password_lookup.run([u'/path/to/somewhere chars=a'], None)
for result in results:
self.assertEquals(result, u'a' * password.DEFAULT_LENGTH)
def test_lock_been_held(self):
# pretend the lock file is here
password.os.path.exists = lambda x: True
try:
with patch.object(builtins, 'open', mock_open(read_data=b'hunter42 salt=87654321\n')) as m:
# should timeout here
results = self.password_lookup.run([u'/path/to/somewhere chars=anything'], None)
self.fail("Lookup didn't timeout when lock already been held")
except AnsibleError:
pass
def test_lock_not_been_held(self):
# pretend now there is password file but no lock
password.os.path.exists = lambda x: x == to_bytes('/path/to/somewhere')
try:
with patch.object(builtins, 'open', mock_open(read_data=b'hunter42 salt=87654321\n')) as m:
# should not timeout here
results = self.password_lookup.run([u'/path/to/somewhere chars=anything'], None)
except AnsibleError:
self.fail('Lookup timeouts when lock is free')
for result in results:
self.assertEqual(result, u'hunter42')