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

[FR] Refactor to more seamlessly support multiple DAC approaches #3407

Merged
merged 22 commits into from
Apr 26, 2024

Conversation

brokensound77
Copy link
Collaborator

@brokensound77 brokensound77 commented Jan 28, 2024

Note: this will be merged into the DAC-feature branch (with future consideration for going into main)

Issues

resolves #1342
resolves #1738

The changes encompassed in this PR are primarily to enable detections-as-code adoption more seamlessly. This is being achieved mostly through decoupling of major components. These changes are considered a major refactor to the existing functionality of the repo (and detection-rules processes). As such, these will be merging to an intermediate DAC-feature branch (along with subsequent PRs) to ensure implementation is accurate and nothing major is breaking.

Add custom rule directory support

Summary

This adds support to allow all a custom rule directory to be supplied via the environment variable DETECTION_RULES_DIR, which exposes most testing, loading, and building functionality of the repo (there are still internal things specific for this repo and internal Elastic team).

Details

The portability of the rules dir is accomplished via a few changes:

  • loading all required config files into a single, portable dataclass (RulesConfig)
  • based on a consolidated _config.yaml file to point to the proper files.
  • replacing all calls to default version_lock to be based on the RulesConfig
  • changing the rule load from default internal rules to account for the envvar DETECTION_RULES_DIR

Steps to configure custom directory

  • copy _config.yaml to the root of the custom rules directory
  • set using the environment variable DETECTION_RULES_DIR
  • setup the directory similar to the example structure:
#     custom-rules
#     ├── _config.yaml
#     └── rules
#         ├── example_rule_1.toml
#         ├── example_rule_2.toml
#     └── etc
#         ├── deprecated_rules.json
#         ├── packages.yml
#         ├── stack-schema-map.yaml
#         └── version.lock.json
#     └── actions
##         ├── action_1.toml
##         ├── action_2.toml
#     └── exceptions
##         ├── exception_1.toml
##         ├── exception_2.toml
  • Which would then require updating custom-rules/_config.yaml with:
    • deprecated_rules: etc/deprecated_rules.json
    • packages: etc/packages.yml
    • stack_schema_map: etc/stack-schema-map.yaml
    • version_lock: etc/version.lock.json
  • the paths in this file are relative to the custom rules directory (DETECTION_RULES_DIR/)
  • run tests by setting envvar
    • CUSTOM_RULES_DIR=custom-rules make test or CUSTOM_RULES_DIR=custom-rules python -m detection_rules test

config to opt out of unit tests and bypass query validation

Summary

This exposes a config file approach to opting in and out of existing unit tests and query validation.

Details

A test config is defined and passed within the _config.yaml or via a direct environment variable. From there glob patterns can be used to opt in (test_only) or opt out (bypass) or unit tests by pattern name.

Additionally, rules can bypass query validation by their rule_id

validation for exception_lists and actions

Summary

Adds objects for exception_lists and actions for basic validation

Details

The dataclasses are equivelent to the Rule and related classes hierarchical structure. For now, that is the limit of the support. We can look to stitching them in a subsequent PR

Other updates

  • adds a GenericCollection for loading and managing actions and exception lists

For more details, refer to the docs/custom-rules.md

Testing

setup a custom dir like:

custom-rules
├── _config.yaml
├── etc
│   ├── deprecated_rules.json
│   ├── packages.yml
│   ├── stack-schema-map.yaml
│   ├── test_config.yaml
│   └── version.lock.jso
└── rules
    ├── collection_posh_audio_capture.toml
    └── command_and_control_cobalt_strike_default_teamserver_cert.toml
  • they can be any legit rule
  • update config with:
# detection-rules config file

rule_dirs:
  - rules
files:
  deprecated_rules: etc/deprecated_rules.json
  packages: etc/packages.yml
  stack_schema_map: etc/stack-schema-map.yaml
  version_lock: etc/version.lock.json

testing:
  config: etc/test_config.yaml
  • configure various combinations of bypass and test_only for unit tests and rule_ids in validation within the custom test config
  • run make test for all combinations

@brokensound77 brokensound77 added the enhancement New feature or request label Jan 28, 2024
@brokensound77 brokensound77 self-assigned this Jan 28, 2024
Mikaayenson

This comment was marked as resolved.

@brokensound77

This comment was marked as resolved.

@Mikaayenson

This comment was marked as resolved.

@brokensound77

This comment was marked as resolved.

@Mikaayenson
Copy link
Collaborator

Mikaayenson commented Jan 31, 2024

If a customer wants to apply the same rule to multiple custom_dir aka different stacks/clients do they have to duplicate that rule in every folder?

Imagine I want some rules applied to one stack vs another (organized by folder). I think based what's available so far, a user could create custom_base_rules, custom_stack1_rules, custom_stack2_rules folders. They could then in some other manner push custom_base_rules and custom_stack1_rules to one stack and then custom_base_rules and custom_stack2_rules to another stack.

Subsequently, if they had to make a change to that common file, would then then have to update each instance in every subfolder?

Similarily, for packages.yml, do we expect customers to create custom code to sync this file across their custom directories? It may not be a huge lift for one custom_dir, but it may be cumbersome for many without automation.

@Mikaayenson

This comment was marked as resolved.

@Mikaayenson
Copy link
Collaborator

Note: If we have people manage a separate stack-schema-map, will they also have to manage schema and manifest updates?

@brokensound77
Copy link
Collaborator Author

BYOS (schema) within the directories for integrations, etc.

@brokensound77
Copy link
Collaborator Author

@Mikaayenson

If a customer wants to apply the same rule to multiple custom_dir aka different stacks/clients do they have to duplicate that rule in every folder?

Remember, think of the repo and stack updates as completely decoupled. Meaning, if they wanted to push to multiple stacks, the could test once, and push the same folder 3 different times via a single CI/CD workflow.

If the idea is to have 3 unique sets of rules, then yes, by design, 3 directories and config sets. In this scenario, they can handle shared rules using git

Note: If we have people manage a separate stack-schema-map, will they also have to manage schema and manifest updates?

This is a good call out - either they will need to get us to approve it, where it would be globally accessible. Or, even better, we update this process to allow the custom dir to portably include schemas too

@Mikaayenson
Copy link
Collaborator

FWIW, #3430 is another example of why we may want to enable custom schemas. We have an instance where we need to leverage the integration name to populate related integrations, but aren't using the integration for schema validation since we dont use new fields from the integration. In this case, we may not need it in our repo, but others may.

@Mikaayenson
Copy link
Collaborator

@brokensound77 Is it true to say that a user has to change their auth config file or environment variables in between deploying to different environments? It seems like an easy enabler would be to allow them to specify environment for credentials in the auth config or maybe have the custom rules directory discern the correct credentials based on prefix or something?

Id want to just run CUSTOM_RULES_DIR=custom-rules make test then CUSTOM_RULES_DIR=custom-rules_2 make test back to back without having to change or even worry about credentials in between. This would also set us up for the future when remote validation is available.

brokensound77 and others added 3 commits February 20, 2024 20:11
…ions

[FR] Add support to decouple actions and exceptions
…-validation

[FR] Add support for configurable tests and validation
@brokensound77 brokensound77 changed the title [FR] Add custom rule directory support [FR] Refactor to more seamlessly support multiple DAC approaches Mar 13, 2024
# Conflicts:
#	detection_rules/devtools.py
#	detection_rules/packaging.py
#	tests/test_all_rules.py
Mikaayenson

This comment was marked as resolved.

@brokensound77

This comment was marked as resolved.

@brokensound77
Copy link
Collaborator Author

brokensound77 commented Apr 24, 2024

Thanks for the feedback @Mikaayenson

I still think we should normalize the yml / yaml extensions.

agree, but in a subsequent PR (or better, targeting main) (🏈 )

What about handling credentials between stacks #3407 (comment).

not a bad idea but out of scope. I would consider adding that targeting main, and just allow the CLI to be launched with -c <explicit-config> or alternatively the default config with multiple entries, and the entry selected at launch

I think this #3407 (review) still needs to be addressed about The create_rule, import_rules, and rule_prompt methods should be updated to support the CUSTOM_RULES_DIR, which all currently use RULES_DIR.

refer to my previous comment here:

* https://github.com/elastic/detection-rules/pull/3407#pullrequestreview-1953539086 multi-collection and single-collection to use custom dirs from RULES_CONFIG
   * this includes [revising](https://github.com/elastic/detection-rules/pull/3407/commits/c1a263c519457388a9840c7c92d9b762e23ebebb) use of the rule_loader default global constants and decoupling them as needed

Did you see this #3407 (review), this was feedback from infosec.

folder structure is dictated by users, especially since the loader is recursive. There is nothing special we need to do to support it. In fact, I think a better approach is using a filter in the loader to handle those, based on the data

Same thing with this #3407 (comment). Resolved, but no reply. Both are feedback from infosec.

this was also already answered in my previous comment:

* consider https://github.com/elastic/detection-rules/pull/3407#issuecomment-2025974382 a callback function to allow folder structuring on save

🏈

Note: I didn't see this community #3407 (comment) on your list or a reply to it re: sub indices etc.

From my previous comment:

* BYOS
   * including reviewing https://github.com/elastic/detection-rules/pull/3407#issuecomment-2026577079

🏈

Copy link
Collaborator

@eric-forte-elastic eric-forte-elastic left a comment

Choose a reason for hiding this comment

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

Great peer review 👍

Copy link
Collaborator

@Mikaayenson Mikaayenson left a comment

Choose a reason for hiding this comment

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

:shipit:

# Conflicts:
#	detection_rules/cli_utils.py
#	detection_rules/rule.py
#	detection_rules/rule_loader.py
#	docs/developing.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request python Internal python for the repository schema
Projects
None yet
3 participants