-
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
Add a warning log message to keep track of bluekeep scanning/exploits attempts #114
Conversation
I look for clients that open the "MS_T120" virtual channel, which is the first step to trigger bluekeep. This is a pretty good heuristic, and works with this PoC: https://github.com/Ekultek/BlueKeep using the PyRDP still crashes, but it's after the log statement, so the statistic still registers. Note: it does not work with the |
…checking for the MS_T120 channel
Added a commit to fix a bug I introduced. |
pyrdp/mitm/MCSMITM.py
Outdated
@@ -102,6 +106,9 @@ def onConnectInitial(self, pdu: MCSConnectInitialPDU): | |||
|
|||
if rdpClientDataPDU.networkData: | |||
self.state.channelDefinitions = rdpClientDataPDU.networkData.channelDefinitions | |||
if "MS_T120" in map(lambda channelDef: channelDef.name, rdpClientDataPDU.networkData.channelDefinitions): | |||
self.log.warning("Client tries to open virtual channel 'MS_T120', which most likely means it either" |
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.
This log statement is quite a mouthful. The point is to be able to interpret this information (someone's trying to use BlueKeep) very quickly. Wouldn't something like "BlueKeep exploit attempt" be more simple, easier to read and say essentially the same thing?
Also, this seems like INFO level to me. Warning should be for when something went wrong with the actual program or the connection, but it's not something that's very serious. Someone trying to use BlueKeep is information that is actually desirable in a honeypot.
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 I wasnt sure for the warning vs info. I thought warning was good because what follows will probably make pyrdp crash. What do you think?
As for the message, it being long doesnt seem like a problem to me, in fact I think it helps people not knowing details about bluekeep. I dont have strong feelings aboat this however
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's more about the form than the content. The message is a full english sentence with 22 words. The most important keyword is at the end (BlueKeep), so I have to read a whole 20 words to get the actual meaning. What's actually useful is "BlueKeep attempt" and optionally "channel MS_T120".
I just think the faster you can recognize individual log message types, the better, so they should be concise while still providing the requisite information. This is what I think someone scrolling through a huge text file full of logs would rather have.
Fair enough, I'll change it. What about warning vs info?
…On Wed., Jun. 12, 2019, 23:40 Francis Labelle, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pyrdp/mitm/MCSMITM.py
<#114 (comment)>:
> @@ -102,6 +106,9 @@ def onConnectInitial(self, pdu: MCSConnectInitialPDU):
if rdpClientDataPDU.networkData:
self.state.channelDefinitions = rdpClientDataPDU.networkData.channelDefinitions
+ if "MS_T120" in map(lambda channelDef: channelDef.name, rdpClientDataPDU.networkData.channelDefinitions):
+ self.log.warning("Client tries to open virtual channel 'MS_T120', which most likely means it either"
It's more about the form than the content. The message is a full english
sentence with 22 words. The most important keyword is at the end
(BlueKeep), so I have to read a whole 20 words to get the actual meaning.
What's actually useful is "BlueKeep attempt" and optionally "channel
MS_T120".
I just think the faster you can recognize individual log message types,
the better, so they should be concise while still providing the requisite
information. This is what I think someone scrolling through a huge text
file full of logs would rather have.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#114?email_source=notifications&email_token=ADPMNL24MDO3LAABJXFCEQDP2G6RNA5CNFSM4HWLUQR2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB3MSY7I#discussion_r293194865>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADPMNLY4DF7MVLI466ZSISDP2G6RNANCNFSM4HWLUQRQ>
.
|
I would say this is INFO level, since even though it makes pyrdp crash, its not a protocol error / unexpected packet. |
Isnt it exactly what it is, an invalid packet?
(What follows)
…On Thu., Jun. 13, 2019, 09:41 Maxime Carbonneau, ***@***.***> wrote:
I would say this is INFO level, since even though it makes pyrdp crash,
its not a protocol error / unexpected packet.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#114?email_source=notifications&email_token=ADPMNLZECJX7IGI4QITYJ63P2JFB7A5CNFSM4HWLUQR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXTXIXY#issuecomment-501707871>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADPMNL4YYE3IQJ4X4OVHEJTP2JFB7ANCNFSM4HWLUQRQ>
.
|
@xshill Is it the right place to put this log statement?
Implements #113