Skip to content
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

[Filebeat] Add module for Kibana audit logs #22696

Merged
merged 14 commits into from
Dec 15, 2020

Conversation

legrego
Copy link
Member

@legrego legrego commented Nov 20, 2020

👶 First beats PR

This is my first time submitting a PR against this repository, so I'm sorry if I've missed something obvious.

What does this PR do?

Adds support to Filebeat for ingesting Kibana's ECS Audit Log, available in 7.11+.

Specifically, this adds a new audit fileset to the existing kibana module, defining a new pipeline, additional fields, and tests.

Why is it important?

Having first-class support for ingesting stack audit logs is critical to improving observability.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

Setup and run Kibana with ECS Audit Logging enabled (instructions).

# kibana.yml
xpack.security.audit.enabled: true
xpack.security.audit.appender:
  kind: file
  path: './kibana_audit.json'
  layout:
    kind: json

Configure filebeat's kibana module to read the audit log:

# Module: kibana
# Docs: https://www.elastic.co/guide/en/beats/filebeat/master/filebeat-module-kibana.html

- module: kibana
  # Server logs
  log:
    enabled: false

    # Set custom paths for the log files. If left empty,
    # Filebeat will choose the paths depending on your OS.
    #var.paths:

  # Audit logs
  audit:
    enabled: true

    # Set custom paths for the log files. If left empty,
    # Filebeat will choose the paths depending on your OS.
    var.paths: ["/path/to/kibana_audit.json"]

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Nov 20, 2020
@legrego legrego force-pushed the kibana-audit-module branch from 929a926 to 32fe416 Compare November 20, 2020 20:27
@elasticmachine
Copy link
Collaborator

elasticmachine commented Nov 20, 2020

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #22696 updated

  • Start Time: 2020-12-15T11:50:41.170+0000

  • Duration: 45 min 13 sec

Test stats 🧪

Test Results
Failed 0
Passed 5122
Skipped 574
Total 5696

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 5122
Skipped 574
Total 5696

@andresrc andresrc added the Team:Services (Deprecated) Label for the former Integrations-Services team label Nov 21, 2020
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Nov 21, 2020
@legrego legrego force-pushed the kibana-audit-module branch 11 times, most recently from c5837d9 to ab260bd Compare December 4, 2020 14:37
@legrego legrego force-pushed the kibana-audit-module branch from ab260bd to d3cafc4 Compare December 4, 2020 17:38
@legrego legrego changed the title Add module for Kibana audit logs [Filebeat] Add module for Kibana audit logs Dec 4, 2020
@legrego legrego marked this pull request as ready for review December 4, 2020 19:07
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-services (Team:Services)

@legrego legrego requested a review from thomheymann December 4, 2020 19:42
Copy link
Member

@P1llus P1llus left a comment

Choose a reason for hiding this comment

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

Not a full review, but these are the ones that popped out to me at first.

filebeat/module/kibana/_meta/fields.yml Outdated Show resolved Hide resolved
filebeat/module/kibana/audit/ingest/pipeline-json.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@thomheymann thomheymann left a comment

Choose a reason for hiding this comment

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

Looks great!

Verified that ECS audit logs are imported correctly.

Couple comments but I don't know enough about processors so might not be applicable.

filebeat/module/kibana/_meta/fields.yml Outdated Show resolved Hide resolved
- add_fields:
target: ''
fields:
ecs.version: 1.6.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this something we should log from Kibana instead?
If we want to update to a later version of ECS that should be controlled by Kibana, not the Filebeat module.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question; I think this would make sense for Kibana to log instead of Filebeat. I copied this over from the ES config, but it probably doesn't make sense for us to follow suit here.

Copy link
Contributor

@thomheymann thomheymann Dec 9, 2020

Choose a reason for hiding this comment

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

Cool, I'll create a PR to log that in Kibana: elastic/kibana#85390

Copy link
Member Author

Choose a reason for hiding this comment

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

I came across this when testing. It appears that Filebeat is expecting that it'll set an explicit version itself:

self.assert_explicit_ecs_version_set(module, fileset)

I worked around this, but I don't know if that's "ok" or not 😕

- rename:
if: ctx.kibana._audit_temp.kibana.delete_from_spaces != null
target_field: kibana.delete_from_spaces
field: kibana._audit_temp.kibana.delete_from_spaces
Copy link
Contributor

@thomheymann thomheymann Dec 8, 2020

Choose a reason for hiding this comment

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

I'm a bit worried about picking fields within kibana manually since it's supposed to be possible for plugin developer to add whatever fields they need and I would expect that to be ingested using the Filebeat module.

https://github.com/elastic/kibana/blob/master/x-pack/plugins/security/server/audit/audit_events.ts#L50-L53

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm a bit worried about picking fields within kibana manually since it's supposed to be free for login developer to add whatever fields they need and I would expect that to be ingested using the Filebeat module.

I'm torn on this too. On one hand, it would be great if this module "just worked" whenever a new field was added. On the other hand, unknown fields will be dynamically mapped, which might not be what we want. @P1llus are there recommendations on how filesets should handle dynamic fields? Should we accept them so they are mapped dynamically, or should we attempt to keep the mapping in sync with changes in Kibana going forward?

Copy link
Member

Choose a reason for hiding this comment

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

The general way we approach it, is ensuring that all fields coming from filebeat modules would be appended under modulename.filesetname.*.
I usually copy the whole event under the correct field root first (in this case kibana.audit), then rename the ones that are moved to ECS while keeping the rest under kibana.audit

In terms of dynamic mapping, I would recommend to map as many fields as possible, however if you know that there is certain fields with complex or everchanging field names, you can apply the "flattened" type to the top field for that usecase, though please don't set the root field kibana.audit as flattened, you could also have a specific field if you want, in which you put all custom data.

Hope that makes sense @legrego @thomheymann

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not a fan of changing the JSON structure from what's logged since it is already in the format that we want and it would be super confusing for users otherwise.

I'm starting to think that it then might be better to restrict the ECS schema in Kibana so that only known fields are allowed to be logged which we can map manually in the Filebeat pipeline.

Is there a way of defining this pipeline using an npm module in the Kibana repository or can we publish a Go module from within Kibana? I'm a bit worried about these things being dislocated and going out of sync. (Developers adding fields without updating Filebeat pipeline)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm starting to think that it then might be better to restrict the ECS schema in Kibana so that only known fields are allowed to be logged which we can map manually in the Filebeat pipeline.

++ I think this is the safest approach for now too, until we get to the point where we actually require dynamic fields that can't be known ahead of time.

Is there a way of defining this pipeline using an npm module in the Kibana repository or can we publish a Go module from within Kibana? I'm a bit worried about these things being dislocated and going out of sync. (Developers adding fields without updating Filebeat pipeline)

I wonder if there's something we could do within Kibana to enforce this. If we had the audit logger run alongside the functional test suites, we could inspect it to ensure that it's not capturing anything unexpected.

If it wouldn't be so expensive to do, we could do a runtime enforcement, but I don't think that's worth the overhead at this point.

Copy link
Contributor

@thomheymann thomheymann Dec 14, 2020

Choose a reason for hiding this comment

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

I've typed the kibana namespace in elastic/kibana#85451

@legrego
Copy link
Member Author

legrego commented Dec 9, 2020

@P1llus am I not allowed to use the error property in my output? I updated my tests (locally), and it's failing because I have a log entry that makes use of error, but the test runner appears to forbid that:

assert "error" not in obj, "not error expected but got: {}.\n The related error message is: {}".format(
obj, obj["error"].get("message"))

E AssertionError: not error expected but got: {'agent': {'name': '190239f3e16e', 'id': 'a0fccfb3-b282-42e1-a1a1-9b0f1e1460b6', 'type': 'filebeat', 'ephemeral_id': '26878692-25b8-4908-81de-1ef96fa949cb', 'version': '8.0.0'}, 'process': {'pid': 20699}, 'log': {'file': {'path': '/go/src/github.com/elastic/beats/filebeat/module/kibana/audit/test/test-audit-711.log'}, 'offset': 2213}, 'fileset': {'name': 'audit'}, 'message': 'Failed attempt to access saved objects', 'error': {'code': 'Error', 'message': 'unauthorized'}, 'input': {'type': 'log'}, 'trace': {'id': '6f0cc64c-0096-411e-bcd1-3fab36731e39'}, '@timestamp': '2020-12-09T12:04:35.549-05:00', 'ecs': {'version': '1.6.0'}, 'related': {'user': ['default']}, 'service': {'type': 'kibana'}, 'host': {'name': '190239f3e16e'}, 'event': {'ingested': '2020-12-09T17:51:38.856978593Z', 'timezone': '-02:00', 'created': '2020-12-09T17:51:36.660Z', 'kind': 'event', 'module': 'kibana', 'action': 'saved_object_find', 'category': 'database', 'type': 'access', 'dataset': 'kibana.audit', 'outcome': 'failure'}, 'kibana': {'space_id': 'marketing'}, 'user': {'roles': ['default'], 'name': 'default'}}.

Copy link
Contributor

@thomheymann thomheymann left a comment

Choose a reason for hiding this comment

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

LGTM - Thanks for taking this on!

@legrego legrego merged commit 4474b4e into elastic:master Dec 15, 2020
@legrego legrego deleted the kibana-audit-module branch December 15, 2020 15:51
legrego added a commit to legrego/beats that referenced this pull request Dec 15, 2020
Co-authored-by: Thom Heymann <190132+thomheymann@users.noreply.github.com>
legrego added a commit that referenced this pull request Dec 16, 2020
Co-authored-by: Thom Heymann <190132+thomheymann@users.noreply.github.com>

Co-authored-by: Thom Heymann <190132+thomheymann@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Filebeat Filebeat Team:Services (Deprecated) Label for the former Integrations-Services team v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants