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

🧹Cleanup and rename analytic keys #11124

Closed
wants to merge 14 commits into from

Conversation

pemontto
Copy link
Contributor

Required items, please complete

Change(s):

  • Cleanup and rename incorrectly named analytic keys

Reason for Change(s):

  • Data is invalid and can breaks rule pipelines. It looks like some of these fields came from the API or deployed rules instead of templates.

Version Updated: ✅

Testing Completed: ✅

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

  • See guidance below

@pemontto pemontto requested review from a team as code owners September 13, 2024 09:55
@pemontto pemontto requested a review from a team as a code owner September 13, 2024 09:58
@pemontto
Copy link
Contributor Author

There are also additional (and redundant) keys in Solutions/Microsoft Entra ID/Analytic Rules/nrt_FirstAppOrServicePrincipalCredential.yaml, threatAnalysisTactics and threatAnalysisTechniques. Not sure if there are future plans for these fields specifically?

tactics:
  - DefenseEvasion
relevantTechniques:
  - T1550.001
[..]
threatAnalysisTactics: [ "DefenseEvasion" ]
threatAnalysisTechniques: [ "T1550.001" ]

@v-atulyadav v-atulyadav added Solution Solution specialty review needed Analytic Rules labels Sep 13, 2024
@v-prasadboke
Copy link
Contributor

Hello @pemontto, Can you try to resolve falling validations

@pemontto
Copy link
Contributor Author

Accidentally forced pushed without commit. Re-opening with fixed tactics

@pemontto pemontto reopened this Sep 23, 2024
@pemontto pemontto force-pushed the fix-analytic-keys branch 2 times, most recently from 6035f6f to c3366ce Compare September 23, 2024 18:01
@pemontto
Copy link
Contributor Author

pemontto commented Sep 23, 2024

@v-prasadboke, looking good now. Haven't dug into how the validator works, but it still wasn't happy with T1659 being considered InitialAccess. For now I've left T1659 in there commented out for future reference. Regardless, we're in better stead that we were before when they were under the wrong key 👍

@pemontto
Copy link
Contributor Author

pemontto commented Oct 4, 2024

@v-prasadboke anything else need here?

@pemontto
Copy link
Contributor Author

@v-prasadboke any feedback, or is this OK to be merged?

@v-prasadboke
Copy link
Contributor

Hello @pemontto, Please repackage the Solutions.

Please go through this documentation to package the solutions
https://github.com/Azure/Azure-Sentinel/blob/master/Tools/Create-Azure-Sentinel-Solution/V3/README.md

@pemontto
Copy link
Contributor Author

pemontto commented Nov 5, 2024

@v-prasadboke repackaged these with no other changes.

@pemontto
Copy link
Contributor Author

@v-prasadboke do you want/need me to repackage this again, or do you want to merge master into this branch?

@pemontto
Copy link
Contributor Author

@v-prasadboke I can combine this and #11199 into a single MR if you'd prefer?

@@ -39,13 +39,12 @@ entityMappings:
fieldMappings:
- identifier: HostName
columnName: hosts_s
sentinelEntitiesMappings: null
Copy link
Contributor

Choose a reason for hiding this comment

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

irrelevant deletion
packaging tool automatically removed this field

@@ -4,7 +4,7 @@ description: 'Digital Shadows Analytic rule for generating Microsoft Sentinel in
severity: Medium
requiredDataConnectors:
- connectorId: DigitalShadows
dataTypes:
Copy link
Contributor

Choose a reason for hiding this comment

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

irrelevant removal of whitespace
packaging tool automatically removes this blankspace

@@ -6,7 +6,6 @@ requiredDataConnectors:
- connectorId: MimecastSIEMAPI
dataTypes:
- MimecastSIEM_CL
enabled: true
Copy link
Contributor

Choose a reason for hiding this comment

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

irrelevant deletion
tool automatically removes this

@v-prasadboke
Copy link
Contributor

v-prasadboke commented Dec 19, 2024

Also we cannot remove this properties from the rules as it may lead to incorrect format in future

As for Solution with typo correction for use requiretechniques instead of relevanttechniques I'll raise a new PR for the same

until then you can close all of your PR's 😊😊
I have added a proper comment to those

Thanks and sorry for the delay in response

@v-prasadboke
Copy link
Contributor

v-prasadboke commented Dec 30, 2024

We wanted to check on the status of PR #11124. PR is pending for more than 10+ days. Please let us know if you need any assistance to review this PR. Per our standard operating procedures if no response is received in the next 7 business days, we will close this PR. Thank you for your cooperation.

@v-prasadboke
Copy link
Contributor

Since we have not received a response in the last 7 days, we are closing your PR #11124 per our standard operating procedures. If you still need support for this issue, you can re-open the PR at any time.

If you do re-open, we simply request that you ensure the PR has response to the last request. Thank you for your cooperation.

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.

5 participants