diff options
author | Matt Martz <matt@sivel.net> | 2019-10-11 12:32:01 -0500 |
---|---|---|
committer | Toshio Kuratomi <a.badger@gmail.com> | 2019-10-11 10:32:00 -0700 |
commit | 16684f118715a52e1c46d437652add9ca36423de (patch) | |
tree | 9686e5cdf31c904a6e7562f01f5e3b6db380a282 | |
parent | d961f676c01023a6a21503df16ba551a550e515b (diff) | |
download | ansible-16684f118715a52e1c46d437652add9ca36423de.tar.gz |
[stable-2.6] Wrap CLI passwords as AnsibleUnsafeText (#63352) (#63393)
* [stable-2.6] 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 76d652f7c8..474c77e761 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 @@ -329,8 +329,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 @@ -338,17 +336,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 059258a3c8..0815d38c93 100644 --- a/lib/ansible/template/__init__.py +++ b/lib/ansible/template/__init__.py @@ -204,7 +204,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 b4723cd83a..8683751263 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 @@ -632,3 +633,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')) |