metasploit-framework icon indicating copy to clipboard operation
metasploit-framework copied to clipboard

Improve validation and error handling for forgeticket module

Open cgranleese-r7 opened this issue 2 years ago • 3 comments

Note

If a user had their features set datastore_fallbacks false as well as saved options. The options would continue to fail to validate and would accept any value and not honour the options regex check. This error does not occur with the features set datastore_fallbacks true, therefore I'll leave this in draft for now until I I can figure out a way to fix this.

This PR adds validation to some of the options within the forge_ticket.rb module. As well as improving error affordance when setting options. Adding examples to the API to aid in making errors more transparent for users, and works for both inline options as well as setting options via the set command, also updates some tests to reflect the changes in error outputs.

Before

[-] Msf::OptionValidateError The following options failed to validate: SPN 

After

[-] The following options failed to validate: Value 'test' is not valid for option 'SPN'. Example value: MSSqlSvc/host.domain.local:1433.

features set datastore_fallbacks true

image

features set datastore_fallbacks false

image

Verification

Needs testing with the datastore_fallbacks set to true then false

  • [ ] Below steps tested with features set datastore_fallbacks false
  • [ ] Below steps tested with features set datastore_fallbacks true

Testing steps:

  • [ ] Start msfconsole
  • [ ] Use auxiliary/admin/kerberos/forge_ticket
  • [ ] Attempt to set invalid option for DOMAIN_SID - set DOMAIN_SID test
  • [ ] Attempt to set invalid option for SPN - set SPN test
  • [ ] Verify the appropriate error message are returned
  • [ ] Verify the step above only this time using inline options run spn=test domain_sid=test

cgranleese-r7 avatar Nov 16 '22 10:11 cgranleese-r7

This will most likely land into master after the kerberos branch is merged, taking off the feature label for now :+1:

adfoster-r7 avatar Jan 20 '23 13:01 adfoster-r7

@cgranleese-r7 Looks like this needs a rebase

adfoster-r7 avatar Jan 31 '23 17:01 adfoster-r7

Can take a look at this shortly if no one has picked it up before then otherwise feel free to grab it.

gwillcox-r7 avatar Feb 01 '23 23:02 gwillcox-r7