-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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
Enum Ad Users -> Database #4392
Conversation
|
||
begin | ||
q = query(search_filter, max_search, fields) | ||
if q.nil? || q[:results].empty? |
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 exceptions should only occur in the query
method call, so only it should be protected. This if
should moveout of the begin rescue
:
begin
q = query(search_filter, max_search, fields)
rescue ::RuntimeError, ::Rex::Post::Meterpreter::RequestError => e
# Can't bind or in a network w/ limited accounts
print_errror(e.message)
return
end
if q.nil? || q[:results].empty?
return
end
I got a bit carried away and added lots of functionality based on @darkoperator's similar module :o |
Happy you liked my work ;) |
Thanks for the PR! Works when I use it from an Administrator account:
But fails on a regular Domain User session:
Does the module requires a session with administration privileges? If it's the case worths to point the module description and/or make some check in the code, since the error isn't very explanatory. |
HRESULT 0x8007052E is something to do with a login failure? It shouldn't require administrative privileges at all, a normal user can enumerate domain users via LDAP. Are you getting in via cached local credentials? Perhaps the user's password has expired etc? It could be that your Domain DNS is a bit funky? Try |
mmm trying to figure out what is going on with this duplicate key error |
Asked ppl about reporting just username as credentials. Since @limhoff-r7 didn't any comment about that, I'm assuming it's right. Anyway, I'm going to wait to get some feedback about the raised exception when running the module a second time. |
Hmm, I'm not hitting that error when running it twice against the same session, or even against a second session here. I don't really understand the internals of CREDS enough apart from its obviously creating a duplicate composite key of workspace_id and public_id. The workspace presumably being the current workspace, and public_id being generated somewhere in the gem? Storing just the username makes sense, in smb_login you can use |
@Meatballs1 yup, I asked for feedback to ppl, hopefully they |
@Meatballs1 lol, beating me to the punch. this has been on my lsit to do for a long time. It was marked down for after I added LDAP support to Meterpreter...which O did. Not sure If I should be happy or sad that the things I was going to do keep getting done by other people first =P |
i'll take a peek at this today if I can |
@jvazquez-r7 based on line numbers, i suspect your version of metasploit-credential may be out of date, try |
end | ||
|
||
def run | ||
fields = ['sAMAccountName', 'userAccountControl', 'lockoutTime', 'mail', 'primarygroupid', 'description'] |
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.
if this array of fields is never getting modified it should probably be a constant
@jvazquez-r7 @Meatballs1 issued a PR against your PR branch with some code style cleanups per my comments above. Also pulls in latest master, including gem updates for metasploit-credential. With these changes I don't see any errors occuring. |
ooookay, after arguing with the github ui, i think i got the pr setup right this time...sorry about that |
Feature/metaballs1/enum ad users
All changes working here, thanks @dmaloney-r7 |
And the duplicate error was due to my database being in kinda weird state, so I guess it is ready to go, fast check and landing, thanks @dmaloney-r7 and @Meatballs1 ! |
well, holding, @dmaloney-r7 already assigned himself, maybe wants to double check, giving him chance to review, otherwise will land along the next days :) |
ack'ed with @dmaloney-r7, I'm going to do last check and landing ! Thanks @Meatballs1 and @dmaloney-r7 for the hard work here! |
Last test;
|
Cool ta :) |
Dumps AD Users to the database. Tries to detect Disabled and Locked out accounts from the userAccountControl. Disabled appears to be set here, but normal account lockouts don't seem to set in the flags.
Not sure where Metasploit::Credentials stores that info, or how it uses it, doesn't bother showing it with creds... :)
n.b. I tried using smb_login with DB_ALL_USERS true, but that didn't seem to work...
Verification