fix: Better success messages for pyrdp-convert #369
Conversation
If we don't use the NLA redirection feature and the server doesn't support downgrade attacks then the best we can do is steal the hash. Some ASN.1 BER improvements were required as well. Fixes GoSecure#358 Co-authored-by: Olivier Bilodeau <obilodeau@gosecure.net>
Improved the situation with GoSecure#294
|
Tested this but I stumble upon the same problems as #366. Moving to the next one. |
|
Now that #366 is merged. I started to look into this one tonight. Unfortunately, this PR doesn't work with the replay handler because, in it, it says: Line 84 in 44e94a9 Since handler is None in that case, then calling This should have been caught by tests. I'll rework them to make sure it is. |
|
The error should trigger with tests now. I added nice little JSON tests while we are at it. The history is a bit of a mess because I pulled this PR in my master back in November so when I pushed back a lot of things went up with it. That said, the GitHub diff is clean so I'm leaving this that way. In any case, I'll probably squash-merge. |
This will make tests results nasty
|
Ok, so all errors were swallowed so adding tests didn't help. Now I'm adjusting exit codes but this will make existing tests fail I think. I'll see what I do about that tomorrow. Zzz |
|
Ohh, actually this seems to be exactly what I wanted!
Now time to fix that missing |
The required refactoring would be a too large undertaking for the current release window
* 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>
While making my other PRs, I noticed that the pyrdp-convert success message was not very consistent, so I wrote a bit of code to make it consistent.
It will print the filename when outputting to the same folder, or the absolute file path if a prefix is specified. For multiple streams, each file will be printed after its conversion.
The way this was done was by exposing the output filename to the
RDPReplayervia a property that gets it from the transport layer (either aConversionLayeror aFileLayer)We could make this a bit sturdier by having a common base class for both
ConversionLayerandFileLayerthat enforces the presence of afilenamemember, but for now both have it, so it works.