* [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
1 | 1 |
new file mode 100644 |
... | ... |
@@ -0,0 +1,6 @@ |
0 |
+bugfixes: |
|
1 |
+- > |
|
2 |
+ **security issue** - Convert CLI provided passwords to text initially, to |
|
3 |
+ prevent unsafe context being lost when converting from bytes->text during |
|
4 |
+ post processing of PlayContext. This prevents CLI provided passwords from |
|
5 |
+ being incorrectly templated (CVE-2019-14856) |
... | ... |
@@ -42,7 +42,7 @@ from ansible.parsing.dataloader import DataLoader |
42 | 42 |
from ansible.release import __version__ |
43 | 43 |
from ansible.utils.path import unfrackpath |
44 | 44 |
from ansible.utils.vars import load_extra_vars, load_options_vars |
45 |
-from ansible.utils.unsafe_proxy import AnsibleUnsafeBytes |
|
45 |
+from ansible.utils.unsafe_proxy import AnsibleUnsafeText |
|
46 | 46 |
from ansible.vars.manager import VariableManager |
47 | 47 |
from ansible.parsing.vault import PromptVaultSecret, get_file_vault_secret |
48 | 48 |
|
... | ... |
@@ -323,8 +323,6 @@ class CLI(with_metaclass(ABCMeta, object)): |
323 | 323 |
if op.ask_pass: |
324 | 324 |
sshpass = getpass.getpass(prompt="SSH password: ") |
325 | 325 |
become_prompt = "%s password[defaults to SSH password]: " % become_prompt_method |
326 |
- if sshpass: |
|
327 |
- sshpass = to_bytes(sshpass, errors='strict', nonstring='simplerepr') |
|
328 | 326 |
else: |
329 | 327 |
become_prompt = "%s password: " % become_prompt_method |
330 | 328 |
|
... | ... |
@@ -332,17 +330,15 @@ class CLI(with_metaclass(ABCMeta, object)): |
332 | 332 |
becomepass = getpass.getpass(prompt=become_prompt) |
333 | 333 |
if op.ask_pass and becomepass == '': |
334 | 334 |
becomepass = sshpass |
335 |
- if becomepass: |
|
336 |
- becomepass = to_bytes(becomepass) |
|
337 | 335 |
except EOFError: |
338 | 336 |
pass |
339 | 337 |
|
340 | 338 |
# we 'wrap' the passwords to prevent templating as |
341 | 339 |
# they can contain special chars and trigger it incorrectly |
342 | 340 |
if sshpass: |
343 |
- sshpass = AnsibleUnsafeBytes(sshpass) |
|
341 |
+ sshpass = AnsibleUnsafeText(to_text(sshpass)) |
|
344 | 342 |
if becomepass: |
345 |
- becomepass = AnsibleUnsafeBytes(becomepass) |
|
343 |
+ becomepass = AnsibleUnsafeText(to_text(becomepass)) |
|
346 | 344 |
|
347 | 345 |
return (sshpass, becomepass) |
348 | 346 |
|
0 | 4 |
new file mode 100755 |
... | ... |
@@ -0,0 +1,21 @@ |
0 |
+#!/usr/bin/env python |
|
1 |
+# Copyright (c) 2019 Matt Martz <matt@sivel.net> |
|
2 |
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) |
|
3 |
+ |
|
4 |
+# Make coding more python3-ish |
|
5 |
+from __future__ import (absolute_import, division, print_function) |
|
6 |
+__metaclass__ = type |
|
7 |
+ |
|
8 |
+import os |
|
9 |
+ |
|
10 |
+import pexpect |
|
11 |
+ |
|
12 |
+os.environ['ANSIBLE_NOCOLOR'] = '1' |
|
13 |
+out = pexpect.run( |
|
14 |
+ 'ansible localhost -m debug -a msg="{{ ansible_password }}" -k', |
|
15 |
+ events={ |
|
16 |
+ 'SSH password:': '{{ 1 + 2 }}\n' |
|
17 |
+ } |
|
18 |
+) |
|
19 |
+ |
|
20 |
+assert b'{{ 1 + 2 }}' in out |
... | ... |
@@ -26,6 +26,7 @@ from ansible.module_utils.six import string_types |
26 | 26 |
from ansible.playbook.attribute import FieldAttribute |
27 | 27 |
from ansible.template import Templar |
28 | 28 |
from ansible.playbook import base |
29 |
+from ansible.utils.unsafe_proxy import AnsibleUnsafeBytes, AnsibleUnsafeText |
|
29 | 30 |
|
30 | 31 |
from units.mock.loader import DictDataLoader |
31 | 32 |
|
... | ... |
@@ -626,3 +627,10 @@ class TestBaseSubClass(TestBase): |
626 | 626 |
ds = {'test_attr_method_missing': a_string} |
627 | 627 |
bsc = self._base_validate(ds) |
628 | 628 |
self.assertEquals(bsc.test_attr_method_missing, a_string) |
629 |
+ |
|
630 |
+ def test_get_validated_value_string_rewrap_unsafe(self): |
|
631 |
+ value = AnsibleUnsafeText(u'bar') |
|
632 |
+ ds = {'test_attr_string': value} |
|
633 |
+ bsc = self._base_validate(ds) |
|
634 |
+ self.assertIsInstance(bsc.test_attr_string, AnsibleUnsafeText) |
|
635 |
+ self.assertEquals(bsc.test_attr_string, AnsibleUnsafeText(u'bar')) |