diff options
| -rw-r--r-- | changelogs/fragments/vault_tmp_file.yml | 2 | ||||
| -rw-r--r-- | lib/ansible/executor/process/worker.py | 11 | ||||
| -rw-r--r-- | lib/ansible/parsing/dataloader.py | 14 | ||||
| -rw-r--r-- | lib/ansible/parsing/vault/__init__.py | 2 | ||||
| -rw-r--r-- | test/integration/targets/vault/files/test_assemble/nonsecret.txt | 1 | ||||
| -rw-r--r-- | test/integration/targets/vault/files/test_assemble/secret.vault | 7 | ||||
| -rwxr-xr-x | test/integration/targets/vault/runme.sh | 3 | ||||
| -rw-r--r-- | test/integration/targets/vault/test_dangling_temp.yml | 34 |
8 files changed, 72 insertions, 2 deletions
diff --git a/changelogs/fragments/vault_tmp_file.yml b/changelogs/fragments/vault_tmp_file.yml new file mode 100644 index 0000000000..1eaea6f3e7 --- /dev/null +++ b/changelogs/fragments/vault_tmp_file.yml @@ -0,0 +1,2 @@ +bugfixes: + - Ensure DataLoader temp files are removed at appropriate times and that we observe the LOCAL_TMP setting. diff --git a/lib/ansible/executor/process/worker.py b/lib/ansible/executor/process/worker.py index 2cf9bbb45a..3dde10d9a8 100644 --- a/lib/ansible/executor/process/worker.py +++ b/lib/ansible/executor/process/worker.py @@ -90,6 +90,10 @@ class WorkerProcess(multiprocessing.Process): # set to /dev/null self._new_stdin = os.devnull + # NOTE: this works due to fork, if switching to threads this should change to per thread storage of temp files + # clear var to ensure we only delete files for this child + self._loader._tempfiles = set() + def run(self): ''' Called when the process is started. Pushes the result onto the @@ -159,6 +163,8 @@ class WorkerProcess(multiprocessing.Process): except: display.debug(u"WORKER EXCEPTION: %s" % to_text(e)) display.debug(u"WORKER TRACEBACK: %s" % to_text(traceback.format_exc())) + finally: + self._clean_up() display.debug("WORKER PROCESS EXITING") @@ -169,3 +175,8 @@ class WorkerProcess(multiprocessing.Process): # ps.print_stats() # with open('worker_%06d.stats' % os.getpid(), 'w') as f: # f.write(s.getvalue()) + + def _clean_up(self): + # NOTE: see note in init about forks + # ensure we cleanup all temp files for this worker + self._loader.cleanup_all_tmp_files() diff --git a/lib/ansible/parsing/dataloader.py b/lib/ansible/parsing/dataloader.py index 0b9a3ff9f2..2606bb26c6 100644 --- a/lib/ansible/parsing/dataloader.py +++ b/lib/ansible/parsing/dataloader.py @@ -54,8 +54,16 @@ class DataLoader: ''' def __init__(self): + self._basedir = '.' + + # NOTE: not effective with forks as the main copy does not get updated. + # avoids rereading files self._FILE_CACHE = dict() + + # NOTE: not thread safe, also issues with forks not returning data to main proc + # so they need to be cleaned independantly. See WorkerProcess for example. + # used to keep track of temp files for cleaning self._tempfiles = set() # initialize the vault stuff with an empty password @@ -325,7 +333,7 @@ class DataLoader: def _create_content_tempfile(self, content): ''' Create a tempfile containing defined content ''' - fd, content_tempfile = tempfile.mkstemp() + fd, content_tempfile = tempfile.mkstemp(dir=C.DEFAULT_LOCAL_TMP) f = os.fdopen(fd, 'wb') content = to_bytes(content) try: @@ -388,6 +396,10 @@ class DataLoader: self._tempfiles.remove(file_path) def cleanup_all_tmp_files(self): + """ + Removes all temporary files that DataLoader has created + NOTE: not thread safe, forks also need special handling see __init__ for details. + """ for f in self._tempfiles: try: self.cleanup_tmp_file(f) diff --git a/lib/ansible/parsing/vault/__init__.py b/lib/ansible/parsing/vault/__init__.py index 2ac1291dfa..4f4fb0ef4e 100644 --- a/lib/ansible/parsing/vault/__init__.py +++ b/lib/ansible/parsing/vault/__init__.py @@ -850,7 +850,7 @@ class VaultEditor: # Create a tempfile root, ext = os.path.splitext(os.path.realpath(filename)) - fd, tmp_path = tempfile.mkstemp(suffix=ext) + fd, tmp_path = tempfile.mkstemp(suffix=ext, dir=C.DEFAULT_LOCAL_TMP) os.close(fd) cmd = self._editor_shell_command(tmp_path) diff --git a/test/integration/targets/vault/files/test_assemble/nonsecret.txt b/test/integration/targets/vault/files/test_assemble/nonsecret.txt new file mode 100644 index 0000000000..320b6b4ca0 --- /dev/null +++ b/test/integration/targets/vault/files/test_assemble/nonsecret.txt @@ -0,0 +1 @@ +THIS IS OK diff --git a/test/integration/targets/vault/files/test_assemble/secret.vault b/test/integration/targets/vault/files/test_assemble/secret.vault new file mode 100644 index 0000000000..fd27856491 --- /dev/null +++ b/test/integration/targets/vault/files/test_assemble/secret.vault @@ -0,0 +1,7 @@ +$ANSIBLE_VAULT;1.1;AES256 +37626439373465656332623633333336353334326531333666363766303339336134313136616165 +6561333963343739386334653636393363396366396338660a663537666561643862343233393265 +33336436633864323935356337623861663631316530336532633932623635346364363338363437 +3365313831366365350a613934313862313538626130653539303834656634353132343065633162 +34316135313837623735653932663139353164643834303534346238386435373832366564646236 +3461333465343434666639373432366139363566303564643066 diff --git a/test/integration/targets/vault/runme.sh b/test/integration/targets/vault/runme.sh index 1dee54a9e5..a0d9b7d99e 100755 --- a/test/integration/targets/vault/runme.sh +++ b/test/integration/targets/vault/runme.sh @@ -487,3 +487,6 @@ ansible-playbook "$@" -i invalid_format/inventory --vault-id invalid_format/vaul EXPECTED_ERROR='Vault format unhexlify error: Odd-length string' ansible-playbook "$@" -i invalid_format/inventory --vault-id invalid_format/vault-secret invalid_format/broken-group-vars-tasks.yml 2>&1 | grep "${EXPECTED_ERROR}" + +# Ensure we don't leave unencrypted temp files dangling +ansible-playbook -v "$@" --vault-password-file vault-password test_dangling_temp.yml diff --git a/test/integration/targets/vault/test_dangling_temp.yml b/test/integration/targets/vault/test_dangling_temp.yml new file mode 100644 index 0000000000..71a9d73aaf --- /dev/null +++ b/test/integration/targets/vault/test_dangling_temp.yml @@ -0,0 +1,34 @@ +- hosts: localhost + gather_facts: False + vars: + od: "{{output_dir|default('/tmp')}}/test_vault_assemble" + tasks: + - name: create target directory + file: + path: "{{od}}" + state: directory + + - name: assemble_file file with secret + assemble: + src: files/test_assemble + dest: "{{od}}/dest_file" + remote_src: no + mode: 0600 + + - name: remove assembled file with secret (so nothing should have unencrypted secret) + file: path="{{od}}/dest_file" state=absent + + - name: find temp files with secrets + find: + paths: '{{temp_paths}}' + contains: 'VAULT TEST IN WHICH BAD THING HAPPENED' + recurse: yes + register: badthings + vars: + temp_paths: "{{[lookup('env', 'TMP'), lookup('env', 'TEMP'), hardcoded]|flatten(1)|unique|list}}" + hardcoded: ['/tmp', '/var/tmp'] + + - name: ensure we failed to find any + assert: + that: + - badthings['matched'] == 0 |