Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bugs for TLS PCAPs and refactor pyrdp-convert code #311

Merged
merged 15 commits into from
Aug 3, 2021
Merged

Conversation

xshill
Copy link
Collaborator

@xshill xshill commented Apr 22, 2021

Big refactor of the pyrdp-convert code and some bug fixes. Most notably:

  • The pyrdp-convert code has its own module now (like the player), the launch script only bootstraps this code.
  • Classes and functions have been grouped together in separate files instead of having everything in one file.
  • We use Scapy's TCP / TLS reconstruction feature instead of doing it manually (which is complicated). Basically, Scapy will first reconstruct all the TLS packets with a generic session (the packets aren't decrypted). Then, we pass all those reconstructed TLS packets in a different session that has the master secret to decrypt them. This works despite the fact that the first couple RDP packets don't actually use TLS.

@xshill xshill linked an issue Apr 22, 2021 that may be closed by this pull request
Copy link
Collaborator

@obilodeau obilodeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Large diff makes this difficult to review. I'm assuming a lot of code was moved without being changed. I'm going to do functional tests right now and report back.

pyrdp/player/RenderingEventHandler.py Outdated Show resolved Hide resolved
@obilodeau obilodeau added this to the v1.1.0 milestone Apr 28, 2021
@obilodeau
Copy link
Collaborator

obilodeau commented Apr 28, 2021

I ran the convert tool on a couple of pcaps and I find odd file structures being generated:

$ find
.
./certs
./20212819120351_10.x.y.z-10.i.j.k.pyrdp
./filesystems
./filesystems/replay
./filesystems/replay/device1
./filesystems/replay/device4
./filesystems/replay/device2
./filesystems/replay/device5
./filesystems/replay/device3
./20213019120329_10.k.l.m-10.m.n.o.pyrdp
./replay
./replay/mimikatz_trunk.zip
./replay/mimikatz_trunk_1.zip

There shouldn't be a replay/ directory under filesystems/ and the mimikatz files should be in a files/ folder not a replay one.

@obilodeau
Copy link
Collaborator

obilodeau commented Apr 28, 2021

Another issue, with one pcap I have, Python segfaults with a stack overflow error:

$ pyrdp-convert.py -f problematic.pcap 
[*] Analyzing PCAP 'problematic.pcap' ...
Fatal Python error: _Py_CheckRecursiveCall: Cannot recover from stack overflow.
Python runtime state: initialized

Current thread 0x00007f4cc6121740 (most recent call first):
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/crypto/cipher_stream.py", line 75 in __setattr__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/crypto/cipher_stream.py", line 119 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/session.py", line 100 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/session.py", line 259 in snapshot
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/session.py", line 885 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/base_classes.py", line 389 in __call__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 121 in m2i
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 163 in getfield
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 951 in do_dissect
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 994 in dissect
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 159 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/session.py", line 913 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 311 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/base_classes.py", line 389 in __call__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 576 in do_dissect_payload
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 999 in dissect
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 159 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/session.py", line 913 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 311 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/base_classes.py", line 389 in __call__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 576 in do_dissect_payload
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 999 in dissect
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 159 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/session.py", line 913 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 311 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/base_classes.py", line 389 in __call__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 576 in do_dissect_payload
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 999 in dissect
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 159 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/session.py", line 913 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 311 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/base_classes.py", line 389 in __call__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 576 in do_dissect_payload

[...]

Running in ipdb it took 100% CPU for several minutes. Slowly increased CPU usage until my out of memory manager (OOM) killed it.

Investigating an upstream issue.

@obilodeau
Copy link
Collaborator

With scapy 2.4.4 (latest is 2.4.5) on that same pcap, memory consumed by Python increases up to 8GB RSS memory. At this point, something bubbles up and Python is terminated. This is for a relatively straightforward 4MB pcap. Something is wrong.

Apr 28 15:49:21 barachois earlyoom[907]: low memory! at or below SIGTERM limits: mem 10.00%, swap 100.00%
Apr 28 15:49:21 barachois earlyoom[907]: sending SIGTERM to process 139490 uid 1000 "pyrdp-convert.p": badness 874, VmRSS 11901 MiB
Apr 28 15:49:21 barachois earlyoom[907]: process exited after 0.5 seconds
$ python -V
Python 3.9.4

Wait a second. I wasn't specifying a TLS master secret log...

@obilodeau
Copy link
Collaborator

In the code, the TLS secret verification happens after that stacktrace or OOM reap so it didn't matter if I passed the file or not. The issue is still present.

@xshill
Copy link
Collaborator Author

xshill commented Apr 29, 2021

Another issue, with one pcap I have, Python segfaults with a stack overflow error:

$ pyrdp-convert.py -f problematic.pcap 
[*] Analyzing PCAP 'problematic.pcap' ...
Fatal Python error: _Py_CheckRecursiveCall: Cannot recover from stack overflow.
Python runtime state: initialized

Current thread 0x00007f4cc6121740 (most recent call first):
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/crypto/cipher_stream.py", line 75 in __setattr__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/crypto/cipher_stream.py", line 119 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/session.py", line 100 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/session.py", line 259 in snapshot
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/session.py", line 885 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/base_classes.py", line 389 in __call__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 121 in m2i
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 163 in getfield
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 951 in do_dissect
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 994 in dissect
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 159 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/session.py", line 913 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 311 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/base_classes.py", line 389 in __call__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 576 in do_dissect_payload
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 999 in dissect
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 159 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/session.py", line 913 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 311 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/base_classes.py", line 389 in __call__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 576 in do_dissect_payload
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 999 in dissect
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 159 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/session.py", line 913 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 311 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/base_classes.py", line 389 in __call__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 576 in do_dissect_payload
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 999 in dissect
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/packet.py", line 159 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/session.py", line 913 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 311 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/base_classes.py", line 389 in __call__
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/scapy/layers/tls/record.py", line 576 in do_dissect_payload

[...]

Running in ipdb it took 100% CPU for several minutes. Slowly increased CPU usage until my out of memory manager (OOM) killed it.

Investigating an upstream issue.

This is all scapy stuff, we would need the actual PyRDP line that provokes this. Also, how big is the PCAP?

@obilodeau
Copy link
Collaborator

This is all scapy stuff, we would need the actual PyRDP line that provokes this.

The stacktrace is too long, it finishes before it leads to pyrdp. With the debugger it makes me think it is at the sniff() call (PCAPConverter.py around line 52) but as I said earlier with the debugger I don't get a Python fatal error, instead my OOM kills the process before that (at around 12GB of RSS RAM consumed).

Also, how big is the PCAP?

Except for the file output problem, all the tests I have done today and documented here were on a 4 megabyte pcap with 5 TCP streams. 2 or 3 of which gathers certificates and 2 real connections (same source and dst IP). Manually splitting the pcap to layer 7 PDUs works fine.

@obilodeau
Copy link
Collaborator

More tests tonight:

  • Tested the branch in our docker build with Python 3.8 and scapy 2.4.4: same issue
  • Reduced the numbers of TLS secrets: same issue

The problem is in scapy. Using AsyncSniffer I can get a feeling of where the problem happens but adding packet summary and timestamps like this (there's an unnecessary packet counter in that patch that I thought would be useful):

diff --git a/pyrdp/convert/PCAPConverter.py b/pyrdp/convert/PCAPConverter.py
index 9fb5e3b..3caf3c9 100644
--- a/pyrdp/convert/PCAPConverter.py
+++ b/pyrdp/convert/PCAPConverter.py
@@ -49,7 +49,21 @@ class PCAPConverter(Converter):
     def listSessions(self) -> List[Tuple[int, PCAPStream]]:
         print(f"[*] Analyzing PCAP '{self.inputFile}' ...")
         bind_layers(TCP, TLS)
-        pcap = sniff(offline=str(self.inputFile), session=TCPSession)
+        #import ipdb; ipdb.set_trace()
+        #pcap = sniff(offline=str(self.inputFile), session=TCPSession)
+        pcap = rdpcap(str(self.inputFile))
+        pkt_count = len(pcap)
+        #pcap = sniff(offline=str(self.inputFile), session=TCPSession, count=pkt_count)
+        def callb(x):
+            print(f"{x.time}")
+            print(x.summary())
+
+        sniffer = AsyncSniffer(offline=str(self.inputFile), session=TCPSession, count=pkt_count, prn=callb)
+        #sniffer = AsyncSniffer(offline=str(self.inputFile), session=TCPSession, count=pkt_count, prn=lambda x: x.summary())
+        sniffer.start()
+        #import ipdb; ipdb.set_trace()
+        sniffer.join()
+        pcap = sniffer.results
 
         sessions = pcap.sessions(tcp_both)
 

rdpcap finishes so there is no problem there but the session reconstruction code is not in there...

@obilodeau
Copy link
Collaborator

After a long investigation, we can say that the pcap issue is from scapy and is a corner-case that can be worked around by exporting PDUs from wireshark instead. The problem will be tracked here: #318. In order to move forward with 1.1.0 we will proceed with the review ignoring the specific scapy problems.

@obilodeau obilodeau mentioned this pull request Jul 28, 2021
@obilodeau
Copy link
Collaborator

After some initial attempts at reproducing the issue I thought it was no longer there but it only affects replay output type.

pyrdp-convert.py -f replay -o test/ wrong-output-names.pcap

I'll be back with a fix hopefully

@obilodeau
Copy link
Collaborator

I fixed the path issues I was worried about. However, there's a regression with the pcap to mp4 conversion now. I'll look at it tomorrow.

@obilodeau
Copy link
Collaborator

obilodeau commented Jul 30, 2021

I might have an initial fix. Some test cases the fix needs to go through:

  • pcap to replay: files are saved with proper directory structure
  • pcap to replay with -o with directory: files are saved with proper directory structure inside the proper directory structure
  • pcap to replay with -o w/ prefix: files are saved with the proper directory structure inside a prefix
  • pcap to mp4: no transfer artifacts, file is saved in current directory with unique name w/ timestamp and session
  • pcap to mp4 with -o w/ directory: no transfer artifacts, file is saved in given directory with unique name w/ timestamp and session
  • pcap to mp4 with -o w/ prefix: no transfer artifacts, file is saved with unique name w/ timestamp and session and prefix
  • conversion speed to mp4 not slowed down too much
  • pcap to json w/o -o never worked before this refactor
  • pcap to json w/ -o never worked before this refactor

@obilodeau
Copy link
Collaborator

I fixed the master branch's problem with this:

--- a/bin/pyrdp-convert.py
+++ b/bin/pyrdp-convert.py
@@ -66,7 +67,7 @@ def getSink(format: str, outfile: str, progress=None) -> (str, str):
         sys.exit(1)
 
     sink, ext = HANDLERS[format]
-    outfile += f".{ext}"
+    outfile = str(outfile / f".{ext}")
     return sink(outfile, progress=progress) if sink else None, outfile
 

and I can confirm that files were extracted in the current master so I won't consider this a regression of that PR but something to fix for later.

@obilodeau
Copy link
Collaborator

Confirmed that conversion speed on master was slow as well. See #329. Confirmed that file artifacts were generated on master as well, will fix later (if ever). So I only need to make sure that a mp4 file is actually being generated.

@obilodeau
Copy link
Collaborator

Anything converted from a pcap with the output option (-o) will create a directory structure. Deleting associated test tasks.

@obilodeau
Copy link
Collaborator

obilodeau commented Jul 30, 2021

Latest commits fixed Mp4 output but JSON output doesn't work. If that's a regression from master I'll try to fix it, if it never worked, I'll merge this and open a ticket. I updated task list with a couple of comments above to reflect current situation.

@obilodeau
Copy link
Collaborator

In master, previous to this merge, pcap to json wasn't supported. I don't feel bad if this branch doesn't fix that. I opened #331 to track that and offered a workaround in it.

This was the last thing to look at so this branch can be merged now.

@obilodeau obilodeau merged commit 43b26a6 into master Aug 3, 2021
@obilodeau obilodeau deleted the pcap-fix branch August 6, 2021 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pyrdp-convert: Fails to extract replay with multiple connections in PCAP
2 participants