Browse code

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

* 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)
* addresses CVE-2018-16859

Matt Davis authored on 2018/11/17 10:09:20
Showing 8 changed files
1 1
new file mode 100644
... ...
@@ -0,0 +1,2 @@
0
+bugfixes:
1
+- Windows - prevent sensitive content from appearing in scriptblock logging (CVE 2018-16859)
... ...
@@ -850,7 +850,8 @@ def _find_module_utils(module_name, b_module_data, module_path, module_args, tas
850 850
         # FUTURE: smuggle this back as a dict instead of serializing here; the connection plugin may need to modify it
851 851
         module_json = json.dumps(exec_manifest)
852 852
 
853
-        b_module_data = exec_wrapper.replace(b"$json_raw = ''", b"$json_raw = @'\r\n%s\r\n'@" % to_bytes(module_json))
853
+        # delimit the payload JSON from the wrapper to keep sensitive contents out of scriptblocks (which can be logged)
854
+        b_module_data = to_bytes(exec_wrapper) + b'\0\0\0\0' + to_bytes(module_json)
854 855
 
855 856
     elif module_substyle == 'jsonargs':
856 857
         module_args_json = to_bytes(json.dumps(module_args))
857 858
new file mode 100644
858 859
new file mode 100644
... ...
@@ -0,0 +1,7 @@
0
+&chcp.com 65001 > $null
1
+$exec_wrapper_str = $input | Out-String
2
+$split_parts = $exec_wrapper_str.Split(@("`0`0`0`0"), 2, [StringSplitOptions]::RemoveEmptyEntries)
3
+If (-not $split_parts.Length -eq 2) { throw "invalid payload" }
4
+Set-Variable -Name json_raw -Value $split_parts[1]
5
+$exec_wrapper = [ScriptBlock]::Create($split_parts[0])
6
+&$exec_wrapper
... ...
@@ -126,12 +126,13 @@ class ActionModule(ActionBase):
126 126
             exec_data = None
127 127
             # WinRM requires a special wrapper to work with environment variables
128 128
             if self._connection.transport == "winrm":
129
-                pay = self._connection._create_raw_wrapper_payload(script_cmd,
130
-                                                                   env_dict)
131
-                exec_data = exec_wrapper.replace(b"$json_raw = ''",
132
-                                                 b"$json_raw = @'\r\n%s\r\n'@"
133
-                                                 % to_bytes(pay))
134
-                script_cmd = "-"
129
+                pay = self._connection._create_raw_wrapper_payload(script_cmd, env_dict)
130
+                exec_data = to_bytes(exec_wrapper) + b"\0\0\0\0" + to_bytes(pay)
131
+
132
+                # build the necessary exec wrapper command
133
+                # FUTURE: this still doesn't let script work on Windows with non-pipelined connections or
134
+                # full manual exec of KEEP_REMOTE_FILES
135
+                script_cmd = self._connection._shell.build_module_command(env_string='', shebang='#!powershell', cmd='')
135 136
 
136 137
             result.update(self._low_level_execute_command(cmd=script_cmd, in_data=exec_data, sudoable=True, chdir=chdir))
137 138
 
... ...
@@ -103,6 +103,7 @@ import traceback
103 103
 import json
104 104
 import tempfile
105 105
 import subprocess
106
+import xml.etree.ElementTree as ET
106 107
 
107 108
 HAVE_KERBEROS = False
108 109
 try:
... ...
@@ -550,14 +551,17 @@ class Connection(ConnectionBase):
550 550
         return (result.status_code, result.std_out, result.std_err)
551 551
 
552 552
     def is_clixml(self, value):
553
-        return value.startswith(b"#< CLIXML")
553
+        return value.startswith(b"#< CLIXML\r\n")
554 554
 
555 555
     # hacky way to get just stdout- not always sure of doc framing here, so use with care
556 556
     def parse_clixml_stream(self, clixml_doc, stream_name='Error'):
557
-        clear_xml = clixml_doc.replace(b'#< CLIXML\r\n', b'')
558
-        doc = xmltodict.parse(clear_xml)
559
-        lines = [l.get('#text', '').replace('_x000D__x000A_', '') for l in doc.get('Objs', {}).get('S', {}) if l.get('@S') == stream_name]
560
-        return '\r\n'.join(lines)
557
+        clixml = ET.fromstring(clixml_doc.split(b"\r\n", 1)[-1])
558
+        namespace_match = re.match(r'{(.*)}', clixml.tag)
559
+        namespace = "{%s}" % namespace_match.group(1) if namespace_match else ""
560
+
561
+        strings = clixml.findall("./%sS" % namespace)
562
+        lines = [e.text.replace('_x000D__x000A_', '') for e in strings if e.attrib.get('S') == stream_name]
563
+        return to_bytes('\r\n'.join(lines))
561 564
 
562 565
     # FUTURE: determine buffer size at runtime via remote winrm config?
563 566
     def _put_file_stdin_iterator(self, in_path, out_path, buffer_size=250000):
... ...
@@ -44,6 +44,7 @@ import base64
44 44
 import os
45 45
 import re
46 46
 import shlex
47
+import pkgutil
47 48
 
48 49
 from ansible.errors import AnsibleError
49 50
 from ansible.module_utils._text import to_native, to_text
... ...
@@ -78,8 +79,10 @@ begin {
78 78
     # stream JSON including become_pw, ps_module_payload, bin_module_payload, become_payload, write_payload_path, preserve directives
79 79
     # exec runspace, capture output, cleanup, return module output
80 80
 
81
-    # NB: do not adjust the following line- it is replaced when doing non-streamed module output
82
-    $json_raw = ''
81
+    # only init and stream in $json_raw if it wasn't set by the enclosing scope
82
+    if (-not $(Get-Variable "json_raw" -ErrorAction SilentlyContinue)) {
83
+        $json_raw = ''
84
+    }
83 85
 }
84 86
 process {
85 87
     $input_as_string = [string]$input
... ...
@@ -2022,9 +2025,11 @@ class ShellModule(ShellBase):
2022 2022
         return self._encode_script(script)
2023 2023
 
2024 2024
     def build_module_command(self, env_string, shebang, cmd, arg_path=None):
2025
+        bootstrap_wrapper = pkgutil.get_data("ansible.executor.powershell", "bootstrap_wrapper.ps1")
2026
+
2025 2027
         # pipelining bypass
2026 2028
         if cmd == '':
2027
-            return '-'
2029
+            return self._encode_script(script=bootstrap_wrapper, strict_mode=False, preserve_rc=False)
2028 2030
 
2029 2031
         # non-pipelining
2030 2032
 
... ...
@@ -2032,8 +2037,11 @@ class ShellModule(ShellBase):
2032 2032
         cmd_parts = list(map(to_text, cmd_parts))
2033 2033
         if shebang and shebang.lower() == '#!powershell':
2034 2034
             if not self._unquote(cmd_parts[0]).lower().endswith('.ps1'):
2035
+                # we're running a module via the bootstrap wrapper
2035 2036
                 cmd_parts[0] = '"%s.ps1"' % self._unquote(cmd_parts[0])
2036
-            cmd_parts.insert(0, '&')
2037
+            wrapper_cmd = "type " + cmd_parts[0] + " | " + self._encode_script(script=bootstrap_wrapper,
2038
+                                                                               strict_mode=False, preserve_rc=False)
2039
+            return wrapper_cmd
2037 2040
         elif shebang and shebang.startswith('#!'):
2038 2041
             cmd_parts.insert(0, shebang[2:])
2039 2042
         elif not shebang:
... ...
@@ -1,7 +1,7 @@
1 1
 - set_fact:
2 2
     become_test_username: ansible_become_test
3 3
     become_test_admin_username: ansible_become_admin
4
-    gen_pw: password123! + {{ lookup('password', '/dev/null chars=ascii_letters,digits length=8') }}
4
+    gen_pw: "{{ 'password123!' + lookup('password', '/dev/null chars=ascii_letters,digits length=8') }}"
5 5
 
6 6
 - name: create unprivileged user
7 7
   win_user:
... ...
@@ -29,6 +29,10 @@
29 29
   - SeInteractiveLogonRight
30 30
   - SeBatchLogonRight
31 31
 
32
+- name: fetch current target date/time for log filtering
33
+  raw: '[datetime]::now | Out-String'
34
+  register: test_starttime
35
+
32 36
 - name: execute tests and ensure that test user is deleted regardless of success/failure
33 37
   block:
34 38
   - name: ensure current user is not the become user
... ...
@@ -219,6 +223,7 @@
219 219
     assert:
220 220
       that:
221 221
       - whoami_out is successful
222
+      - become_test_username in whoami_out.stdout
222 223
     when: os_version.stdout_lines[0] == "async"
223 224
 
224 225
   - name: test failure with string become invalid key
... ...
@@ -320,6 +325,18 @@
320 320
       - nonascii_output.stdout_lines[0] == 'über den Fußgängerübergang gehen'
321 321
       - nonascii_output.stderr == ''
322 322
 
323
+  - name: get PS events containing password or module args created since test start
324
+    raw: |
325
+      $dt=[datetime]"{{ test_starttime.stdout|trim }}"
326
+      (Get-WinEvent -LogName Microsoft-Windows-Powershell/Operational |
327
+      ? { $_.TimeCreated -ge $dt -and $_.Message -match "{{ gen_pw }}|whoami" }).Count
328
+    register: ps_log_count
329
+
330
+  - name: assert no PS events contain password or module args
331
+    assert:
332
+      that:
333
+      - ps_log_count.stdout | int == 0
334
+
323 335
 # FUTURE: test raw + script become behavior once they're running under the exec wrapper again
324 336
 # FUTURE: add standalone playbook tests to include password prompting and play become keywords
325 337