-
Notifications
You must be signed in to change notification settings - Fork 473
[O365] Update the mapping of ECS message
field for ComplianceDLPExchange
events
#14587
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
base: main
Are you sure you want to change the base?
Conversation
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
packages/o365/changelog.yml
Outdated
- version: "2.18.6" | ||
- version: "2.19.0" | ||
changes: | ||
- description: Stricter enforcement of maximum age limits. | ||
type: bugfix | ||
link: https://github.com/elastic/integrations/pull/14567 | ||
- description: Populate `message` field from the O365 Audit Log fields instead of `Subject` field in ComplianceDLPExchange events to better reflect Alert Titles. | ||
type: enhancement | ||
link: https://github.com/elastic/integrations/pull/14587 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we replacing an existing changelog ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, this is happened because #14567 was merged after i raised this PR. Need to take pull from main
.
packages/o365/changelog.yml
Outdated
- version: "2.18.6" | ||
- version: "2.19.0" | ||
changes: | ||
- description: Stricter enforcement of maximum age limits. | ||
type: bugfix | ||
link: https://github.com/elastic/integrations/pull/14567 | ||
- description: Populate `message` field from the O365 Audit Log fields instead of `Subject` field in ComplianceDLPExchange events to better reflect Alert Titles. | ||
type: enhancement | ||
link: https://github.com/elastic/integrations/pull/14587 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add new changelog entry instead of updating existing one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to take pull from main as #14567 was merged few days back.
- script: | ||
lang: painless | ||
description: Construct a message from operation, user and subject fields for 'ComplianceDLPExchange' events | ||
if: 'ctx.event?.code != null && ctx.event?.code == "ComplianceDLPExchange"' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if: 'ctx.event?.code != null && ctx.event?.code == "ComplianceDLPExchange"' | |
if: ctx.event?.code == "ComplianceDLPExchange" |
description: Construct a message from operation, user and subject fields for 'ComplianceDLPExchange' events | ||
if: 'ctx.event?.code != null && ctx.event?.code == "ComplianceDLPExchange"' | ||
source: > | ||
def operation = ctx.event?.action != null ? ctx.event.action : ctx.o365audit?.Operation; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much earlier in the pipeline we have:
integrations/packages/o365/data_stream/audit/elasticsearch/ingest_pipeline/default.yml
Lines 163 to 166 in 648e999
- rename: | |
field: o365audit.Operation | |
target_field: event.action | |
ignore_missing: true |
I think L#738 this is same as: def operation = ctx.event?.action
if: 'ctx.event?.code != null && ctx.event?.code == "ComplianceDLPExchange"' | ||
source: > | ||
def operation = ctx.event?.action != null ? ctx.event.action : ctx.o365audit?.Operation; | ||
def user = ctx.user?.id != null ? ctx.user.id : ctx.o365audit?.UserId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, based on
integrations/packages/o365/data_stream/audit/elasticsearch/ingest_pipeline/default.yml
Lines 154 to 158 in 648e999
- convert: | |
field: o365audit.UserId | |
target_field: user.id | |
type: string | |
ignore_missing: true |
def operation = ctx.event?.action != null ? ctx.event.action : ctx.o365audit?.Operation; | ||
def user = ctx.user?.id != null ? ctx.user.id : ctx.o365audit?.UserId; | ||
def subject = ctx.o365audit?.ExchangeMetaData?.Subject != null ? ctx.o365audit.ExchangeMetaData.Subject : (ctx.email?.subject != null ? ctx.email.subject : ''); | ||
ctx.message = "Office365 Alert: " + operation + " detected in email sent by " + user + " with subject '" + subject + "'"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to be this only handles part of the suggested fix: #12598 (comment) ?
I don't see any message of the format DLP Alert: {Operation} detected in email
or Office 365 alert: {Operation} detected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raqueltabuyo can you please update the issue as we discussed?
@kcreddy i had a discussion with her and confirmed the format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see this clarified in the issue. Can this be done?
@@ -730,6 +730,22 @@ processors: | |||
ignore_missing: true | |||
if: 'ctx.event?.code == "SecurityComplianceAlerts" && ctx.rule?.ruleset == "MalwareFamily"' | |||
# DLP Schema | |||
- script: | |||
lang: painless |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add tag
- set: | ||
field: message | ||
value: 'DLP Alert: Unable to construct message due to script failure.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the processor reaches a failure as all conditions are handled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, yes then there's no meaning of the on_failure
block, @kcreddy can we keep it for just unexpected script failures?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update README as per CI error:
| o365.audit.ExchangeMetaData.Sent | | date |
+| o365.audit.ExchangeMetaData.Subject | | ketword |
| o365.audit.ExchangeMetaData.To | | keyword |
Error: checking package failed: checking readme files are up-to-date failed: files do not match
[o365] run_tests_package failed
def operation = ctx.event?.action != null ? ctx.event.action : ctx.o365audit?.Operation; | ||
def user = ctx.user?.id != null ? ctx.user.id : ctx.o365audit?.UserId; | ||
def subject = ctx.o365audit?.ExchangeMetaData?.Subject != null ? ctx.o365audit.ExchangeMetaData.Subject : (ctx.email?.subject != null ? ctx.email.subject : ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One query here,
In scenarios when ctx.event.action == null, ctx.user.id == null and ctx.o365audit.ExchangeMetaData?.Subject == null
are we guaranteed to have the alternatives populated ? If not, we might end up with an incomplete message with the current logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kcreddy, what do you think, should we have a fallback message just in-case ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be worth to have <unknown>
. But I will leave it to @raqueltabuyo to confirm.
Also waiting for #14587 (comment)
🚀 Benchmarks reportPackage
|
Data stream | Previous EPS | New EPS | Diff (%) | Result |
---|---|---|---|---|
audit |
1712.33 | 1190.48 | -521.85 (-30.48%) | 💔 |
To see the full report comment with /test benchmark fullreport
source: > | ||
def operation = ctx.event?.action; | ||
def user = ctx.user?.id; | ||
def subject = ctx.o365audit?.ExchangeMetaData?.Subject != null ? ctx.o365audit.ExchangeMetaData.Subject : (ctx.email?.subject != null ? ctx.email.subject : ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This kind of density is why ternaries are problematic.
If this must be done, suggest instead the slightly less unpleasant:
def subject = ctx.o365audit?.ExchangeMetaData?.Subject ?: (ctx.email?.subject != null ? ctx.email.subject : '');
|
💚 Build Succeeded
History
|
Proposed commit message
Checklist
changelog.yml
file.How to test this PR locally
Related issues
message
Field with Alert Titles for DLP Exchange Alerts #12598