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: Better success messages for pyrdp-convert #369

Merged
merged 23 commits into from
Jan 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
761d611
fix: make conversion success message uniform
alxbl Oct 27, 2021
76b15df
fix: typo in conversion layer sink
alxbl Oct 27, 2021
d484d91
chore(363): replace notify2 by py-notifier.
alxbl Oct 25, 2021
e5588ce
chore: remove dbus dependencies
alxbl Oct 27, 2021
0efbdd1
cleanup: remove notify-osd
alxbl Oct 29, 2021
8c6c6f9
Removed leftover comment in Dockerfile
obilodeau Nov 26, 2021
5785bd8
Updated CHANGELOG
obilodeau Nov 26, 2021
24f3477
Capture the NetNTLM hash if server enforces NLA (#367)
lubiedo Nov 26, 2021
477c572
Updated CHANGELOG
obilodeau Nov 26, 2021
6112252
Some type hint improvements
obilodeau Nov 26, 2021
719064a
Not longer assuming every connection will have VirtualChannels
obilodeau Nov 26, 2021
0e98272
Updated CHANGELOG
obilodeau Nov 26, 2021
45ffe9b
Added CI/CD tests for pyrdp-convert JSON output
obilodeau Jan 6, 2022
4061666
Merge branch 'master' into convert-msg
obilodeau Jan 6, 2022
494316a
fixup: adjusted filepath
obilodeau Jan 6, 2022
a64e2b3
Added pcap to json tests, removed worthless Windows test
obilodeau Jan 6, 2022
2d9e000
fixup: another file name mistake
obilodeau Jan 6, 2022
c91a9f5
pyrdp-convert: Added some exit code propagation on exceptions
obilodeau Jan 6, 2022
837f046
a bit of a dirty way to get rid of the handler.cleanup crash
obilodeau Jan 6, 2022
b9ed8d3
Added a test on the existance of the replay file
obilodeau Jan 6, 2022
db604a0
fixup: test filename
obilodeau Jan 6, 2022
da223b8
fixup: PCAP to JSON test was broken, validator needed tweaking too
obilodeau Jan 6, 2022
9bacc20
Updated CHANGELOG
obilodeau Jan 6, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion bin/pyrdp-convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,5 @@
sys.stderr.write("Unknown file extension. (Supported: .pcap, .pyrdp)")
sys.exit(1)

converter.process()
exitCode = converter.process()
sys.exit(exitCode)
8 changes: 6 additions & 2 deletions pyrdp/convert/PCAPConverter.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def process(self):
if self.listOnly:
return

exitCode = 0
for startTimeStamp, stream in streams:
try:
self.processStream(startTimeStamp, stream)
Expand All @@ -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}' ...")
Expand Down Expand Up @@ -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}'")
14 changes: 12 additions & 2 deletions pyrdp/convert/RDPReplayer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)

Expand Down Expand Up @@ -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
11 changes: 11 additions & 0 deletions test/validate_json.sh
Original file line number Diff line number Diff line change
@@ -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