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.

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

[Bug] [DaC] Metadata maturity field default mismatch and poor enforcement of rule naming conventions #4282

Open
eric-forte-elastic opened this issue Dec 4, 2024 · 1 comment · May be fixed by #4285
Assignees
Labels
bug Something isn't working Team: TRADE

Comments

@eric-forte-elastic
Copy link
Contributor

eric-forte-elastic commented Dec 4, 2024

Describe the Bug

Summary

Thanks to @SHolzhauer for finding this (see thread for more detail)! There is a mismatch in detection rules with the file naming defaults and metadata defaults.

  • In the import-rules-to-repo logic path the maturity metadata is hardcoded to development currently meta = {'creation_date': creation_date, 'updated_date': creation_date, 'maturity': 'development'} and the dates are hardcoded to today: creation_date = datetime.date.today().strftime("%Y/%m/%d")

  • In export-rules the logic path has the dates hardcoded to today as well if none provided (which there will not be from Kibana), but maturity is overwritten to be production contents = TOMLRuleContents.from_rule_resource(rule_resource, maturity="production")

This can cause arbitrary metadata changes in cases where the rule name does not match the expected <tactic>_<rule_name> convention, which also is not enforced outside of the CLI rule creation prompt, which some users may not use. In this case, the existing metadata in the repo will not be used and will be overridden with these defaults causing arbitrary rule file content changes depending on which path is used for the import/export.

An immediate fix could be to load all of the custom rules and check for existing metadata by rule ID, and use this if it exists. Additionally, this could be used to mitigate the rule naming convention issues. However, this could lead to significant performance degradation so we will want to put some thought into a more performant approach to accomplish the same result.

Furthermore, a warning should be printed if a rule is discovered that does not follow the expected naming convention, along with a the expected/correct file name to use.

To Reproduce

  1. Create a rule in detection rules, not using the rule creation CLI where the file name does not match the naming convention and where the metadata created and modified dates are a date prior to the current day.
  2. Import this rule into Kibana (or ndjson)
  3. Export this rule from Kibana (or ndjson)
  4. Observe that there are now 2 rules with the same rule ID. One with the correct file name and one with the original name. Additionally, observe that the created and modified dates has now been updated to today with no rule changed.

E.g.

Image

Expected Behavior

  1. Create a rule in detection rules, not using the rule creation CLI where the file name does not match the naming convention and where the metadata created and modified dates are a date prior to the current day.
  2. Import this rule into Kibana (or ndjson)
  3. Export this rule from Kibana (or ndjson)
  4. Observe that there is only one rule, with the metadata and filename of what was originally written.

Screenshots

No response

Desktop - OS

None

Desktop - Version

No response

Additional Context

No response

@eric-forte-elastic eric-forte-elastic added bug Something isn't working Team: TRADE labels Dec 4, 2024
@eric-forte-elastic eric-forte-elastic self-assigned this Dec 4, 2024
@SHolzhauer
Copy link
Contributor

@eric-forte-elastic you mention that

a warning should be printed if a rule is discovered that does not follow the expected naming convention, along with a the expected/correct file name to use.

and

An immediate fix could be to load all of the custom rules and check for existing metadata by rule ID, and use this if it exists. Additionally, this could be used to mitigate the rule naming convention issues. However, this could lead to significant performance degradation

Thinking about these two points, what if the import-rules command performs the naming validation and reject/fails for rules with an invalid naming. That would ensure the file naming matches and later down the line with export-rules you don't end up with duplicates.
This would leave the metadata issue to be resolved for which I don't yet have any insight.

Although if we keep to proper "as-code" methodology, the test command should always be used prior to the import-rules (or other methods of deployment). So if that already fails on the naming then it does not need to be part of the actual deployment steps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Team: TRADE
Projects
None yet
2 participants