-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[SIEM] Office 365 module #16386
[SIEM] Office 365 module #16386
Conversation
Pinging @elastic/siem (Team:SIEM) |
Forgot to make it a draft PR. This includes the changes in #16244 |
@leehinman can you please take a look at the ECS mappings in this PR. |
@andrewkroh @leehinman this is still a WIP (should be a draft, I clicked the wrong button and made it a PR), but I will push the final mappings soon. |
62f0ff8
to
c77f2d7
Compare
774e66b
to
483e3bb
Compare
Ready for final reviews / ECS validation. I've mapped all the API schemas that made sense to me. Please have a look only at the last commit ( Docs / dashboards will be added on a separate PR after this is merged. |
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.
Looking really good, couple of minor things and questions below. I did notice that we could probably make use of the new event.category, iam. Not sure if we want to do that now, or after ECS 1.5 is officially released.
type: keyword | ||
|
||
- name: CreationTime | ||
type: keyword |
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.
should this be 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.
This is the original date from the API, which is already parsed to date as @timestamp
. I'd like to keep the fields under o365.audit
as raw as possible.
type: keyword | ||
|
||
- name: ClientIPAddress | ||
type: keyword |
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.
should this be type ip?
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 that case we risk that if the API provides something that is not a valid IP, it won't be possible to ingest the event. This is already converted to type ip in client.ip
.
- name: ClientInfoString | ||
type: keyword | ||
|
||
- name: ClientIP |
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.
Are the ClientIP & ClientIPAddress fields different?
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.
Not 100% sure.
ClientIP:
The IP address of the device that was used when the activity was logged. The IP address is displayed in either an IPv4 or IPv6 address format.
For some services, the value displayed in this property might be the IP address for a trusted application (for example, Office on the web apps) calling into the service on behalf of a user and not the IP address of the device used by person who performed the activity.
Also, for Azure Active Directory-related events, the IP address isn't logged and the value for the ClientIP property is null.
ClientIPAddress:
The IP address of the device that was used when the operation was logged. The IP address is displayed in either an IPv4 or IPv6 address format.
What I did is set client.ip
from both. Keeping the value from ClientIPAddress
if it exists, as I understand sometimes ClientIP
can be the IP of an intermediate service.
In my log samples, the few times ClientIPAddress exists, it has the same value as ClientIP (::1
, which is useless).
{from: 'o365audit.SharePointMetaData.From', to: 'user.id'}, | ||
{from: 'o365audit.SharePointMetaData.FileName', to: 'file.name'}, | ||
{from: 'o365audit.SharePointMetaData.FilePathUrl', to: 'url.original'}, | ||
{from: 'o365audit.SharePointMetaData.UniqueId', to: 'file.inode'}, |
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 maybe a duplicate? UniqueId & UniqueID?
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's documented as UniqueId
but the API is producing UniqueID
. Was just being a little too defensive in here, just in case they realize the discrepancy and change the generated field instead of the docs.
if (maxSeverity > -1) { | ||
evt.Put("event.severity", maxSeverity); | ||
} | ||
evt.Put("event.outcome", allowed || isBlockOverride(evt)? "success" : "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 think if allowed is true we want to set event.outcome to "success" not true. .... Oh, I see where this gets mapped from true -> success later. I wonder if it would be better to have the second half or the || statement also set a boolean value. That might make it more consistent and easier to follow a year from now.
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 operator precedence of javascript is different from what you might expect. This expression is equivalent to:
( allowed || isBlockOverride(evt) )? "success" : "failure"
I've added parentheses around it to make it more clear.
"organization.name": "testsiem.onmicrosoft.com", | ||
"server.address": "HE1PR0102MB3228 (15.20.2707.017)", | ||
"service.type": "o365", | ||
"user.hash": "NT AUTHORITY\\SYSTEM (Microsoft.Exchange.ServiceHost)", |
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.
hmmm. this doesn't look like a hash to me.
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.
Right. I was too eager to try to map as many fields as possible. The source field "UserKey" is really just an alternate user ID (also with different meanings across systems). I think it's better not to map it to ECS at all.
x-pack/filebeat/module/o365/audit/test/02-exchange-item.log-expected.json
Outdated
Show resolved
Hide resolved
"event.type": "access", | ||
"file.inode": "9cc7be1c-dd5a-4895-b7cb-757de6d51b42", | ||
"file.name": "Customers Financial Data.docx", | ||
"file.owner": "Alan Smithee", |
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.
might be nice to have this in related.user as well
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.
Good idea. Done.
"@timestamp": "2020-02-28T09:42:45.000Z", | ||
"client.address": "79.159.10.151", | ||
"client.ip": "79.159.10.151", | ||
"event.action": "GroupCreation", |
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.
groupcreate would probably be event.category = iam, event.type = ["create", "group"]
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.
Thanks for the suggestion. I wasn't aware of this new category value.
@@ -165,4 +165,4 @@ | |||
"zeek.connection.state_message": "No SYN seen, just midstream traffic (a 'partial connection' that was not later closed).", | |||
"zeek.session_id": "Cc6NJ3GRlfjE44I3h" | |||
} | |||
] | |||
] |
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.
Probably shouldn't be part of this PR.
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.
Good catch
Please add an overview dashboard to accompany the module and highlight some of the key signals coming from the data (activity over time, geo data, failures, etc). Thanks! |
ebc53a0
to
7ff69a6
Compare
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. The pipeline really looks comprehensive. 👍
var outcome = evt.Get("event.outcome"); | ||
// As event.type is an array, this sets both the traditional | ||
// "authentication_success"/"authentication_failure" | ||
// and the soon to be ECS standard "start". |
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.
event.type: start
is no longer "soon to be" 😄 .
jenkins, test this |
This includes a new fileset, o365.audit, that uses the o365audit input to ingest logs using the Office 365 Management API.
Avoid error when trying to dissect.
Max retention is 7 days, that's 168h, not 178.
This patches the o365audit input to accept resource and authentication_endpoint configuration options without a scheme.
4949b21
to
4a6ea69
Compare
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.
2 small items. But I would be OK with this going in as is.
x-pack/filebeat/module/o365/audit/test/15-azuread-sts-logon.log-expected.json
Show resolved
Hide resolved
"event.module": "o365", | ||
"event.outcome": "success", | ||
"event.provider": "Yammer", | ||
"event.type": "group", |
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.
would it be possible to have "creation" added to event.type. So it would be event.cateogory = iam, event.type = "creation, group"
This includes a new fileset, o365.audit, that uses the o365audit input to ingest logs using the Office 365 Management API. (cherry picked from commit 1cc1d33)
What does this PR do?
This includes a new fileset, o365.audit, that uses the o365audit input
to ingest logs using the Office 365 Management API.
TODO
Map more API types.
Validate ECS mappings for SIEM app. (Looking good to me.)
Documentation.
Simplified testing logs.
Dashboard.
Closes [Meta] Module to ingest Office365 audit events #16196
Depends New input for Office 365 audit logs #16244