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

8 files changed, 54 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 c725db0d00..bf10d5a007 100644
--- a/lib/ansible/cli/__init__.py
+++ b/lib/ansible/cli/__init__.py
@@ -29,7 +29,7 @@ from ansible.release import __version__
 from ansible.utils.collection_loader import AnsibleCollectionLoader, get_collection_name_from_path, set_collection_playbook_paths
 from ansible.utils.display import Display
 from ansible.utils.path import unfrackpath
-from ansible.utils.unsafe_proxy import AnsibleUnsafeBytes
+from ansible.utils.unsafe_proxy import AnsibleUnsafeText
 from ansible.vars.manager import VariableManager
 
 try:
@@ -240,8 +240,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
 
@@ -249,17 +247,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 1503c82ea7..fa4b11afe1 100644
--- a/lib/ansible/template/__init__.py
+++ b/lib/ansible/template/__init__.py
@@ -272,7 +272,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 dc3509577e..0b51271772 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
 
@@ -620,3 +621,12 @@ 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):
+        attribute = FieldAttribute(isa='string')
+        value = AnsibleUnsafeText(u'bar')
+        templar = Templar(None)
+        bsc = self.ClassUnderTest()
+        result = bsc.get_validated_value('foo', attribute, value, templar)
+        self.assertIsInstance(result, AnsibleUnsafeText)
+        self.assertEquals(result, AnsibleUnsafeText(u'bar'))