-
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
Update gpp.rb to display GPO name #12258
Conversation
GPO files on SYSVOL do only include the GPO GUID, not the GPO name defined by the administrator. This modification makes this gpp module make an ADSI query to retrieve all of the domain's GPOs, and compare their GUID. If one GUID matches, then we know the GPO name and we can display it. On a pentest, a client is much more interested by knowing the GPO name rather than the obscure GUID. The ADSI query relies on meterpreter "extapi" extension.
@@ -241,7 +266,19 @@ def parse_xml(xmlfile) | |||
tables = Rex::Parser::GPP.create_tables(results, filetype, xmlfile[:domain], xmlfile[:dc]) | |||
|
|||
tables.each do |table| | |||
print_good table.to_s | |||
# We have to manually format the results as we want to insert a new line |
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.
Is there a reason that using table << ['NAME', xmlfile[:name]] if xmlfile.member?(:name)
to add a row to the table doesn't work?
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.
I simply did not think about it ^^ I wanted to insert this new field on top of other values. However the method you suggested is much simpler and cleaner, and I commited your idea to the code. The "NAME" field is added so it goes to the end instead, but the code simplification is worth it. Thanks :-)
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.
Another (perhaps more correct option) would be to have Rex::Parser::GPP.create_tables
take the name as an option parameter and add it to the table around https://github.com/rapid7/metasploit-framework/blob/master/lib/rex/parser/group_policy_preferences.rb#L102
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.
Also, thanks for making the change! The code is definitely much cleaner for it.
Directly insert the new value in the "table", instead of modifying the screen output manually. Simpler and cleaner, thanks @acammack-r7 !
Aesthetics modification
Release NotesThis improves the windows/gather/credentials/gpp module by displaying the name as well as the GUID for Group Policy Objects (GPO). It relies on the 'extapi' extension for Meterpreter in order to perform ADSI queries for obtaining the name to GUID mapping. |
What the changes do:
GPO files on SYSVOL do only include the GPO GUID, not the GPO name defined by the administrator. This modification makes this gpp module make an ADSI query to retrieve all of the domain's GPOs, and compare their GUID. If one GUID matches, then we know the GPO name and we can display it. On a pentest, a client is much more interested by knowing the GPO name rather than the obscure GUID. The ADSI query relies on meterpreter "extapi" extension.
Verification
run post/windows/gather/credentials/gpp
Example
The field NAME Local admin is added at the begining, where "Local admin" is the GPO name in this example.