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

xshill · 2021-04-22T15:21:37Z

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.

…ction

bin/pyrdp-convert.py

obilodeau

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

obilodeau · 2021-04-28T19:20:35Z

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 · 2021-04-28T19:30:58Z

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 · 2021-04-28T19:54:40Z

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 · 2021-04-28T19:58:10Z

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.

obilodeau · 2021-04-28T20:03:25Z

Pointers for a fix:

xshill · 2021-04-29T00:16:57Z

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 · 2021-04-29T00:44:27Z

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 · 2021-04-29T03:51:09Z

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 · 2021-06-28T15:36:07Z

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 · 2021-07-29T19:42:46Z

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

However there's a regression in the pcap -> mp4 conversion right now...

obilodeau · 2021-07-30T03:39:43Z

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 · 2021-07-30T18:19:41Z

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 · 2021-07-30T18:50:10Z

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 · 2021-07-30T19:15:06Z

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 · 2021-07-30T20:53:02Z

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

obilodeau · 2021-07-30T21:03:55Z

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 · 2021-07-31T03:57:41Z

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.

Francis Labelle added 5 commits January 7, 2021 16:38

Add descriptionf or RDPReplayer class

00aa233

Show session number in prints and handle missing random

efad3ef

Merge branch 'master' into pcap-fix

75daa01

Refactor into pyrdp-convert into multiple files and fix TCP reconstru…

cb6611b

…ction

Add comment

ea67724

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

pyrdp-convert: Fails to extract replay with multiple connections in PCAP #221

Closed

Fix test?

52c0f8d

obilodeau reviewed Apr 28, 2021

View reviewed changes

bin/pyrdp-convert.py Show resolved Hide resolved

obilodeau reviewed Apr 28, 2021

View reviewed changes

pyrdp/player/RenderingEventHandler.py Outdated Show resolved Hide resolved

obilodeau added this to the v1.1.0 milestone Apr 28, 2021

obilodeau mentioned this pull request Apr 28, 2021

Change pyrdp-convert output paths. #312

Closed

obilodeau mentioned this pull request Jun 7, 2021

maximum recursion depth exceeded with scapy 2.4.4 secdev/scapy#3145

Closed

yraiesets mentioned this pull request Jun 23, 2021

PyRDP's dependency problem : sniff() function of Scapy module. #318

Open

Copyright bump

450506b

obilodeau mentioned this pull request Jul 28, 2021

Pyrdp-converter #324

Closed

obilodeau added 4 commits July 29, 2021 23:36

Fix for bad output files in the pcap-fix refactoring

8f43702

However there's a regression in the pcap -> mp4 conversion right now...

Announced breaking change in the CHANGELOG

b91f9f9

Converting to a replay? Create an outDir because many files!

a030ae5

cleanup: not accessed anywhere

61d484e

obilodeau mentioned this pull request Jul 30, 2021

MP4 conversion speed improvement opportunity #329

Open

obilodeau added 3 commits July 30, 2021 17:01

Fixed mp4 conversion

8592236

pyrdp-convert: aligned CLI help with behavior

b5fa6ed

refactoring var name

b3ac604

Copyright date bumps for changed files in this PR

fd8105a

obilodeau approved these changes Aug 3, 2021

View reviewed changes

obilodeau merged commit 43b26a6 into master Aug 3, 2021

obilodeau deleted the pcap-fix branch August 6, 2021 17:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

xshill commented Apr 22, 2021

obilodeau left a comment

obilodeau commented Apr 28, 2021 •

edited

Loading

obilodeau commented Apr 28, 2021 •

edited

Loading

obilodeau commented Apr 28, 2021

obilodeau commented Apr 28, 2021

obilodeau commented Apr 28, 2021

xshill commented Apr 29, 2021

obilodeau commented Apr 29, 2021

obilodeau commented Apr 29, 2021

obilodeau commented Jun 28, 2021

obilodeau commented Jul 29, 2021

obilodeau commented Jul 30, 2021

obilodeau commented Jul 30, 2021 •

edited

Loading

obilodeau commented Jul 30, 2021

obilodeau commented Jul 30, 2021

obilodeau commented Jul 30, 2021

obilodeau commented Jul 30, 2021 •

edited

Loading

obilodeau commented Jul 31, 2021

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

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

Conversation

xshill commented Apr 22, 2021

obilodeau left a comment

Choose a reason for hiding this comment

obilodeau commented Apr 28, 2021 • edited Loading

obilodeau commented Apr 28, 2021 • edited Loading

obilodeau commented Apr 28, 2021

obilodeau commented Apr 28, 2021

obilodeau commented Apr 28, 2021

xshill commented Apr 29, 2021

obilodeau commented Apr 29, 2021

obilodeau commented Apr 29, 2021

obilodeau commented Jun 28, 2021

obilodeau commented Jul 29, 2021

obilodeau commented Jul 30, 2021

obilodeau commented Jul 30, 2021 • edited Loading

obilodeau commented Jul 30, 2021

obilodeau commented Jul 30, 2021

obilodeau commented Jul 30, 2021

obilodeau commented Jul 30, 2021 • edited Loading

obilodeau commented Jul 31, 2021

obilodeau commented Apr 28, 2021 •

edited

Loading

obilodeau commented Apr 28, 2021 •

edited

Loading

obilodeau commented Jul 30, 2021 •

edited

Loading

obilodeau commented Jul 30, 2021 •

edited

Loading