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] [DAC] further decouple reliance on default rule dir locations #3654

Conversation

eric-forte-elastic
Copy link
Contributor

@eric-forte-elastic eric-forte-elastic commented May 7, 2024

Issues

#3619

Summary

The primary purpose of this PR is to remove the hard coding of rules directories and replace them with the directories loaded from a config file. Either the default one, or a user generated one.

Additionally, there are a few other considerations that are related to the implementation:

  • This PR should make BBR rules directories optional
  • In the config, we should have a separate path list for BBR rules dirs as they are handled differently
  • This PR updates the current loading of rules to support a list of paths to be loaded, instead of one path.
  • This PR updates the example config generation to match the formatting specified from the above considerations.

Open Design Quetsions:

  • In the config, should also have an RTA section (expected to be optional as well)?
    • Currently this is not done, but maybe it should be?
  • Need to decide whether or not deprecated rules are optional and whether or not we should make this be a configurable directory location (as opposed to _deprecated relative to the rules directory.)
    • Current decision is to store deprecated rules in _deprecated folder relative to rules directory.

Testing

To test one will need to run the following commands with both the default config and a custom config.

  • make build
  • make test-cli
  • make test-remote-cli

Testing Results

Custom config

make

make_build.txt

make test-cli

make_test_cli.txt

make test-remote-cli

detection-rules on  3619-frdac-further-decouple-reliance-on-default-rule-dir-locations [$✘!?] is  v0.1.0 via  v3.12.3 (detection-rules-build) on  eric.forte
❯ make test-remote-cli
Executing test_remote_cli script...
Running detection-rules remote CLI tests...
Performing a quick rule alerts search...
Requires .detection-rules-cfg.json credentials file set.
Loaded config file: /home/forteea1/Code/clean_mains/detection-rules/.detection-rules-cfg.json

█▀▀▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄   ▄      █▀▀▄ ▄  ▄ ▄   ▄▄▄ ▄▄▄
█  █ █▄▄  █  █▄▄ █    █   █  █ █ █▀▄ █      █▄▄▀ █  █ █   █▄▄ █▄▄
█▄▄▀ █▄▄  █  █▄▄ █▄▄  █  ▄█▄ █▄█ █ ▀▄█      █ ▀▄ █▄▄█ █▄▄ █▄▄ ▄▄█

kibana_user: eric.forte
kibana_password:
No alerts detected
Performing a rule export...
Loaded config file: /home/forteea1/Code/clean_mains/detection-rules/.detection-rules-cfg.json

█▀▀▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄   ▄      █▀▀▄ ▄  ▄ ▄   ▄▄▄ ▄▄▄
█  █ █▄▄  █  █▄▄ █    █   █  █ █ █▀▄ █      █▄▄▀ █  █ █   █▄▄ █▄▄
█▄▄▀ █▄▄  █  █▄▄ █▄▄  █  ▄█▄ █▄█ █ ▀▄█      █ ▀▄ █▄▄█ █▄▄ █▄▄ ▄▄█

kibana_user: eric.forte
kibana_password:
- skipping First Time Seen AWS Secret Value Accessed in Secrets Manager - ValidationError
3 rules exported
2 rules converted
2 saved to tmp-export
1 errors saved to tmp-export/_errors.txt
adfind_command_activity.toml  clearing_windows_console_history.toml  _errors.txt
Removing generated files...
Detection-rules CLI tests completed!

@eric-forte-elastic eric-forte-elastic changed the base branch from main to DAC-feature May 7, 2024 20:49
@eric-forte-elastic
Copy link
Contributor Author

LGTM!

I think we should run the make test-cli. there may be a few gaps still missing.

Agreed, I think it passed for me due to a specific config setup in for my environment. Will take a look 👀

@Mikaayenson Mikaayenson self-requested a review May 10, 2024 23:23
@eric-forte-elastic
Copy link
Contributor Author

Couple thoughts:

  1. We should probably update the custom_rules setup-config to match the bbr rule changes made in here.
  2. When we load optional directories, we should probably add resiliency to handle when its none
load_directories(DEFAULT_PREBUILT_RULES_DIRS)
load_directories(DEFAULT_PREBUILT_BBR_DIRS)

will error if either are set to NoneType

Good catch! Added in a default to not be None in the creation and adding resiliency
bbr_rules_dirs: Optional[List[Path]] = field(default_factory=list)

@Mikaayenson Mikaayenson self-requested a review May 13, 2024 20:55
@@ -89,6 +90,8 @@ a combination of both.
In addition to the formats mentioned using `create-rule`, this will also accept an `.ndjson`/`jsonl` file
containing multiple rules (as would be the case with a bulk export).

The `-s/--save-directory` is an optional parameter to specify a non default directory to place imported rules. If it is not specified, the first directory specified in the rules config will be used.
Copy link
Contributor

Choose a reason for hiding this comment

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

This will give people the ability to place rules where they want which is nice!

Copy link
Contributor

@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.

⭐ Great work. It lgtm. Especially after making the final changes based on the make test files.

Copy link

@traut traut left a comment

Choose a reason for hiding this comment

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

looks good!

@eric-forte-elastic eric-forte-elastic merged commit d0a2e28 into DAC-feature May 14, 2024
14 checks passed
@eric-forte-elastic eric-forte-elastic deleted the 3619-frdac-further-decouple-reliance-on-default-rule-dir-locations branch May 14, 2024 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community detections-as-code enhancement New feature or request python Internal python for the repository Team: TRADE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FR][DAC] further decouple reliance on default rule dir locations
3 participants