Alerts in correlations [Experminental] #1040

riysaxen-amzn · 2024-05-22T00:12:14Z

Description

This is the initial PR for Alerts in Correlations, it has correlation alerts index management logic and Notification Service integration
A seperate PR will be raised for the new APIs

Issues Resolved

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Riya <[email protected]>

Signed-off-by: Riya Saxena <[email protected]>

eirsep · 2024-06-03T19:41:46Z

what is the intent of adding [Experimental] in PR? is that an opensearch project guideline?

eirsep · 2024-06-03T19:42:18Z

...va/org/opensearch/securityanalytics/correlation/alert/notifications/NotificationService.java

+    /**
+     * Extension function for publishing a notification to a channel in the Notification plugin.
+     */
+    public void sendNotification(String configId, String severity, String subject, String notificationMessageText) throws IOException {


what is config id?

configId is the id of Notification configuration, same as destinationId

eirsep · 2024-06-03T19:43:33Z

...va/org/opensearch/securityanalytics/correlation/alert/notifications/NotificationService.java

+            }
+            @Override
+            public void onFailure(Exception e) {
+                logger.error("Failed while sending a notification: " + e.toString());


can we log information about detector id or config id or whatever is required to do an RCA on why notification failed by just looking at this log

Signed-off-by: Riya Saxena <[email protected]>

riysaxen-amzn · 2024-06-06T00:20:35Z

what is the intent of adding [Experimental] in PR? is that an opensearch project guideline?

this is an experimental feature, thought to add this

eirsep · 2024-06-06T00:27:08Z

src/main/java/org/opensearch/securityanalytics/correlation/JoinEngine.java

    public JoinEngine(Client client, PublishFindingsRequest request, NamedXContentRegistry xContentRegistry,
-                      long corrTimeWindow, TransportCorrelateFindingAction.AsyncCorrelateFindingAction correlateFindingAction,
-                      LogTypeService logTypeService, boolean enableAutoCorrelations) {
+                      long corrTimeWindow, TimeValue indexTimeout, TransportCorrelateFindingAction.AsyncCorrelateFindingAction correlateFindingAction,


where is index timeout being set?

In JoinEngine

this.clusterService.getClusterSettings().addSettingsUpdateConsumer(SecurityAnalyticsSettings.INDEX_TIMEOUT, it -> indexTimeout = it);

sbcd90 · 2024-06-06T22:08:22Z

src/main/java/org/opensearch/securityanalytics/SecurityAnalyticsPlugin.java

@@ -53,6 +54,8 @@
 import org.opensearch.script.ScriptService;
 import org.opensearch.securityanalytics.action.*;


remove * imports.

sbcd90 · 2024-06-06T22:15:15Z

src/main/java/org/opensearch/securityanalytics/correlation/alert/CorrelationRuleScheduler.java

+        executorService.shutdown();
+    }
+
+    private void scheduleRule(CorrelationRule correlationRule, List<String> findingIds, TimeValue indexTimeout, String sourceFindingId, User user) {


instead of using java's executorservice can we use job-scheduler plugin?
also, i see for each rule we create a runnable.
Can we instead use an index to create a monitor like object which can be monitored by running a single job?

Signed-off-by: Riya Saxena <[email protected]>

jowg-amazon · 2024-06-11T17:01:25Z

src/main/java/org/opensearch/securityanalytics/correlation/alert/CorrelationAlertService.java

+        // Convert CorrelationAlert to a map
+        try {
+            XContentBuilder builder = XContentFactory.jsonBuilder().startObject();
+            builder.field("correlated_finding_ids", correlationAlert.getCorrelatedFindingIds());


Should use the final variables declared above

jowg-amazon · 2024-06-11T17:02:30Z

src/main/java/org/opensearch/securityanalytics/correlation/alert/CorrelationAlertService.java

+    }
+
+    // logic will be moved to common-utils, once the parsing logic in common-utils is fixed
+    public static CorrelationAlert parse(XContentParser xcp, String id, long version) throws IOException {


Does this still need to be moved to common-utils?

yes, I can move this common-utils at a later point

jowg-amazon · 2024-06-11T17:07:05Z

src/main/java/org/opensearch/securityanalytics/model/CorrelationRuleTrigger.java

+    }
+
+    public List<Action> getActions() {
+//        List<Action> transformedActions = new ArrayList<>();


Is this not needed anymore?

yes, right now, no use but we can have something in future

jowg-amazon · 2024-06-11T17:10:23Z

src/main/java/org/opensearch/securityanalytics/correlation/alert/CorrelationAlertService.java

+    private final NamedXContentRegistry xContentRegistry;
+    private final Client client;
+
+    protected static final String CORRELATED_FINDING_IDS = "correlated_finding_ids";


Is there a reason we're not specifying these in the correlation alert model in common-utils?

the parsing logic in common-utils is nt working as expected therefore for timebeing this is moved here

Signed-off-by: Riya Saxena <[email protected]>

riysaxen-amzn · 2024-06-11T20:51:46Z

CI is passing locally after the latest changes in common-utils built in:

WARNING: Please consider reporting this to the maintainers of org.opensearch.bootstrap.BootstrapForTesting
WARNING: System::setSecurityManager will be removed in a future release
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.gradle.api.internal.tasks.testing.worker.TestWorker (file:/Users/riysaxen/.gradle/wrapper/dists/gradle-8.5-bin/5t9huq95ubn472n8rpzujfbqh/gradle-8.5/lib/plugins/gradle-testing-base-8.5.jar)
WARNING: Please consider reporting this to the maintainers of org.gradle.api.internal.tasks.testing.worker.TestWorker
WARNING: System::setSecurityManager will be removed in a future release
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.gradle.api.internal.tasks.testing.worker.TestWorker (file:/Users/riysaxen/.gradle/wrapper/dists/gradle-8.5-bin/5t9huq95ubn472n8rpzujfbqh/gradle-8.5/lib/plugins/gradle-testing-base-8.5.jar)
WARNING: Please consider reporting this to the maintainers of org.gradle.api.internal.tasks.testing.worker.TestWorker
WARNING: System::setSecurityManager will be removed in a future release
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.gradle.api.internal.tasks.testing.worker.TestWorker (file:/Users/riysaxen/.gradle/wrapper/dists/gradle-8.5-bin/5t9huq95ubn472n8rpzujfbqh/gradle-8.5/lib/plugins/gradle-testing-base-8.5.jar)
WARNING: Please consider reporting this to the maintainers of org.gradle.api.internal.tasks.testing.worker.TestWorker
WARNING: System::setSecurityManager will be removed in a future release
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.gradle.api.internal.tasks.testing.worker.TestWorker (file:/Users/riysaxen/.gradle/wrapper/dists/gradle-8.5-bin/5t9huq95ubn472n8rpzujfbqh/gradle-8.5/lib/plugins/gradle-testing-base-8.5.jar)
WARNING: Please consider reporting this to the maintainers of org.gradle.api.internal.tasks.testing.worker.TestWorker
WARNING: System::setSecurityManager will be removed in a future release
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.gradle.api.internal.tasks.testing.worker.TestWorker (file:/Users/riysaxen/.gradle/wrapper/dists/gradle-8.5-bin/5t9huq95ubn472n8rpzujfbqh/gradle-8.5/lib/plugins/gradle-testing-base-8.5.jar)
WARNING: Please consider reporting this to the maintainers of org.gradle.api.internal.tasks.testing.worker.TestWorker
WARNING: System::setSecurityManager will be removed in a future release
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.gradle.api.internal.tasks.testing.worker.TestWorker (file:/Users/riysaxen/.gradle/wrapper/dists/gradle-8.5-bin/5t9huq95ubn472n8rpzujfbqh/gradle-8.5/lib/plugins/gradle-testing-base-8.5.jar)
WARNING: Please consider reporting this to the maintainers of org.gradle.api.internal.tasks.testing.worker.TestWorker
WARNING: System::setSecurityManager will be removed in a future release

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.5/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

BUILD SUCCESSFUL in 38s

jowg-amazon · 2024-06-11T21:09:27Z

Non blocking but can we add TODO tasks for all the parts that need to be refactored/changed after this PR?

riysaxen-amzn · 2024-06-11T21:20:07Z

Non blocking but can we add TODO tasks for all the parts that need to be refactored/changed after this PR?

yes, there'll be another PR after this for the APIs will update there

* notification for alerting in correlation * correlation alerts mapping change * working code Signed-off-by: Riya Saxena <[email protected]> * alertsInCorrelation without notifciations Signed-off-by: Riya Saxena <[email protected]> * alertsInCorrelation without notifciations Signed-off-by: Riya Saxena <[email protected]> * alertsInCorrelation without notifciations Signed-off-by: Riya Saxena <[email protected]> * alerts in correlations notification service added Signed-off-by: Riya Saxena <[email protected]> * addressing the comments Signed-off-by: Riya Saxena <[email protected]> * addressing the comments Signed-off-by: Riya Saxena <[email protected]> * address the design changes discussed Signed-off-by: Riya Saxena <[email protected]> * address the design changes discussed Signed-off-by: Riya Saxena <[email protected]> * fixed tests Signed-off-by: Riya Saxena <[email protected]> --------- Signed-off-by: Riya <[email protected]> Signed-off-by: Riya Saxena <[email protected]> (cherry picked from commit 62e4453) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

opensearch-trigger-bot · 2024-06-12T00:36:20Z

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:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/security-analytics/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/security-analytics/backport-2.x
# Create a new branch
git switch --create backport-1040-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 62e4453dde8977a43c16c7d7b27ffc4413ec00df
# Push it to GitHub
git push --set-upstream origin backport-1040-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/security-analytics/backport-2.x

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

* notification for alerting in correlation * correlation alerts mapping change * working code Signed-off-by: Riya Saxena <[email protected]> * alertsInCorrelation without notifciations Signed-off-by: Riya Saxena <[email protected]> * alertsInCorrelation without notifciations Signed-off-by: Riya Saxena <[email protected]> * alertsInCorrelation without notifciations Signed-off-by: Riya Saxena <[email protected]> * alerts in correlations notification service added Signed-off-by: Riya Saxena <[email protected]> * addressing the comments Signed-off-by: Riya Saxena <[email protected]> * addressing the comments Signed-off-by: Riya Saxena <[email protected]> * address the design changes discussed Signed-off-by: Riya Saxena <[email protected]> * address the design changes discussed Signed-off-by: Riya Saxena <[email protected]> * fixed tests Signed-off-by: Riya Saxena <[email protected]> --------- Signed-off-by: Riya <[email protected]> Signed-off-by: Riya Saxena <[email protected]> (cherry picked from commit 62e4453)

eirsep · 2024-06-25T08:28:21Z

...va/org/opensearch/securityanalytics/correlation/alert/notifications/NotificationService.java

+    /**
+     * Extension function for publishing a notification to a channel in the Notification plugin.
+     */
+    public void sendNotification(String configId, String severity, String subject, String notificationMessageText) throws IOException {


this call should be event driven. write now we made it async.. cant handle failure

* notification for alerting in correlation * correlation alerts mapping change * working code Signed-off-by: Riya Saxena <[email protected]> * alertsInCorrelation without notifciations Signed-off-by: Riya Saxena <[email protected]> * alertsInCorrelation without notifciations Signed-off-by: Riya Saxena <[email protected]> * alertsInCorrelation without notifciations Signed-off-by: Riya Saxena <[email protected]> * alerts in correlations notification service added Signed-off-by: Riya Saxena <[email protected]> * addressing the comments Signed-off-by: Riya Saxena <[email protected]> * addressing the comments Signed-off-by: Riya Saxena <[email protected]> * address the design changes discussed Signed-off-by: Riya Saxena <[email protected]> * address the design changes discussed Signed-off-by: Riya Saxena <[email protected]> * fixed tests Signed-off-by: Riya Saxena <[email protected]> --------- Signed-off-by: Riya <[email protected]> Signed-off-by: Riya Saxena <[email protected]>

riysaxen-amzn and others added 8 commits March 5, 2024 10:08

notification for alerting in correlation

da73e7e

correlation alerts mapping change

439e0c0

Merge branch 'main' into alertsInCorrelations

e2530f5

Signed-off-by: Riya <[email protected]>

Merge branch 'main' into alertsInCorrelations

a890e73

working code

425831d

Signed-off-by: Riya Saxena <[email protected]>

alertsInCorrelation without notifciations

7e0b257

Signed-off-by: Riya Saxena <[email protected]>

alertsInCorrelation without notifciations

c244a99

Signed-off-by: Riya Saxena <[email protected]>

alertsInCorrelation without notifciations

8603d5a

Signed-off-by: Riya Saxena <[email protected]>

riysaxen-amzn marked this pull request as ready for review May 31, 2024 01:41

riysaxen-amzn requested review from amsiglan, AWSHurneyt, getsaurabh02, lezzago, praveensameneni, sbcd90, eirsep, jowg-amazon, engechas and goyamegh as code owners May 31, 2024 01:41

riysaxen-amzn changed the title ~~Alerts in correlations~~ Alerts in correlations [Experminental] May 31, 2024

alerts in correlations notification service added

29ba163

Signed-off-by: Riya Saxena <[email protected]>

eirsep reviewed Jun 3, 2024

View reviewed changes

riysaxen-amzn added 2 commits June 5, 2024 17:12

addressing the comments

2a4cf26

Signed-off-by: Riya Saxena <[email protected]>

addressing the comments

e23e969

Signed-off-by: Riya Saxena <[email protected]>

riysaxen-amzn requested a review from eirsep June 6, 2024 00:20

eirsep reviewed Jun 6, 2024

View reviewed changes

sbcd90 reviewed Jun 6, 2024

View reviewed changes

riysaxen-amzn added 2 commits June 10, 2024 14:10

address the design changes discussed

1547393

Signed-off-by: Riya Saxena <[email protected]>

address the design changes discussed

abade3f

Signed-off-by: Riya Saxena <[email protected]>

riysaxen-amzn requested review from sbcd90 and eirsep June 10, 2024 21:59

jowg-amazon reviewed Jun 11, 2024

View reviewed changes

fixed tests

5ca1899

Signed-off-by: Riya Saxena <[email protected]>

sbcd90 approved these changes Jun 11, 2024

View reviewed changes

riysaxen-amzn requested a review from jowg-amazon June 11, 2024 21:03

jowg-amazon approved these changes Jun 11, 2024

View reviewed changes

riysaxen-amzn merged commit 62e4453 into opensearch-project:main Jun 11, 2024
3 of 11 checks passed

riysaxen-amzn added the backport 2.x label Jun 11, 2024

opensearch-trigger-bot bot mentioned this pull request Jun 11, 2024

[Backport 2.x] Alerts in correlations [Experminental] #1067

Closed

riysaxen-amzn added backport 2.x and removed backport 2.x labels Jun 12, 2024

opensearch-trigger-bot bot added the failed backport label Jun 12, 2024

eirsep reviewed Jun 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alerts in correlations [Experminental] #1040

Alerts in correlations [Experminental] #1040

riysaxen-amzn commented May 22, 2024 •

edited

Loading

eirsep commented Jun 3, 2024

eirsep Jun 3, 2024

riysaxen-amzn Jun 6, 2024

eirsep Jun 3, 2024

riysaxen-amzn Jun 6, 2024

riysaxen-amzn commented Jun 6, 2024

eirsep Jun 6, 2024

riysaxen-amzn Jun 10, 2024

sbcd90 Jun 6, 2024

riysaxen-amzn Jun 10, 2024

sbcd90 Jun 6, 2024

riysaxen-amzn Jun 10, 2024

jowg-amazon Jun 11, 2024

jowg-amazon Jun 11, 2024

riysaxen-amzn Jun 11, 2024

jowg-amazon Jun 11, 2024

riysaxen-amzn Jun 11, 2024

jowg-amazon Jun 11, 2024

riysaxen-amzn Jun 11, 2024

riysaxen-amzn commented Jun 11, 2024 •

edited

Loading

jowg-amazon commented Jun 11, 2024

riysaxen-amzn commented Jun 11, 2024

opensearch-trigger-bot bot commented Jun 12, 2024

eirsep Jun 25, 2024

		@@ -53,6 +54,8 @@
		import org.opensearch.script.ScriptService;
		import org.opensearch.securityanalytics.action.*;

Alerts in correlations [Experminental] #1040

Alerts in correlations [Experminental] #1040

Conversation

riysaxen-amzn commented May 22, 2024 • edited Loading

Description

Issues Resolved

Check List

eirsep commented Jun 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

riysaxen-amzn commented Jun 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

riysaxen-amzn commented Jun 11, 2024 • edited Loading

jowg-amazon commented Jun 11, 2024

riysaxen-amzn commented Jun 11, 2024

opensearch-trigger-bot bot commented Jun 12, 2024

Choose a reason for hiding this comment

riysaxen-amzn commented May 22, 2024 •

edited

Loading

riysaxen-amzn commented Jun 11, 2024 •

edited

Loading