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

Convert Filebeat system.auth to ECS #9138

Merged
merged 14 commits into from
Nov 27, 2018
Merged

Conversation

webmat
Copy link
Contributor

@webmat webmat commented Nov 16, 2018

Caveats

  • Assuming system auth logs only come from local system, not remote systems. So only populating host.hostname and ignoring device.hostname.
  • ECS is missing a timezone field, but should have one. Currently leaving to beats.timezone.
  • system.auth.ssh.dropped_ip should be at least copied to source.ip.
    • Should we rename it, and mention the dropping of the connection at event.action? Or should we just copy it, and the presence of system.auth.ssh.dropped_ip is what indicates it was dropped?
      • Note that this module is not keeping the original message for ssh events.
  • group field set is not part of ECS, at this time. But based on user.id and user.name semantics, I went with their obvious group.* equivalents. It will be fine ;-)
  • After performing straight field renames on these auth messages, I wonder if we're missing something. For example the first pattern uses only ECS fields, if it wasn't for one last field that hasn't been renamed (system.auth.ssh.method, line 29), I would never know that its ECS event is a message about SSH. Added to [WIP] Filebeat module issues found during ECS conversion #9208

ECS Fields temporarily defined locally

  • host.hostname, added to system/_meta/fields.yml
  • group.id, added to system/_meta/fields.yml
  • group.name, added to system/_meta/fields.yml

Renames

  • system.auth.hostname => host.hostname
  • system.auth.pid => process.pid
  • system.auth.program => process.name
  • system.auth.message => message
  • system.auth.useradd.gid => group.id
  • system.auth.groupadd.gid => group.id
  • system.auth.useradd.uid => user.id
  • system.auth.useradd.user => user.name
  • system.auth.ssh.event => event.action
  • system.auth.ssh.ip => source.ip
  • system.auth.ssh.port => source.port
  • system.auth.ssh.geoip.* => source.geo.*

TODO

  • Changelog
  • ECS-migrations.yml
  • Create field aliases
  • Copy dropped_ip to source_ip via pipeline
  • Test failure: "Cannot write to a field alias [system.auth.timestamp]."
  • Post-rebase test failure: AssertionError: expected 10 events to compare but got 9.
    • "Cannot write to a field alias [system.auth.groupadd.gid]."

Noticed

@ruflin ruflin mentioned this pull request Nov 16, 2018
@tsg tsg added Filebeat Filebeat ecs labels Nov 19, 2018
@webmat webmat added in progress Pull request is currently in progress. module labels Nov 20, 2018
@webmat
Copy link
Contributor Author

webmat commented Nov 20, 2018

@ruflin This one is not ready for review. But I’d like to discuss some of the caveats.

@ruflin
Copy link
Member

ruflin commented Nov 22, 2018

Caveats

  • I think host.hostname is in both cases correct. We should probably have a discussion about what we think device is (not in this PR).
  • +1 on keeping beat.timezone for now. If I understand it correctly, this is the timezone of the host machine?
  • dropped_ip: Sounds like a good case where copy is more accurate then just the alias. As the alias alone would always be there.
  • +1 on group
  • ssh event: Interesting note but would delay discussion as we have at the moment the information needed.

@webmat
Copy link
Contributor Author

webmat commented Nov 23, 2018

Finishing up on this PR, I'm no longer sure about mapping system.auth.ssh.signature to event.hash.

Thoughts, @ruflin, @MikePaquette?

Should event.hash be strictly for ensuring integrity of messages in the pipeline (what I understand from the current ECS description), or can it be used to contain the hash or signature an event is describing?

@webmat webmat force-pushed the ecs-system-auth branch 2 times, most recently from 08d9413 to 6aff55f Compare November 23, 2018 21:37
@webmat
Copy link
Contributor Author

webmat commented Nov 23, 2018

@ruflin Ready for final review.

Prior to merging I'd like opinions on this comment, though #9138 (comment). This signature field is no longer mapped to ECS' event.hash.

Also of note: I thought I would use copy_to for system.auth.ssh.dropped_ip => source.ip, but the copy_to key in the fields.yml doesn't seem to work. Instead I'm doing it via the ingest pipeline.

@webmat webmat changed the title WIP Convert Filebeat system.auth to ECS Convert Filebeat system.auth to ECS Nov 23, 2018
@webmat webmat added review and removed in progress Pull request is currently in progress. labels Nov 23, 2018
@webmat webmat requested a review from ruflin November 23, 2018 21:41
@webmat webmat self-assigned this Nov 23, 2018
@ruflin
Copy link
Member

ruflin commented Nov 24, 2018

@webmat Failures on CI seem to be related.

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Change LGTM but check what happens in the tests.

@webmat
Copy link
Contributor Author

webmat commented Nov 26, 2018

@ruflin Failure fixed. All tests should be green now. Will merge tomorrow unless something comes up

@ruflin
Copy link
Member

ruflin commented Nov 27, 2018

@webmat One general comment for ingest pipeline vs copy_to. I think the advantage of using ingest_pipeline is that at one point it can also be applied later again to a raw log line and we it does not depend on a template to be correct.

Mathieu Martin added 5 commits November 27, 2018 10:48
- system.auth.hostname => host.hostname
- system.auth.pid => process.pid
- system.auth.user => user.name
- system.auth.program => process.name
- system.auth.ssh.ip => source.ip
- system.auth.ssh.port => source.port
- system.auth.ssh.geoip.* => source.geo.*
Note that there's no log in this format being tested by the integration tests,
at this time.
@webmat
Copy link
Contributor Author

webmat commented Nov 27, 2018

Merging without waiting on Darwin. Rest of the Jenkins run is green.

@webmat webmat merged commit ab67b31 into elastic:master Nov 27, 2018
webmat added a commit that referenced this pull request Nov 27, 2018
@webmat webmat deleted the ecs-system-auth branch November 27, 2018 19:59
@webmat
Copy link
Contributor Author

webmat commented Nov 27, 2018

GitHub messed up the squash message 🤦‍♂️

jsoriano added a commit that referenced this pull request Mar 19, 2019
Before migration to ECS (#9138), we could rely on the presence of specific
fields to know the process originating the events, but this is not so reliable
after some of these fields have been moved to common places. Add
process.name also for known messages so we keep this info in a known
place.

Also use event.outcome instead of event.action for the result of the
logged action.

(cherry picked from commit a2e6d3a)
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.

3 participants