Fix 'New Vault password' on vault 'edit' (#35923)

* Fix 'New Vault password' on vault 'edit'

ffe0ddea96 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
This commit is contained in:
Adrian Likins 2018-03-27 14:12:21 -04:00 committed by GitHub
parent cbe2915ba5
commit 6e737c8cb6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 63 additions and 20 deletions

View file

@ -107,7 +107,7 @@ class VaultCLI(CLI):
self.parser.set_usage("usage: %prog rekey [options] file_name") 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 # 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', self.parser.add_option('--encrypt-vault-id', default=[], dest='encrypt_vault_id',
action='store', type='string', action='store', type='string',
help='the vault id used to encrypt (required if more than vault-id is provided)') 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: if not vault_secrets:
raise AnsibleOptionsError("A vault password is required to use Ansible's Vault") 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 encrypt_vault_id = None
# no --encrypt-vault-id self.options.encrypt_vault_id for 'edit' # no --encrypt-vault-id self.options.encrypt_vault_id for 'edit'

View file

@ -657,7 +657,7 @@ class VaultLib:
: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 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 return plaintext
def decrypt_and_get_vault_id(self, vaulttext, filename=None): 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 :kwarg filename: a filename that the data came from. This is only
used to make better error messages in case the data cannot be used to make better error messages in case the data cannot be
decrypted. 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') b_vaulttext = to_bytes(vaulttext, errors='strict', encoding='utf-8')
@ -709,6 +709,7 @@ class VaultLib:
vault_id_matchers = [] vault_id_matchers = []
vault_id_used = None vault_id_used = None
vault_secret_used = None
if vault_id: if vault_id:
display.vvvvv('Found a vault_id (%s) in the vaulttext' % (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) b_plaintext = this_cipher.decrypt(b_vaulttext, vault_secret)
if b_plaintext is not None: if b_plaintext is not None:
vault_id_used = vault_secret_id vault_id_used = vault_secret_id
vault_secret_used = vault_secret
file_slug = '' file_slug = ''
if filename: if filename:
file_slug = ' of "%s"' % filename file_slug = ' of "%s"' % filename
@ -765,7 +767,7 @@ class VaultLib:
msg += " on %s" % to_native(filename) msg += " on %s" % to_native(filename)
raise AnsibleError(msg) raise AnsibleError(msg)
return b_plaintext, vault_id_used return b_plaintext, vault_id_used, vault_secret_used
class VaultEditor: class VaultEditor:
@ -931,7 +933,8 @@ class VaultEditor:
self._edit_file_helper(filename, secret, vault_id=vault_id) self._edit_file_helper(filename, secret, vault_id=vault_id)
def edit_file(self, filename): def edit_file(self, filename):
vault_id_used = None
vault_secret_used = None
# follow the symlink # follow the symlink
filename = self._real_path(filename) filename = self._real_path(filename)
@ -943,7 +946,7 @@ class VaultEditor:
try: try:
# vaulttext gets converted back to bytes, but alas # vaulttext gets converted back to bytes, but alas
# TODO: return the vault_id that worked? # 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: except AnsibleError as e:
raise AnsibleError("%s for %s" % (to_bytes(e), to_bytes(filename))) 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 # 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) # (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 # Keep the same vault-id (and version) as in the header
# 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]
if cipher_name not in CIPHER_WRITE_WHITELIST: if cipher_name not in CIPHER_WRITE_WHITELIST:
# we want to get rid of files encrypted with the AES cipher # 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: 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): 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' % display.vvvvv('Rekeying file "%s" to with new vault-id "%s" and vault secret %s' %
(filename, new_vault_id, new_vault_secret)) (filename, new_vault_id, new_vault_secret))
try: 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: except AnsibleError as e:
raise AnsibleError("%s for %s" % (to_bytes(e), to_bytes(filename))) raise AnsibleError("%s for %s" % (to_bytes(e), to_bytes(filename)))

View file

@ -26,6 +26,22 @@ echo "This is a test file for edit" > "${TEST_FILE_EDIT}"
TEST_FILE_EDIT2="${MYTMPDIR}/test_file_edit2" TEST_FILE_EDIT2="${MYTMPDIR}/test_file_edit2"
echo "This is a test file for edit2" > "${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 # view the vault encrypted password file
ansible-vault view "$@" --vault-id vault-password encrypted-vault-password 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}" head -1 "${TEST_FILE_EDIT}" | grep "${FORMAT_1_1_HEADER}"
# edit a 1.1 format with vault-id, should stay 1.1 # 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}" 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}" head -1 "${TEST_FILE_EDIT}" | grep "${FORMAT_1_1_HEADER}"
ansible-vault encrypt "$@" --vault-id vault_password@vault-password "${TEST_FILE_EDIT2}" 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 # 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}" 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" head -1 "${TEST_FILE_EDIT2}" | grep "${FORMAT_1_2_HEADER};vault_password"

View file

@ -888,6 +888,26 @@ fe3db930508b65e0ff5947e4386b79af8ab094017629590ef6ba486814cf70f8e4ab0ed0c7d2587e
# assert we throw an error # assert we throw an error
self.v.decrypt(b_invalid_ciphertext) 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): def test_decrypt_non_default_1_2(self):
b_expected_plaintext = to_bytes('foo bar\n') b_expected_plaintext = to_bytes('foo bar\n')
vaulttext = '''$ANSIBLE_VAULT;1.2;AES256;ansible_devel vaulttext = '''$ANSIBLE_VAULT;1.2;AES256;ansible_devel