-
Notifications
You must be signed in to change notification settings - Fork 246
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
Fixed licensing issue and added licensing boilerplate #118
Conversation
From what you saw of the licensing packets, are there nice information that could be used to fingerprint clients or might be relevant to log in general? |
@@ -135,7 +135,9 @@ def __init__(self, parser = BasicSecurityParser()): | |||
self.securityHeaderExpected = False | |||
|
|||
def recv(self, data: bytes): | |||
if not self.securityHeaderExpected: | |||
# Licensing happens in the security layer | |||
licensingBytes = b"\x80\x00" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does that correspond to a flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is, but since this isn't parsed data, it is not in the correct format. Would it be better if I refer to this flag, and parse the 0x80\x00 and compare to the flag?
In pyrdp/enum/rdp.py - line 43
SEC_LICENSE_PKT = 0x0080
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah for that @xshill would have a better idea than me. I have an idea but i’m really not sure if its good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The licensingBytes you're looking for IS part of a security header. You shouldn't need to check for that if securityHeaderExpected is set properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Licensing is an optional part of the protocol, and the boolean "securityHeaderExpected" is false when receiving a licensing PDU. securityHeaderExpected is only used to redirect traffic to the securityLayer when we know we are expecting it in the handshake
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, since this is TLS it starts at false.
The only two interesting things I saw were the "clientUserName" and "clientMachineName", so current username and hostname. With Remmina, the client sent "username" instead of a username to bind the certificate to Remmina instead of a specific user. I don't think these two have any value since they can be tampered. Reference : https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpele/c57e4890-9049-421e-9fe8-9a6f9519675a |
The value might come for fingerprinting purposes, even if they can be tampered. Ex: debbie didnt tamper anything, so he was pretty easy to identify. Of course its not urgent, simply a nice to have and good to know |
I have fixed the issue discussed in #104. The actual fix in in SecurityMITM.py, but I have left some boilerplate code inside the (unused) licensing.py if we ever feel like parsing the data.
I had to make a pre-parse validation in order to avoir sending licensing data to the slowpath. I am not quite happy with that method, and I'm open to change it if you have any propositions.