From a14d0f3586617f678b2843a25b521243a99cd589 Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Tue, 26 Sep 2017 12:28:31 -0400 Subject: [PATCH] Use vault_id when encrypted via vault-edit (#30772) * Use vault_id when encrypted via vault-edit On the encryption stage of 'ansible-vault edit --vault-id=someid@passfile somefile', the vault id was not being passed to encrypt() so the files were always saved with the default vault id in the 1.1 version format. When trying to edit that file a second time, also with a --vault-id, the file would be decrypted with the secret associated with the provided vault-id, but since the encrypted file had no vault id in the envelope there would be no match for 'default' secrets. (Only the --vault-id was included in the potential matches, so the vault id actually used to decrypt was not). If that list was empty, there would be an IndexError when trying to encrypted the changed file. This would result in the displayed error: ERROR! Unexpected Exception, this is probably a bug: list index out of range Fix is two parts: 1) use the vault id when encrypting from edit 2) when matching the secret to use for encrypting after edit, include the vault id that was used for decryption and not just the vault id (or lack of vault id) from the envelope. add unit tests for #30575 and intg tests for 'ansible-vault edit' Fixes #30575 --- lib/ansible/parsing/vault/__init__.py | 43 +++++++++++++++--- test/integration/targets/vault/faux-editor.py | 44 +++++++++++++++++++ test/integration/targets/vault/runme.sh | 28 ++++++++++++ test/units/parsing/vault/test_vault_editor.py | 32 +++++++++++++- 4 files changed, 139 insertions(+), 8 deletions(-) create mode 100755 test/integration/targets/vault/faux-editor.py diff --git a/lib/ansible/parsing/vault/__init__.py b/lib/ansible/parsing/vault/__init__.py index e02587e63d..ef08e602fb 100644 --- a/lib/ansible/parsing/vault/__init__.py +++ b/lib/ansible/parsing/vault/__init__.py @@ -491,6 +491,20 @@ class VaultLib: return b_vaulttext def decrypt(self, vaulttext, filename=None): + '''Decrypt a piece of vault encrypted data. + + :arg vaulttext: a string to decrypt. Since vault encrypted data is an + ascii text format this can be either a byte str or unicode string. + :kwarg filename: a filename that the data came from. This is only + used to make better error messages in case the data cannot be + decrypted. + :returns: a byte string containing the decrypted data and the vault-id that was used + + ''' + plaintext, vault_id = self.decrypt_and_get_vault_id(vaulttext, filename=filename) + return plaintext + + def decrypt_and_get_vault_id(self, vaulttext, filename=None): """Decrypt a piece of vault encrypted data. :arg vaulttext: a string to decrypt. Since vault encrypted data is an @@ -498,7 +512,8 @@ class VaultLib: :kwarg filename: a filename that the data came from. This is only used to make better error messages in case the data cannot be decrypted. - :returns: a byte string containing the decrypted data + :returns: a byte string containing the decrypted data and the vault-id that was used + """ b_vaulttext = to_bytes(vaulttext, errors='strict', encoding='utf-8') @@ -536,6 +551,7 @@ class VaultLib: # we check it first. vault_id_matchers = [] + vault_id_used = None if vault_id: display.vvvvv('Found a vault_id (%s) in the vaulttext' % (vault_id)) @@ -563,6 +579,7 @@ class VaultLib: display.vvvv('Trying secret %s for vault_id=%s' % (vault_secret, vault_secret_id)) b_plaintext = this_cipher.decrypt(b_vaulttext, vault_secret) if b_plaintext is not None: + vault_id_used = vault_secret_id display.vvvvv('decrypt succesful with secret=%s and vault_id=%s' % (vault_secret, vault_secret_id)) break except AnsibleError as e: @@ -581,7 +598,7 @@ class VaultLib: msg += " on %s" % filename raise AnsibleError(msg) - return b_plaintext + return b_plaintext, vault_id_used class VaultEditor: @@ -692,6 +709,7 @@ class VaultEditor: # 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)) def _real_path(self, filename): # '-' is special to VaultEditor, dont expand it. @@ -754,7 +772,8 @@ class VaultEditor: try: # vaulttext gets converted back to bytes, but alas - plaintext = self.vault.decrypt(vaulttext) + # TODO: return the vault_id that worked? + plaintext, vault_id_used = self.vault.decrypt_and_get_vault_id(vaulttext) except AnsibleError as e: raise AnsibleError("%s for %s" % (to_bytes(e), to_bytes(filename))) @@ -762,15 +781,25 @@ class VaultEditor: # (duplicates parts of decrypt, but alas...) dummy, dummy, cipher_name, vault_id = parse_vaulttext_envelope(b_vaulttext) - # if we could decrypt, the vault_id should be in secrets + # 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) + + # if we could decrypt, the vault_id should be in secrets or we use vault_id_used # though we could have multiple secrets for a given vault_id, pick the first one - secrets = match_secrets(self.vault.secrets, [vault_id]) + secrets = match_secrets(self.vault.secrets, [vault_id_used, vault_id]) + + if not secrets: + raise AnsibleVaultError('Attempting to encrypt "%s" but no vault secrets were found for vault ids "%s" or "%s"' % + (filename, vault_id, vault_id_used)) + secret = secrets[0][1] + 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, secret, existing_data=plaintext, force_save=True) + self._edit_file_helper(filename, secret, existing_data=plaintext, force_save=True, vault_id=vault_id) else: - self._edit_file_helper(filename, secret, existing_data=plaintext, force_save=False) + self._edit_file_helper(filename, secret, existing_data=plaintext, force_save=False, vault_id=vault_id) def plaintext(self, filename): diff --git a/test/integration/targets/vault/faux-editor.py b/test/integration/targets/vault/faux-editor.py new file mode 100755 index 0000000000..7f9983c07c --- /dev/null +++ b/test/integration/targets/vault/faux-editor.py @@ -0,0 +1,44 @@ +#!/usr/bin/env python +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . +# +# ansible-vault is a script that encrypts/decrypts YAML files. See +# http://docs.ansible.com/playbooks_vault.html for more details. + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import sys +import time +import os + + +def main(args): + path = os.path.abspath(args[1]) + + fo = open(path, 'r+') + + content = fo.readlines() + + content.append('faux editor added at %s\n' % time.time()) + + fo.seek(0) + fo.write(''.join(content)) + fo.close() + + return 0 + + +if __name__ == '__main__': + sys.exit(main(sys.argv[:])) diff --git a/test/integration/targets/vault/runme.sh b/test/integration/targets/vault/runme.sh index 2d4db75267..4adfa83fbc 100755 --- a/test/integration/targets/vault/runme.sh +++ b/test/integration/targets/vault/runme.sh @@ -14,7 +14,14 @@ echo "This is a test file for format 1.2" > "${TEST_FILE_1_2}" TEST_FILE_OUTPUT="${MYTMPDIR}/test_file_output" +TEST_FILE_EDIT="${MYTMPDIR}/test_file_edit" +echo "This is a test file for edit" > "${TEST_FILE_EDIT}" +TEST_FILE_EDIT2="${MYTMPDIR}/test_file_edit2" +echo "This is a test file for edit2" > "${TEST_FILE_EDIT2}" + +FORMAT_1_1_HEADER="\$ANSIBLE_VAULT;1.1;AES256" +FORMAT_1_2_HEADER="\$ANSIBLE_VAULT;1.2;AES256" # old format ansible-vault view "$@" --vault-password-file vault-password-ansible format_1_0_AES.yml @@ -234,6 +241,27 @@ ansible-vault encrypt_string "$@" --vault-password-file "${NEW_VAULT_PASSWORD}" # write to file ansible-vault encrypt_string "$@" --vault-password-file "${NEW_VAULT_PASSWORD}" --name "blippy" "a test string names blippy" --output "${MYTMPDIR}/enc_string_test_file" +# test ansible-vault edit with a faux editor +ansible-vault encrypt "$@" --vault-password-file vault-password "${TEST_FILE_EDIT}" + +# edit a 1.1 format with no vault-id, should stay 1.1 +EDITOR=./faux-editor.py ansible-vault edit "$@" --vault-password-file vault-password "${TEST_FILE_EDIT}" +head -1 "${TEST_FILE_EDIT}" | grep "${FORMAT_1_1_HEADER}" + +# edit a 1.1 format with vault-id, should stay 1.1 +EDITOR=./faux-editor.py ansible-vault edit "$@" --vault-id vault_password@vault-password "${TEST_FILE_EDIT}" +head -1 "${TEST_FILE_EDIT}" | grep "${FORMAT_1_1_HEADER}" + +ansible-vault encrypt "$@" --vault-id vault_password@vault-password "${TEST_FILE_EDIT2}" + +# edit a 1.2 format with vault id, should keep vault id and 1.2 format +EDITOR=./faux-editor.py ansible-vault edit "$@" --vault-id vault_password@vault-password "${TEST_FILE_EDIT2}" +head -1 "${TEST_FILE_EDIT2}" | grep "${FORMAT_1_2_HEADER};vault_password" + +# edit a 1.2 file with no vault-id, should keep vault id and 1.2 format +EDITOR=./faux-editor.py ansible-vault edit "$@" --vault-password-file vault-password "${TEST_FILE_EDIT2}" +head -1 "${TEST_FILE_EDIT2}" | grep "${FORMAT_1_2_HEADER};vault_password" + # test playbooks using vaulted files ansible-playbook test_vault.yml -i ../../inventory -v "$@" --vault-password-file vault-password --list-tasks diff --git a/test/units/parsing/vault/test_vault_editor.py b/test/units/parsing/vault/test_vault_editor.py index a7318505e0..b416bb2c84 100644 --- a/test/units/parsing/vault/test_vault_editor.py +++ b/test/units/parsing/vault/test_vault_editor.py @@ -309,7 +309,7 @@ class TestVaultEditor(unittest.TestCase): self._assert_file_is_link(src_file_link_path, src_file_path) @patch('ansible.parsing.vault.subprocess.call') - def test_edit_file(self, mock_sp_call): + def test_edit_file_no_vault_id(self, mock_sp_call): self._test_dir = self._create_test_dir() src_contents = to_bytes("some info in a file\nyup.") @@ -330,6 +330,36 @@ class TestVaultEditor(unittest.TestCase): new_src_file = open(src_file_path, 'rb') new_src_file_contents = new_src_file.read() + self.assertTrue(b'$ANSIBLE_VAULT;1.1;AES256' in new_src_file_contents) + + src_file_plaintext = ve.vault.decrypt(new_src_file_contents) + self.assertEqual(src_file_plaintext, new_src_contents) + + @patch('ansible.parsing.vault.subprocess.call') + def test_edit_file_with_vault_id(self, mock_sp_call): + self._test_dir = self._create_test_dir() + src_contents = to_bytes("some info in a file\nyup.") + + src_file_path = self._create_file(self._test_dir, 'src_file', content=src_contents) + + new_src_contents = to_bytes("The info is different now.") + + def faux_editor(editor_args): + self._faux_editor(editor_args, new_src_contents) + + mock_sp_call.side_effect = faux_editor + + ve = self._vault_editor() + + ve.encrypt_file(src_file_path, self.vault_secret, + vault_id='vault_secrets') + ve.edit_file(src_file_path) + + new_src_file = open(src_file_path, 'rb') + new_src_file_contents = new_src_file.read() + + self.assertTrue(b'$ANSIBLE_VAULT;1.2;AES256;vault_secrets' in new_src_file_contents) + src_file_plaintext = ve.vault.decrypt(new_src_file_contents) self.assertEqual(src_file_plaintext, new_src_contents)