Skip to content

Parse more fields from elasticsearch audit log#10356

Merged
ycombinator merged 10 commits intoelastic:masterfrom
ycombinator:fb-es-audit-log-more-fields
Jan 30, 2019
Merged

Parse more fields from elasticsearch audit log#10356
ycombinator merged 10 commits intoelastic:masterfrom
ycombinator:fb-es-audit-log-more-fields

Conversation

@ycombinator
Copy link
Copy Markdown
Contributor

@ycombinator ycombinator commented Jan 28, 2019

Follow up to #10352 per #10352 (comment):

While working on this PR I realized that we don't have sample lines for the structured elasticsearch audit log containing a request body (which is supposed to be parsed into the http.request.body.content field). I'm working with @albertzaharovits to get such a sample and will incorporate it into follow up PRs (for master and 6.x).

Accordingly, this PR adds sample lines to the structured and unstructured log file test fixtures for the elasticsearch/audit fileset and teaches the fileset to parse any new fields encountered in these sample lines.

@ycombinator ycombinator added in progress Pull request is currently in progress. module Filebeat Filebeat v7.0.0 Feature:Stack Monitoring labels Jan 28, 2019
@ycombinator ycombinator requested review from a team as code owners January 28, 2019 02:35
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/stack-monitoring

@ycombinator
Copy link
Copy Markdown
Contributor Author

jenkins, test this

@ycombinator ycombinator requested review from ruflin and webmat January 29, 2019 18:12
@ycombinator ycombinator added review and removed in progress Pull request is currently in progress. labels Jan 29, 2019
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Breaking change or was never release?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this qualifies as a breaking change. Previously we thought that the value of realm=[...] was the authenticated user's realm (es.audit.user.realm) but it turns out to be the realm that was used to test against before the user was authenticated or not (es.audit.realm).

The JSON audit logs contain both fields, but the plaintext audit logs only contain the latter one (es.audit.realm). Hence this change.

From a ES mapping perspective, we're introducing a new field here, es.audit.realm so in that sense it's not a breaking change.

Copy link
Copy Markdown
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

LGTM overall, except for message.

Since this is a structured log, without another message field in it, it can be tricky to have a good message field as a result. The current value could at least be stripped out of the timestamp. The rest is important for context (if you think about looking at this log with the log viewer).

So ideally that's the one change I would request here. Although not the end of the world if you don't have time. Getting good message fields across the board will take time.

@webmat
Copy link
Copy Markdown
Contributor

webmat commented Jan 30, 2019

As discussed, never mind about message here ;-)

@ycombinator ycombinator merged commit c42ebe4 into elastic:master Jan 30, 2019
@ycombinator ycombinator deleted the fb-es-audit-log-more-fields branch January 30, 2019 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants