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.

Sign up for GitHub

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

Jump to bottom

Update Suricata for ECS DNS #13329

Merged
merged 4 commits into from
Aug 27, 2019
Merged

Update Suricata for ECS DNS #13329

merged 4 commits into from
Aug 27, 2019

Conversation

andrewkroh
Copy link
Member

@andrewkroh andrewkroh commented Aug 23, 2019 •
edited
Loading

This updates the Suricata module to populate the ECS DNS fields. It does not remove existing suricata.eve.dns.* fields to preserve backward compatibility.

This also enhances the pipeline to handle the Suricata detailed DNS format (aka version 2). It requires that when using EVE DNS version: 2 that formats: [detailed] is used (grouped can be enabled too but it is ignored).

log.original is now populated with the original JSON log data.

source.address and destination.address are now populated.

event.end is populated with the flow.end value now and hence some events that did not contain flow.end no longer have an event.end.

Relates #13320

Needs:

  • dns.question.registered_domain

@andrewkroh andrewkroh force-pushed the feature/fb/suricata-ecs-dns branch 2 times, most recently from 771af5a to 4b71724 Compare August 23, 2019 03:41
@andrewkroh andrewkroh mentioned this pull request Aug 23, 2019
Closed
6 tasks
webmat
webmat reviewed Aug 23, 2019
View reviewed changes
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.

Looking good so far!

I agree with populating event.end strictly with flow.end for flow events.

Can't wait to see the final version!

Have one important comment on DNS v1 answers, afaict it's a non-issue.

x-pack/filebeat/module/suricata/eve/config/eve.yml
return;
}
evt.Put("dns.answers", [answer]);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If it's a DNS version 1 event, this will continue to create a single event per DNS answer still, correct?

I guess for backwards compatibility for users that use Suricata as a source, we don't really have a choice to keep it like that. But the semantics for DNS answers will not match ECS, therefore I'm not sure we'll be able to depend on Suricata DNS version 1 as a source for DNS answers.

I think for now SIEM only looks at queries, however. Correct?

cc @cwurm

Copy link
Contributor

Choose a reason for hiding this comment

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

The SIEM app has two DNS queries:

  1. Top DNS domains table: Terms agg on dns.question.etld_plus_one, sub-aggs on dns.question.name, source.bytes, and destination.bytes
  2. DNS queries KPI: Checks if dns.question.name exists among other things.

Since there is no filter on just queries or just answers I think we will miscount if there are several documents (for query and answer or one per answer).

x-pack/filebeat/module/suricata/eve/ingest/pipeline.yml
@@ -0,0 +1,231 @@
---
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 a straight JSON to YAML conversion, or were there other changes to this 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 added a lowercase filter for the network.transport. And made the aforementioned event.end change.

x-pack/filebeat/module/suricata/eve/test/eve-small.log-expected.json
"ttl": 299,
"type": "A"
}
],
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@andrewkroh
Update Suricata for ECS DNS
f8ba4d3 
This updates the Suricata module to populate the ECS DNS fields. It does not remove existing `suricata.eve.dns.*` fields to preserve backward compatibility.

This also enhances the pipeline to handle the Suricata detailed DNS format (aka version 2). It requires that when using EVE DNS `version: 2` that `formats: [detailed]` is used (`grouped` can be enabled too but it is ignored).

`log.original` is now populated with the original JSON log data.

`source.address` and `destination.address` are now populated.

`event.end` is populated with the `flow.end` value now and hence some events that did not contain `flow.end` no longer have an `event.end`.

Relates elastic#13320
@andrewkroh
Add registered_domain
0f55384
@andrewkroh andrewkroh force-pushed the feature/fb/suricata-ecs-dns branch from 4b71724 to 0f55384 Compare August 26, 2019 16:02
@andrewkroh andrewkroh marked this pull request as ready for review August 26, 2019 16:12
@elasticmachine
Copy link
Collaborator

Pinging @elastic/siem

webmat
webmat approved these changes Aug 26, 2019
View reviewed changes
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.

LGTM, once Changelog is rebased

@webmat
Copy link
Contributor

webmat commented Aug 27, 2019

Travis flailing with the exact same error as #13324, so pretty sure it's unrelated:

======================================================================
ERROR: Test stopping filebeat under load: wait for all events being published.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/go/src/github.com/elastic/beats/filebeat/tests/system/test_shutdown.py", line 57, in test_shutdown_wait_ok
    max_timeout=15)
  File "/go/src/github.com/elastic/beats/filebeat/tests/system/../../../libbeat/tests/system/beat/beat.py", line 387, in wait_log_contains
    name=name)
  File "/go/src/github.com/elastic/beats/filebeat/tests/system/../../../libbeat/tests/system/beat/beat.py", line 352, in wait_until
    "Waited {} seconds.".format(max_timeout))
TimeoutError: Timeout waiting for 'log_contains' to be true. Waited 15 seconds.
----------------------------------------------------------------------
XML: /go/src/github.com/elastic/beats/filebeat/build/TEST-system.xml
[success] 17.56% test_autodiscover.TestAutodiscover.test_default_settings: 93.9637s
[success] 8.25% test_autodiscover.TestAutodiscover.test_docker: 44.1770s
[error] 4.71% test_shutdown.Test.test_shutdown_wait_ok: 25.2150s

@andrewkroh andrewkroh merged commit 835e063 into elastic:master Aug 27, 2019
@adriansr adriansr mentioned this pull request Sep 25, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cwurm cwurm cwurm left review comments

@webmat webmat webmat approved these changes

Assignees
No one assigned
Labels
ecs Filebeat Filebeat review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@andrewkroh @elasticmachine @webmat @cwurm