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

Update "TI Map" analytics rules for performance #9181

Closed
wants to merge 23 commits into from

Conversation

kfriede
Copy link
Contributor

@kfriede kfriede commented Oct 9, 2023

Required items, please complete

Change(s):

  • Changed query to put any applicable WHERE filters before any SUMMARIZE calls for performance.
  • Added ThreatIntelligenceTaxii data connector on missing analytics.

Reason for Change(s):

  • Many of these queries would fail on real-world datasets due to summarization of unnecessary data.
  • ThreatIntelligenceTaxii is a valid provider of ThreatIntelligenceIndicator table data and should be included

Version Updated:

  • Yes

Testing Completed:

  • Yes, as much as possible (don't have all data sources to test appropriately for some rules)

Checked that the validations are passing and have addressed any issues that are present:

  • Yes

@kfriede kfriede requested review from a team as code owners October 9, 2023 16:47
@kfriede
Copy link
Contributor Author

kfriede commented Oct 9, 2023

To whoever ends up picking this up, the KQL validation failures is due to Line 397 of https://github.com/Azure/Azure-Sentinel/blob/master/.script/tests/KqlvalidationsTests/KqlValidationTests.cs

The regex being used is overly-specific and not conducive for the performance improvements being made here.

@v-rbajaj
Copy link
Contributor

Hi @kfriede, thanks for raising this PR. We will review this PR by 13 Oct 2023.

@kfriede
Copy link
Contributor Author

kfriede commented Oct 10, 2023

This traces back to commit 16564be from @aprakash13. Query layouts looked largely performance-optimized before this change.

@v-rbajaj
Copy link
Contributor

v-rbajaj commented Oct 13, 2023

Hi @kfriede, for KQL validation, we will see what can be done in this case.

We would need you to repackage this solution using this tool https://github.com/Azure/Azure-Sentinel/tree/master/Tools/Create-Azure-Sentinel-Solution/V3

Also update the release notes for the same.

Thanks

@v-rbajaj
Copy link
Contributor

Hi @kfriede, please provide update on above comment.

@v-rbajaj
Copy link
Contributor

Hi @kfriede, please provide some update on this PR

@kfriede
Copy link
Contributor Author

kfriede commented Oct 23, 2023

Hi @v-rbajaj, apologies I was out last week. I will look at repackaging the solution for these edits

Edit: Solution has been repackaged for 3.0.2

@v-rbajaj
Copy link
Contributor

Hi @v-rbajaj, apologies I was out last week. I will look at repackaging the solution for these edits

Edit: Solution has been repackaged for 3.0.2

Thanks @kfriede, will review the PR by 27 Oct, 2023.

@@ -6,7 +6,7 @@
"config": {
"isWizard": false,
"basics": {
"description": "<img src=\"https://raw.githubusercontent.com/Azure/Azure-Sentinel/master/Logos/Azure_Sentinel.svg\"width=\"75px\" height=\"75px\">\n\n**Note:** Please refer to the following before installing the solution: \r \n • Review the solution [Release Notes](https://github.com/Azure/Azure-Sentinel/tree/master/Solutions/Threat Intelligence/ReleaseNotes.md)\r \n • There may be [known issues](https://aka.ms/sentinelsolutionsknownissues) pertaining to this Solution, please refer to them before installing.\n\nThe Threat Intelligence solution contains data connectors for import of threat indicators into Microsoft Sentinel, analytic rules for matching TI data with event data, workbook, and hunting queries. Threat indicators can be malicious IP's, URL's, filehashes, domains, email addresses etc.\n\n**Data Connectors:** 4, **Workbooks:** 1, **Analytic Rules:** 38, **Hunting Queries:** 5\n\n[Learn more about Microsoft Sentinel](https://aka.ms/azuresentinel) | [Learn more about Solutions](https://aka.ms/azuresentinelsolutionsdoc)",
"description": "<img src=\"https://raw.githubusercontent.com/Azure/Azure-Sentinel/master/Logos/Azure_Sentinel.svg\"width=\"75px\" height=\"75px\">\n\n**Note:** _There may be [known issues](https://aka.ms/sentinelsolutionsknownissues) pertaining to this Solution, please refer to them before installing._\n\nThe Threat Intelligence solution contains data connectors for import of threat indicators into Microsoft Sentinel, analytic rules for matching TI data with event data, workbook, and hunting queries. Threat indicators can be malicious IP's, URL's, filehashes, domains, email addresses etc.\n\n**Workbooks:** 1, **Analytic Rules:** 38, **Hunting Queries:** 5\n\n[Learn more about Microsoft Sentinel](https://aka.ms/azuresentinel) | [Learn more about Solutions](https://aka.ms/azuresentinelsolutionsdoc)",
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @kfriede, Please revert this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@v-rbajaj done!

@kfriede kfriede force-pushed the update-ti-analytics branch from 1dbaca4 to 4e2bde4 Compare October 27, 2023 12:55
@v-rbajaj
Copy link
Contributor

Hi @kfriede, thanks. We are currently looking into the KQL validations and once that is fixed, we will revisit this PR by 2 Nov 2023.

@v-rbajaj
Copy link
Contributor

v-rbajaj commented Nov 2, 2023

Hi @kfriede, we are still investigating on KQL validations, will get back to by 06 Nov 2023.

@kfriede
Copy link
Contributor Author

kfriede commented Nov 13, 2023

Hi @v-rbajaj, it's been over 1 week since your last update. Is this still under review?

@v-atulyadav
Copy link
Contributor

Hi @kfriede,
Apologies for the inconvenience, but due to unavailability, the review has been delayed. We assure you that you will receive an update on this by the end of the day tomorrow. Thank you.

@v-rbajaj
Copy link
Contributor

Hi @kfriede, we are working on the KQL changes and we will soon provide an update.

@v-rbajaj v-rbajaj mentioned this pull request Nov 16, 2023
@kfriede kfriede force-pushed the update-ti-analytics branch from 4e2bde4 to f07b36b Compare November 22, 2023 14:24
@kfriede
Copy link
Contributor Author

kfriede commented Nov 22, 2023

@v-rbajaj after refactoring, files now give: Kqlvalidations.Tests.KqlValidationTests.Validate_DetectionQueries_SkippedTemplatesDoNotHaveValidKql(fileName: "IPEntity_DuoSecurity.yaml", encodedFilePath: "L2hvbWUvdnN0cy93b3JrLzEvcy9Tb2x1dGlvbnMvVGhyZWF0IE"...) in testing

@v-rbajaj
Copy link
Contributor

Hi @kfriede, sorry for the above commits, was trying to fix few errors from KQL validations. We are checking on it, will get back to you by 29 Nov 2023.

@v-rbajaj
Copy link
Contributor

Hi @kfriede, can we merge #9369 as the changes there look similar and all the KQL validations have been fixed by @blauwers.

If there are any changes you want to suggest on #9369 or want to contribute your changes, can you please comment on that PR?

@kfriede
Copy link
Contributor Author

kfriede commented Nov 29, 2023 via email

@v-rbajaj v-rbajaj marked this pull request as draft November 30, 2023 06:51
@v-rbajaj
Copy link
Contributor

Thanks @kfriede, I have changed the PR status from open to draft and will close later once #9369 is merged.

@v-rbajaj
Copy link
Contributor

Closing this PR as #9369 is merged.

@v-rbajaj v-rbajaj closed this Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Analytic Rules Solution Solution specialty review needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants