-
Notifications
You must be signed in to change notification settings - Fork 467
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
[o365] Mapping, parsing of o365.audit fields Platform and Data.* #8571
[o365] Mapping, parsing of o365.audit fields Platform and Data.* #8571
Conversation
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
🌐 Coverage report
|
multi_fields: | ||
- name: sip | ||
type: ip | ||
- name: at | ||
type: date |
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.
Is this kind of multi-field approach valid?
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.
Actually, no. I thought I'd found a good way to leave the full data where it was and expose individual fields with the least surprising names, but looking in Kibana I'm only seeing the one flattened field so I think that's not the way to go.
Now I'm planning to do it like this:
o365.audit.Data
typegroup
(of parsed fields of typesip
,date
, andkeyword
)o365.audit.DataRest
typeflattened
(fields not broken out aso365.audit.Data.*
)
And I guess now it should definitely get a major version bump, because it's not just a type change and field additions, it's moving data that was under o365.audit.Data
to elsewhere.
What do you think?
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.
Done in a new commit.
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.
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.
Okay, thanks, that looks good. Done here in a new commit.
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 but please get opinion from @P1llus.
7a03763
to
7cdaeca
Compare
🚀 Benchmarks reportTo see the full report comment with |
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.
Added some minor comments or thoughts, though feel free to let me know if they don't make sense :)
@@ -1090,6 +1108,64 @@ processors: | |||
field: o365audit.YammerNetworkId | |||
type: string | |||
ignore_missing: true | |||
- json: | |||
field: o365audit.Data | |||
if: ctx.o365audit?.containsKey('Data') |
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.
So we can confirm this is never a string? You could always change this to startsWith a bracket if you are unsure.
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.
The documentation isn't that explicit but based on examples and online discussion it seems to always be JSON. I'll add the startsWith check anyway.
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.
On second thought, I think it's better to let that processor fail if it's not JSON data. It does seem to always be JSON, and if we're parsing and indexing it it doesn't seem worth retaining a field for a non-JSON string.
- convert: | ||
field: o365audit.Data.sip | ||
type: ip | ||
ignore_missing: true | ||
- date: | ||
field: o365audit.Data.at | ||
target_field: o365audit.Data.at | ||
formats: | ||
- ISO8601 | ||
if: ctx.o365audit?.Data?.at != null | ||
- date: | ||
field: o365audit.Data.md | ||
target_field: o365audit.Data.md | ||
formats: | ||
- ISO8601 | ||
if: ctx.o365audit?.Data?.md != null | ||
- date: | ||
field: o365audit.Data.te | ||
target_field: o365audit.Data.te | ||
formats: | ||
- ISO8601 | ||
if: ctx.o365audit?.Data?.te != null | ||
- date: | ||
field: o365audit.Data.ts | ||
target_field: o365audit.Data.ts | ||
formats: | ||
- ISO8601 | ||
if: ctx.o365audit?.Data?.ts != null | ||
- date: | ||
field: o365audit.Data.ttdt | ||
target_field: o365audit.Data.ttdt | ||
formats: | ||
- ISO8601 | ||
if: ctx.o365audit?.Data?.ttdt != null |
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.
None of these matches any date type ECS fields?
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.
Some might, but I haven't been able to find any documentation of these fields.
Users want access to them but we don't have dependable information about their meaning, which may also vary significantly.
- json: | ||
field: o365audit.Data | ||
if: ctx.o365audit?.containsKey('Data') | ||
- rename: |
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.
There is no real reason to still keep the string, so let's just remove it instead? We are moving it to a separate field, breaking it for anyone that might have needed to unpack it either way?
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.
The reason is that there might be other keys in there that we don't know about.
It's a pity that o365.audit.Data
changes type, but o365.audit.Data.knownfield
and o365.audit.Data.flattened.otherfield
seems better than
"lon": "GrantAdminPermission", | ||
"op": "GrantAdminPermission", |
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.
Multiple of these fields look like good candidates for things like event.action, unless that is already set with a better value?
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.
In this case we do have "event.action": "AlertTriggered"
. More generally, we don't have documentation for the individual fields and although we have a list of typical keys, the meanings may vary.
@P1llus I responded to each point, but in summary: |
Hi @chrisberkhout, please update your branch with the latest contents from main branch. There was an important PR merged updating the CI pipelines. Thanks! |
edc9def
to
ed1e5d0
Compare
…wn key names/types.
ed1e5d0
to
aea1939
Compare
💚 Build Succeeded
|
Package o365 - 2.0.0 containing this change is available at https://epr.elastic.co/search?package=o365 |
Proposed commit message
Data
notesAccording to the Office 365 Management Activity API documentation, the
Data
parameter appears in events for the Security and Compliance Alerts schema and the Automated investigation and response events in Office 365, Main investigation schema. It is described only as "The detailed data blob of the alert or alert entity" or "Data string which contains more details about investigation entities, and information about alerts related to the investigation. Entities are available in a separate node within the data blob."A list of fields and types inside the
Data
JSON blob was provided by a user in #4319 (comment). Example values I could find matched the user-supplied list.Known example values for the
Data
parameterI found a couple of examples in this forum post:
And a couple more in existing test data for this integration:
Platform
notesAccording to the Office 365 Management Activity API schema documentation, the
Platform
parameter appears various AIP (Azure Information Protection) events. It is described there as "Device platform (Win, OSX, Android, iOS)", or "The platform on which the activity happened. For example, Windows."A user reported the field in an event with
"Workload": "PublicEndpoint"
,"Application": "Outlook"
and"Operation": "SensitivityLabelApplied"
, however this example came via the deprecatedo365audit
input rather than the currentcel
input, and is not covered in the Office 365 Management Activity API documentation.The reported values were both strings and integer codes.
There are other pages in the Office 365 Management API documentation that provide more details about AIP events. This content can be found in the
Aip*.md
files in MicrosoftDocs/office-365-management-api. It lists integer codes and their corresponding meanings, so those codes are substituted where found.Checklist
changelog.yml
file.How to test this PR locally
Related issues