From 6e737c8cb66df1500dba1c74369314dd8f65867c Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Tue, 27 Mar 2018 14:12:21 -0400 Subject: [PATCH] Fix 'New Vault password' on vault 'edit' (#35923) * Fix 'New Vault password' on vault 'edit' ffe0ddea96bbe8ac27af816e58667c212e74688e introduce a change on 'ansible-vault edit' that tried to check for --encrypt-vault-id in that mode. But '--encrypt-vault-id' is not intended for 'edit' since the 'edit' should always reuse the vault secret that was used to decrypt the text. Change cli to not check for --encrypt-vault-id on 'edit'. VaultLib.decrypt_and_get_vault_id() was change to return the vault secret used to decrypt (in addition to vault_id and the plaintext). VaultEditor.edit_file() will now use 'vault_secret_used' as returned from decrypt_and_get_vault_id() so that an edited file always gets reencrypted with the same secret, regardless of any vault id configuration or cli options. Fixes #35834 --- lib/ansible/cli/vault.py | 4 ++-- lib/ansible/parsing/vault/__init__.py | 32 +++++++++++-------------- test/integration/targets/vault/runme.sh | 27 +++++++++++++++++++++ test/units/parsing/vault/test_vault.py | 20 ++++++++++++++++ 4 files changed, 63 insertions(+), 20 deletions(-) diff --git a/lib/ansible/cli/vault.py b/lib/ansible/cli/vault.py index 4b7188384b..cbc56b826c 100644 --- a/lib/ansible/cli/vault.py +++ b/lib/ansible/cli/vault.py @@ -107,7 +107,7 @@ class VaultCLI(CLI): self.parser.set_usage("usage: %prog rekey [options] file_name") # For encrypting actions, we can also specify which of multiple vault ids should be used for encrypting - if self.action in ['create', 'encrypt', 'encrypt_string', 'rekey']: + if self.action in ['create', 'encrypt', 'encrypt_string', 'rekey', 'edit']: self.parser.add_option('--encrypt-vault-id', default=[], dest='encrypt_vault_id', action='store', type='string', help='the vault id used to encrypt (required if more than vault-id is provided)') @@ -181,7 +181,7 @@ class VaultCLI(CLI): if not vault_secrets: raise AnsibleOptionsError("A vault password is required to use Ansible's Vault") - if self.action in ['encrypt', 'encrypt_string', 'create', 'edit']: + if self.action in ['encrypt', 'encrypt_string', 'create']: encrypt_vault_id = None # no --encrypt-vault-id self.options.encrypt_vault_id for 'edit' diff --git a/lib/ansible/parsing/vault/__init__.py b/lib/ansible/parsing/vault/__init__.py index cca3164763..7a68166d13 100644 --- a/lib/ansible/parsing/vault/__init__.py +++ b/lib/ansible/parsing/vault/__init__.py @@ -657,7 +657,7 @@ class VaultLib: :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) + plaintext, vault_id, vault_secret = self.decrypt_and_get_vault_id(vaulttext, filename=filename) return plaintext def decrypt_and_get_vault_id(self, vaulttext, filename=None): @@ -668,7 +668,7 @@ 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 and the vault-id that was used + :returns: a byte string containing the decrypted data and the vault-id vault-secret that was used """ b_vaulttext = to_bytes(vaulttext, errors='strict', encoding='utf-8') @@ -709,6 +709,7 @@ class VaultLib: vault_id_matchers = [] vault_id_used = None + vault_secret_used = None if vault_id: display.vvvvv('Found a vault_id (%s) in the vaulttext' % (vault_id)) @@ -737,6 +738,7 @@ class VaultLib: b_plaintext = this_cipher.decrypt(b_vaulttext, vault_secret) if b_plaintext is not None: vault_id_used = vault_secret_id + vault_secret_used = vault_secret file_slug = '' if filename: file_slug = ' of "%s"' % filename @@ -765,7 +767,7 @@ class VaultLib: msg += " on %s" % to_native(filename) raise AnsibleError(msg) - return b_plaintext, vault_id_used + return b_plaintext, vault_id_used, vault_secret_used class VaultEditor: @@ -931,7 +933,8 @@ class VaultEditor: self._edit_file_helper(filename, secret, vault_id=vault_id) def edit_file(self, filename): - + vault_id_used = None + vault_secret_used = None # follow the symlink filename = self._real_path(filename) @@ -943,7 +946,7 @@ class VaultEditor: try: # vaulttext gets converted back to bytes, but alas # TODO: return the vault_id that worked? - plaintext, vault_id_used = self.vault.decrypt_and_get_vault_id(vaulttext) + plaintext, vault_id_used, vault_secret_used = self.vault.decrypt_and_get_vault_id(vaulttext) except AnsibleError as e: raise AnsibleError("%s for %s" % (to_bytes(e), to_bytes(filename))) @@ -956,21 +959,14 @@ class VaultEditor: # 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_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] - + # 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, secret, existing_data=plaintext, force_save=True, vault_id=vault_id) + self._edit_file_helper(filename, vault_secret_used, existing_data=plaintext, + force_save=True, vault_id=vault_id) else: - self._edit_file_helper(filename, secret, existing_data=plaintext, force_save=False, vault_id=vault_id) + self._edit_file_helper(filename, vault_secret_used, existing_data=plaintext, + force_save=False, vault_id=vault_id) def plaintext(self, filename): @@ -996,7 +992,7 @@ class VaultEditor: display.vvvvv('Rekeying file "%s" to with new vault-id "%s" and vault secret %s' % (filename, new_vault_id, new_vault_secret)) try: - plaintext, vault_id_used = self.vault.decrypt_and_get_vault_id(vaulttext) + plaintext, vault_id_used, _dummy = self.vault.decrypt_and_get_vault_id(vaulttext) except AnsibleError as e: raise AnsibleError("%s for %s" % (to_bytes(e), to_bytes(filename))) diff --git a/test/integration/targets/vault/runme.sh b/test/integration/targets/vault/runme.sh index ad890e5806..5b9748926e 100755 --- a/test/integration/targets/vault/runme.sh +++ b/test/integration/targets/vault/runme.sh @@ -26,6 +26,22 @@ 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}" +# test case for https://github.com/ansible/ansible/issues/35834 +# (being prompted for new password on vault-edit with no configured passwords) + +TEST_FILE_EDIT3="${MYTMPDIR}/test_file_edit3" +echo "This is a test file for edit3" > "${TEST_FILE_EDIT3}" + +# ansible-config view +ansible-config view + +# ansisle-config +ansible-config dump --only-changed +ansible-vault encrypt "$@" --vault-id vault-password "${TEST_FILE_EDIT3}" +# EDITOR=./faux-editor.py ansible-vault edit "$@" "${TEST_FILE_EDIT3}" +EDITOR=./faux-editor.py ansible-vault edit --vault-id vault-password -vvvvv "${TEST_FILE_EDIT3}" +echo $? + # view the vault encrypted password file ansible-vault view "$@" --vault-id vault-password encrypted-vault-password @@ -336,11 +352,22 @@ EDITOR=./faux-editor.py ansible-vault edit "$@" --vault-password-file vault-pass head -1 "${TEST_FILE_EDIT}" | grep "${FORMAT_1_1_HEADER}" # edit a 1.1 format with vault-id, should stay 1.1 +cat "${TEST_FILE_EDIT}" EDITOR=./faux-editor.py ansible-vault edit "$@" --vault-id vault_password@vault-password "${TEST_FILE_EDIT}" +cat "${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}" +# verify that we aren't prompted for a new vault password on edit if we are running interactively (ie, with prompts) +# have to use setsid nd --ask-vault-pass to force a prompt to simulate. +# See https://github.com/ansible/ansible/issues/35834 +setsid sh -c 'tty; echo password |ansible-vault edit --ask-vault-pass vault_test.yml' < /dev/null > log 2>&1 && : +grep 'New Vault password' log && : +WRONG_RC=$? +echo "The stdout log had 'New Vault password' in it and it is not supposed to. rc of grep was $WRONG_RC (1 is expected)" +[ $WRONG_RC -eq 1 ] + # 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" diff --git a/test/units/parsing/vault/test_vault.py b/test/units/parsing/vault/test_vault.py index 5d06aaa516..7035a225fb 100644 --- a/test/units/parsing/vault/test_vault.py +++ b/test/units/parsing/vault/test_vault.py @@ -888,6 +888,26 @@ fe3db930508b65e0ff5947e4386b79af8ab094017629590ef6ba486814cf70f8e4ab0ed0c7d2587e # assert we throw an error self.v.decrypt(b_invalid_ciphertext) + def test_decrypt_and_get_vault_id(self): + b_expected_plaintext = to_bytes('foo bar\n') + vaulttext = '''$ANSIBLE_VAULT;1.2;AES256;ansible_devel +65616435333934613466373335363332373764363365633035303466643439313864663837393234 +3330656363343637313962633731333237313636633534630a386264363438363362326132363239 +39363166646664346264383934393935653933316263333838386362633534326664646166663736 +6462303664383765650a356637643633366663643566353036303162386237336233393065393164 +6264''' + + vault_secrets = self._vault_secrets_from_password('ansible_devel', 'ansible') + v = vault.VaultLib(vault_secrets) + + b_vaulttext = to_bytes(vaulttext) + + b_plaintext, vault_id_used, vault_secret_used = v.decrypt_and_get_vault_id(b_vaulttext) + + self.assertEqual(b_expected_plaintext, b_plaintext) + self.assertEqual(vault_id_used, 'ansible_devel') + self.assertEqual(vault_secret_used.text, 'ansible') + def test_decrypt_non_default_1_2(self): b_expected_plaintext = to_bytes('foo bar\n') vaulttext = '''$ANSIBLE_VAULT;1.2;AES256;ansible_devel