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 DomainEntity_EmailUrlInfo.yaml #11369

Closed
wants to merge 17 commits into from
Closed

Conversation

MSJosh
Copy link
Contributor

@MSJosh MSJosh commented Oct 31, 2024

Removed materialize as it causes a memory issue on larger data sets. Improved performance of query by splitting the logic.

Required items, please complete

Change(s):
-Removed materialize() functions
-Split single query into two different queries to improve performance
-Cleaned up sub queries

Reason for Change(s):

  • For larger data sets this would lead to memory issues and would fail to run or lead to shorter look back periods.
  • Resolves Issue 11340

Version Updated:

  • 1.0.2

Testing Completed:

  • Yes

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

  • Yes

Removed materialize as it causes a memory issue on larger data sets.  Improved performance of query by splitting the logic.
@MSJosh MSJosh requested review from a team as code owners October 31, 2024 16:20
@MSJosh
Copy link
Contributor Author

MSJosh commented Oct 31, 2024 via email

@v-prasadboke v-prasadboke self-assigned this Nov 4, 2024
@v-prasadboke v-prasadboke added the Solution Solution specialty review needed label Nov 4, 2024
@v-prasadboke
Copy link
Contributor

Hi Josh,

I saw your mail as you said it worked on alpine ski house
But can you check and fix this if possible

@v-prasadboke
Copy link
Contributor

Hello @MSJosh, Just discussed about this internally.

We need to use the template which is mentioned in the validation check

**Errors: Content needs to use the latest Threat Intelligence data. Sample queries to get the latest Threat Intelligence data:
ThreatIntelligenceIndicator
| where TimeGenerated >= ago(ioc_lookBack)
| summarize LatestIndicatorTime = arg_max(TimeGenerated, *) by IndicatorId
| where Active == true and ExpirationDateTime > now()

    or

    ThreatIntelligenceIndicator
    | where TimeGenerated >= ago(ioc_lookBack)
    | summarize LatestIndicatorTime = arg_max(TimeGenerated, *) by IndicatorId
    | where ExpirationDateTime > now() and Active == true**

Reason being, above query fetches the latest TI data or logs.
Neglecting this would fetch the expired data or false data.

@MSJosh
Copy link
Contributor Author

MSJosh commented Nov 15, 2024 via email

@v-prasadboke
Copy link
Contributor

Not understanding what you need me to do. Are you wanting to modify the code to put the | summarize LatestIndicatorTime = arg_max(TimeGenerated, ) by IndicatorId | where Active == true and ExpirationDateTime > now() In the portions where I have wrote for TI??

________________________________ From: v-prasadboke @.
> Sent: Friday, November 15, 2024 5:58 AM To: Azure/Azure-Sentinel @.> Cc: Josh Kolka @.>; Mention @.*> Subject: Re: [Azure/Azure-Sentinel] Update DomainEntity_EmailUrlInfo.yaml (PR #11369) Hello @MSJoshhttps://github.com/MSJosh, Just discussed about this internally. We need to use the template which is mentioned in the validation check **Errors: Content needs to use the latest Threat Intelligence data. Sample queries to get the latest Threat Intelligence data: ThreatIntelligenceIndicator | where TimeGenerated >= ago(ioc_lookBack) | summarize LatestIndicatorTime = arg_max(TimeGenerated, ) by IndicatorId | where Active == true and ExpirationDateTime > now() or ThreatIntelligenceIndicator | where TimeGenerated >= ago(ioc_lookBack) | summarize LatestIndicatorTime = arg_max(TimeGenerated, ) by IndicatorId | where ExpirationDateTime > now() and Active == true Reason being, above query fetches the latest TI data or logs. Neglecting this would fetch the expired data or false data. — Reply to this email directly, view it on GitHub<#11369 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A4XLFSM4ZQYTD4KK3BO43EL2AXHW5AVCNFSM6AAAAABQ6UYJ6OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINZYGU3DAMRXG4. You are receiving this because you were mentioned.

Yes

@MSJosh
Copy link
Contributor Author

MSJosh commented Nov 21, 2024

@v-prasadboke Looks like I have everything fixed minus one item. I did a new pull to fix the one issue found. I fixed the issue in pull request 11468, where I added Workday as a source as the team didn't have it added as part of the new CCP solution for Workday.

Copy link
Contributor Author

@MSJosh MSJosh left a comment

Choose a reason for hiding this comment

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

Please run KQL validation against this. Looks like it was an issue with the verifier.

Copy link
Contributor Author

@MSJosh MSJosh left a comment

Choose a reason for hiding this comment

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

Running verifier again

@v-prasadboke
Copy link
Contributor

Hello @MSJosh, Can you package this solution once
I tried to pull commits to my local but I'm facing some issue

Please package the solution using V3 tool once.
https://github.com/Azure/Azure-Sentinel/blob/master/Tools/Create-Azure-Sentinel-Solution/V3/README.md

@MSJosh
Copy link
Contributor Author

MSJosh commented Dec 3, 2024

@v-prasadboke please take a look and validate. There should be a new build 3.09 available in this latest build. The arm-ttk check is flagging but has nothing to do with the changes made for these detections and looks like they are historical configs that have existed for some time.

@MSJosh MSJosh closed this Dec 9, 2024
@MSJosh
Copy link
Contributor Author

MSJosh commented Dec 9, 2024

Other commit went live and created a fresh pull for workday to keep it clean.

@v-prasadboke
Copy link
Contributor

Thanks for the update Josh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Solution Solution specialty review needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants