-
Notifications
You must be signed in to change notification settings - Fork 515
[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
Conversation
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
| - 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/data_stream/audit/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
packages/o365/data_stream/audit/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
packages/o365/data_stream/audit/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
packages/o365/data_stream/audit/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
packages/o365/data_stream/audit/elasticsearch/ingest_pipeline/default.yml
Show resolved
Hide resolved
packages/o365/data_stream/audit/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
kcreddy
left a comment
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)
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 everything is null, I would change the message to "Office365 Alert", without giving additional details as the rest is unknown, but we still alert the user of the existence of a suspicious behavior in their O365 environment.
🚀 Benchmarks reportTo see the full report comment with |
| 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
|
|
kcreddy
left a comment
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.
LGTM for my comments.
raqueltabuyo
left a comment
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.
LGTM
|
Package o365 - 2.19.0 containing this change is available at https://epr.elastic.co/package/o365/2.19.0/ |
…change` events (elastic#14587) This PR updates the mapping of message ECS field for the ComplianceDLPExchange events from mixture of o365 fields.




Proposed commit message
Checklist
changelog.ymlfile.How to test this PR locally
Related issues
messageField with Alert Titles for DLP Exchange Alerts #12598