fix: Better success and error messages for pyrdp-convert (#369)

* fix: make conversion success message uniform
* fix: typo in conversion layer sink
* Added pcap to json tests, removed worthless Windows test
* Added CI/CD tests for pyrdp-convert JSON and replay outputs
* pyrdp-convert: Added some exit code propagation on exceptions

Co-authored-by: Olivier Bilodeau <obilodeau@gosecure.net>
Co-authored-by: Alexandre Beaulieu <alex@segfault.me>

.github/workflows/ci.yml

@@ -88,6 +88,30 @@ jobs:
         working-directory: ./
         run: file test_convert.mp4 | grep "MP4 Base Media"
 
+      - name: pyrdp-convert.py replay to JSON
+        working-directory: ./
+        run: coverage run --append bin/pyrdp-convert.py test/files/test_convert.pyrdp -f json
+
+      - name: Verify the replay to JSON file
+        working-directory: ./
+        run: ./test/validate_json.sh test_convert.json
+
+      - name: pyrdp-convert.py PCAP to JSON
+        working-directory: ./
+        run: coverage run --append bin/pyrdp-convert.py test/files/test_session.pcap -f json
+
+      - name: Verify the PCAP to JSON file
+        working-directory: ./
+        run: ./test/validate_json.sh "20200319000716_192.168.38.1:20989-192.168.38.1:3389.json"
+
+      - name: pyrdp-convert.py PCAP to replay
+        working-directory: ./
+        run: coverage run --append bin/pyrdp-convert.py test/files/test_session.pcap -f replay
+
+      - name: Verify that the replay file exists
+        working-directory: ./
+        run: file -E "20200319000716_192.168.38.1:20989-192.168.38.1:3389.pyrdp"
+
       - name: Run unit tests
         working-directory: ./
         run: coverage run --append -m unittest discover -v

CHANGELOG.adoc

@@ -22,7 +22,7 @@ For a detailed view of what has changed, refer to the {uri-repo}/commits/master[
 * `pyrdp-convert` video conversion is now 6x faster! (See {uri-issue}349[#349])
 * `pyrdp-convert` video format can be viewed during encoding and will play even if the conversion process crashes or is halted ({uri-issue}352[#352], {uri-issue}353[#353])
 * `pyrdp-convert` can now extract session information including keyboard and mouse movement information in JSON from pcap and PDUs ({uri-issue}331[#331], {uri-issues}366[#366])
-* Better error reporting for `pyrdp-convert` ({uri-issue}361[#361])
+* `pyrdp-convert` has better success messages, error reporting and exit status ({uri-issue}361[#361], {uri-issue}369[#369])
 * Minor CLI improvements
 * Improved type hints
 * Updated instructions to extract the RDP certificate and private key ({uri-issue}345[#345])
@@ -44,6 +44,8 @@ For a detailed view of what has changed, refer to the {uri-repo}/commits/master[
 * The slim flavor of our Docker image is now provided for the ARM and ARM64 platforms ({uri-issue}346[#346])
 * Docker images are now built and pushed via GitHub Actions ({uri-issue}334[#334], {uri-issue}341[#341])
 * Added an automated video conversion test to CI configuration ({uri-issue}349[#349])
+* Added an automated JSON conversion test to CI configuration with some validation ({uri-issue}369[#369])
+* Added an automated replay conversion test to CI configuration ({uri-issue}369[#369])
 
 
 == v1.1.0 - 2021-08-05

bin/pyrdp-convert.py

@@ -92,4 +92,5 @@
         sys.stderr.write("Unknown file extension. (Supported: .pcap, .pyrdp)")
         sys.exit(1)
 
-    converter.process()
+    exitCode = converter.process()
+    sys.exit(exitCode)

pyrdp/convert/PCAPConverter.py

@@ -43,6 +43,7 @@ def process(self):
         if self.listOnly:
             return
 
+        exitCode = 0
         for startTimeStamp, stream in streams:
             try:
                 self.processStream(startTimeStamp, stream)
@@ -51,6 +52,8 @@ def process(self):
                 print() # newline
                 print(trace)
                 print(f"[-] Failed: {e}")
+                exitCode = 1
+        return exitCode
 
     def listSessions(self) -> List[Tuple[int, PCAPStream]]:
         print(f"[*] Analyzing PCAP '{self.inputFile}' ...")
@@ -117,8 +120,9 @@ def processStream(self, startTimeStamp: int, stream: PCAPStream):
 
         try:
             replayer.tcp.recordConnectionClose()
-            handler.cleanup()
+            if handler:
+                handler.cleanup()
         except struct.error:
             sys.stderr.write("[!] Couldn't close the session cleanly. Make sure that --src and --dst are correct.")
 
-        print(f"\n[+] Successfully wrote all files to '{self.outputPrefix}'")
+        print(f"\n[+] Successfully wrote '{replayer.filename}'")

pyrdp/convert/RDPReplayer.py

@@ -44,6 +44,10 @@ def __init__(self, handler: BaseEventHandler):
         self.player = PlayerLayer()
         self.player.addObserver(handler)
 
+    @property
+    def filename(self):
+        return self.sink.filename
+
     def sendBytes(self, data: bytes):
         self.player.recv(data)
 
@@ -66,8 +70,9 @@ def sendBytesStub(_: bytes):
 
         state = RDPMITMState(config, sessionID)
 
-        transport = ConversionLayer(handler) if handler else FileLayer(output_directory / (sessionID + '.' + HANDLERS['replay'][1]))
-        rec = OfflineRecorder([transport], state)
+        self.transport = ConversionLayer(handler) if handler else FileLayer(output_directory / (sessionID + '.' + HANDLERS['replay'][1]))
+
+        rec = OfflineRecorder([self.transport], state)
 
         super().__init__(log, log, config, state, rec)
 
@@ -98,3 +103,8 @@ def startTLS(self, onTlsReady):
 
     def sendPayload(self):
         pass
+
+    @property
+    def filename(self):
+        """The destination filename for this replay"""
+        return self.transport.filename

test/validate_json.sh

@@ -0,0 +1,11 @@
+#!/bin/bash -x
+
+if [[ `cat $@ | jq -e ".info | length"` -lt 4 ]]; then
+  echo "JSON conversion failed: info structure too small"
+  exit 1
+fi
+
+if [[ `cat $@ | jq -e ".events | length"` -lt 25 ]]; then
+  echo "JSON conversion failed: not enough events"
+  exit 2
+fi