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 mitre attack based auto-correlations support in correlation engine #532

Merged
merged 2 commits into from
Sep 6, 2023

Conversation

sbcd90
Copy link
Collaborator

@sbcd90 sbcd90 commented Sep 5, 2023

Description

Findings in security-analytics link sigma rules to logs. Each sigma rule is linked to one or more mitre tactics & techniques. this info along with mitre cti relationships are used to generate auto-correlations among findings.
this pr add mitre attack based auto-correlations support in correlation engine. https://github.com/mitre/cti

image

Issues Resolved

[List any issues this PR will resolve]

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: Subhobrata Dey <sbcd90@gmail.com>
@eirsep
Copy link
Member

eirsep commented Sep 6, 2023

plz add more elaborate description

@@ -0,0 +1,9625 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

how can we verify this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>
@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Merging #532 (9cd032b) into main (4d63f38) will decrease coverage by 0.26%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               main     #532      +/-   ##
============================================
- Coverage     25.72%   25.47%   -0.26%     
- Complexity      938      942       +4     
============================================
  Files           252      253       +1     
  Lines         10768    10895     +127     
  Branches       1197     1223      +26     
============================================
+ Hits           2770     2775       +5     
- Misses         7749     7873     +124     
+ Partials        249      247       -2     
Files Changed Coverage Δ
...arch/securityanalytics/correlation/JoinEngine.java 0.00% <0.00%> (ø)
...ics/transport/TransportCorrelateFindingAction.java 0.00% <0.00%> (ø)
...h/securityanalytics/util/AutoCorrelationsRepo.java 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

try {
generateAutoCorrelations(detector, finding);
} catch (IOException ex) {
correlateFindingAction.onFailures(ex);
Copy link
Member

Choose a reason for hiding this comment

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

can we log error message that auto correlation has failed

int idx = 0;
for (MultiSearchResponse.Item response : responses) {
if (response.isFailure()) {
log.info(response.getFailureMessage());
Copy link
Member

Choose a reason for hiding this comment

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

why info log? error or debug would be better. along with description message of what this failure is from?

Copy link
Member

@eirsep eirsep left a comment

Choose a reason for hiding this comment

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

I am approving this
but can we take a follow up action item to add code comments.
This will make the repo and this feature more maintainable and understandable to community

@sbcd90
Copy link
Collaborator Author

sbcd90 commented Sep 6, 2023

updated to a more verbose description. added review comments to issue #502

@sbcd90 sbcd90 merged commit 32d5aa1 into opensearch-project:main Sep 6, 2023
12 of 16 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 6, 2023
#532)

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>
(cherry picked from commit 32d5aa1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants