From 47bcdf5952b21277380cf14464f3a45d41ea0be7 Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Mon, 24 Aug 2015 17:53:12 +0530 Subject: [PATCH 1/7] Remove incorrect copy-pasted comment --- lib/ansible/parsing/vault/__init__.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/ansible/parsing/vault/__init__.py b/lib/ansible/parsing/vault/__init__.py index 55346166b6a..306454cb8de 100644 --- a/lib/ansible/parsing/vault/__init__.py +++ b/lib/ansible/parsing/vault/__init__.py @@ -12,11 +12,6 @@ # # You should have received a copy of the GNU General Public License # along with Ansible. If not, see . -# -# ansible-pull is a script that runs ansible in local mode -# after checking out a playbooks directory from source repo. There is an -# example playbook to bootstrap this script in the examples/ dir which -# installs ansible and sets it up to run on cron. # Make coding more python3-ish from __future__ import (absolute_import, division, print_function) From 017566a2d9b3297c34c78fc7d38697ade20e1e56 Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Tue, 25 Aug 2015 14:54:23 +0530 Subject: [PATCH 2/7] Use AES256 if the cipher is not write-whitelisted --- lib/ansible/parsing/vault/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ansible/parsing/vault/__init__.py b/lib/ansible/parsing/vault/__init__.py index 306454cb8de..b12e11816e2 100644 --- a/lib/ansible/parsing/vault/__init__.py +++ b/lib/ansible/parsing/vault/__init__.py @@ -132,11 +132,11 @@ class VaultLib: if self.is_encrypted(b_data): raise AnsibleError("data is already encrypted") - if not self.cipher_name: + if not self.cipher_name or self.cipher_name not in CIPHER_WRITE_WHITELIST: self.cipher_name = u"AES256" cipher_class_name = u'Vault{0}'.format(self.cipher_name) - if cipher_class_name in globals() and self.cipher_name in CIPHER_WHITELIST: + if cipher_class_name in globals(): Cipher = globals()[cipher_class_name] this_cipher = Cipher() else: From f91ad3dabe5e5eb20f6b6b5ee875b4a8da3db5ea Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Wed, 26 Aug 2015 18:20:57 +0530 Subject: [PATCH 3/7] Don't pass the cipher around so much It's unused and unnecessary; VaultLib can decide for itself what cipher to use when encrypting. There's no need (and no provision) for the user to override the cipher via options, so there's no need for code to see if that has been done either. --- lib/ansible/cli/vault.py | 16 ++++++---------- lib/ansible/parsing/vault/__init__.py | 18 +++++------------- 2 files changed, 11 insertions(+), 23 deletions(-) diff --git a/lib/ansible/cli/vault.py b/lib/ansible/cli/vault.py index 393bbdd50b0..c68e620a183 100644 --- a/lib/ansible/cli/vault.py +++ b/lib/ansible/cli/vault.py @@ -30,7 +30,6 @@ class VaultCLI(CLI): """ Vault command line class """ VALID_ACTIONS = ("create", "decrypt", "edit", "encrypt", "rekey", "view") - CIPHER = 'AES256' def __init__(self, args, display=None): @@ -91,15 +90,13 @@ class VaultCLI(CLI): if len(self.args) > 1: raise AnsibleOptionsError("ansible-vault create can take only one filename argument") - cipher = getattr(self.options, 'cipher', self.CIPHER) - this_editor = VaultEditor(cipher, self.vault_pass, self.args[0]) + this_editor = VaultEditor(self.vault_pass, self.args[0]) this_editor.create_file() def execute_decrypt(self): - cipher = getattr(self.options, 'cipher', self.CIPHER) for f in self.args: - this_editor = VaultEditor(cipher, self.vault_pass, f) + this_editor = VaultEditor(self.vault_pass, f) this_editor.decrypt_file() self.display.display("Decryption successful") @@ -107,20 +104,19 @@ class VaultCLI(CLI): def execute_edit(self): for f in self.args: - this_editor = VaultEditor(None, self.vault_pass, f) + this_editor = VaultEditor(self.vault_pass, f) this_editor.edit_file() def execute_view(self): for f in self.args: - this_editor = VaultEditor(None, self.vault_pass, f) + this_editor = VaultEditor(self.vault_pass, f) this_editor.view_file() def execute_encrypt(self): - cipher = getattr(self.options, 'cipher', self.CIPHER) for f in self.args: - this_editor = VaultEditor(cipher, self.vault_pass, f) + this_editor = VaultEditor(self.vault_pass, f) this_editor.encrypt_file() self.display.display("Encryption successful") @@ -136,7 +132,7 @@ class VaultCLI(CLI): __, new_password = self.ask_vault_passwords(ask_vault_pass=False, ask_new_vault_pass=True, confirm_new=True) for f in self.args: - this_editor = VaultEditor(None, self.vault_pass, f) + this_editor = VaultEditor(self.vault_pass, f) this_editor.rekey_file(new_password) self.display.display("Rekey successful") diff --git a/lib/ansible/parsing/vault/__init__.py b/lib/ansible/parsing/vault/__init__.py index b12e11816e2..9e49e134042 100644 --- a/lib/ansible/parsing/vault/__init__.py +++ b/lib/ansible/parsing/vault/__init__.py @@ -230,13 +230,11 @@ class VaultEditor: # file I/O, ditto read_file(self, filename) and launch_editor(self, filename) # ... "Don't Repeat Yourself", etc. - def __init__(self, cipher_name, password, filename): - # instantiates a member variable for VaultLib - self.cipher_name = cipher_name + def __init__(self, password, filename): self.password = password self.filename = filename - def _edit_file_helper(self, existing_data=None, cipher=None, force_save=False): + def _edit_file_helper(self, existing_data=None, force_save=False): # make sure the umask is set to a sane value old_umask = os.umask(0o077) @@ -257,8 +255,6 @@ class VaultEditor: # create new vault this_vault = VaultLib(self.password) - if cipher: - this_vault.cipher_name = cipher # encrypt new data and write out to tmp enc_data = this_vault.encrypt(tmpdata) @@ -279,7 +275,7 @@ class VaultEditor: raise AnsibleError("%s exists, please use 'edit' instead" % self.filename) # Let the user specify contents and save file - self._edit_file_helper(cipher=self.cipher_name) + self._edit_file_helper() def decrypt_file(self): @@ -311,9 +307,9 @@ class VaultEditor: # let the user edit the data and save if this_vault.cipher_name not in CIPHER_WRITE_WHITELIST: # we want to get rid of files encrypted with the AES cipher - self._edit_file_helper(existing_data=dec_data, cipher=None, force_save=True) + self._edit_file_helper(existing_data=dec_data, force_save=True) else: - self._edit_file_helper(existing_data=dec_data, cipher=this_vault.cipher_name, force_save=False) + self._edit_file_helper(existing_data=dec_data, force_save=False) def view_file(self): @@ -339,7 +335,6 @@ class VaultEditor: tmpdata = self.read_data(self.filename) this_vault = VaultLib(self.password) - this_vault.cipher_name = self.cipher_name if not this_vault.is_encrypted(tmpdata): enc_data = this_vault.encrypt(tmpdata) self.write_data(enc_data, self.filename) @@ -358,9 +353,6 @@ class VaultEditor: # create new vault new_vault = VaultLib(new_password) - # we want to force cipher to the default - #new_vault.cipher_name = this_vault.cipher_name - # re-encrypt data and re-write file enc_data = new_vault.encrypt(dec_data) self.write_data(enc_data, self.filename) From a27c5741a1a52f78a7ee25dddbf30e78ecce6a4f Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Wed, 26 Aug 2015 18:23:17 +0530 Subject: [PATCH 4/7] Remove inaccurate outdated comment --- lib/ansible/parsing/vault/__init__.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/ansible/parsing/vault/__init__.py b/lib/ansible/parsing/vault/__init__.py index 9e49e134042..d932c94160c 100644 --- a/lib/ansible/parsing/vault/__init__.py +++ b/lib/ansible/parsing/vault/__init__.py @@ -225,10 +225,6 @@ class VaultLib: class VaultEditor: - # uses helper methods for write_file(self, filename, data) - # to write a file so that code isn't duplicated for simple - # file I/O, ditto read_file(self, filename) and launch_editor(self, filename) - # ... "Don't Repeat Yourself", etc. def __init__(self, password, filename): self.password = password From 20fd9224bbf82df0260f700e5dcddbefe9965bde Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Wed, 26 Aug 2015 19:17:37 +0530 Subject: [PATCH 5/7] Pass the filename to the individual VaultEditor methods, not __init__ Now we don't have to recreate VaultEditor objects for each file, and so on. It also paves the way towards specifying separate input and output files later. --- lib/ansible/cli/vault.py | 27 ++++++------- lib/ansible/parsing/vault/__init__.py | 57 +++++++++++++-------------- 2 files changed, 39 insertions(+), 45 deletions(-) diff --git a/lib/ansible/cli/vault.py b/lib/ansible/cli/vault.py index c68e620a183..d28abacd5ac 100644 --- a/lib/ansible/cli/vault.py +++ b/lib/ansible/cli/vault.py @@ -83,6 +83,8 @@ class VaultCLI(CLI): if not self.vault_pass: raise AnsibleOptionsError("A password is required to use Ansible's Vault") + self.editor = VaultEditor(self.vault_pass) + self.execute() def execute_create(self): @@ -90,36 +92,30 @@ class VaultCLI(CLI): if len(self.args) > 1: raise AnsibleOptionsError("ansible-vault create can take only one filename argument") - this_editor = VaultEditor(self.vault_pass, self.args[0]) - this_editor.create_file() + self.editor.create_file(self.args[0]) def execute_decrypt(self): for f in self.args: - this_editor = VaultEditor(self.vault_pass, f) - this_editor.decrypt_file() + self.editor.decrypt_file(f) - self.display.display("Decryption successful") + self.display.display("Decryption successful", stderr=True) def execute_edit(self): - for f in self.args: - this_editor = VaultEditor(self.vault_pass, f) - this_editor.edit_file() + self.editor.edit_file(f) def execute_view(self): for f in self.args: - this_editor = VaultEditor(self.vault_pass, f) - this_editor.view_file() + self.editor.view_file(f) def execute_encrypt(self): for f in self.args: - this_editor = VaultEditor(self.vault_pass, f) - this_editor.encrypt_file() + self.editor.encrypt_file(f) - self.display.display("Encryption successful") + self.display.display("Encryption successful", stderr=True) def execute_rekey(self): for f in self.args: @@ -132,7 +128,6 @@ class VaultCLI(CLI): __, new_password = self.ask_vault_passwords(ask_vault_pass=False, ask_new_vault_pass=True, confirm_new=True) for f in self.args: - this_editor = VaultEditor(self.vault_pass, f) - this_editor.rekey_file(new_password) + self.editor.rekey_file(new_password, f) - self.display.display("Rekey successful") + self.display.display("Rekey successful", stderr=True) diff --git a/lib/ansible/parsing/vault/__init__.py b/lib/ansible/parsing/vault/__init__.py index d932c94160c..c9f2c4a4f66 100644 --- a/lib/ansible/parsing/vault/__init__.py +++ b/lib/ansible/parsing/vault/__init__.py @@ -226,11 +226,10 @@ class VaultLib: class VaultEditor: - def __init__(self, password, filename): + def __init__(self, password): self.password = password - self.filename = filename - def _edit_file_helper(self, existing_data=None, force_save=False): + def _edit_file_helper(self, filename, existing_data=None, force_save=False): # make sure the umask is set to a sane value old_umask = os.umask(0o077) @@ -257,62 +256,62 @@ class VaultEditor: self.write_data(enc_data, tmp_path) # shuffle tmp file into place - self.shuffle_files(tmp_path, self.filename) + self.shuffle_files(tmp_path, filename) # and restore umask os.umask(old_umask) - def create_file(self): + def create_file(self, filename): """ create a new encrypted file """ check_prereqs() - if os.path.isfile(self.filename): - raise AnsibleError("%s exists, please use 'edit' instead" % self.filename) + if os.path.isfile(filename): + raise AnsibleError("%s exists, please use 'edit' instead" % filename) # Let the user specify contents and save file - self._edit_file_helper() + self._edit_file_helper(filename) - def decrypt_file(self): + def decrypt_file(self, filename): check_prereqs() - if not os.path.isfile(self.filename): - raise AnsibleError("%s does not exist" % self.filename) + if not os.path.isfile(filename): + raise AnsibleError("%s does not exist" % filename) - tmpdata = self.read_data(self.filename) + tmpdata = self.read_data(filename) this_vault = VaultLib(self.password) if this_vault.is_encrypted(tmpdata): dec_data = this_vault.decrypt(tmpdata) if dec_data is None: raise AnsibleError("Decryption failed") else: - self.write_data(dec_data, self.filename) + self.write_data(dec_data, filename) else: - raise AnsibleError("%s is not encrypted" % self.filename) + raise AnsibleError("%s is not encrypted" % filename) - def edit_file(self): + def edit_file(self, filename): check_prereqs() # decrypt to tmpfile - tmpdata = self.read_data(self.filename) + tmpdata = self.read_data(filename) this_vault = VaultLib(self.password) dec_data = this_vault.decrypt(tmpdata) # let the user edit the data and save if this_vault.cipher_name not in CIPHER_WRITE_WHITELIST: # we want to get rid of files encrypted with the AES cipher - self._edit_file_helper(existing_data=dec_data, force_save=True) + self._edit_file_helper(filename, existing_data=dec_data, force_save=True) else: - self._edit_file_helper(existing_data=dec_data, force_save=False) + self._edit_file_helper(filename, existing_data=dec_data, force_save=False) - def view_file(self): + def view_file(self, filename): check_prereqs() # decrypt to tmpfile - tmpdata = self.read_data(self.filename) + tmpdata = self.read_data(filename) this_vault = VaultLib(self.password) dec_data = this_vault.decrypt(tmpdata) _, tmp_path = tempfile.mkstemp() @@ -322,27 +321,27 @@ class VaultEditor: call(self._pager_shell_command(tmp_path)) os.remove(tmp_path) - def encrypt_file(self): + def encrypt_file(self, filename): check_prereqs() - if not os.path.isfile(self.filename): - raise AnsibleError("%s does not exist" % self.filename) + if not os.path.isfile(filename): + raise AnsibleError("%s does not exist" % filename) - tmpdata = self.read_data(self.filename) + tmpdata = self.read_data(filename) this_vault = VaultLib(self.password) if not this_vault.is_encrypted(tmpdata): enc_data = this_vault.encrypt(tmpdata) - self.write_data(enc_data, self.filename) + self.write_data(enc_data, filename) else: - raise AnsibleError("%s is already encrypted" % self.filename) + raise AnsibleError("%s is already encrypted" % filename) - def rekey_file(self, new_password): + def rekey_file(self, new_password, filename): check_prereqs() # decrypt - tmpdata = self.read_data(self.filename) + tmpdata = self.read_data(filename) this_vault = VaultLib(self.password) dec_data = this_vault.decrypt(tmpdata) @@ -351,7 +350,7 @@ class VaultEditor: # re-encrypt data and re-write file enc_data = new_vault.encrypt(dec_data) - self.write_data(enc_data, self.filename) + self.write_data(enc_data, filename) def read_data(self, filename): f = open(filename, "rb") From c4b2540ecc7e215ff7278bc3712dcbd0e010f04a Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Wed, 26 Aug 2015 19:52:20 +0530 Subject: [PATCH 6/7] Update tests for VaultEditor API changes --- test/units/parsing/vault/test_vault_editor.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/units/parsing/vault/test_vault_editor.py b/test/units/parsing/vault/test_vault_editor.py index 2ddf3de27a2..d1fa0d07a05 100644 --- a/test/units/parsing/vault/test_vault_editor.py +++ b/test/units/parsing/vault/test_vault_editor.py @@ -81,7 +81,7 @@ class TestVaultEditor(unittest.TestCase): pass def test_methods_exist(self): - v = VaultEditor(None, None, None) + v = VaultEditor(None) slots = ['create_file', 'decrypt_file', 'edit_file', @@ -103,8 +103,8 @@ class TestVaultEditor(unittest.TestCase): tmp_file = tempfile.NamedTemporaryFile() os.unlink(tmp_file.name) - ve = VaultEditor(None, "ansible", tmp_file.name) - ve.create_file() + ve = VaultEditor("ansible") + ve.create_file(tmp_file.name) self.assertTrue(os.path.exists(tmp_file.name)) @@ -120,12 +120,12 @@ class TestVaultEditor(unittest.TestCase): with v10_file as f: f.write(to_bytes(v10_data)) - ve = VaultEditor(None, "ansible", v10_file.name) + ve = VaultEditor("ansible") # make sure the password functions for the cipher error_hit = False try: - ve.decrypt_file() + ve.decrypt_file(v10_file.name) except errors.AnsibleError as e: error_hit = True @@ -148,12 +148,12 @@ class TestVaultEditor(unittest.TestCase): with v11_file as f: f.write(to_bytes(v11_data)) - ve = VaultEditor(None, "ansible", v11_file.name) + ve = VaultEditor("ansible") # make sure the password functions for the cipher error_hit = False try: - ve.decrypt_file() + ve.decrypt_file(v11_file.name) except errors.AnsibleError as e: error_hit = True @@ -180,12 +180,12 @@ class TestVaultEditor(unittest.TestCase): with v10_file as f: f.write(to_bytes(v10_data)) - ve = VaultEditor(None, "ansible", v10_file.name) + ve = VaultEditor("ansible") # make sure the password functions for the cipher error_hit = False try: - ve.rekey_file('ansible2') + ve.rekey_file('ansible2', v10_file.name) except errors.AnsibleError as e: error_hit = True From b84053019a496f460a75cc8c336b6ca64cc423e6 Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Wed, 26 Aug 2015 19:54:59 +0530 Subject: [PATCH 7/7] Make the filename the first argument to rekey_file --- lib/ansible/cli/vault.py | 2 +- lib/ansible/parsing/vault/__init__.py | 2 +- test/units/parsing/vault/test_vault_editor.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ansible/cli/vault.py b/lib/ansible/cli/vault.py index d28abacd5ac..bae7377750b 100644 --- a/lib/ansible/cli/vault.py +++ b/lib/ansible/cli/vault.py @@ -128,6 +128,6 @@ class VaultCLI(CLI): __, new_password = self.ask_vault_passwords(ask_vault_pass=False, ask_new_vault_pass=True, confirm_new=True) for f in self.args: - self.editor.rekey_file(new_password, f) + self.editor.rekey_file(f, new_password) self.display.display("Rekey successful", stderr=True) diff --git a/lib/ansible/parsing/vault/__init__.py b/lib/ansible/parsing/vault/__init__.py index c9f2c4a4f66..c9d4372e7b5 100644 --- a/lib/ansible/parsing/vault/__init__.py +++ b/lib/ansible/parsing/vault/__init__.py @@ -336,7 +336,7 @@ class VaultEditor: else: raise AnsibleError("%s is already encrypted" % filename) - def rekey_file(self, new_password, filename): + def rekey_file(self, filename, new_password): check_prereqs() diff --git a/test/units/parsing/vault/test_vault_editor.py b/test/units/parsing/vault/test_vault_editor.py index d1fa0d07a05..e943b008685 100644 --- a/test/units/parsing/vault/test_vault_editor.py +++ b/test/units/parsing/vault/test_vault_editor.py @@ -185,7 +185,7 @@ class TestVaultEditor(unittest.TestCase): # make sure the password functions for the cipher error_hit = False try: - ve.rekey_file('ansible2', v10_file.name) + ve.rekey_file(v10_file.name, 'ansible2') except errors.AnsibleError as e: error_hit = True