split PS wrapper and payload (CVE-2018-16859) (#49143)

* prevent scriptblock logging from logging payload contents
* added tests to verify no payload contents in PS Operational event log
* fix script action to send split-aware wrapper
* fix CLIXML error parser (return to -EncodedCommand exposed problems with it)
This commit is contained in:
Matt Davis 2018-11-26 15:28:47 -08:00 committed by GitHub
parent 29e2454f7c
commit 2f8d3fcf41
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 66 additions and 24 deletions

View file

@ -0,0 +1,2 @@
bugfixes:
- Windows - prevent sensitive content from appearing in scriptblock logging (CVE 2018-16859)

View file

@ -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))

View file

@ -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

View file

@ -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))

View file

@ -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

View file

@ -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):

View file

@ -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,9 +79,11 @@ 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
# 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:

View file

@ -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