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

Make enum options case normalizing #19660

Merged
merged 1 commit into from
Dec 13, 2024
Merged

Make enum options case normalizing #19660

merged 1 commit into from
Dec 13, 2024
+31 −2

Conversation

zeroSteiner
Copy link
Contributor

@zeroSteiner zeroSteiner commented Nov 18, 2024 •
edited by cgranleese-r7
Loading

This closes #19450 by introducing changes to the #valid? and #normalize methods of the OptEnum class to make it case-insensitive. This should make it slightly easier for users to set opinions by not having to remember the exact casing of opinion values. As a good example, the ldap_query module has an OUTPUT_FORMAT with options of csv and json. Since those words are acronyms and common file extensions, either csv or CSV would make sense in the context. Now users don't need to remember that exact detail. The casing of the option is normalized to what the module author specified though, so the value can still be matched exactly in the module code.

Testing

  • Use the ldap_query module
  • Set the OUTPUT_FORMAT option to CSV and see it get normalized to csv
  • See the module still works, showing the normalized value was used

Demo

Old:

metasploit-framework (S:0 J:0) auxiliary(gather/ldap_query) > set OUTPUT_FORMAT CSV
[-] The following options failed to validate: Value 'CSV' is not valid for option 'OUTPUT_FORMAT'.
OUTPUT_FORMAT => table
metasploit-framework (S:0 J:0) auxiliary(gather/ldap_query) >

New:

metasploit-framework (S:0 J:0) auxiliary(gather/ldap_query) > set OUTPUT_FORMAT CSV
OUTPUT_FORMAT => csv
metasploit-framework (S:0 J:0) auxiliary(gather/ldap_query) >

adfoster-r7
adfoster-r7 reviewed Nov 21, 2024
View reviewed changes
lib/msf/core/opt_enum.rb
@@ -44,6 +53,10 @@ def desc

protected

def case_sensitive?
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the last time I looked into this, there weren't any options that had case sensitive enums 🤔

Should this just be an invariant to the OptEnum initialize method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you asking if we should always enforce that the values are unique when ignoring case or that it's a kwarg? I know we looked into it and saw nothing would need case sensitivity but I wasn't confident that would always be the case in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

Enforcing that it should be unique when ignoring case, so that this always holds true: "I wasn't confident that would always be the case in the future"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shouldn't need to be enforced because this function will detect if it is necessary. If it is necessary in the future, then it will behave as it does now and the user will have to specify the option exactly as it's defined.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @adfoster-r7 just wanted to check in to see if you still had any concerns here?

Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be nice to cache the result of def case_sensitive? but not a blocker

@cgranleese-r7 cgranleese-r7 self-assigned this Dec 13, 2024
@cgranleese-r7
Copy link
Contributor

cgranleese-r7 commented Dec 13, 2024 •
edited
Loading

Everything seems to be working as expected 👍

Specs

SPEC_OPTS='--tag acceptance' SPEC_HELPER_LOAD_METASPLOIT=false bundle exec rspec ./spec/acceptance/ldap_spec.rb:

Finished in 20.5 seconds (files took 3.31 seconds to load)
12 examples, 0 failures

bundle exec rspec spec/lib/msf/core/opt_enum_spec.rb

Finished in 0.04386 seconds (files took 6.48 seconds to load)
19 examples, 0 failures

MSFConsole testing

OUTPUT_FORMAT set to csv

msf6 auxiliary(gather/ldap_query) > set output_format csv
output_format => csv
msf6 auxiliary(gather/ldap_query) > run

[+] Successfully bound to the LDAP server via existing SESSION!
[*] 127.0.0.1:389 Discovered base DN: DC=ldap,DC=example,DC=com
Name,Attributes
"badpwdcount","0"
"description","Built-in account for administering the computer/domain"
"dn","CN=Administrator,CN=Users,DC=ldap,DC=example,DC=com"
"lastlogoff","1601-01-01 00:00:00 UTC"
"lastlogon","1601-01-01 00:00:00 UTC"
"logoncount","0"
"memberof","CN=Domain Admins,CN=Users,DC=ldap,DC=example,DC=com || CN=Schema Admins,CN=Users,DC=ldap,DC=example,DC=com || CN=Enterprise Admins,CN=Users,DC=ldap,DC=example,DC=com || CN=Group Policy Creator Owners,CN=Users,DC=ldap,DC=example,DC=com || CN=Administrators,CN=Builtin,DC=ldap,DC=example,DC=com"
"name","Administrator"
"objectsid","S-1-5-21-815333893-3774685697-946021116-500"
"pwdlastset",""
"samaccountname","Administrator"
"useraccountcontrol","512"

Name,Attributes
"badpwdcount","0"
"dn","CN=LDAP,OU=Domain Controllers,DC=ldap,DC=example,DC=com"
"lastlogoff","1601-01-01 00:00:00 UTC"
"lastlogon","1601-01-01 00:00:00 UTC"
"logoncount","0"
"name","LDAP"
"objectsid","S-1-5-21-815333893-3774685697-946021116-1000"
"pwdlastset",""
"samaccountname","LDAP$"
"useraccountcontrol","532480"

Name,Attributes
"badpwdcount","0"
"description","Built-in account for guest access to the computer/domain"
"dn","CN=Guest,CN=Users,DC=ldap,DC=example,DC=com"
"lastlogoff","1601-01-01 00:00:00 UTC"
"lastlogon","1601-01-01 00:00:00 UTC"
"logoncount","0"
"memberof","CN=Guests,CN=Builtin,DC=ldap,DC=example,DC=com"
"name","Guest"
"objectsid","S-1-5-21-815333893-3774685697-946021116-501"
"pwdlastset",""
"samaccountname","Guest"
"useraccountcontrol","66082"

Name,Attributes
"badpwdcount","0"
"description","Key Distribution Center Service Account"
"dn","CN=krbtgt,CN=Users,DC=ldap,DC=example,DC=com"
"lastlogoff","1601-01-01 00:00:00 UTC"
"lastlogon","1601-01-01 00:00:00 UTC"
"logoncount","0"
"memberof","CN=Denied RODC Password Replication Group,CN=Users,DC=ldap,DC=example,DC=com"
"name","krbtgt"
"objectsid","S-1-5-21-815333893-3774685697-946021116-502"
"pwdlastset",""
"samaccountname","krbtgt"
"useraccountcontrol","514"

[*] Query returned 4 results.
[*] Auxiliary module execution completed

OUTPUT_FORMAT set to CSV:

msf6 auxiliary(gather/ldap_query) > set output_format CSV
output_format => csv
msf6 auxiliary(gather/ldap_query) > run

[+] Successfully bound to the LDAP server via existing SESSION!
[*] 127.0.0.1:389 Discovered base DN: DC=ldap,DC=example,DC=com
Name,Attributes
"badpwdcount","0"
"description","Built-in account for administering the computer/domain"
"dn","CN=Administrator,CN=Users,DC=ldap,DC=example,DC=com"
"lastlogoff","1601-01-01 00:00:00 UTC"
"lastlogon","1601-01-01 00:00:00 UTC"
"logoncount","0"
"memberof","CN=Domain Admins,CN=Users,DC=ldap,DC=example,DC=com || CN=Schema Admins,CN=Users,DC=ldap,DC=example,DC=com || CN=Enterprise Admins,CN=Users,DC=ldap,DC=example,DC=com || CN=Group Policy Creator Owners,CN=Users,DC=ldap,DC=example,DC=com || CN=Administrators,CN=Builtin,DC=ldap,DC=example,DC=com"
"name","Administrator"
"objectsid","S-1-5-21-815333893-3774685697-946021116-500"
"pwdlastset",""
"samaccountname","Administrator"
"useraccountcontrol","512"

Name,Attributes
"badpwdcount","0"
"dn","CN=LDAP,OU=Domain Controllers,DC=ldap,DC=example,DC=com"
"lastlogoff","1601-01-01 00:00:00 UTC"
"lastlogon","1601-01-01 00:00:00 UTC"
"logoncount","0"
"name","LDAP"
"objectsid","S-1-5-21-815333893-3774685697-946021116-1000"
"pwdlastset",""
"samaccountname","LDAP$"
"useraccountcontrol","532480"

Name,Attributes
"badpwdcount","0"
"description","Built-in account for guest access to the computer/domain"
"dn","CN=Guest,CN=Users,DC=ldap,DC=example,DC=com"
"lastlogoff","1601-01-01 00:00:00 UTC"
"lastlogon","1601-01-01 00:00:00 UTC"
"logoncount","0"
"memberof","CN=Guests,CN=Builtin,DC=ldap,DC=example,DC=com"
"name","Guest"
"objectsid","S-1-5-21-815333893-3774685697-946021116-501"
"pwdlastset",""
"samaccountname","Guest"
"useraccountcontrol","66082"

Name,Attributes
"badpwdcount","0"
"description","Key Distribution Center Service Account"
"dn","CN=krbtgt,CN=Users,DC=ldap,DC=example,DC=com"
"lastlogoff","1601-01-01 00:00:00 UTC"
"lastlogon","1601-01-01 00:00:00 UTC"
"logoncount","0"
"memberof","CN=Denied RODC Password Replication Group,CN=Users,DC=ldap,DC=example,DC=com"
"name","krbtgt"
"objectsid","S-1-5-21-815333893-3774685697-946021116-502"
"pwdlastset",""
"samaccountname","krbtgt"
"useraccountcontrol","514"

[*] Query returned 4 results.
[*] Auxiliary module execution completed

@cgranleese-r7 cgranleese-r7 added enhancement rn-enhancement release notes enhancement labels Dec 13, 2024
@cgranleese-r7 cgranleese-r7 merged commit 90066b3 into rapid7:master Dec 13, 2024
71 checks passed
@cgranleese-r7
Copy link
Contributor

Release Notes

Updates OptEnum to validate values without being case sensitive while preserving the case the author was expecting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adfoster-r7 adfoster-r7 adfoster-r7 left review comments

@jheysel-r7 jheysel-r7 jheysel-r7 left review comments

@cgranleese-r7 cgranleese-r7 cgranleese-r7 approved these changes

Assignees

@cgranleese-r7 cgranleese-r7

Labels
enhancement rn-enhancement release notes enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Case Insensitive OptEnum
4 participants
@zeroSteiner @cgranleese-r7 @jheysel-r7 @adfoster-r7