adds support for alerts and triggers on group by based sigma rules #545

eirsep · 2023-09-07T05:35:33Z

Description

For each group by rule a bucket level monitor is created.
Bucket level monitor doesn't support the current detector triggers which cater focussedly to per document alerting
This change uses workflow's chained monitor findings feature to feed findings generated by all bucket level monitors as source data to a match all doc level monitor. The chained findings monitor simply creates findings for all docs which match the bucket level monitor rule and honors the detector trigger condition creating alerts if there are any group by rules mentioned in the trigger.

Behaviour of non-group by rule matching is unaffected.

codecov · 2023-09-07T08:20:10Z

Codecov Report

Merging #545 (3684997) into main (1003936) will decrease coverage by 0.13%.
Report is 1 commits behind head on main.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               main     #545      +/-   ##
============================================
- Coverage     25.12%   24.99%   -0.13%     
- Complexity      942      943       +1     
============================================
  Files           255      255              
  Lines         11064    11125      +61     
  Branches       1231     1242      +11     
============================================
+ Hits           2780     2781       +1     
- Misses         8032     8093      +61     
+ Partials        252      251       -1

Files Changed	Coverage Δ
...earch/securityanalytics/model/DetectorTrigger.java	`54.85% <0.00%> (-1.29%)`	⬇️
...yanalytics/settings/SecurityAnalyticsSettings.java	`96.00% <ø> (ø)`
...lytics/transport/TransportIndexDetectorAction.java	`0.00% <0.00%> (ø)`
...ensearch/securityanalytics/util/DetectorUtils.java	`0.00% <0.00%> (ø)`
...search/securityanalytics/util/WorkflowService.java	`0.00% <0.00%> (ø)`

... and 1 file with indirect coverage changes

sbcd90 · 2023-09-07T19:03:50Z

src/test/java/org/opensearch/securityanalytics/resthandler/DetectorMonitorRestApiIT.java

@@ -940,7 +941,7 @@ else if (ruleId == minRuleId) {
        // Assert findings
        assertNotNull(getFindingsBody);
        // 8 findings from doc level rules, and 3 findings for aggregation (sum, max and min)
-        assertEquals(11, getFindingsBody.get("total_findings"));
+        assertEquals(19, getFindingsBody.get("total_findings"));


should we hide the findings generated by match of _id: * queries?

been thinking about this
Need to add logic to avoid fetching findings from bucket level monitors which are feeding findings to chained findings doc monitor.

let's go with this for now
will do de-dupe in a quick follow up.

sbcd90 · 2023-09-07T19:05:00Z

src/test/java/org/opensearch/securityanalytics/resthandler/DetectorMonitorRestApiIT.java

@@ -1409,7 +1410,7 @@ public void testCreateDetector_verifyWorkflowExecutionBucketLevelDocLevelMonitor
        Map<String, Object> getFindingsBody = entityAsMap(getFindingsResponse);

        assertNotNull(getFindingsBody);
-        assertEquals(6, getFindingsBody.get("total_findings"));
+        assertEquals(10, getFindingsBody.get("total_findings"));


same comment as https://github.com/opensearch-project/security-analytics/pull/545/files#r1319008403

let's go with this for now
will do de-dupe in a quick follow up.

Signed-off-by: Surya Sashank Nistala <[email protected]>

…mentioned in detector triggers Signed-off-by: Surya Sashank Nistala <[email protected]>

sbcd90 · 2023-09-08T19:35:17Z

src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java

@@ -785,7 +850,7 @@ private IndexMonitorRequest createBucketLevelMonitorRequest(
         triggers.add(bucketLevelTrigger1);
         } **/

-        Monitor monitor = new Monitor(monitorId, Monitor.NO_VERSION, detector.getName(), false, detector.getSchedule(), detector.getLastUpdateTime(), null,
+        Monitor monitor = new Monitor(monitorId, Monitor.NO_VERSION, detector.getName() + UUID.randomUUID(), false, detector.getSchedule(), detector.getLastUpdateTime(), null,


is this change needed? i reverted this & integ tests still work fine.

Signed-off-by: Surya Sashank Nistala <[email protected]>

eirsep · 2023-09-08T21:35:58Z

#558

opensearch-trigger-bot · 2023-09-08T21:36:51Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-545-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 f4be879cd1486329fac5f2df1e251d3b544cd48e
# Push it to GitHub
git push --set-upstream origin backport/backport-545-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-545-to-2.x.

opensearch-trigger-bot · 2023-09-08T21:36:52Z

The backport to 2.10 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.10 2.10
# Navigate to the new working tree
cd .worktrees/backport-2.10
# Create a new branch
git switch --create backport/backport-545-to-2.10
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 f4be879cd1486329fac5f2df1e251d3b544cd48e
# Push it to GitHub
git push --set-upstream origin backport/backport-545-to-2.10
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.10

Then, create a pull request where the base branch is 2.10 and the compare/head branch is backport/backport-545-to-2.10.

…pensearch-project#545) * fix test Signed-off-by: Surya Sashank Nistala <[email protected]> * adds support for alerts and triggers on group by based rules Signed-off-by: Surya Sashank Nistala <[email protected]> * change setting workflow usage enabled to true by default Signed-off-by: Surya Sashank Nistala <[email protected]> * turn off workflow setting for test Signed-off-by: Surya Sashank Nistala <[email protected]> * revert new line Signed-off-by: Surya Sashank Nistala <[email protected]> * support feeding findings to chained finding monitors ONLY from rules mentioned in detector triggers Signed-off-by: Surya Sashank Nistala <[email protected]> * revert naming convention change for bucket level monitors Signed-off-by: Surya Sashank Nistala <[email protected]> --------- Signed-off-by: Surya Sashank Nistala <[email protected]>

…igma rules (#545) (#559) * adds support for alerts and triggers on group by based sigma rules (#545) * fix test Signed-off-by: Surya Sashank Nistala <[email protected]> * adds support for alerts and triggers on group by based rules Signed-off-by: Surya Sashank Nistala <[email protected]> * change setting workflow usage enabled to true by default Signed-off-by: Surya Sashank Nistala <[email protected]> * turn off workflow setting for test Signed-off-by: Surya Sashank Nistala <[email protected]> * revert new line Signed-off-by: Surya Sashank Nistala <[email protected]> * support feeding findings to chained finding monitors ONLY from rules mentioned in detector triggers Signed-off-by: Surya Sashank Nistala <[email protected]> * revert naming convention change for bucket level monitors Signed-off-by: Surya Sashank Nistala <[email protected]> --------- Signed-off-by: Surya Sashank Nistala <[email protected]> * fix compile issue from main and 2.x dependency divergence Signed-off-by: Surya Sashank Nistala <[email protected]> --------- Signed-off-by: Surya Sashank Nistala <[email protected]>

…igma rules (#545) (#559) * adds support for alerts and triggers on group by based sigma rules (#545) * fix test Signed-off-by: Surya Sashank Nistala <[email protected]> * adds support for alerts and triggers on group by based rules Signed-off-by: Surya Sashank Nistala <[email protected]> * change setting workflow usage enabled to true by default Signed-off-by: Surya Sashank Nistala <[email protected]> * turn off workflow setting for test Signed-off-by: Surya Sashank Nistala <[email protected]> * revert new line Signed-off-by: Surya Sashank Nistala <[email protected]> * support feeding findings to chained finding monitors ONLY from rules mentioned in detector triggers Signed-off-by: Surya Sashank Nistala <[email protected]> * revert naming convention change for bucket level monitors Signed-off-by: Surya Sashank Nistala <[email protected]> --------- Signed-off-by: Surya Sashank Nistala <[email protected]> * fix compile issue from main and 2.x dependency divergence Signed-off-by: Surya Sashank Nistala <[email protected]> --------- Signed-off-by: Surya Sashank Nistala <[email protected]> (cherry picked from commit 24fe8d4)

…sigma rules (#545) (#559) (#560) Signed-off-by: Surya Sashank Nistala <[email protected]>

eirsep requested review from amsiglan, AWSHurneyt, getsaurabh02, lezzago, praveensameneni and sbcd90 as code owners September 7, 2023 05:35

sbcd90 reviewed Sep 7, 2023

View reviewed changes

eirsep added 5 commits September 7, 2023 15:51

fix test

d14a9cd

Signed-off-by: Surya Sashank Nistala <[email protected]>

adds support for alerts and triggers on group by based rules

8ddf0bf

Signed-off-by: Surya Sashank Nistala <[email protected]>

change setting workflow usage enabled to true by default

7fcc7e0

Signed-off-by: Surya Sashank Nistala <[email protected]>

turn off workflow setting for test

85cb0f0

Signed-off-by: Surya Sashank Nistala <[email protected]>

revert new line

ac2323a

Signed-off-by: Surya Sashank Nistala <[email protected]>

eirsep force-pushed the bucket branch from 32f5658 to 655064b Compare September 7, 2023 23:14

support feeding findings to chained finding monitors ONLY from rules …

ea025b0

…mentioned in detector triggers Signed-off-by: Surya Sashank Nistala <[email protected]>

eirsep force-pushed the bucket branch from 655064b to ea025b0 Compare September 8, 2023 10:07

sbcd90 reviewed Sep 8, 2023

View reviewed changes

revert naming convention change for bucket level monitors

3684997

Signed-off-by: Surya Sashank Nistala <[email protected]>

eirsep force-pushed the bucket branch from 6f5ad45 to 3684997 Compare September 8, 2023 20:22

sbcd90 approved these changes Sep 8, 2023

View reviewed changes

eirsep added backport 2.x backport 2.10 labels Sep 8, 2023

getsaurabh02 approved these changes Sep 8, 2023

View reviewed changes

eirsep merged commit f4be879 into opensearch-project:main Sep 8, 2023

sbcd90 pushed a commit that referenced this pull request Sep 9, 2023

Backport 2.10 adds support for alerts and triggers on group by based …

a59bb63

…sigma rules (#545) (#559) (#560) Signed-off-by: Surya Sashank Nistala <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adds support for alerts and triggers on group by based sigma rules #545

adds support for alerts and triggers on group by based sigma rules #545

eirsep commented Sep 7, 2023

codecov bot commented Sep 7, 2023 •

edited

Loading

sbcd90 Sep 7, 2023

eirsep Sep 7, 2023 •

edited

Loading

sbcd90 Sep 7, 2023

eirsep Sep 7, 2023 •

edited

Loading

sbcd90 Sep 8, 2023

eirsep commented Sep 8, 2023

opensearch-trigger-bot bot commented Sep 8, 2023

opensearch-trigger-bot bot commented Sep 8, 2023

adds support for alerts and triggers on group by based sigma rules #545

adds support for alerts and triggers on group by based sigma rules #545

Conversation

eirsep commented Sep 7, 2023

Description

codecov bot commented Sep 7, 2023 • edited Loading

Codecov Report

sbcd90 Sep 7, 2023

Choose a reason for hiding this comment

eirsep Sep 7, 2023 • edited Loading

Choose a reason for hiding this comment

sbcd90 Sep 7, 2023

Choose a reason for hiding this comment

eirsep Sep 7, 2023 • edited Loading

Choose a reason for hiding this comment

sbcd90 Sep 8, 2023

Choose a reason for hiding this comment

eirsep commented Sep 8, 2023

opensearch-trigger-bot bot commented Sep 8, 2023

opensearch-trigger-bot bot commented Sep 8, 2023

codecov bot commented Sep 7, 2023 •

edited

Loading

eirsep Sep 7, 2023 •

edited

Loading

eirsep Sep 7, 2023 •

edited

Loading