diff --git a/changelogs/fragments/ps_sb_logging.yaml b/changelogs/fragments/ps_sb_logging.yaml new file mode 100644 index 00000000000..78241df4494 --- /dev/null +++ b/changelogs/fragments/ps_sb_logging.yaml @@ -0,0 +1,2 @@ +bugfixes: +- Windows - prevent sensitive content from appearing in scriptblock logging (CVE 2018-16859) diff --git a/lib/ansible/executor/module_common.py b/lib/ansible/executor/module_common.py index 969ca8d046c..4f4a43890b1 100644 --- a/lib/ansible/executor/module_common.py +++ b/lib/ansible/executor/module_common.py @@ -912,7 +912,8 @@ def _find_module_utils(module_name, b_module_data, module_path, module_args, tas # FUTURE: smuggle this back as a dict instead of serializing here; the connection plugin may need to modify it module_json = json.dumps(exec_manifest) - b_module_data = exec_wrapper.replace(b"$json_raw = ''", b"$json_raw = @'\r\n%s\r\n'@" % to_bytes(module_json)) + # delimit the payload JSON from the wrapper to keep sensitive contents out of scriptblocks (which can be logged) + b_module_data = to_bytes(exec_wrapper) + b'\0\0\0\0' + to_bytes(module_json) elif module_substyle == 'jsonargs': module_args_json = to_bytes(json.dumps(module_args)) diff --git a/lib/ansible/executor/powershell/__init__.py b/lib/ansible/executor/powershell/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/lib/ansible/executor/powershell/bootstrap_wrapper.ps1 b/lib/ansible/executor/powershell/bootstrap_wrapper.ps1 new file mode 100644 index 00000000000..cfad82703db --- /dev/null +++ b/lib/ansible/executor/powershell/bootstrap_wrapper.ps1 @@ -0,0 +1,7 @@ +&chcp.com 65001 > $null +$exec_wrapper_str = $input | Out-String +$split_parts = $exec_wrapper_str.Split(@("`0`0`0`0"), 2, [StringSplitOptions]::RemoveEmptyEntries) +If (-not $split_parts.Length -eq 2) { throw "invalid payload" } +Set-Variable -Name json_raw -Value $split_parts[1] +$exec_wrapper = [ScriptBlock]::Create($split_parts[0]) +&$exec_wrapper diff --git a/lib/ansible/plugins/action/script.py b/lib/ansible/plugins/action/script.py index 76e0d5c9f7e..26b8fd038d6 100644 --- a/lib/ansible/plugins/action/script.py +++ b/lib/ansible/plugins/action/script.py @@ -126,12 +126,13 @@ class ActionModule(ActionBase): exec_data = None # WinRM requires a special wrapper to work with environment variables if self._connection.transport == "winrm": - pay = self._connection._create_raw_wrapper_payload(script_cmd, - env_dict) - exec_data = exec_wrapper.replace(b"$json_raw = ''", - b"$json_raw = @'\r\n%s\r\n'@" - % to_bytes(pay)) - script_cmd = "-" + pay = self._connection._create_raw_wrapper_payload(script_cmd, env_dict) + exec_data = to_bytes(exec_wrapper) + b"\0\0\0\0" + to_bytes(pay) + + # build the necessary exec wrapper command + # FUTURE: this still doesn't let script work on Windows with non-pipelined connections or + # full manual exec of KEEP_REMOTE_FILES + script_cmd = self._connection._shell.build_module_command(env_string='', shebang='#!powershell', cmd='') result.update(self._low_level_execute_command(cmd=script_cmd, in_data=exec_data, sudoable=True, chdir=chdir)) diff --git a/lib/ansible/plugins/connection/psrp.py b/lib/ansible/plugins/connection/psrp.py index c3249cfb4a1..7aa0d909e22 100644 --- a/lib/ansible/plugins/connection/psrp.py +++ b/lib/ansible/plugins/connection/psrp.py @@ -278,6 +278,7 @@ class Connection(ConnectionBase): # starting a new interpreter to save on time b_command = base64.b64decode(cmd.split(" ")[-1]) script = to_text(b_command, 'utf-16-le') + in_data = to_text(in_data, errors="surrogate_or_strict", nonstring="passthru") display.vvv("PSRP: EXEC %s" % script, host=self._psrp_host) else: # in other cases we want to execute the cmd as the script diff --git a/lib/ansible/plugins/connection/winrm.py b/lib/ansible/plugins/connection/winrm.py index 127c76dfd39..2d43c5f916a 100644 --- a/lib/ansible/plugins/connection/winrm.py +++ b/lib/ansible/plugins/connection/winrm.py @@ -103,6 +103,7 @@ import traceback import json import tempfile import subprocess +import xml.etree.ElementTree as ET HAVE_KERBEROS = False try: @@ -550,14 +551,17 @@ class Connection(ConnectionBase): return (result.status_code, result.std_out, result.std_err) def is_clixml(self, value): - return value.startswith(b"#< CLIXML") + return value.startswith(b"#< CLIXML\r\n") # hacky way to get just stdout- not always sure of doc framing here, so use with care def parse_clixml_stream(self, clixml_doc, stream_name='Error'): - clear_xml = clixml_doc.replace(b'#< CLIXML\r\n', b'') - doc = xmltodict.parse(clear_xml) - lines = [l.get('#text', '').replace('_x000D__x000A_', '') for l in doc.get('Objs', {}).get('S', {}) if l.get('@S') == stream_name] - return '\r\n'.join(lines) + clixml = ET.fromstring(clixml_doc.split(b"\r\n", 1)[-1]) + namespace_match = re.match(r'{(.*)}', clixml.tag) + namespace = "{%s}" % namespace_match.group(1) if namespace_match else "" + + strings = clixml.findall("./%sS" % namespace) + lines = [e.text.replace('_x000D__x000A_', '') for e in strings if e.attrib.get('S') == stream_name] + return to_bytes('\r\n'.join(lines)) # FUTURE: determine buffer size at runtime via remote winrm config? def _put_file_stdin_iterator(self, in_path, out_path, buffer_size=250000): diff --git a/lib/ansible/plugins/shell/powershell.py b/lib/ansible/plugins/shell/powershell.py index 8490cb5b145..283b8420c1d 100644 --- a/lib/ansible/plugins/shell/powershell.py +++ b/lib/ansible/plugins/shell/powershell.py @@ -44,6 +44,7 @@ import base64 import os import re import shlex +import pkgutil from ansible.errors import AnsibleError from ansible.module_utils._text import to_text @@ -78,8 +79,10 @@ begin { # stream JSON including become_pw, ps_module_payload, bin_module_payload, become_payload, write_payload_path, preserve directives # exec runspace, capture output, cleanup, return module output - # NB: do not adjust the following line- it is replaced when doing non-streamed module output - $json_raw = '' + # only init and stream in $json_raw if it wasn't set by the enclosing scope + if (-not $(Get-Variable "json_raw" -ErrorAction SilentlyContinue)) { + $json_raw = '' + } } process { $input_as_string = [string]$input @@ -929,9 +932,11 @@ namespace AnsibleBecome $become_exec_wrapper = { chcp.com 65001 > $null $ProgressPreference = "SilentlyContinue" - $exec_wrapper_str = [System.Console]::In.ReadToEnd() - $exec_wrapper = [ScriptBlock]::Create($exec_wrapper_str) - &$exec_wrapper + $raw = [System.Console]::In.ReadToEnd() + $split_parts = $raw.Split(@("`0`0`0`0"), 0) + If (-not $split_parts.Length -eq 2) { throw "invalid payload" } + $json_raw = $split_parts[1] + &([ScriptBlock]::Create($split_parts[0])) } $exec_wrapper = { @@ -955,10 +960,9 @@ $exec_wrapper = { # stream JSON including become_pw, ps_module_payload, bin_module_payload, become_payload, write_payload_path, preserve directives # exec runspace, capture output, cleanup, return module output. Do not change this as it is set become before being passed to the # become process. - $json_raw = "" - If (-not $json_raw) { - Write-Error "no input given" -Category InvalidArgument + if (-not $(Get-Variable "json_raw" -ErrorAction SilentlyContinue)) { + Write-Error "no payload supplied" -Category InvalidArgument } $payload = ConvertTo-HashtableFromPsCustomObject -myPsObject (ConvertFrom-Json $json_raw) @@ -1095,7 +1099,7 @@ Function Run($payload) { # wrapper which calls our read wrapper passed through stdin. Cannot use 'powershell -' as # the $ErrorActionPreference is always set to Stop and cannot be changed $payload_string = $payload | ConvertTo-Json -Depth 99 -Compress - $exec_wrapper = $exec_wrapper.ToString().Replace('$json_raw = ""', "`$json_raw = '$payload_string'") + $exec_wrapper = $exec_wrapper.ToString() + "`0`0`0`0" + $payload_string $rc = 0 $exec_command = [Convert]::ToBase64String([System.Text.Encoding]::Unicode.GetBytes($become_exec_wrapper.ToString())) @@ -1584,9 +1588,11 @@ class ShellModule(ShellBase): return self._encode_script(script) def build_module_command(self, env_string, shebang, cmd, arg_path=None): + bootstrap_wrapper = pkgutil.get_data("ansible.executor.powershell", "bootstrap_wrapper.ps1") + # pipelining bypass if cmd == '': - return '-' + return self._encode_script(script=bootstrap_wrapper, strict_mode=False, preserve_rc=False) # non-pipelining @@ -1594,8 +1600,11 @@ class ShellModule(ShellBase): cmd_parts = list(map(to_text, cmd_parts)) if shebang and shebang.lower() == '#!powershell': if not self._unquote(cmd_parts[0]).lower().endswith('.ps1'): + # we're running a module via the bootstrap wrapper cmd_parts[0] = '"%s.ps1"' % self._unquote(cmd_parts[0]) - cmd_parts.insert(0, '&') + wrapper_cmd = "type " + cmd_parts[0] + " | " + self._encode_script(script=bootstrap_wrapper, + strict_mode=False, preserve_rc=False) + return wrapper_cmd elif shebang and shebang.startswith('#!'): cmd_parts.insert(0, shebang[2:]) elif not shebang: diff --git a/test/integration/targets/win_become/tasks/main.yml b/test/integration/targets/win_become/tasks/main.yml index 1f2f241884a..c9a3a02282e 100644 --- a/test/integration/targets/win_become/tasks/main.yml +++ b/test/integration/targets/win_become/tasks/main.yml @@ -1,7 +1,7 @@ - set_fact: become_test_username: ansible_become_test become_test_admin_username: ansible_become_admin - gen_pw: password123! + {{ lookup('password', '/dev/null chars=ascii_letters,digits length=8') }} + gen_pw: "{{ 'password123!' + lookup('password', '/dev/null chars=ascii_letters,digits length=8') }}" - name: create unprivileged user win_user: @@ -29,6 +29,10 @@ - SeInteractiveLogonRight - SeBatchLogonRight +- name: fetch current target date/time for log filtering + raw: '[datetime]::now | Out-String' + register: test_starttime + - name: execute tests and ensure that test user is deleted regardless of success/failure block: - name: ensure current user is not the become user @@ -199,6 +203,7 @@ assert: that: - whoami_out is successful + - become_test_username in whoami_out.stdout - name: test failure with string become invalid key vars: *become_vars @@ -312,6 +317,18 @@ - nonascii_output.stdout_lines[0] == 'über den Fußgängerübergang gehen' - nonascii_output.stderr == '' + - name: get PS events containing password or module args created since test start + raw: | + $dt=[datetime]"{{ test_starttime.stdout|trim }}" + (Get-WinEvent -LogName Microsoft-Windows-Powershell/Operational | + ? { $_.TimeCreated -ge $dt -and $_.Message -match "{{ gen_pw }}|whoami" }).Count + register: ps_log_count + + - name: assert no PS events contain password or module args + assert: + that: + - ps_log_count.stdout | int == 0 + # FUTURE: test raw + script become behavior once they're running under the exec wrapper again # FUTURE: add standalone playbook tests to include password prompting and play become keywords