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.

Sign up for GitHub

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

Jump to bottom

(#358) Catch NTLM hash if non-SSP authentication #367

Merged
merged 26 commits into from
Nov 26, 2021
Merged

(#358) Catch NTLM hash if non-SSP authentication #367

merged 26 commits into from
Nov 26, 2021

Conversation

lubiedo
Copy link
Contributor

@lubiedo lubiedo commented Oct 26, 2021

This PR will enable PyRDP to catch the NTLM Hash during a client's connection even when it was not executed with CredSSP authentication enabled and the server enforces it. To achieve this it will cut connection with the server and create a challenge PDU for the client to respond with an authentication message.

cc @obilodeau

@lubiedo
Copy link
Contributor Author

lubiedo commented Nov 2, 2021

Getting closer to finishing this PR. @obilodeau: let me know what you think and if you have some changes for me to add to this.

@lubiedo lubiedo changed the title Catch NTLM hash if non-SSP authentication (#358) Catch NTLM hash if non-SSP authentication Nov 11, 2021
@lubiedo lubiedo linked an issue Nov 11, 2021 that may be closed by this pull request
Grab the Net-NTLM hash before closing an NLA connection #358
Closed
obilodeau
obilodeau reviewed Nov 16, 2021
View reviewed changes
Copy link
Member

@obilodeau obilodeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things are taking shape. Here's my review. A lot of little things, nothing major. Great work!

pyrdp/core/ber.py Outdated Show resolved Hide resolved
pyrdp/mitm/RDPMITM.py Outdated Show resolved Hide resolved
pyrdp/mitm/RDPMITM.py Outdated Show resolved Hide resolved
pyrdp/mitm/X224MITM.py Outdated Show resolved Hide resolved
pyrdp/mitm/state.py Outdated Show resolved Hide resolved
pyrdp/pdu/rdp/ntlmssp.py Show resolved Hide resolved
pyrdp/pdu/rdp/ntlmssp.py Show resolved Hide resolved
pyrdp/pdu/rdp/ntlmssp.py Show resolved Hide resolved
pyrdp/security/nla.py Outdated Show resolved Hide resolved
pyrdp/security/nla.py Show resolved Hide resolved
obilodeau
obilodeau reviewed Nov 17, 2021
View reviewed changes
pyrdp/mitm/RDPMITM.py Outdated Show resolved Hide resolved
obilodeau
obilodeau reviewed Nov 23, 2021
View reviewed changes
Copy link
Member

@obilodeau obilodeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another round of review/comments.

pyrdp/security/nla.py Outdated Show resolved Hide resolved
pyrdp/security/nla.py Outdated Show resolved Hide resolved
pyrdp/security/nla.py Outdated Show resolved Hide resolved
pyrdp/parser/rdp/ntlmssp.py Show resolved Hide resolved
pyrdp/parser/rdp/ntlmssp.py Outdated Show resolved Hide resolved
@obilodeau
Copy link
Member

Another important problem showed up. There seems to be a problem if we don't already have a copy of the server's certificate:

[2021-11-23 16:50:03,586] - INFO - Ellyn175837 - pyrdp.mitm.connections.x224 - Server requires CredSSP. Closing connection with server and attempting to capture NTLM hashes.
Unhandled Error
Traceback (most recent call last):
  File "/usr/lib/python3.9/asyncio/base_events.py", line 596, in run_forever
    self._run_once()
  File "/usr/lib/python3.9/asyncio/base_events.py", line 1890, in _run_once
    handle._run()
  File "/usr/lib/python3.9/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/twisted/internet/asyncioreactor.py", line 273, in _onTimer
    self.runUntilCurrent()
--- <exception caught here> ---
  File "/home/olivier/Documents/gosecure/src/pyrdp/venv/lib/python3.9/site-packages/twisted/internet/base.py", line 994, in runUntilCurrent
    call.func(*call.args, **call.kw)
  File "/home/olivier/Documents/gosecure/src/pyrdp/pyrdp/mitm/RDPMITM.py", line 229, in doClientTls
    privKey, certFile = self.certs.lookup(cert)
builtins.UnboundLocalError: local variable 'cert' referenced before assignment

obilodeau
obilodeau reviewed Nov 23, 2021
View reviewed changes
pyrdp/mitm/RDPMITM.py Outdated Show resolved Hide resolved
@obilodeau
Copy link
Member

Another important problem showed up. There seems to be a problem if we don't already have a copy of the server's certificate:

I might have a fix for this in the works.

@lubiedo
Copy link
Contributor Author

lubiedo commented Nov 24, 2021

Thanks for the reviews @obilodeau! Will work on them ASAP.

@lubiedo
Copy link
Contributor Author

lubiedo commented Nov 24, 2021

Fix for major issue #367 (comment) fixed with commit 527a03b

obilodeau
obilodeau reviewed Nov 25, 2021
View reviewed changes
pyrdp/mitm/X224MITM.py Outdated Show resolved Hide resolved
obilodeau
obilodeau reviewed Nov 25, 2021
View reviewed changes
pyrdp/mitm/X224MITM.py Outdated Show resolved Hide resolved
obilodeau
obilodeau reviewed Nov 25, 2021
View reviewed changes
Copy link
Member

@obilodeau obilodeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor suggestions/comments

pyrdp/mitm/X224MITM.py Show resolved Hide resolved
pyrdp/mitm/X224MITM.py Outdated Show resolved Hide resolved
pyrdp/parser/rdp/ntlmssp.py Outdated Show resolved Hide resolved
pyrdp/parser/rdp/ntlmssp.py Show resolved Hide resolved
pyrdp/parser/rdp/ntlmssp.py Show resolved Hide resolved
@lubiedo lubiedo added the enhancement New feature or request label Nov 26, 2021
obilodeau
obilodeau approved these changes Nov 26, 2021
View reviewed changes
Copy link
Member

@obilodeau obilodeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR sparks joy! 🌟

@obilodeau obilodeau merged commit 25b3ee0 into GoSecure:master Nov 26, 2021
This was referenced Nov 26, 2021
Open
Merged
@obilodeau obilodeau mentioned this pull request Jan 5, 2022
Closed
obilodeau added a commit to alxbl/pyrdp that referenced this pull request Jan 6, 2022
@lubiedo @obilodeau
Capture the NetNTLM hash if server enforces NLA (GoSecure#367)
24f3477 
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>
obilodeau added a commit that referenced this pull request Jan 17, 2022
@lubiedo @obilodeau
Capture the NetNTLM hash if server enforces NLA (#367)
d8408a8 
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 #358

Co-authored-by: Olivier Bilodeau <obilodeau@gosecure.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@obilodeau obilodeau obilodeau approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Grab the Net-NTLM hash before closing an NLA connection
2 participants
@lubiedo @obilodeau