From be17ba67b22f05db6684d448be8c4cf3afbb6b22 Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Mon, 25 Apr 2016 17:13:42 +0100 Subject: [PATCH] Prevent race in key setup for accelerate daemon (#15299) Ensure that initial setup in creating the key directory for ansible accelerate mode keys, and generation/storage of the key for a particular host are completed in a multiprocess safe manner. Creating directories/files and then assigning permissions and contents to them means that paths may exist and satisfy the os.path.exists() method in python for other processes before they are usable. Use a combination of locking around operations with use of unique named files and an OS file system move to ensure that the conditions of checking whether a file or directory exists, where it is potentially created by another thread, will only succeed when the file has both the correct contents and permissions. Fixes #13850 --- lib/ansible/utils/encrypt.py | 49 +++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/lib/ansible/utils/encrypt.py b/lib/ansible/utils/encrypt.py index d00a89bb1c6..991476c5de7 100644 --- a/lib/ansible/utils/encrypt.py +++ b/lib/ansible/utils/encrypt.py @@ -20,6 +20,8 @@ __metaclass__ = type import os import stat +import tempfile +import multiprocessing import time import warnings @@ -68,6 +70,9 @@ from ansible.errors import AnsibleError __all__ = ['do_encrypt'] +_LOCK = multiprocessing.Lock() + + def do_encrypt(result, encrypt, salt_size=None, salt=None): if PASSLIB_AVAILABLE: try: @@ -95,8 +100,15 @@ def key_for_hostname(hostname): key_path = os.path.expanduser(C.ACCELERATE_KEYS_DIR) if not os.path.exists(key_path): - os.makedirs(key_path, mode=0o700) - os.chmod(key_path, int(C.ACCELERATE_KEYS_DIR_PERMS, 8)) + # avoid race with multiple forks trying to create paths on host + # but limit when locking is needed to creation only + with(_LOCK): + if not os.path.exists(key_path): + # use a temp directory and rename to ensure the directory + # searched for only appears after permissions applied. + tmp_dir = tempfile.mkdtemp(dir=os.path.dirname(key_path)) + os.chmod(tmp_dir, int(C.ACCELERATE_KEYS_DIR_PERMS, 8)) + os.rename(tmp_dir, key_path) elif not os.path.isdir(key_path): raise AnsibleError('ACCELERATE_KEYS_DIR is not a directory.') @@ -107,19 +119,26 @@ def key_for_hostname(hostname): # use new AES keys every 2 hours, which means fireball must not allow running for longer either if not os.path.exists(key_path) or (time.time() - os.path.getmtime(key_path) > 60*60*2): - key = AesKey.Generate(size=256) - fd = os.open(key_path, os.O_WRONLY | os.O_CREAT, int(C.ACCELERATE_KEYS_FILE_PERMS, 8)) - fh = os.fdopen(fd, 'w') - fh.write(str(key)) - fh.close() - return key - else: - if stat.S_IMODE(os.stat(key_path).st_mode) != int(C.ACCELERATE_KEYS_FILE_PERMS, 8): - raise AnsibleError('Incorrect permissions on the key file for this host. Use `chmod 0%o %s` to correct this issue.' % (int(C.ACCELERATE_KEYS_FILE_PERMS, 8), key_path)) - fh = open(key_path) - key = AesKey.Read(fh.read()) - fh.close() - return key + # avoid race with multiple forks trying to create key + # but limit when locking is needed to creation only + with(_LOCK): + if not os.path.exists(key_path) or (time.time() - os.path.getmtime(key_path) > 60*60*2): + key = AesKey.Generate() + # use temp file to ensure file only appears once it has + # desired contents and permissions + with tempfile.NamedTemporaryFile(mode='w', dir=os.path.dirname(key_path), delete=False) as fh: + tmp_key_path = fh.name + fh.write(str(key)) + os.chmod(tmp_key_path, int(C.ACCELERATE_KEYS_FILE_PERMS, 8)) + os.rename(tmp_key_path, key_path) + return key + + if stat.S_IMODE(os.stat(key_path).st_mode) != int(C.ACCELERATE_KEYS_FILE_PERMS, 8): + raise AnsibleError('Incorrect permissions on the key file for this host. Use `chmod 0%o %s` to correct this issue.' % (int(C.ACCELERATE_KEYS_FILE_PERMS, 8), key_path)) + fh = open(key_path) + key = AesKey.Read(fh.read()) + fh.close() + return key def keyczar_encrypt(key, msg): return key.Encrypt(msg.encode('utf-8'))