Improve validation and error handling for forgeticket module

[cgranleese-r7]

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

features set datastore_fallbacks false

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

[adfoster-r7]

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

[adfoster-r7]

@cgranleese-r7 Looks like this needs a rebase

[gwillcox-r7]

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

[dwelch-r7]

So I'm not getting exactly the same output as you are but I think that's in part to some of the options no longer being required?

Either way I can take another look when the conflicts have been resolved

[dwelch-r7]

Suggested change

	OptString.new('DOMAIN', [ false, 'The Domain (upper case) Ex: DEMO.LOCAL' ]),
	OptString.new('DOMAIN_SID', [ false, 'The Domain SID, Ex: S-1-5-21-1755879683-3641577184-3486455962'], regex: /^S-\d-\d+-(\d+-){1,14}\d+$/, examples: %w[S-1-5-21-1755879683-3641577184-3486455962 S-1-5-21-1180699209-877415012-3182924384-1004]),
	OptString.new('DOMAIN', [ true, 'The Domain (upper case) Ex: DEMO.LOCAL' ]),
	OptString.new('DOMAIN_SID', [ true, 'The Domain SID, Ex: S-1-5-21-1755879683-3641577184-3486455962'], regex: /^S-\d-\d+-(\d+-){1,14}\d+$/, examples: %w[S-1-5-21-1755879683-3641577184-3486455962 S-1-5-21-1180699209-877415012-3182924384-1004]),

Not sure how/why these got changed, pretty sure they should be required

[dwelch-r7]

Suggested change

	OptInt.new('DURATION', [ false, 'Duration of the ticket in days', 3650]),
	OptInt.new('DURATION', [ true, 'Duration of the ticket in days', 3650]),

[github-actions]

Thanks for your contribution to Metasploit Framework! We've looked at this pull request, and we agree that it seems like a good addition to Metasploit, but it looks like it is not quite ready to land. We've labeled it attic and closed it for now.

What does this generally mean? It could be one or more of several things:

• It doesn't look like there has been any activity on this pull request in a while
• We may not have the proper access or equipment to test this pull request, or the contributor doesn't have time to work on it right now.
• Sometimes the implementation isn't quite right and a different approach is necessary.

We would love to land this pull request when it's ready. If you have a chance to address all comments, we would be happy to reopen and discuss how to merge this!

[adfoster-r7]

Will attic for now until we can pick this up again in the new year when we've got the other priorities out for this year 👍