-
Notifications
You must be signed in to change notification settings - Fork 14k
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 generic LDAP hashdump module #13906
Conversation
Do you have any steps that you could provide us on setting up an environment that would be suitable for testing and demonstrating this module? Is this something we can spin up with like OpenLDAP and are there any configuration settings that need to be made in order for it to be vulnerable? Any assistance you can provide us here will speed up the processing of your contribution. |
@smcintyre-r7: That was copied from my module doc. We had our own target. That said, I've more recently been able to set up vCenter Server or just plain OpenLDAP in a pinch. |
Thanks for your pull request! Before this pull request can be merged, it must pass the checks of our automated linting tools. We use Rubocop and msftidy to ensure the quality of our code. This can be ran from the root directory of Metasploit:
You can automate most of these changes with the
Please update your branch after these have been made, and reach out if you have any problems. |
Hi @smcintyre-r7, I have added a test scenario using a dockerized OpenLDAP server, that is made intentionally insecure. HTH Otherwise I've tested the module on numerous LDAP servers in wild, various brands. |
I intend to add support for multiple naming contexts within one target |
Currently performing a mass test, identifying and fixing edge cases. |
Okay, that sounds great thank you! It definitely looks like you're actively working on this, so I'm going to go ahead and convert it to a draft PR and when you're all done and ready for it to be reviewed just press the "Ready for review" button on GitHub towards the bottom of the PR. |
Hi @smcintyre-r7 , @wvu-r7 I believe the code is ready for a review. I've been doing very large test on various LDAP servers and possibly caught or worked around majority of the issues. Due to performance I made the module as Scanner to leverage multi threading. |
Is there a chance to make it draft PR again? I found one more thing to improve - pwdhistory and passwordhistory handling. |
Done! Convert it back when you're ready. |
@smcintyre-r7 no more changes planned |
Fantastic effort! Thank you for doing this. |
Please for a patience, since I started with ruby 2-3 weeks ago. Can’t write idiomatic ruby yet. |
Apparently I'm not catching all the cases with the |
You'd need to use a debugger, I generally like the one built into RubyMine. |
Ok, I found the issue. The Net::LDAP#open does 3 things:
Apparently the bind operation is not controlled by the connect_timeout and the code may hang on certain servers. |
@smcintyre-r7, @wvu-r7 ready for the review. |
For the monkey patch in the last commit I have submitted an issue to the net-ldap library: |
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.
Left a few comments through out. A few things I noticed:
- All of the
CheckCode
uses should be changed tofail_with
, you don't define acheck
method so there's no place those values are going. Usingfail_with
instead will report why the module can't continue. - The multiple uses of
ldap.search
should be moved into a module functions. They're all pretty similiar and it would reduce the repeat code around the temporary file and timeout handling.
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.
moved the ldap.search to its function and done the other changes. see the conversations.
case @pass_attr.downcase | ||
when 'sambalmpassword' | ||
hash_format = 'lm' | ||
when 'sambalmpassword' | ||
hash_format = 'nt' | ||
else | ||
hash_format = identify_hash(hash) | ||
end | ||
|
||
create_credential(service_details.merge( | ||
username: dn, | ||
private_data: hash, | ||
private_type: :nonreplayable_hash, | ||
jtr_format: identify_hash(hash) | ||
jtr_format: hash_format |
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.
What's the right type for NT or LM hashes? Is it also nonreplayable_hash?
@smcintyre-r7 I cannot use the |
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.
@smcintyre-r7 I cannot use the fail_with. Once called for one host it aborts the whole module run, i.e for the other threads too. This is not desired.
That's right, I've been meaning to see if we can fix that. In the meantime returning like you are is fine, 👍
The questionable Timeout.timeout
has been consolidated down into 2 instances within the module so given the constraints we're operating in with regards to Net::LDAP
that seems reasonable.
I'll start testing this out now, I left a few minor comments but nothing that I can't address myself during testing.
While testing this with the docker container you provided I noticed that the module prints an error on a connection error but it continues to run showing that it's not properly identifying an error that should cause it to just exit. Note the SSL connection error in the following output (because I set
|
@HynekPetrak, @smcintyre-r7: Please ensure that the existing LDAP modules continue to function with the mixin changes. Thanks! ETA: Documentation may or may not need to be updated, too. |
|
author Hynek Petrak <[email protected]> 1595628792 +0200 committer Spencer McIntyre <[email protected]> 1598532753 -0400 Added module to dump hashes from LDAP added hash formatters, documentation, ldap authentication typo sanitizing added scenario for NASDeluxe added few hash attribute examples typo correction Co-authored-by: bcoles <[email protected]> typo correction Co-authored-by: bcoles <[email protected]> typo correction Co-authored-by: bcoles <[email protected]> avoid option name conflicts added test scenario linted linted Dump all nameContexts, not just the first one. Search creds in multiple attributes. attemt to dump special and operational attributes check if ldap bind succeeded sanitize the ldap hashes, skip invalid, remove {crypt} prefix memory optimization for large LDAP servers spaces at eols put header to the ldif loot added other LDAP hash formats, don't save empty ldif, dump root DSE now we handle vmdir case too explictly set md5crypt for $ Converted to scanner to improve performance on large networks krbprincipalkey, memory optimization for ldap.search handle additional hash types be verbose about search errors added per host timeout catch exception from Net::Ldap shorten the param value handle pwdhistory entries added comment about sambapwdhistory value reject shorter empty sambapassordhistory entries reject null nt and lm hashes report assumed clear text passwords refactored timeout for the sake of the loot ignore {SASL} pass-trough auth entries distinguish unresolved hashes from clear passwords print ldap server error message, meaningful loot name correct exception handling handle hashes with eol remove debug line handle pkcs12 in binary form attemt to control timeout on bind operation leave LDAP#bind to be called implicitly in #search remove debug line fixed bug, when pillage broke the outer LDAP#search learning ruby monkey patched ldap connection handling, ignoring bind errors commenting the net:LDAP misbehaviour review fixes review fixes moving ldap.search into a function remove fail_with, store loot from one place, print statistics linting consolidated ldap_new and connect, don't catch exceptions in the mixin Complete the credential creation Co-authored-by: Spencer McIntyre <[email protected]>
573add7
to
aa60b4e
Compare
I just did some git surgery on this PR. I squashed the commit history down from the 59 commits to one, rebased onto master and then updated the module to again use Thanks alot for all of your work on this @HynekPetrak! |
Tested successfully, added those tweaks I mentioned in commit aa60b4e. Test run using the provided docker image:
|
Release NotesNew module |
This PR adds a new module that dumps LDAP data via anonymous or authenticated (added to LDAP mixin) bind.
Looting the dumped LDIF and searching for common attributes that usually hold user credentials, e.g. userPassword.
TODO: test the LDAP authentication
Here is an example running the module against LDAP server of Avaya Communication manager, those server ususally do not require an authentication to dump credentials (not sure if there is any CVE on that, maybe I should create one):