CVE-2020-1740
Description
A flaw was found in Ansible Engine when using Ansible Vault for editing encrypted files. When a user executes "ansible-vault edit", another user on the same computer can read the old and new secret, as it is created in a temporary file with mkstemp and the returned file descriptor is closed and the method write_data is called to write the existing secret in the file. This method will delete the file before recreating it insecurely. All versions in 2.7.x, 2.8.x and 2.9.x branches are believed to be vulnerable.
Affected packages
Versions sourced from the GitHub Security Advisory.
| Package | Affected versions | Patched versions |
|---|---|---|
ansiblePyPI | < 2.7.17 | 2.7.17 |
ansiblePyPI | >= 2.8.0a1, < 2.8.11 | 2.8.11 |
ansiblePyPI | >= 2.9.0a1, < 2.9.7 | 2.9.7 |
Affected products
1Patches
42a563514f070safely use vault to edit secrets (#68644)
2 files changed · +82 −39
changelogs/fragments/vault_tmp_race_fix.yml+2 −0 added@@ -0,0 +1,2 @@ +bugfixes: + - "**security_issue** - create temporary vault file with strict permissions when editing and prevent race condition (CVE-2020-1740)"
lib/ansible/parsing/vault/__init__.py+80 −39 modified@@ -19,6 +19,8 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +import errno +import fcntl import os import random import shlex @@ -27,6 +29,7 @@ import sys import tempfile import warnings + from binascii import hexlify from binascii import unhexlify from binascii import Error as BinasciiError @@ -843,42 +846,48 @@ def _shred_file(self, tmp_path): os.remove(tmp_path) - def _edit_file_helper(self, filename, secret, - existing_data=None, force_save=False, vault_id=None): + def _edit_file_helper(self, filename, secret, existing_data=None, force_save=False, vault_id=None): # Create a tempfile root, ext = os.path.splitext(os.path.realpath(filename)) fd, tmp_path = tempfile.mkstemp(suffix=ext, dir=C.DEFAULT_LOCAL_TMP) - os.close(fd) cmd = self._editor_shell_command(tmp_path) try: if existing_data: - self.write_data(existing_data, tmp_path, shred=False) + self.write_data(existing_data, fd, shred=False) + except Exception: + # if an error happens, destroy the decrypted file + self._shred_file(tmp_path) + raise + finally: + os.close(fd) + try: # drop the user into an editor on the tmp file subprocess.call(cmd) except Exception as e: - # whatever happens, destroy the decrypted file + # if an error happens, destroy the decrypted file self._shred_file(tmp_path) raise AnsibleError('Unable to execute the command "%s": %s' % (' '.join(cmd), to_native(e))) b_tmpdata = self.read_data(tmp_path) # Do nothing if the content has not changed - if existing_data == b_tmpdata and not force_save: - self._shred_file(tmp_path) - return + if force_save or existing_data != b_tmpdata: + + # encrypt new data and write out to tmp + # An existing vaultfile will always be UTF-8, + # so decode to unicode here + b_ciphertext = self.vault.encrypt(b_tmpdata, secret, vault_id=vault_id) + self.write_data(b_ciphertext, tmp_path) - # encrypt new data and write out to tmp - # An existing vaultfile will always be UTF-8, - # so decode to unicode here - b_ciphertext = self.vault.encrypt(b_tmpdata, secret, vault_id=vault_id) - self.write_data(b_ciphertext, tmp_path) + # shuffle tmp file into place + self.shuffle_files(tmp_path, filename) + display.vvvvv(u'Saved edited file "%s" encrypted using %s and vault id "%s"' % (to_text(filename), to_text(secret), to_text(vault_id))) - # shuffle tmp file into place - self.shuffle_files(tmp_path, filename) - display.vvvvv('Saved edited file "%s" encrypted using %s and vault id "%s"' % (filename, secret, vault_id)) + # always shred temp, jic + self._shred_file(tmp_path) def _real_path(self, filename): # '-' is special to VaultEditor, dont expand it. @@ -954,21 +963,17 @@ def edit_file(self, filename): # Figure out the vault id from the file, to select the right secret to re-encrypt it # (duplicates parts of decrypt, but alas...) - dummy, dummy, cipher_name, vault_id = parse_vaulttext_envelope(b_vaulttext, - filename=filename) + dummy, dummy, cipher_name, vault_id = parse_vaulttext_envelope(b_vaulttext, filename=filename) # vault id here may not be the vault id actually used for decrypting # as when the edited file has no vault-id but is decrypted by non-default id in secrets # (vault_id=default, while a different vault-id decrypted) + # we want to get rid of files encrypted with the AES cipher + force_save = (cipher_name not in CIPHER_WRITE_WHITELIST) + # Keep the same vault-id (and version) as in the header - if cipher_name not in CIPHER_WRITE_WHITELIST: - # we want to get rid of files encrypted with the AES cipher - self._edit_file_helper(filename, vault_secret_used, existing_data=plaintext, - force_save=True, vault_id=vault_id) - else: - self._edit_file_helper(filename, vault_secret_used, existing_data=plaintext, - force_save=False, vault_id=vault_id) + self._edit_file_helper(filename, vault_secret_used, existing_data=plaintext, force_save=force_save, vault_id=vault_id) def plaintext(self, filename): @@ -1038,8 +1043,8 @@ def read_data(self, filename): return data - # TODO: add docstrings for arg types since this code is picky about that - def write_data(self, data, filename, shred=True): + def write_data(self, data, thefile, shred=True, mode=0o600): + # TODO: add docstrings for arg types since this code is picky about that """Write the data bytes to given path This is used to write a byte string to a file or stdout. It is used for @@ -1056,28 +1061,64 @@ def write_data(self, data, filename, shred=True): should be a byte string and not a text type. :arg data: the byte string (bytes) data - :arg filename: filename to save 'data' to. + :arg thefile: file descriptor or filename to save 'data' to. :arg shred: if shred==True, make sure that the original data is first shredded so that is cannot be recovered. :returns: None """ # FIXME: do we need this now? data_bytes should always be a utf-8 byte string b_file_data = to_bytes(data, errors='strict') - # get a ref to either sys.stdout.buffer for py3 or plain old sys.stdout for py2 - # We need sys.stdout.buffer on py3 so we can write bytes to it since the plaintext - # of the vaulted object could be anything/binary/etc - output = getattr(sys.stdout, 'buffer', sys.stdout) - - if filename == '-': + # check if we have a file descriptor instead of a path + is_fd = False + try: + is_fd = (isinstance(thefile, int) and fcntl.fcntl(thefile, fcntl.F_GETFD) != -1) + except Exception: + pass + + if is_fd: + # if passed descriptor, use that to ensure secure access, otherwise it is a string. + # assumes the fd is securely opened by caller (mkstemp) + os.ftruncate(thefile, 0) + os.write(thefile, b_file_data) + elif thefile == '-': + # get a ref to either sys.stdout.buffer for py3 or plain old sys.stdout for py2 + # We need sys.stdout.buffer on py3 so we can write bytes to it since the plaintext + # of the vaulted object could be anything/binary/etc + output = getattr(sys.stdout, 'buffer', sys.stdout) output.write(b_file_data) else: - if os.path.isfile(filename): + # file names are insecure and prone to race conditions, so remove and create securely + if os.path.isfile(thefile): if shred: - self._shred_file(filename) + self._shred_file(thefile) else: - os.remove(filename) - with open(filename, "wb") as fh: - fh.write(b_file_data) + os.remove(thefile) + + # when setting new umask, we get previous as return + current_umask = os.umask(0o077) + try: + try: + # create file with secure permissions + fd = os.open(thefile, os.O_CREAT | os.O_EXCL | os.O_RDWR | os.O_TRUNC, mode) + except OSError as ose: + # Want to catch FileExistsError, which doesn't exist in Python 2, so catch OSError + # and compare the error number to get equivalent behavior in Python 2/3 + if ose.errno == errno.EEXIST: + raise AnsibleError('Vault file got recreated while we were operating on it: %s' % to_native(ose)) + + raise AnsibleError('Problem creating temporary vault file: %s' % to_native(ose)) + + try: + # now write to the file and ensure ours is only data in it + os.ftruncate(fd, 0) + os.write(fd, b_file_data) + except OSError as e: + raise AnsibleError('Unable to write to temporary vault file: %s' % to_native(e)) + finally: + # Make sure the file descriptor is always closed and reset umask + os.close(fd) + finally: + os.umask(current_umask) def shuffle_files(self, src, dest): prev = None
ef32a5bf96a8safely use vault to edit secrets (#68644)
2 files changed · +82 −39
changelogs/fragments/vault_tmp_race_fix.yml+2 −0 added@@ -0,0 +1,2 @@ +bugfixes: + - "**security_issue** - create temporary vault file with strict permissions when editing and prevent race condition (CVE-2020-1740)"
lib/ansible/parsing/vault/__init__.py+80 −39 modified@@ -19,6 +19,8 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +import errno +import fcntl import os import random import shlex @@ -27,6 +29,7 @@ import sys import tempfile import warnings + from binascii import hexlify from binascii import unhexlify from binascii import Error as BinasciiError @@ -845,42 +848,48 @@ def _shred_file(self, tmp_path): os.remove(tmp_path) - def _edit_file_helper(self, filename, secret, - existing_data=None, force_save=False, vault_id=None): + def _edit_file_helper(self, filename, secret, existing_data=None, force_save=False, vault_id=None): # Create a tempfile root, ext = os.path.splitext(os.path.realpath(filename)) fd, tmp_path = tempfile.mkstemp(suffix=ext, dir=C.DEFAULT_LOCAL_TMP) - os.close(fd) cmd = self._editor_shell_command(tmp_path) try: if existing_data: - self.write_data(existing_data, tmp_path, shred=False) + self.write_data(existing_data, fd, shred=False) + except Exception: + # if an error happens, destroy the decrypted file + self._shred_file(tmp_path) + raise + finally: + os.close(fd) + try: # drop the user into an editor on the tmp file subprocess.call(cmd) except Exception as e: - # whatever happens, destroy the decrypted file + # if an error happens, destroy the decrypted file self._shred_file(tmp_path) raise AnsibleError('Unable to execute the command "%s": %s' % (' '.join(cmd), to_native(e))) b_tmpdata = self.read_data(tmp_path) # Do nothing if the content has not changed - if existing_data == b_tmpdata and not force_save: - self._shred_file(tmp_path) - return + if force_save or existing_data != b_tmpdata: + + # encrypt new data and write out to tmp + # An existing vaultfile will always be UTF-8, + # so decode to unicode here + b_ciphertext = self.vault.encrypt(b_tmpdata, secret, vault_id=vault_id) + self.write_data(b_ciphertext, tmp_path) - # encrypt new data and write out to tmp - # An existing vaultfile will always be UTF-8, - # so decode to unicode here - b_ciphertext = self.vault.encrypt(b_tmpdata, secret, vault_id=vault_id) - self.write_data(b_ciphertext, tmp_path) + # shuffle tmp file into place + self.shuffle_files(tmp_path, filename) + display.vvvvv(u'Saved edited file "%s" encrypted using %s and vault id "%s"' % (to_text(filename), to_text(secret), to_text(vault_id))) - # shuffle tmp file into place - self.shuffle_files(tmp_path, filename) - display.vvvvv('Saved edited file "%s" encrypted using %s and vault id "%s"' % (filename, secret, vault_id)) + # always shred temp, jic + self._shred_file(tmp_path) def _real_path(self, filename): # '-' is special to VaultEditor, dont expand it. @@ -956,21 +965,17 @@ def edit_file(self, filename): # Figure out the vault id from the file, to select the right secret to re-encrypt it # (duplicates parts of decrypt, but alas...) - dummy, dummy, cipher_name, vault_id = parse_vaulttext_envelope(b_vaulttext, - filename=filename) + dummy, dummy, cipher_name, vault_id = parse_vaulttext_envelope(b_vaulttext, filename=filename) # vault id here may not be the vault id actually used for decrypting # as when the edited file has no vault-id but is decrypted by non-default id in secrets # (vault_id=default, while a different vault-id decrypted) + # we want to get rid of files encrypted with the AES cipher + force_save = (cipher_name not in CIPHER_WRITE_WHITELIST) + # Keep the same vault-id (and version) as in the header - if cipher_name not in CIPHER_WRITE_WHITELIST: - # we want to get rid of files encrypted with the AES cipher - self._edit_file_helper(filename, vault_secret_used, existing_data=plaintext, - force_save=True, vault_id=vault_id) - else: - self._edit_file_helper(filename, vault_secret_used, existing_data=plaintext, - force_save=False, vault_id=vault_id) + self._edit_file_helper(filename, vault_secret_used, existing_data=plaintext, force_save=force_save, vault_id=vault_id) def plaintext(self, filename): @@ -1037,8 +1042,8 @@ def read_data(self, filename): return data - # TODO: add docstrings for arg types since this code is picky about that - def write_data(self, data, filename, shred=True): + def write_data(self, data, thefile, shred=True, mode=0o600): + # TODO: add docstrings for arg types since this code is picky about that """Write the data bytes to given path This is used to write a byte string to a file or stdout. It is used for @@ -1055,28 +1060,64 @@ def write_data(self, data, filename, shred=True): should be a byte string and not a text type. :arg data: the byte string (bytes) data - :arg filename: filename to save 'data' to. + :arg thefile: file descriptor or filename to save 'data' to. :arg shred: if shred==True, make sure that the original data is first shredded so that is cannot be recovered. :returns: None """ # FIXME: do we need this now? data_bytes should always be a utf-8 byte string b_file_data = to_bytes(data, errors='strict') - # get a ref to either sys.stdout.buffer for py3 or plain old sys.stdout for py2 - # We need sys.stdout.buffer on py3 so we can write bytes to it since the plaintext - # of the vaulted object could be anything/binary/etc - output = getattr(sys.stdout, 'buffer', sys.stdout) - - if filename == '-': + # check if we have a file descriptor instead of a path + is_fd = False + try: + is_fd = (isinstance(thefile, int) and fcntl.fcntl(thefile, fcntl.F_GETFD) != -1) + except Exception: + pass + + if is_fd: + # if passed descriptor, use that to ensure secure access, otherwise it is a string. + # assumes the fd is securely opened by caller (mkstemp) + os.ftruncate(thefile, 0) + os.write(thefile, b_file_data) + elif thefile == '-': + # get a ref to either sys.stdout.buffer for py3 or plain old sys.stdout for py2 + # We need sys.stdout.buffer on py3 so we can write bytes to it since the plaintext + # of the vaulted object could be anything/binary/etc + output = getattr(sys.stdout, 'buffer', sys.stdout) output.write(b_file_data) else: - if os.path.isfile(filename): + # file names are insecure and prone to race conditions, so remove and create securely + if os.path.isfile(thefile): if shred: - self._shred_file(filename) + self._shred_file(thefile) else: - os.remove(filename) - with open(filename, "wb") as fh: - fh.write(b_file_data) + os.remove(thefile) + + # when setting new umask, we get previous as return + current_umask = os.umask(0o077) + try: + try: + # create file with secure permissions + fd = os.open(thefile, os.O_CREAT | os.O_EXCL | os.O_RDWR | os.O_TRUNC, mode) + except OSError as ose: + # Want to catch FileExistsError, which doesn't exist in Python 2, so catch OSError + # and compare the error number to get equivalent behavior in Python 2/3 + if ose.errno == errno.EEXIST: + raise AnsibleError('Vault file got recreated while we were operating on it: %s' % to_native(ose)) + + raise AnsibleError('Problem creating temporary vault file: %s' % to_native(ose)) + + try: + # now write to the file and ensure ours is only data in it + os.ftruncate(fd, 0) + os.write(fd, b_file_data) + except OSError as e: + raise AnsibleError('Unable to write to temporary vault file: %s' % to_native(e)) + finally: + # Make sure the file descriptor is always closed and reset umask + os.close(fd) + finally: + os.umask(current_umask) def shuffle_files(self, src, dest): prev = None
685a4b6d3ff7safely use vault to edit secrets (#68644)
2 files changed · +82 −39
changelogs/fragments/vault_tmp_race_fix.yml+2 −0 added@@ -0,0 +1,2 @@ +bugfixes: + - "**security_issue** - create temporary vault file with strict permissions when editing and prevent race condition (CVE-2020-1740)"
lib/ansible/parsing/vault/__init__.py+80 −39 modified@@ -19,6 +19,8 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +import errno +import fcntl import os import random import shlex @@ -27,6 +29,7 @@ import sys import tempfile import warnings + from binascii import hexlify from binascii import unhexlify from binascii import Error as BinasciiError @@ -845,42 +848,48 @@ def _shred_file(self, tmp_path): os.remove(tmp_path) - def _edit_file_helper(self, filename, secret, - existing_data=None, force_save=False, vault_id=None): + def _edit_file_helper(self, filename, secret, existing_data=None, force_save=False, vault_id=None): # Create a tempfile root, ext = os.path.splitext(os.path.realpath(filename)) fd, tmp_path = tempfile.mkstemp(suffix=ext, dir=C.DEFAULT_LOCAL_TMP) - os.close(fd) cmd = self._editor_shell_command(tmp_path) try: if existing_data: - self.write_data(existing_data, tmp_path, shred=False) + self.write_data(existing_data, fd, shred=False) + except Exception: + # if an error happens, destroy the decrypted file + self._shred_file(tmp_path) + raise + finally: + os.close(fd) + try: # drop the user into an editor on the tmp file subprocess.call(cmd) except Exception as e: - # whatever happens, destroy the decrypted file + # if an error happens, destroy the decrypted file self._shred_file(tmp_path) raise AnsibleError('Unable to execute the command "%s": %s' % (' '.join(cmd), to_native(e))) b_tmpdata = self.read_data(tmp_path) # Do nothing if the content has not changed - if existing_data == b_tmpdata and not force_save: - self._shred_file(tmp_path) - return + if force_save or existing_data != b_tmpdata: + + # encrypt new data and write out to tmp + # An existing vaultfile will always be UTF-8, + # so decode to unicode here + b_ciphertext = self.vault.encrypt(b_tmpdata, secret, vault_id=vault_id) + self.write_data(b_ciphertext, tmp_path) - # encrypt new data and write out to tmp - # An existing vaultfile will always be UTF-8, - # so decode to unicode here - b_ciphertext = self.vault.encrypt(b_tmpdata, secret, vault_id=vault_id) - self.write_data(b_ciphertext, tmp_path) + # shuffle tmp file into place + self.shuffle_files(tmp_path, filename) + display.vvvvv(u'Saved edited file "%s" encrypted using %s and vault id "%s"' % (to_text(filename), to_text(secret), to_text(vault_id))) - # shuffle tmp file into place - self.shuffle_files(tmp_path, filename) - display.vvvvv(u'Saved edited file "%s" encrypted using %s and vault id "%s"' % (to_text(filename), to_text(secret), to_text(vault_id))) + # always shred temp, jic + self._shred_file(tmp_path) def _real_path(self, filename): # '-' is special to VaultEditor, dont expand it. @@ -956,21 +965,17 @@ def edit_file(self, filename): # Figure out the vault id from the file, to select the right secret to re-encrypt it # (duplicates parts of decrypt, but alas...) - dummy, dummy, cipher_name, vault_id = parse_vaulttext_envelope(b_vaulttext, - filename=filename) + dummy, dummy, cipher_name, vault_id = parse_vaulttext_envelope(b_vaulttext, filename=filename) # vault id here may not be the vault id actually used for decrypting # as when the edited file has no vault-id but is decrypted by non-default id in secrets # (vault_id=default, while a different vault-id decrypted) + # we want to get rid of files encrypted with the AES cipher + force_save = (cipher_name not in CIPHER_WRITE_WHITELIST) + # Keep the same vault-id (and version) as in the header - if cipher_name not in CIPHER_WRITE_WHITELIST: - # we want to get rid of files encrypted with the AES cipher - self._edit_file_helper(filename, vault_secret_used, existing_data=plaintext, - force_save=True, vault_id=vault_id) - else: - self._edit_file_helper(filename, vault_secret_used, existing_data=plaintext, - force_save=False, vault_id=vault_id) + self._edit_file_helper(filename, vault_secret_used, existing_data=plaintext, force_save=force_save, vault_id=vault_id) def plaintext(self, filename): @@ -1040,8 +1045,8 @@ def read_data(self, filename): return data - # TODO: add docstrings for arg types since this code is picky about that - def write_data(self, data, filename, shred=True): + def write_data(self, data, thefile, shred=True, mode=0o600): + # TODO: add docstrings for arg types since this code is picky about that """Write the data bytes to given path This is used to write a byte string to a file or stdout. It is used for @@ -1058,28 +1063,64 @@ def write_data(self, data, filename, shred=True): should be a byte string and not a text type. :arg data: the byte string (bytes) data - :arg filename: filename to save 'data' to. + :arg thefile: file descriptor or filename to save 'data' to. :arg shred: if shred==True, make sure that the original data is first shredded so that is cannot be recovered. :returns: None """ # FIXME: do we need this now? data_bytes should always be a utf-8 byte string b_file_data = to_bytes(data, errors='strict') - # get a ref to either sys.stdout.buffer for py3 or plain old sys.stdout for py2 - # We need sys.stdout.buffer on py3 so we can write bytes to it since the plaintext - # of the vaulted object could be anything/binary/etc - output = getattr(sys.stdout, 'buffer', sys.stdout) - - if filename == '-': + # check if we have a file descriptor instead of a path + is_fd = False + try: + is_fd = (isinstance(thefile, int) and fcntl.fcntl(thefile, fcntl.F_GETFD) != -1) + except Exception: + pass + + if is_fd: + # if passed descriptor, use that to ensure secure access, otherwise it is a string. + # assumes the fd is securely opened by caller (mkstemp) + os.ftruncate(thefile, 0) + os.write(thefile, b_file_data) + elif thefile == '-': + # get a ref to either sys.stdout.buffer for py3 or plain old sys.stdout for py2 + # We need sys.stdout.buffer on py3 so we can write bytes to it since the plaintext + # of the vaulted object could be anything/binary/etc + output = getattr(sys.stdout, 'buffer', sys.stdout) output.write(b_file_data) else: - if os.path.isfile(filename): + # file names are insecure and prone to race conditions, so remove and create securely + if os.path.isfile(thefile): if shred: - self._shred_file(filename) + self._shred_file(thefile) else: - os.remove(filename) - with open(filename, "wb") as fh: - fh.write(b_file_data) + os.remove(thefile) + + # when setting new umask, we get previous as return + current_umask = os.umask(0o077) + try: + try: + # create file with secure permissions + fd = os.open(thefile, os.O_CREAT | os.O_EXCL | os.O_RDWR | os.O_TRUNC, mode) + except OSError as ose: + # Want to catch FileExistsError, which doesn't exist in Python 2, so catch OSError + # and compare the error number to get equivalent behavior in Python 2/3 + if ose.errno == errno.EEXIST: + raise AnsibleError('Vault file got recreated while we were operating on it: %s' % to_native(ose)) + + raise AnsibleError('Problem creating temporary vault file: %s' % to_native(ose)) + + try: + # now write to the file and ensure ours is only data in it + os.ftruncate(fd, 0) + os.write(fd, b_file_data) + except OSError as e: + raise AnsibleError('Unable to write to temporary vault file: %s' % to_native(e)) + finally: + # Make sure the file descriptor is always closed and reset umask + os.close(fd) + finally: + os.umask(current_umask) def shuffle_files(self, src, dest): prev = None
28f9fbdb5e28safely use vault to edit secrets (#68644)
2 files changed · +82 −39
changelogs/fragments/vault_tmp_race_fix.yml+2 −0 added@@ -0,0 +1,2 @@ +bugfixes: + - "**security_issue** - create temporary vault file with strict permissions when editing and prevent race condition (CVE-2020-1740)"
lib/ansible/parsing/vault/__init__.py+80 −39 modified@@ -19,6 +19,8 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +import errno +import fcntl import os import random import shlex @@ -27,6 +29,7 @@ import sys import tempfile import warnings + from binascii import hexlify from binascii import unhexlify from binascii import Error as BinasciiError @@ -845,42 +848,48 @@ def _shred_file(self, tmp_path): os.remove(tmp_path) - def _edit_file_helper(self, filename, secret, - existing_data=None, force_save=False, vault_id=None): + def _edit_file_helper(self, filename, secret, existing_data=None, force_save=False, vault_id=None): # Create a tempfile root, ext = os.path.splitext(os.path.realpath(filename)) fd, tmp_path = tempfile.mkstemp(suffix=ext, dir=C.DEFAULT_LOCAL_TMP) - os.close(fd) cmd = self._editor_shell_command(tmp_path) try: if existing_data: - self.write_data(existing_data, tmp_path, shred=False) + self.write_data(existing_data, fd, shred=False) + except Exception: + # if an error happens, destroy the decrypted file + self._shred_file(tmp_path) + raise + finally: + os.close(fd) + try: # drop the user into an editor on the tmp file subprocess.call(cmd) except Exception as e: - # whatever happens, destroy the decrypted file + # if an error happens, destroy the decrypted file self._shred_file(tmp_path) raise AnsibleError('Unable to execute the command "%s": %s' % (' '.join(cmd), to_native(e))) b_tmpdata = self.read_data(tmp_path) # Do nothing if the content has not changed - if existing_data == b_tmpdata and not force_save: - self._shred_file(tmp_path) - return + if force_save or existing_data != b_tmpdata: + + # encrypt new data and write out to tmp + # An existing vaultfile will always be UTF-8, + # so decode to unicode here + b_ciphertext = self.vault.encrypt(b_tmpdata, secret, vault_id=vault_id) + self.write_data(b_ciphertext, tmp_path) - # encrypt new data and write out to tmp - # An existing vaultfile will always be UTF-8, - # so decode to unicode here - b_ciphertext = self.vault.encrypt(b_tmpdata, secret, vault_id=vault_id) - self.write_data(b_ciphertext, tmp_path) + # shuffle tmp file into place + self.shuffle_files(tmp_path, filename) + display.vvvvv(u'Saved edited file "%s" encrypted using %s and vault id "%s"' % (to_text(filename), to_text(secret), to_text(vault_id))) - # shuffle tmp file into place - self.shuffle_files(tmp_path, filename) - display.vvvvv(u'Saved edited file "%s" encrypted using %s and vault id "%s"' % (to_text(filename), to_text(secret), to_text(vault_id))) + # always shred temp, jic + self._shred_file(tmp_path) def _real_path(self, filename): # '-' is special to VaultEditor, dont expand it. @@ -956,21 +965,17 @@ def edit_file(self, filename): # Figure out the vault id from the file, to select the right secret to re-encrypt it # (duplicates parts of decrypt, but alas...) - dummy, dummy, cipher_name, vault_id = parse_vaulttext_envelope(b_vaulttext, - filename=filename) + dummy, dummy, cipher_name, vault_id = parse_vaulttext_envelope(b_vaulttext, filename=filename) # vault id here may not be the vault id actually used for decrypting # as when the edited file has no vault-id but is decrypted by non-default id in secrets # (vault_id=default, while a different vault-id decrypted) + # we want to get rid of files encrypted with the AES cipher + force_save = (cipher_name not in CIPHER_WRITE_WHITELIST) + # Keep the same vault-id (and version) as in the header - if cipher_name not in CIPHER_WRITE_WHITELIST: - # we want to get rid of files encrypted with the AES cipher - self._edit_file_helper(filename, vault_secret_used, existing_data=plaintext, - force_save=True, vault_id=vault_id) - else: - self._edit_file_helper(filename, vault_secret_used, existing_data=plaintext, - force_save=False, vault_id=vault_id) + self._edit_file_helper(filename, vault_secret_used, existing_data=plaintext, force_save=force_save, vault_id=vault_id) def plaintext(self, filename): @@ -1040,8 +1045,8 @@ def read_data(self, filename): return data - # TODO: add docstrings for arg types since this code is picky about that - def write_data(self, data, filename, shred=True): + def write_data(self, data, thefile, shred=True, mode=0o600): + # TODO: add docstrings for arg types since this code is picky about that """Write the data bytes to given path This is used to write a byte string to a file or stdout. It is used for @@ -1058,28 +1063,64 @@ def write_data(self, data, filename, shred=True): should be a byte string and not a text type. :arg data: the byte string (bytes) data - :arg filename: filename to save 'data' to. + :arg thefile: file descriptor or filename to save 'data' to. :arg shred: if shred==True, make sure that the original data is first shredded so that is cannot be recovered. :returns: None """ # FIXME: do we need this now? data_bytes should always be a utf-8 byte string b_file_data = to_bytes(data, errors='strict') - # get a ref to either sys.stdout.buffer for py3 or plain old sys.stdout for py2 - # We need sys.stdout.buffer on py3 so we can write bytes to it since the plaintext - # of the vaulted object could be anything/binary/etc - output = getattr(sys.stdout, 'buffer', sys.stdout) - - if filename == '-': + # check if we have a file descriptor instead of a path + is_fd = False + try: + is_fd = (isinstance(thefile, int) and fcntl.fcntl(thefile, fcntl.F_GETFD) != -1) + except Exception: + pass + + if is_fd: + # if passed descriptor, use that to ensure secure access, otherwise it is a string. + # assumes the fd is securely opened by caller (mkstemp) + os.ftruncate(thefile, 0) + os.write(thefile, b_file_data) + elif thefile == '-': + # get a ref to either sys.stdout.buffer for py3 or plain old sys.stdout for py2 + # We need sys.stdout.buffer on py3 so we can write bytes to it since the plaintext + # of the vaulted object could be anything/binary/etc + output = getattr(sys.stdout, 'buffer', sys.stdout) output.write(b_file_data) else: - if os.path.isfile(filename): + # file names are insecure and prone to race conditions, so remove and create securely + if os.path.isfile(thefile): if shred: - self._shred_file(filename) + self._shred_file(thefile) else: - os.remove(filename) - with open(filename, "wb") as fh: - fh.write(b_file_data) + os.remove(thefile) + + # when setting new umask, we get previous as return + current_umask = os.umask(0o077) + try: + try: + # create file with secure permissions + fd = os.open(thefile, os.O_CREAT | os.O_EXCL | os.O_RDWR | os.O_TRUNC, mode) + except OSError as ose: + # Want to catch FileExistsError, which doesn't exist in Python 2, so catch OSError + # and compare the error number to get equivalent behavior in Python 2/3 + if ose.errno == errno.EEXIST: + raise AnsibleError('Vault file got recreated while we were operating on it: %s' % to_native(ose)) + + raise AnsibleError('Problem creating temporary vault file: %s' % to_native(ose)) + + try: + # now write to the file and ensure ours is only data in it + os.ftruncate(fd, 0) + os.write(fd, b_file_data) + except OSError as e: + raise AnsibleError('Unable to write to temporary vault file: %s' % to_native(e)) + finally: + # Make sure the file descriptor is always closed and reset umask + os.close(fd) + finally: + os.umask(current_umask) def shuffle_files(self, src, dest): prev = None
Vulnerability mechanics
Generated by null/stub on May 9, 2026. Inputs: CWE entries + fix-commit diffs from this CVE's patches. Citations validated against bundle.
References
18- github.com/advisories/GHSA-vcg8-98q8-g7mjghsaADVISORY
- lists.fedoraproject.org/archives/list/package-announce%40lists.fedoraproject.org/message/DKPA4KC3OJSUFASUYMG66HKJE7ADNGFW/mitrevendor-advisoryx_refsource_FEDORA
- lists.fedoraproject.org/archives/list/package-announce%40lists.fedoraproject.org/message/MRRYUU5ZBLPBXCYG6CFP35D64NP2UB2S/mitrevendor-advisoryx_refsource_FEDORA
- lists.fedoraproject.org/archives/list/package-announce%40lists.fedoraproject.org/message/WQVOQD4VAIXXTVQAJKTN7NUGTJFE2PCB/mitrevendor-advisoryx_refsource_FEDORA
- nvd.nist.gov/vuln/detail/CVE-2020-1740ghsaADVISORY
- security.gentoo.org/glsa/202006-11ghsavendor-advisoryx_refsource_GENTOOWEB
- www.debian.org/security/2021/dsa-4950mitrevendor-advisoryx_refsource_DEBIAN
- bugzilla.redhat.com/show_bug.cgighsax_refsource_CONFIRMWEB
- github.com/ansible/ansible/commit/28f9fbdb5e281976e33f443193047068afb97a9bghsaWEB
- github.com/ansible/ansible/commit/2a563514f070a0a8ba64aebf6bce21194be96c73ghsaWEB
- github.com/ansible/ansible/commit/685a4b6d3ff72186d2b4ffce73172a5446a71cccghsaWEB
- github.com/ansible/ansible/commit/ef32a5bf96a89107986375516285253c1380d7efghsaWEB
- github.com/ansible/ansible/issues/67798ghsax_refsource_CONFIRMWEB
- github.com/pypa/advisory-database/tree/main/vulns/ansible/PYSEC-2020-12.yamlghsaWEB
- lists.debian.org/debian-lts-announce/2020/05/msg00005.htmlghsamailing-listx_refsource_MLISTWEB
- lists.fedoraproject.org/archives/list/package-announce@lists.fedoraproject.org/message/DKPA4KC3OJSUFASUYMG66HKJE7ADNGFWghsaWEB
- lists.fedoraproject.org/archives/list/package-announce@lists.fedoraproject.org/message/MRRYUU5ZBLPBXCYG6CFP35D64NP2UB2SghsaWEB
- lists.fedoraproject.org/archives/list/package-announce@lists.fedoraproject.org/message/WQVOQD4VAIXXTVQAJKTN7NUGTJFE2PCBghsaWEB
News mentions
0No linked articles in our index yet.