Fix python meterpreter network method exceptions for OSX #651

adfoster-r7 · 2023-06-05T09:48:03Z

Fixes python meterpreter network method exceptions for OSX, and adds unit tests / CI automation

Fixes the following errors in test/meterpreter that I originally spotted running on CI

-            known_failures: [
-              [
-                [
-                  "[-] [should return network interfaces] FAILED: should return network interfaces",
-                  "[-] [should return network interfaces] Exception: Rex::Post::Meterpreter::RequestError : stdapi_net_config_get_interfaces: Operation failed: Python exception: TypeError",
-                  "[-] [should have an interface that matches session_host] FAILED: should have an interface that matches session_host",
-                  "[-] [should have an interface that matches session_host] Exception: Rex::Post::Meterpreter::RequestError : stdapi_net_config_get_interfaces: Operation failed: Python exception: TypeError"
-                ],
-                {
-                  if: [
-                    [
-                      :meterpreter_runtime_version,
-                      :==,
-                      "python3.8"
-                    ],
-                    :or,
-                    [
-                      :meterpreter_runtime_version,
-                      :==,
-                      "python3.6"
-                    ]
-                  ]
-                }
-              ]
-            ]
+            known_failures: []

adfoster-r7 · 2023-06-05T10:11:19Z

python/meterpreter/README.md

+Running unit tests:
+
+```
+python -m unittest discover -v ./tests


I prefer pytest, but I thought only depending on the inbuilt testing library would be easier for contributors

adfoster-r7 · 2023-06-05T10:15:19Z

python/meterpreter/ext_server_stdapi.py

+        if 'flags_str' in iface_info:
+            iface_tlv += tlv_pack(TLV_TYPE_INTERFACE_FLAGS, iface_info['flags_str'])


OSX was returning the integer flag value here, whilst linux was returning the string flag names. Now the implementations return both separately

This seems to be introducing new data into the TLV, is there value for both vs just shifting to a more consistent type retuned in flags?

TLV_TYPE_INTERFACE_FLAGS has always been a string:

TLV_TYPE_INTERFACE_FLAGS = TLV_META_TYPE_STRING | 1403

Previously the OSX code path was was extracting out the integer value for flags instead of a string like in the Linux codepath, i.e. in the osx code path attempted to write out 8836 instead of "UP,BROADCAST,SMART,RUNNING,SIMPLEX,MULTICAST" from the below ifconfig output

en5: flags=8863<UP,BROADCAST,SMART,RUNNING,SIMPLEX,MULTICAST> mtu 1500

This is now updated to send the string flag names for both OSX and Linux

Got it, stdapi_net_config_get_interfaces_via_netlink was returning flags as a set of strings names and stdapi_net_config_get_interfaces_via_osx_ifconfig was returning flags as numeric values now

Now both return flags as numeric values and flags_str as string names and only flags_str is being placed in the response as a TLV_TYPE_INTERFACE_FLAGS packet.

adfoster-r7 · 2023-06-05T10:15:45Z

python/meterpreter/ext_server_stdapi.py

@@ -2271,19 +2272,20 @@ def stdapi_net_config_get_interfaces_via_osx_ifconfig():
    proc_h = subprocess.Popen('/sbin/ifconfig', stdout=subprocess.PIPE, stderr=subprocess.PIPE)
    if proc_h.wait():
        raise Exception('ifconfig exited with non-zero status')
-    output = proc_h.stdout.read()
+    output = str(proc_h.stdout.read())


Required in newer versions of Python - when the later code attempts to split this result with for line in output.split('\n') the following exception is raised:

[-] method stdapi_net_config_get_interfaces resulted in an error Traceback (most recent call last): File "<string>", line 1725, in create_response File "ext_server_stdapi.py", line 2191, in stdapi_net_config_get_interfaces interfaces = stdapi_net_config_get_interfaces_via_osx_ifconfig() File "ext_server_stdapi.py", line 2293, in stdapi_net_config_get_interfaces_via_osx_ifconfig for line in output.split('\n'): TypeError: a bytes-like object is required, not 'str'

adfoster-r7 · 2023-06-05T10:23:25Z

python/meterpreter/ext_server_stdapi.py

@@ -2524,7 +2526,7 @@ def stdapi_net_config_get_routes_via_osx_netstat():
            continue
        if destination == 'default':
            destination = all_nets
-        if re.match('link#\d+', gateway) or re.match('([0-9a-f]{1,2}:){5}[0-9a-f]{1,2}', gateway):
+        if re.match('link#\\d+', gateway) or re.match('([0-9a-f]{1,2}:){5}[0-9a-f]{1,2}', gateway):


Fixes:

test_stdapi_net_config_get_interfaces_via_osx_ifconfig (test_ext_server_stdapi.TestExtServerStdApi.test_stdapi_net_config_get_interfaces_via_osx_ifconfig) ... :2529: DeprecationWarning: invalid escape sequence '\d'

adfoster-r7 · 2023-06-05T10:36:14Z

python/meterpreter/ext_server_stdapi.py

@@ -2487,7 +2489,7 @@ def stdapi_net_config_get_routes_via_osx_netstat():
    proc_h = subprocess.Popen(['/usr/sbin/netstat', '-rn'], stdout=subprocess.PIPE, stderr=subprocess.PIPE)


Looks like there's a file descriptor leak here, will put up a separate PR for this

$ python3 -m unittest discover -v ./tests test_stdapi_net_config_get_interfaces (test_ext_server_stdapi.TestExtServerStdApi.test_stdapi_net_config_get_interfaces) ... <string>:2183: ResourceWarning: unclosed file <_io.BufferedReader name=3> Object allocated at (most recent call last): File "/usr/local/Cellar/python@3.11/3.11.2_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/subprocess.py", lineno 1014 self.stdout = io.open(c2pread, 'rb', bufsize) <string>:2183: ResourceWarning: unclosed file <_io.BufferedReader name=5> Object allocated at (most recent call last): File "/usr/local/Cellar/python@3.11/3.11.2_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/subprocess.py", lineno 1019 self.stderr = io.open(errread, 'rb', bufsize)

adfoster-r7 · 2023-06-05T10:50:23Z

python/meterpreter/tests/test_ext_server_stdapi.py

+    def test_stdapi_net_config_get_interfaces(self):
+        request = bytes()
+        response = bytes()
+        self.assertErrorSuccess("stdapi_net_config_get_interfaces", request, response)


I'll skip verifying the exact bytes for now, this was just an MVP "make sure it doesn't explode" test for now

python/meterpreter/README.md

adfoster-r7 force-pushed the fix-python-meterpreter-osx-network-method-exceptions branch 2 times, most recently from 4670631 to cc35943 Compare June 5, 2023 10:10

adfoster-r7 commented Jun 5, 2023

View reviewed changes

adfoster-r7 force-pushed the fix-python-meterpreter-osx-network-method-exceptions branch from cc35943 to f684570 Compare June 5, 2023 10:12

adfoster-r7 commented Jun 5, 2023

View reviewed changes

Fix python meterpreter network method exceptions

c494d9e

adfoster-r7 force-pushed the fix-python-meterpreter-osx-network-method-exceptions branch from f684570 to c494d9e Compare June 5, 2023 10:44

adfoster-r7 commented Jun 5, 2023

View reviewed changes

adfoster-r7 force-pushed the fix-python-meterpreter-osx-network-method-exceptions branch 19 times, most recently from ae7f5fb to 66740e7 Compare June 5, 2023 12:08

Add automated tests for Python Meterpreter

a1166ac

adfoster-r7 force-pushed the fix-python-meterpreter-osx-network-method-exceptions branch from 66740e7 to a1166ac Compare June 5, 2023 12:11

adfoster-r7 mentioned this pull request Jun 7, 2023

Fix python meterpreter subprocess deadlock and file descriptor leak #653

Merged

dwelch-r7 reviewed Jun 8, 2023

View reviewed changes

python/meterpreter/README.md Show resolved Hide resolved

smcintyre-r7 assigned space-r7 Jun 9, 2023

space-r7 removed their assignment Jun 9, 2023

cgranleese-r7 self-assigned this Jun 14, 2023

cgranleese-r7 merged commit c6a4850 into rapid7:master Jun 14, 2023

This was referenced Jun 14, 2023

Fix python meterpreter crashing when extracting osx network configuration rapid7/metasploit-framework#18097

Merged

Add python 2.5,2.6,3.1,3.2 test automation #658

Open

This was referenced Jul 24, 2023

Add Meterpreter sanity tests to CI rapid7/metasploit-framework#18210

Merged

Add unit tests to PHP Meterpreter #668

Open

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix python meterpreter network method exceptions for OSX #651

Fix python meterpreter network method exceptions for OSX #651

adfoster-r7 commented Jun 5, 2023 •

edited

Loading

adfoster-r7 Jun 5, 2023

adfoster-r7 Jun 5, 2023

jmartin-tech Jun 5, 2023

adfoster-r7 Jun 5, 2023

jmartin-tech Jun 5, 2023 •

edited

Loading

adfoster-r7 Jun 5, 2023 •

edited

Loading

adfoster-r7 Jun 5, 2023 •

edited

Loading

adfoster-r7 Jun 5, 2023

adfoster-r7 Jun 5, 2023

		if 'flags_str' in iface_info:
		iface_tlv += tlv_pack(TLV_TYPE_INTERFACE_FLAGS, iface_info['flags_str'])

		@@ -2487,7 +2489,7 @@ def stdapi_net_config_get_routes_via_osx_netstat():
		proc_h = subprocess.Popen(['/usr/sbin/netstat', '-rn'], stdout=subprocess.PIPE, stderr=subprocess.PIPE)

Fix python meterpreter network method exceptions for OSX #651

Fix python meterpreter network method exceptions for OSX #651

Conversation

adfoster-r7 commented Jun 5, 2023 • edited Loading

adfoster-r7 Jun 5, 2023

Choose a reason for hiding this comment

adfoster-r7 Jun 5, 2023

Choose a reason for hiding this comment

jmartin-tech Jun 5, 2023

Choose a reason for hiding this comment

adfoster-r7 Jun 5, 2023

Choose a reason for hiding this comment

jmartin-tech Jun 5, 2023 • edited Loading

Choose a reason for hiding this comment

adfoster-r7 Jun 5, 2023 • edited Loading

Choose a reason for hiding this comment

adfoster-r7 Jun 5, 2023 • edited Loading

Choose a reason for hiding this comment

adfoster-r7 Jun 5, 2023

Choose a reason for hiding this comment

adfoster-r7 Jun 5, 2023

Choose a reason for hiding this comment

adfoster-r7 commented Jun 5, 2023 •

edited

Loading

jmartin-tech Jun 5, 2023 •

edited

Loading

adfoster-r7 Jun 5, 2023 •

edited

Loading

adfoster-r7 Jun 5, 2023 •

edited

Loading