diff --git a/lib/ansible/parsing/vault/__init__.py b/lib/ansible/parsing/vault/__init__.py index b5dd432ec5..bb89cd64e0 100644 --- a/lib/ansible/parsing/vault/__init__.py +++ b/lib/ansible/parsing/vault/__init__.py @@ -29,6 +29,7 @@ import tempfile import warnings from binascii import hexlify from binascii import unhexlify +from binascii import Error as BinasciiError from hashlib import md5 from hashlib import sha256 from io import BytesIO @@ -105,6 +106,10 @@ class AnsibleVaultPasswordError(AnsibleVaultError): pass +class AnsibleVaultFormatError(AnsibleError): + pass + + def is_encrypted(data): """ Test if this is vault encrypted data blob @@ -148,20 +153,7 @@ def is_encrypted_file(file_obj, start_pos=0, count=-1): file_obj.seek(current_position) -def parse_vaulttext_envelope(b_vaulttext_envelope, default_vault_id=None): - """Retrieve information about the Vault and clean the data - - When data is saved, it has a header prepended and is formatted into 80 - character lines. This method extracts the information from the header - and then removes the header and the inserted newlines. The string returned - is suitable for processing by the Cipher classes. - - :arg b_vaulttext: byte str containing the data from a save file - :returns: a byte str suitable for passing to a Cipher class's - decrypt() function. - """ - # used by decrypt - default_vault_id = default_vault_id or C.DEFAULT_VAULT_IDENTITY +def _parse_vaulttext_envelope(b_vaulttext_envelope, default_vault_id=None): b_tmpdata = b_vaulttext_envelope.splitlines() b_tmpheader = b_tmpdata[0].strip().split(b';') @@ -169,7 +161,6 @@ def parse_vaulttext_envelope(b_vaulttext_envelope, default_vault_id=None): b_version = b_tmpheader[1].strip() cipher_name = to_text(b_tmpheader[2].strip()) vault_id = default_vault_id - # vault_id = None # Only attempt to find vault_id if the vault file is version 1.2 or newer # if self.b_version == b'1.2': @@ -181,6 +172,37 @@ def parse_vaulttext_envelope(b_vaulttext_envelope, default_vault_id=None): return b_ciphertext, b_version, cipher_name, vault_id +def parse_vaulttext_envelope(b_vaulttext_envelope, default_vault_id=None, filename=None): + """Parse the vaulttext envelope + + When data is saved, it has a header prepended and is formatted into 80 + character lines. This method extracts the information from the header + and then removes the header and the inserted newlines. The string returned + is suitable for processing by the Cipher classes. + + :arg b_vaulttext: byte str containing the data from a save file + :kwarg default_vault_id: The vault_id name to use if the vaulttext does not provide one. + :kwarg filename: The filename that the data came from. This is only + used to make better error messages in case the data cannot be + decrypted. This is optional. + :returns: A tuple of byte str of the vaulttext suitable to pass to parse_vaultext, + a byte str of the vault format version, + the name of the cipher used, and the vault_id. + :raises: AnsibleVaultFormatError: if the vaulttext_envelope format is invalid + """ + # used by decrypt + default_vault_id = default_vault_id or C.DEFAULT_VAULT_IDENTITY + + try: + return _parse_vaulttext_envelope(b_vaulttext_envelope, default_vault_id) + except Exception as exc: + msg = "Vault envelope format error" + if filename: + msg += ' in %s' % (filename) + msg += ': %s' % exc + raise AnsibleVaultFormatError(msg) + + def format_vaulttext_envelope(b_ciphertext, cipher_name, version=None, vault_id=None): """ Add header and format to 80 columns @@ -222,6 +244,41 @@ def format_vaulttext_envelope(b_ciphertext, cipher_name, version=None, vault_id= return b_vaulttext +def _unhexlify(b_data): + try: + return unhexlify(b_data) + except (BinasciiError, TypeError) as exc: + raise AnsibleVaultFormatError('Vault format unhexlify error: %s' % exc) + + +def _parse_vaulttext(b_vaulttext): + b_vaulttext = _unhexlify(b_vaulttext) + b_salt, b_crypted_hmac, b_ciphertext = b_vaulttext.split(b"\n", 2) + b_salt = _unhexlify(b_salt) + b_ciphertext = _unhexlify(b_ciphertext) + + return b_ciphertext, b_salt, b_crypted_hmac + + +def parse_vaulttext(b_vaulttext): + """Parse the vaulttext + + :arg b_vaulttext: byte str containing the vaulttext (ciphertext, salt, crypted_hmac) + :returns: A tuple of byte str of the ciphertext suitable for passing to a + Cipher class's decrypt() function, a byte str of the salt, + and a byte str of the crypted_hmac + :raises: AnsibleVaultFormatError: if the vaulttext format is invalid + """ + # SPLIT SALT, DIGEST, AND DATA + try: + return _parse_vaulttext(b_vaulttext) + except AnsibleVaultFormatError: + raise + except Exception as exc: + msg = "Vault vaulttext format error: %s" % exc + raise AnsibleVaultFormatError(msg) + + def verify_secret_is_not_empty(secret, msg=None): '''Check the secret against minimal requirements. @@ -609,7 +666,8 @@ class VaultLib: msg += "%s is not a vault encrypted file" % filename raise AnsibleError(msg) - b_vaulttext, dummy, cipher_name, vault_id = parse_vaulttext_envelope(b_vaulttext) + b_vaulttext, dummy, cipher_name, vault_id = parse_vaulttext_envelope(b_vaulttext, + filename=filename) # create the cipher object, note that the cipher used for decrypt can # be different than the cipher used for encrypt @@ -665,6 +723,13 @@ class VaultLib: vault_id_used = vault_secret_id display.vvvvv('decrypt succesful with secret=%s and vault_id=%s' % (vault_secret, vault_secret_id)) break + except AnsibleVaultFormatError as exc: + msg = "There was a vault format error" + if filename: + msg += ' in %s' % (filename) + msg += ': %s' % exc + display.warning(msg) + raise except AnsibleError as e: display.vvvv('Tried to use the vault secret (%s) to decrypt (%s) but it failed. Error: %s' % (vault_secret_id, filename, e)) @@ -862,7 +927,8 @@ class VaultEditor: # 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) + 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 @@ -1136,7 +1202,7 @@ class VaultAES: 'switch to the newer VaultAES256 format', version='2.3') # http://stackoverflow.com/a/14989032 - b_vaultdata = unhexlify(b_vaulttext) + b_vaultdata = _unhexlify(b_vaulttext) b_salt = b_vaultdata[len(b'Salted__'):16] b_ciphertext = b_vaultdata[16:] @@ -1289,7 +1355,7 @@ class VaultAES256: hmac = HMAC(b_key2, hashes.SHA256(), CRYPTOGRAPHY_BACKEND) hmac.update(b_ciphertext) try: - hmac.verify(unhexlify(b_crypted_hmac)) + hmac.verify(_unhexlify(b_crypted_hmac)) except InvalidSignature as e: raise AnsibleVaultError('HMAC verification failed: %s' % e) @@ -1351,11 +1417,8 @@ class VaultAES256: @classmethod def decrypt(cls, b_vaulttext, secret): - # SPLIT SALT, DIGEST, AND DATA - b_vaulttext = unhexlify(b_vaulttext) - b_salt, b_crypted_hmac, b_ciphertext = b_vaulttext.split(b"\n", 2) - b_salt = unhexlify(b_salt) - b_ciphertext = unhexlify(b_ciphertext) + + b_ciphertext, b_salt, b_crypted_hmac = parse_vaulttext(b_vaulttext) # TODO: would be nice if a VaultSecret could be passed directly to _decrypt_* # (move _gen_key_initctr() to a AES256 VaultSecret or VaultContext impl?) diff --git a/test/integration/targets/vault/invalid_format/README.md b/test/integration/targets/vault/invalid_format/README.md new file mode 100644 index 0000000000..cbbc07a965 --- /dev/null +++ b/test/integration/targets/vault/invalid_format/README.md @@ -0,0 +1 @@ +Based on https://github.com/yves-vogl/ansible-inline-vault-issue diff --git a/test/integration/targets/vault/invalid_format/broken-group-vars-tasks.yml b/test/integration/targets/vault/invalid_format/broken-group-vars-tasks.yml new file mode 100644 index 0000000000..71dbacc0a3 --- /dev/null +++ b/test/integration/targets/vault/invalid_format/broken-group-vars-tasks.yml @@ -0,0 +1,23 @@ +--- +- hosts: broken-group-vars + gather_facts: false + tasks: + - name: EXPECTED FAILURE + debug: + msg: "some_var_that_fails: {{ some_var_that_fails }}" + + - name: EXPECTED FAILURE Display hostvars + debug: + msg: "{{inventory_hostname}} hostvars: {{ hostvars[inventory_hostname] }}" + + +# ansible-vault --vault-password-file=vault-secret encrypt_string test +# !vault | +# $ANSIBLE_VAULT;1.1;AES256 +# 64323332393930623633306662363165386332376638653035356132646165663632616263653366 +# 6233383362313531623238613461323861376137656265380a366464663835633065616361636231 +# 39653230653538366165623664326661653135306132313730393232343432333635326536373935 +# 3366323866663763660a323766383531396433663861656532373663373134376263383263316261 +# 3137 + +# $ ansible-playbook -i inventory --vault-password-file=vault-secret tasks.yml diff --git a/test/integration/targets/vault/invalid_format/broken-host-vars-tasks.yml b/test/integration/targets/vault/invalid_format/broken-host-vars-tasks.yml new file mode 100644 index 0000000000..9afbd58e14 --- /dev/null +++ b/test/integration/targets/vault/invalid_format/broken-host-vars-tasks.yml @@ -0,0 +1,7 @@ +--- +- hosts: broken-host-vars + gather_facts: false + tasks: + - name: EXPECTED FAILURE Display hostvars + debug: + msg: "{{inventory_hostname}} hostvars: {{ hostvars[inventory_hostname] }}" diff --git a/test/integration/targets/vault/invalid_format/group_vars/broken-group-vars.yml b/test/integration/targets/vault/invalid_format/group_vars/broken-group-vars.yml new file mode 100644 index 0000000000..5f4774310d --- /dev/null +++ b/test/integration/targets/vault/invalid_format/group_vars/broken-group-vars.yml @@ -0,0 +1,8 @@ +$ANSIBLE_VAULT;1.1;AES256 +64306566356165343030353932383461376334336665626135343932356431383134306338353664 +6435326361306561633165633536333234306665346437330a366265346466626464396264393262 +34616366626565336637653032336465363165363334356535353833393332313239353736623237 +6434373738633039650a353435303366323139356234616433613663626334643939303361303764 +3636363333333333333333333 +36313937643431303637353931366363643661396238303530323262326334343432383637633439 +6365373237336535353661356430313965656538363436333836 diff --git a/test/integration/targets/vault/invalid_format/host_vars/broken-host-vars.example.com/vars b/test/integration/targets/vault/invalid_format/host_vars/broken-host-vars.example.com/vars new file mode 100644 index 0000000000..2d309eb5e8 --- /dev/null +++ b/test/integration/targets/vault/invalid_format/host_vars/broken-host-vars.example.com/vars @@ -0,0 +1,11 @@ +--- +example_vars: + some_key: + another_key: some_value + bad_vault_dict_key: !vault | + $ANSIBLE_VAULT;1.1;AES256 + 64323332393930623633306662363165386332376638653035356132646165663632616263653366 + 623338xyz2313531623238613461323861376137656265380a366464663835633065616361636231 + 3366323866663763660a323766383531396433663861656532373663373134376263383263316261 + 3137 + diff --git a/test/integration/targets/vault/invalid_format/inventory b/test/integration/targets/vault/invalid_format/inventory new file mode 100644 index 0000000000..e6e259a406 --- /dev/null +++ b/test/integration/targets/vault/invalid_format/inventory @@ -0,0 +1,5 @@ +[broken-group-vars] +broken.example.com + +[broken-host-vars] +broken-host-vars.example.com diff --git a/test/integration/targets/vault/invalid_format/original-broken-host-vars b/test/integration/targets/vault/invalid_format/original-broken-host-vars new file mode 100644 index 0000000000..6be696b5cb --- /dev/null +++ b/test/integration/targets/vault/invalid_format/original-broken-host-vars @@ -0,0 +1,6 @@ +$ANSIBLE_VAULT;1.1;AES256 +64323332393930623633306662363165386332376638653035356132646165663632616263653366 +6233383362313531623238613461323861376137656265380a366464663835633065616361636231 +3366323866663763660a323766383531396433663861656532373663373134376263383263316261 +3137 + diff --git a/test/integration/targets/vault/invalid_format/original-group-vars.yml b/test/integration/targets/vault/invalid_format/original-group-vars.yml new file mode 100644 index 0000000000..817557be5d --- /dev/null +++ b/test/integration/targets/vault/invalid_format/original-group-vars.yml @@ -0,0 +1,2 @@ +--- +some_var_that_fails: blippy diff --git a/test/integration/targets/vault/invalid_format/some-vars b/test/integration/targets/vault/invalid_format/some-vars new file mode 100644 index 0000000000..e841a262a7 --- /dev/null +++ b/test/integration/targets/vault/invalid_format/some-vars @@ -0,0 +1,6 @@ +$ANSIBLE_VAULT;1.1;AES256 +37303462633933386339386465613039363964643466663866356261313966663465646262636333 +3965643566363764356563363334363431656661636634380a333837343065326239336639373238 +64316236383836383434366662626339643561616630326137383262396331396538363136323063 +6236616130383264620a613863373631316234656236323332633166623738356664353531633239 +3533 diff --git a/test/integration/targets/vault/invalid_format/vault-secret b/test/integration/targets/vault/invalid_format/vault-secret new file mode 100644 index 0000000000..4406e35cd5 --- /dev/null +++ b/test/integration/targets/vault/invalid_format/vault-secret @@ -0,0 +1 @@ +enemenemu \ No newline at end of file diff --git a/test/integration/targets/vault/runme.sh b/test/integration/targets/vault/runme.sh index 891d30e981..0895f4ffe5 100755 --- a/test/integration/targets/vault/runme.sh +++ b/test/integration/targets/vault/runme.sh @@ -23,6 +23,7 @@ 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" + VAULT_PASSWORD_FILE=vault-password # new format, view, using password client script ansible-vault view "$@" --vault-id vault-password@test-vault-client.py format_1_1_AES256.yml @@ -367,3 +368,9 @@ WRONG_RC=$? echo "rc was $WRONG_RC (1 is expected)" [ $WRONG_RC -eq 1 ] +# test invalid format ala https://github.com/ansible/ansible/issues/28038 +EXPECTED_ERROR='Vault format unhexlify error: Non-hexadecimal digit found' +ansible-playbook "$@" -i invalid_format/inventory --vault-id invalid_format/vault-secret invalid_format/broken-host-vars-tasks.yml 2>&1 | grep "${EXPECTED_ERROR}" + +EXPECTED_ERROR='Vault format unhexlify error: Odd-length string' +ansible-playbook "$@" -i invalid_format/inventory --vault-id invalid_format/vault-secret invalid_format/broken-group-vars-tasks.yml 2>&1 | grep "${EXPECTED_ERROR}" diff --git a/test/units/parsing/vault/test_vault.py b/test/units/parsing/vault/test_vault.py index ac5fc001c8..814bcce191 100644 --- a/test/units/parsing/vault/test_vault.py +++ b/test/units/parsing/vault/test_vault.py @@ -41,6 +41,62 @@ from units.mock.loader import DictDataLoader from units.mock.vault_helper import TextVaultSecret +class TestUnhexlify(unittest.TestCase): + def test(self): + b_plain_data = b'some text to hexlify' + b_data = hexlify(b_plain_data) + res = vault._unhexlify(b_data) + self.assertEquals(res, b_plain_data) + + def test_odd_length(self): + b_data = b'123456789abcdefghijklmnopqrstuvwxyz' + + self.assertRaisesRegexp(vault.AnsibleVaultFormatError, + '.*Vault format unhexlify error.*', + vault._unhexlify, + b_data) + + def test_nonhex(self): + b_data = b'6z36316566653264333665333637623064303639353237620a636366633565663263336335656532' + + self.assertRaisesRegexp(vault.AnsibleVaultFormatError, + '.*Vault format unhexlify error.*Non-hexadecimal digit found', + vault._unhexlify, + b_data) + + +class TestParseVaulttext(unittest.TestCase): + def test(self): + vaulttext_envelope = u'''$ANSIBLE_VAULT;1.1;AES256 +33363965326261303234626463623963633531343539616138316433353830356566396130353436 +3562643163366231316662386565383735653432386435610a306664636137376132643732393835 +63383038383730306639353234326630666539346233376330303938323639306661313032396437 +6233623062366136310a633866373936313238333730653739323461656662303864663666653563 +3138''' + + b_vaulttext_envelope = to_bytes(vaulttext_envelope, errors='strict', encoding='utf-8') + b_vaulttext, b_version, cipher_name, vault_id = vault.parse_vaulttext_envelope(b_vaulttext_envelope) + res = vault.parse_vaulttext(b_vaulttext) + self.assertIsInstance(res[0], bytes) + self.assertIsInstance(res[1], bytes) + self.assertIsInstance(res[2], bytes) + + def test_non_hex(self): + vaulttext_envelope = u'''$ANSIBLE_VAULT;1.1;AES256 +3336396J326261303234626463623963633531343539616138316433353830356566396130353436 +3562643163366231316662386565383735653432386435610a306664636137376132643732393835 +63383038383730306639353234326630666539346233376330303938323639306661313032396437 +6233623062366136310a633866373936313238333730653739323461656662303864663666653563 +3138''' + + b_vaulttext_envelope = to_bytes(vaulttext_envelope, errors='strict', encoding='utf-8') + b_vaulttext, b_version, cipher_name, vault_id = vault.parse_vaulttext_envelope(b_vaulttext_envelope) + self.assertRaisesRegexp(vault.AnsibleVaultFormatError, + '.*Vault format unhexlify error.*Non-hexadecimal digit found', + vault.parse_vaulttext, + b_vaulttext_envelope) + + class TestVaultSecret(unittest.TestCase): def test(self): secret = vault.VaultSecret()