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] Improve ECS categorization in iptables module #16637

Merged
merged 3 commits into from
Mar 17, 2020

Conversation

leehinman
Copy link
Contributor

@leehinman leehinman commented Feb 26, 2020

  • event.category
  • event.kind
  • event.type
  • related.ip
  • convert pipeline to yaml

Closes #16166

@leehinman leehinman added enhancement Filebeat Filebeat needs_backport PR is waiting to be backported to other branches. Team:SIEM ecs labels Feb 26, 2020
@leehinman leehinman requested a review from a team as a code owner February 26, 2020 20:21
@elasticmachine
Copy link
Collaborator

Pinging @elastic/siem (Team:SIEM)

"event.module": "iptables",
"event.outcome": "deny",
Copy link
Member

Choose a reason for hiding this comment

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

I think it still makes sense to populate this if we know what action was taken, but use success or failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll ask for some ECS input. My understanding was that allowed/denied would move to event.type. So you could have:
event.type = ["connection", "denied"]

and then event.outcome could be success or failure, based on if you were successful in denying the connection.

Choose a reason for hiding this comment

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

@leehinman that's correct.

Copy link
Member

Choose a reason for hiding this comment

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

and then event.outcome could be success or failure, based on if you were successful in denying the connection.

So isn't the fact that iptables says it blocked the connection evidence enough to set the event.outcome to failure?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I'd have to agree with @andrewkroh here. When we model the iptables events as category "network" and type "connection", looking for event.outcome:failure I would expect to find the connections denied. Not events where a firewall "failed to block a connection" -- can this actually happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If iptables failed to block the connection I'd be very surprised if we got something in the iptables log saying so. At best we might get something from kernel logger. So I'd expect every allowed/denied to end up with event.outcome = success. Which doesn't make event.outcome very useful.

Choose a reason for hiding this comment

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

out of band yes, its possible (websense & some IPS's used to work like that - it sees badness - sends a spoofed reset oob to kill the connection, and it didn't always work).

from an observers standpoint wouldn't the "deny" be event.action? with outcome being success/deny (with a strong default to succeed as oob is likely fairly limited in production these days).

iptables is an ugly one as it can be e.g. ubiquiti as a network firewall, or a host based firewall - where the connection event (e.g. from auditbeat) would have a event.outcome as fail, but e.g. filebeat pulling in the iptables event would be event.action:deny | block | reset, etc. with the event.outcome being specific to the action being taken (in this case the block by the firewall component)

Copy link

@dainperkins dainperkins Mar 5, 2020

Choose a reason for hiding this comment

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

sorry - "denied" can be one of the values in the event.type (in addition to e.g. connection), as well event.action: deny

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I'm retreating from my previous comment. I agree with setting event.outcome:successful when a firewall denies a connection.

Examples of event.outcome:failure would be cases like @dainperkins mentions, where for example sending a TCP rst fails to kill the connection. Whether or not a given firewall correctly reports this is not relevant in the bigger picture (we'd simply never set it in this case).

There's other well known cases where the agent's action is not successful, like Endpoint trying to kill a process, but the process is already gone.

Copy link
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.

Here's a few suggestions that are applicable now:

  • Please make event.category an array of values like event.type, even when there's only one value.
  • I would recommend populating event.action with any raw value as per iptable. Perhaps lowercase it first to make it easier on the eyes. But essentially the goal of event.action not having "allowed values" is for allowing to relay the information as specified by the source. So event.action should be "allow", "deny", and if I understand correctly "drop_input" or any value that's in this position in the logs.
  • Then also map these event actions to an additional value in the event.type array, whichever fits best, either "allowed" or "denied". The value "connection" already in the array is good 👍.

Your PR didn't modify this, so feel free to address here or not. But it looks like the fields could be array fields: iptables.fragment_flags and iptables.tcp.flags?

Here's a few additional suggestions that are applicable to ECS 1.5 (so totally fine to address in a later PR, IMO):

  • iptables.ubiquiti.*_zone likely map to observer.ingress/egress.zone
  • iptables.ubiquiti.rule_* likely map to fields in the rule.* field set

So overall, I'd say only the first 3 bullet points need to be addressed here, and I'd give it my 👍 for mapping to ECS 1.4. The rest of the feedback can be addressed later :-)

@@ -3,20 +3,32 @@
"destination.ip": "10.4.0.5",
"destination.mac": "90:10:20:76:8d:20",
"destination.port": 443,
"event.action": "d",
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this should be deny or something more than d.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is implementing @webmat 's suggestion. The original value from ubiquiti iptables is "d" or "a"

Copy link
Member

Choose a reason for hiding this comment

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

I think expanding it to a word will be more user friendly. I have a feeling users will think it's some kind of bug like me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure. switched to accept/drop which is what is displayed in ubiquiti gui

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree with switching to being more readable.

The ECS recommendation is only a high level for "this should be driven by the source". But when the underlying source transport is too compact like this "a" & "d", I think it's totally fine to expand it. Expanding it to what's in their UI is actually a great resolution here.

@@ -2074,6 +2074,8 @@
type: keyword
ignore_above: 1024
description: Hostname of the observer.
- name: egress.zone
- name: ingress.zone
Copy link
Contributor

Choose a reason for hiding this comment

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

These fields already exist in master since the upgrade to ECS 1.5.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adriansr Thanks for catching that. Should be fixed now.

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

LGTM

- event.category
- event.kind
- event.type
- related.ip
- convert pipeline to yaml

Closes elastic#16166
- event.action, map to accept/drop like gui
- lowercase event.action
- fix tcp_flags grok to get all entries
- make iptables.tcp.flags an array
- make iptables.fragment_flags an array
- make event.category an array
- map iptables.ubiquiti.output_zone -> oberserver.egress.zone
- map iptables.ubiquiti.input_zone -> observer.ingress.zone
- map iptables.ubiquiti.rule_set -> rule.name
- map iptables.ubiquiti.rule_number -> rule.id
- rebase to master
@leehinman leehinman merged commit d9c83df into elastic:master Mar 17, 2020
@leehinman leehinman deleted the 16166_iptables_ecs_1.4 branch March 17, 2020 19:53
@leehinman leehinman added v7.7.0 and removed needs_backport PR is waiting to be backported to other branches. labels Mar 17, 2020
leehinman added a commit to leehinman/beats that referenced this pull request Mar 17, 2020
* Improve ECS categorization in iptables module

- event.action, map to accept/drop like gui
- event.category
- event.kind
- event.type
- observer.egress.zone
- observer.ingress.zone
- related.ip
- rule.id
- rule.name
- convert pipeline to yaml
- fix tcp_flags grok to get all entries
- make iptables.tcp.flags an array
- make iptables.fragment_flags an array

Closes elastic#16166

(cherry picked from commit d9c83df)
leehinman added a commit that referenced this pull request Mar 19, 2020
…7064)

* Improve ECS categorization in iptables module

- event.action, map to accept/drop like gui
- event.category
- event.kind
- event.type
- observer.egress.zone
- observer.ingress.zone
- related.ip
- rule.id
- rule.name
- convert pipeline to yaml
- fix tcp_flags grok to get all entries
- make iptables.tcp.flags an array
- make iptables.fragment_flags an array

Closes #16166

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

[Filebeat] Upgrade iptables module to ECS 1.4
7 participants