String encrypted with ansible-vault encrypt_string will be false if evaluated with `is string`

KvistA-ELS commented 2 years ago

Summary

If I encrypt a string with ansible-vault and tests if it's a string, it will return false. Even though printing it works just fine. It looks like it's not decrypted if it's being evaluated with is string or like.

Issue Type

Bug Report

Component Name

ansible-vault

Ansible Version

Seems to affect multiple versions
Multiple versions tested and affected

ansible 2.10.5
ansible-playbook [core 2.11.3]

Configuration

$ ansible-config dump --only-changed

OS / Environment

Tested on Ubuntu and CentOS

Steps to Reproduce

Note: vault password is vault

- name: Test ansible-vault
  hosts: localhost
  connection: local
  gather_facts: false
  vars:
    clear_password: SeCrEtInClEaRvIeW
    encrypted_password: !vault |
          $ANSIBLE_VAULT;1.1;AES256
          38316339343730613936653332373065343765343139643430346231353137363338303465396630
          6330396466376265373831303935623432636532346439330a636438393430313832636231353835
          64333733363362353233643962313730363363393035373239323665336238313461373864363730
          3135613738636132330a623561633730363262666534313638623830306265613437323630366431
          3465

  tasks:
    - name: "Print clear_password"
      debug:
        msg: "{{ clear_password }}"

    - name: "Print encrypted_password"
      debug:
        msg: "{{ encrypted_password }}"

    - name: "Check if clear_password is string"
      debug:
        msg: "{{ clear_password is string }}"

    - name: "Check if encrypted_password is string"
      debug:
        msg: "{{ encrypted_password is string }}"

Expected Results

I expect Check if encrypted_password is string to be true and not false.

Actual Results

TASK [Print clear_password] ***************************************************************************************************************************************************************************************
task path: /tmp/ansible-test/playbook.yaml:16
ok: [localhost] => {
    "msg": "SeCrEtInClEaRvIeW"
}

TASK [Print encrypted_password] ***********************************************************************************************************************************************************************************
task path: /tmp/ansible-test/playbook.yaml:20
Trying secret <ansible.parsing.vault.PromptVaultSecret object at 0x7f94c4178890> for vault_id=default
ok: [localhost] => {
    "msg": "h1dd3N 53Cr37"
}

TASK [Check if clear_password is string] **************************************************************************************************************************************************************************
task path: /tmp/ansible-test/playbook.yaml:24
ok: [localhost] => {
    "msg": true
}

TASK [Check if encrypted_password is string] **********************************************************************************************************************************************************************
task path: /tmp/ansible-test/playbook.yaml:28
ok: [localhost] => {
    "msg": false
}
META: ran handlers
META: ran handlers

PLAY RECAP ********************************************************************************************************************************************************************************************************
localhost                  : ok=4    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

Code of Conduct

[X] I agree to follow the Ansible Code of Conduct

ansibot commented 2 years ago

Files identified in the description:

lib/ansible/parsing/vault

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

mkrizek commented 2 years ago

The type of the encrypted string is AnsibleVaultEncryptedUnicode and while it fails the is string test it is considered a string. There is an integration test that tests a lot of string operations on AnsibleVaultEncryptedUnicode in https://github.com/ansible/ansible/blob/devel/test/integration/targets/ansible-vault/single_vault_as_string.yml. There is an explanation why is string does not work on AnsibleVaultEncryptedUnicode: https://github.com/ansible/ansible/blob/12734fa21c08a0ce8c84e533abdc560db2eb1955/test/integration/targets/ansible-vault/single_vault_as_string.yml#L51-L52

Is there any specific operation that works on a string but not on AnsibleVaultEncryptedUnicode that you need?

KvistA-ELS commented 2 years ago

Hi @mkrizek

I'm trying to use a 3rd party role that uses is defined and is string on a password to check it before trying to use it. Don't know what the thoughts have been - just hit this issue cause I wanna store the password in ansible-vault :/

I would have imagined that if the vault variable were accessed for is string it would be decrypted and then be a string. I don't see why it would make sense to have a test run on an encrypted vault string, but maybe I'm just not aware of a case where it makes sense?

/Anders

zerwes commented 2 years ago

I can confirm this. I stumbled over it in a third party playbook too, where a config file template is handling generic values. After checking for bool and none, it checks for strings and sequences. Unfortunately the vaulted string fails the is string test and succeeds at the is sequence check, ending up in a messed up config.

In fact this is a usual way of creating generic config templates, I use it quite often too, i.e.

My workaround for now is to declare a var for the vault and then assign this casted to string to the real var in use:

  vars:
    _encrypted_password: !vault |
          $ANSIBLE_VAULT;1.1;AES256
          38316339343730613936653332373065343765343139643430346231353137363338303465396630
          6330396466376265373831303935623432636532346439330a636438393430313832636231353835
          64333733363362353233643962313730363363393035373239323665336238313461373864363730
          3135613738636132330a623561633730363262666534313638623830306265613437323630366431
          3465
    encrypted_password: "{{ _encrypted_password | string }}"

ansible [core 2.12.2] python version = 3.9.2 jinja version = 2.11.3

zerwes commented 2 years ago

Update: the 3.party playbook I mentioned is https://github.com/Rosa-Luxemburgstiftung-Berlin/pdns-ansible/blob/master/templates/pdns.conf.j2#L12:L14

I don't see why it would make sense to have a test run on an encrypted vault string,

Here a API key can be configured, so it makes sense to encrypt it. And for the config template, it makes sense to run tests against the values in order to stay generic and create valid config files ...

bcoca commented 2 years ago

fyi, strings are also sequences in python, in any case this would require an update to the underlying class:

class AnsibleVaultEncryptedUnicode(Sequence, AnsibleBaseYAMLObject)

sivel commented 2 years ago

We now have a vault_encrypted test. And since single vault values can only be strings, you can also test with:

is vault_encrypted

Ultimately the AnsibleVaultEncryptedUnicode implementation will change again, hopefully in 2.14 to utilize the new data tagging feature on the roadmap. At which point, I would expect it to pass both the is string and is vault_encrypted jinja2 tests.

zerwes commented 2 years ago

fyi, strings are also sequences in python

Yep, thx, I know, but usually the is sequence check will come after the is string check and will only be processed if the first fails ...

zerwes commented 2 years ago

We now have a vault_encrypted test

IMHO this is no option, as usually the template is a piece that comes from the developer and the vars from user (ok, in these cases the terms do not really match, but I hope they clarify the distinction regarding the source and the handling of the input), and the user might decide to vault some vars. As a developer I would expect a string to be a string, indifferent if it is vaulted or not ...

bcoca commented 2 years ago

The 'simple' fix is having AnsibleVaultEncryptedUnicode inherit from String but this is not really that simple as we rely on it 'not being a string' for JSON/YAML serialization.

Another approach would be to 'override' the string test and add AnsibleVaultEncryptedUnicode as a valid string class.

zerwes commented 2 years ago

Just hat a quick glance at the code ... Looks that the main point is that on encryption a text will be converted to bytes: https://github.com/ansible/ansible/blob/2cc74b04c49338b48af070ddd811b25b5d801c12/lib/ansible/parsing/vault/__init__.py#L599 while the decrypt function in https://github.com/ansible/ansible/blob/2cc74b04c49338b48af070ddd811b25b5d801c12/lib/ansible/parsing/vault/__init__.py#L637 returns a byte text from https://github.com/ansible/ansible/blob/2cc74b04c49338b48af070ddd811b25b5d801c12/lib/ansible/parsing/vault/__init__.py#L750 So, to say it succinctly and pictorially, it eats apples, digests them in a vault and poops out pears ... So, to stay in a pictorially context, if you compare vaults with a escrow service, the job is not done right, it is not returning exactly what has been entrusted. The input that has been converted using to_bytes from https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/common/text/converters.py#L33 should be decrypted using to_text https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/common/text/converters.py#L150 on return ...

As I said, this is not a deep inspection, just a short glimpse without deep knowledge of the implications ...

But generally speaking: AFAIR in versions <= 2.9 this behaved like expected. On a refactoring the old behavior should be respected and a test like is text in https://github.com/ansible/ansible/blob/devel/test/integration/targets/ansible-vault/single_vault_as_string.yml#L52 should not just been skipped ...

ansible / ansible