-
Notifications
You must be signed in to change notification settings - Fork 14k
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
Improve validation and error handling for forgeticket module #17268
base: master
Are you sure you want to change the base?
Improve validation and error handling for forgeticket module #17268
Conversation
3cfd7ba
to
ff818be
Compare
d4c3138
to
b2932d6
Compare
This will most likely land into master after the kerberos branch is merged, taking off the feature label for now 👍 |
@cgranleese-r7 Looks like this needs a rebase |
b2932d6
to
95a33fe
Compare
95a33fe
to
a661cdf
Compare
Can take a look at this shortly if no one has picked it up before then otherwise feel free to grab it. |
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.
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]), |
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.
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
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('SPN', [ false, 'The Service Principal Name (Only used for silver ticket) Ex: MSSqlSvc/host.domain.local:1434'], conditions: %w[ACTION == FORGE_SILVER], regex: %r{.*/.*}, examples: %w[MSSqlSvc/host.domain.local:1433 MSSqlSvc/host.domain.local:1434]), | ||
OptInt.new('DURATION', [ false, 'Duration of the ticket in days', 3650]), |
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.
OptInt.new('DURATION', [ false, 'Duration of the ticket in days', 3650]), | |
OptInt.new('DURATION', [ true, 'Duration of the ticket in days', 3650]), |
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
After
features set datastore_fallbacks true
features set datastore_fallbacks false
Verification
Needs testing with the datastore_fallbacks set to true then false
features set datastore_fallbacks false
features set datastore_fallbacks true
Testing steps:
msfconsole
auxiliary/admin/kerberos/forge_ticket
DOMAIN_SID
-set DOMAIN_SID test
SPN
-set SPN test
run spn=test domain_sid=test