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

Add a config system for Panther detections #950

Merged
merged 4 commits into from
Dec 4, 2023

Conversation

jof
Copy link
Contributor

@jof jof commented Nov 16, 2023

This adds a fork-friendly way to define some configuration values for detections inside of this repository to consume.

Additionally, this implements this configuration scheme for all existing uses of example.com as a way to demonstrate how to consume the API and use it with some tests and Mocks.

In the current state:

  • Organizational users of Panther are directed to make private forks of this repo
  • Plugin code makes use of hard-coded configuration values directly inside the python source

This is leading to:

  • Organizational users modifying the detection python sources to set configuration values that make sense for them
  • Upstream changes landing into python sources
  • Merging of upstream branches into the forks are causing merge conflicts

The hope of the configuration system proposed in here is that the public fork (panther-labs/panther-analysis) of this repo contains a bunch of reasonable default values inside of global_helpers/panther_config_defaults.py and a mostly empty global_helpers/panther_config_overrides.py.

Then, at lookup-time, users can import and reference panther_config.config; accesses to attributes of this object will first look for values in panther_config_overrides.py and fall back to the defaults in panther_config_defaults.py.
Importantly, for testing, these configuration values should probably get imported into the top-most scope of each rule, such that they can be easily overridden with Mocks: content for the test cases.

@jof jof marked this pull request as ready for review November 16, 2023 06:28
@jof jof requested a review from a team November 16, 2023 06:28
@jof jof force-pushed the jof/public/config branch from 33cc510 to cbab551 Compare November 16, 2023 06:28
Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome PR, thanks for opening it. Should add this file .gitattributes with ours ownership so it always overrides the upstream?

Copy link
Contributor Author

@jof jof Nov 16, 2023

Choose a reason for hiding this comment

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

Yeah, I think that probably makes sense. From my user-perspective, we only ever merge upstream into our fork (or rebase). So long as we're never merging in the other direction (fork->upstream), this seems like the right choice.

Will add.

@jof jof force-pushed the jof/public/config branch 2 times, most recently from 748bc44 to 137c10e Compare November 18, 2023 07:42
@jof jof mentioned this pull request Nov 20, 2023
Copy link
Contributor

@arielkr256 arielkr256 left a comment

Choose a reason for hiding this comment

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

@jof I like the idea of centralizing config values, but want to make sure the changes don't break things in other environments. The 3 new global helper functions need to be added to the pack files for the log types where rules have been updated so that environments using packs get the new helper functions.

@jof jof force-pushed the jof/public/config branch from 137c10e to 14a43b4 Compare November 20, 2023 23:22
@jof
Copy link
Contributor Author

jof commented Nov 20, 2023

@arielkr256 Good call on the Packs -- I updated the affected ones with the new modules

@jof jof requested a review from arielkr256 November 22, 2023 19:04
@jof jof force-pushed the jof/public/config branch from 14a43b4 to 6d17228 Compare November 22, 2023 19:04
@jof jof requested a review from a team November 22, 2023 19:04
@jof jof force-pushed the jof/public/config branch from 6d17228 to 6800a1e Compare November 27, 2023 23:01
Copy link
Contributor

@arielkr256 arielkr256 left a comment

Choose a reason for hiding this comment

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

We were able to confirm these changes will not overwrite configs for customers using packs, good to go!

@arielkr256 arielkr256 merged commit fd2574b into panther-labs:main Dec 4, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants