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

Fix errors in filebeat Zeek dashboard and README files. Update field descriptions. Add notice.log support. #10916

Merged
merged 17 commits into from
Feb 27, 2019
Merged

Conversation

alakahakai
Copy link

Update Filebeat Zeek dashboard to address {issue}10915[10915]

@alakahakai alakahakai requested a review from a team as a code owner February 23, 2019 04:33
@alakahakai alakahakai requested a review from a team February 23, 2019 04:33
@elasticmachine
Copy link
Collaborator

Pinging @elastic/secops

@FrankHassanabad
Copy link

FrankHassanabad commented Feb 23, 2019

Nit: One small thing I noticed when setting up zeek was that in the readme you need

networks.cfg

Rather than network.cfg

For: /etc/bro/network.cfg

So I think it should be: /etc/bro/networks.cfg

In the readme.

@alakahakai
Copy link
Author

Nit: One small thing I noticed when setting up zeek was that in the readme you need

networks.cfg

Rather than network.cfg

For: /etc/bro/network.cfg

So I think it should be: /etc/bro/networks.cfg

In the readme.

Yes. Sorry for the typo.

@alakahakai alakahakai changed the title Update Filebeat Zeek dashboard Update Filebeat Zeek dashboard and README files Feb 23, 2019
@alakahakai alakahakai changed the title Update Filebeat Zeek dashboard and README files Update Filebeat Zeek dashboard and README files. Add notice.log support. Feb 23, 2019
@alakahakai alakahakai requested a review from a team as a code owner February 23, 2019 23:09
@alakahakai alakahakai added the Filebeat Filebeat label Feb 23, 2019
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.

Noted a few things that need to change.

Good stuff overall, though. And I love the idea of doing the field renames right in Beats. It's lightweight and allows for the use of Beats processors on the predictable ECS field names, instead of the arbitrary field names of each different sources.

@alakahakai alakahakai requested a review from webmat February 26, 2019 00:15
@alakahakai alakahakai changed the title Update Filebeat Zeek dashboard and README files. Add notice.log support. Fix errors in filebeat Zeek dashboard and README files. Update field descriptions. Add notice.log support. Feb 26, 2019
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.

Almost done.

I think we should avoid painless for simple cases such as when copying a value.

Once that's fixed, I'm good with the PR

@@ -1 +1,3 @@
{"ts":1547188415.857497,"uid":"CAcJw21BbVedgFnYH3","id.orig_h":"192.168.86.167","id.orig_p":38339,"id.resp_h":"192.168.86.1","id.resp_p":53,"proto":"udp","service":"dns","duration":0.076967,"orig_bytes":75,"resp_bytes":178,"conn_state":"SF","local_orig":true,"local_resp":true,"missed_bytes":0,"history":"Dd","orig_pkts":1,"orig_ip_bytes":103,"resp_pkts":1,"resp_ip_bytes":206,"tunnel_parents":[]}
{"ts":1547188416.857497,"uid":"CAcJw21BbVedgFnYH4","id.orig_h":"192.168.86.167","id.orig_p":38340,"id.resp_h":"8.8.8.8","id.resp_p":53,"proto":"udp","service":"dns","duration":0.076967,"orig_bytes":75,"resp_bytes":178,"conn_state":"SF","local_orig":true,"local_resp":false,"missed_bytes":0,"history":"Dd","orig_pkts":1,"orig_ip_bytes":103,"resp_pkts":1,"resp_ip_bytes":206,"tunnel_parents":[]}
{"ts":1547188417.857497,"uid":"CAcJw21BbVedgFnYH5","id.orig_h":"4.4.2.2","id.orig_p":383341,"id.resp_h":"8.8.8.8","id.resp_p":53,"proto":"udp","service":"dns","duration":0.076967,"orig_bytes":75,"resp_bytes":178,"conn_state":"SF","local_orig":false,"local_resp":false,"missed_bytes":0,"history":"Dd","orig_pkts":1,"orig_ip_bytes":103,"resp_pkts":1,"resp_ip_bytes":206,"tunnel_parents":[]}
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️ moar test cases! 😂

{
"script": {
"lang": "painless",
"source": "ctx.destination.ip = ctx['destination']['address'];",
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for not catching this earlier, but this copy and the following two can be done with set and the same if.

We should avoid using Painless in cases like this, as it adds unnecessary overhead (even if small).

Check out an example in Suricata module

Copy link
Author

@alakahakai alakahakai Feb 26, 2019

Choose a reason for hiding this comment

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

It was using set processors before. I did not see there was an "if" for set, and set processor was creating null value fields. I will fix this.

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

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.

Oops, the test failures appear related.

When Elasticsearch hangs like this, it may mean one of the pipelines or your template is at fault. Ping me if you need help diagnosing this.

@alakahakai
Copy link
Author

Oh jeez, never noticed either ;-)

This actually was not the problem. There was a typo in suricata pipeline that caused a parse error, so the test was stuck.

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.

This PR shouldn't change which version of the stack is used in master.

This is my last complaint for real, this time ;-)

testing/environments/snapshot.yml Outdated Show resolved Hide resolved
@alakahakai
Copy link
Author

I think this needs to be backported to 7.x. Will open a new PR for that.

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

@alakahakai alakahakai merged commit 9cc9725 into elastic:master Feb 27, 2019
@alakahakai alakahakai deleted the zeek-kibana-dashboard-fix branch February 27, 2019 19:49
alakahakai pushed a commit that referenced this pull request Feb 28, 2019
* Update Zeek dashboard and README.md. Add support for notice.log. Update field descriptions
alakahakai pushed a commit that referenced this pull request Feb 28, 2019
* Fix errors in filebeat Zeek dashboard and README files. Update field descriptions. Add notice.log support. (#10916)
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