diff options
author | Matt Martz <matt@sivel.net> | 2019-10-11 12:31:10 -0500 |
---|---|---|
committer | Toshio Kuratomi <a.badger@gmail.com> | 2019-10-11 10:31:10 -0700 |
commit | 40618d70e61af1123907a5fb246cc4fd35f1e5c3 (patch) | |
tree | 7a20dc3e4e8571e8608ff26b5864523176a7f0d2 | |
parent | 90e74dd2600e5cc42dd9b4f4656f3d651c4ce5c4 (diff) | |
download | ansible-40618d70e61af1123907a5fb246cc4fd35f1e5c3.tar.gz |
[stable-2.7] Wrap CLI passwords as AnsibleUnsafeText (#63352) (#63392)
* [stable-2.7] Wrap CLI passwords as AnsibleUnsafeText (#63352)
* isa string should rewrap as unsafe in get_validated_value
* _is_unsafe shouldn't be concerned with underlying types
* Start with passwords as text, instead of bytes
* Remove unused imports
* Add changelog fragment
* Update changelog with CVE.
(cherry picked from commit baeff7462d5d877b6849aa78f50860e7d10ce950)
Co-authored-by: Matt Martz <matt@sivel.net>
* Update tests
-rw-r--r-- | changelogs/fragments/dont-template-cli-passwords.yml | 6 | ||||
-rw-r--r-- | lib/ansible/cli/__init__.py | 10 | ||||
-rw-r--r-- | lib/ansible/template/__init__.py | 2 | ||||
-rw-r--r-- | test/integration/targets/cli/aliases | 2 | ||||
-rwxr-xr-x | test/integration/targets/cli/runme.sh | 7 | ||||
-rw-r--r-- | test/integration/targets/cli/setup.yml | 4 | ||||
-rwxr-xr-x | test/integration/targets/cli/test-cli.py | 21 | ||||
-rw-r--r-- | test/units/playbook/test_base.py | 8 |
8 files changed, 52 insertions, 8 deletions
diff --git a/changelogs/fragments/dont-template-cli-passwords.yml b/changelogs/fragments/dont-template-cli-passwords.yml new file mode 100644 index 0000000000..ddaccc07af --- /dev/null +++ b/changelogs/fragments/dont-template-cli-passwords.yml @@ -0,0 +1,6 @@ +bugfixes: +- > + **security issue** - Convert CLI provided passwords to text initially, to + prevent unsafe context being lost when converting from bytes->text during + post processing of PlayContext. This prevents CLI provided passwords from + being incorrectly templated (CVE-2019-14856) diff --git a/lib/ansible/cli/__init__.py b/lib/ansible/cli/__init__.py index e941743ac7..2d59ec28a6 100644 --- a/lib/ansible/cli/__init__.py +++ b/lib/ansible/cli/__init__.py @@ -42,7 +42,7 @@ from ansible.parsing.dataloader import DataLoader from ansible.release import __version__ from ansible.utils.path import unfrackpath from ansible.utils.vars import load_extra_vars, load_options_vars -from ansible.utils.unsafe_proxy import AnsibleUnsafeBytes +from ansible.utils.unsafe_proxy import AnsibleUnsafeText from ansible.vars.manager import VariableManager from ansible.parsing.vault import PromptVaultSecret, get_file_vault_secret @@ -323,8 +323,6 @@ class CLI(with_metaclass(ABCMeta, object)): if op.ask_pass: sshpass = getpass.getpass(prompt="SSH password: ") become_prompt = "%s password[defaults to SSH password]: " % become_prompt_method - if sshpass: - sshpass = to_bytes(sshpass, errors='strict', nonstring='simplerepr') else: become_prompt = "%s password: " % become_prompt_method @@ -332,17 +330,15 @@ class CLI(with_metaclass(ABCMeta, object)): becomepass = getpass.getpass(prompt=become_prompt) if op.ask_pass and becomepass == '': becomepass = sshpass - if becomepass: - becomepass = to_bytes(becomepass) except EOFError: pass # we 'wrap' the passwords to prevent templating as # they can contain special chars and trigger it incorrectly if sshpass: - sshpass = AnsibleUnsafeBytes(sshpass) + sshpass = AnsibleUnsafeText(to_text(sshpass)) if becomepass: - becomepass = AnsibleUnsafeBytes(becomepass) + becomepass = AnsibleUnsafeText(to_text(becomepass)) return (sshpass, becomepass) diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py index 5535f53eea..d428930a21 100644 --- a/lib/ansible/template/__init__.py +++ b/lib/ansible/template/__init__.py @@ -220,7 +220,7 @@ class AnsibleContext(Context): for item in val: if self._is_unsafe(item): return True - elif isinstance(val, string_types) and hasattr(val, '__UNSAFE__'): + elif hasattr(val, '__UNSAFE__'): return True return False diff --git a/test/integration/targets/cli/aliases b/test/integration/targets/cli/aliases new file mode 100644 index 0000000000..6b71e884a1 --- /dev/null +++ b/test/integration/targets/cli/aliases @@ -0,0 +1,2 @@ +needs/target/setup_pexpect +shippable/posix/group3 diff --git a/test/integration/targets/cli/runme.sh b/test/integration/targets/cli/runme.sh new file mode 100755 index 0000000000..d9e846256f --- /dev/null +++ b/test/integration/targets/cli/runme.sh @@ -0,0 +1,7 @@ +#!/usr/bin/env bash + +set -eux + +ANSIBLE_ROLES_PATH=../ ansible-playbook setup.yml + +python test-cli.py diff --git a/test/integration/targets/cli/setup.yml b/test/integration/targets/cli/setup.yml new file mode 100644 index 0000000000..9f6ab11741 --- /dev/null +++ b/test/integration/targets/cli/setup.yml @@ -0,0 +1,4 @@ +- hosts: localhost + gather_facts: no + roles: + - setup_pexpect diff --git a/test/integration/targets/cli/test-cli.py b/test/integration/targets/cli/test-cli.py new file mode 100755 index 0000000000..9893d6652e --- /dev/null +++ b/test/integration/targets/cli/test-cli.py @@ -0,0 +1,21 @@ +#!/usr/bin/env python +# Copyright (c) 2019 Matt Martz <matt@sivel.net> +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +# Make coding more python3-ish +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import os + +import pexpect + +os.environ['ANSIBLE_NOCOLOR'] = '1' +out = pexpect.run( + 'ansible localhost -m debug -a msg="{{ ansible_password }}" -k', + events={ + 'SSH password:': '{{ 1 + 2 }}\n' + } +) + +assert b'{{ 1 + 2 }}' in out diff --git a/test/units/playbook/test_base.py b/test/units/playbook/test_base.py index f52fd9e3f3..6d7c96fe3e 100644 --- a/test/units/playbook/test_base.py +++ b/test/units/playbook/test_base.py @@ -26,6 +26,7 @@ from ansible.module_utils.six import string_types from ansible.playbook.attribute import FieldAttribute from ansible.template import Templar from ansible.playbook import base +from ansible.utils.unsafe_proxy import AnsibleUnsafeBytes, AnsibleUnsafeText from units.mock.loader import DictDataLoader @@ -626,3 +627,10 @@ class TestBaseSubClass(TestBase): ds = {'test_attr_method_missing': a_string} bsc = self._base_validate(ds) self.assertEquals(bsc.test_attr_method_missing, a_string) + + def test_get_validated_value_string_rewrap_unsafe(self): + value = AnsibleUnsafeText(u'bar') + ds = {'test_attr_string': value} + bsc = self._base_validate(ds) + self.assertIsInstance(bsc.test_attr_string, AnsibleUnsafeText) + self.assertEquals(bsc.test_attr_string, AnsibleUnsafeText(u'bar')) |