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 network direction processor to Zeek and Suricata modules #24620

Merged
merged 11 commits into from
Jun 29, 2021

Conversation

legoguy1000
Copy link
Contributor

@legoguy1000 legoguy1000 commented Mar 18, 2021

What does this PR do?

Adds the add_network_direction processor to the Zeek & Suricata module filesets using the internal_networks variable. The internal_networks variable is set to default to [ private ] and the add_network_direction will only run if that varaible is defined.

Why is it important?

The add_network_direction process adds the network.direction to documents which allows users to easily filter on traffic based on direction without needing to know specific IP subnets.

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.

Author's Checklist

How to test this PR locally

cd beats/x-pack/filebeat
GENERATE=true TESTING_FILEBEAT_MODULES=suricata,zeek mage -v pythonIntegTest

Related issues

N/A

Use cases

Screenshots

Logs

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Mar 18, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Mar 18, 2021

💚 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: P1llus commented: /test

  • Start Time: 2021-06-29T11:55:40.436+0000

  • Duration: 115 min 44 sec

  • Commit: 28e08cc

Test stats 🧪

Test Results
Failed 0
Passed 14229
Skipped 2311
Total 16540

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 14229
Skipped 2311
Total 16540

@legoguy1000 legoguy1000 changed the title [Filebeat] Add network direction processor to Zeek, Sonicwall (Firewall), Suricata modules [Filebeat] Add network direction processor to Zeek and Suricata modules Mar 18, 2021
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Mar 18, 2021
@legoguy1000 legoguy1000 marked this pull request as ready for review March 18, 2021 15:26
@elasticmachine
Copy link
Collaborator

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@legoguy1000
Copy link
Contributor Author

@andrewkroh I think I'm almost done with this PR but I think I found a bug and want to check with you first. I noticed on the Snort and Sonicwall JS scripts, for source and dest IP it was using the fld_append function. This causes the field to be an array/list even if its a single value. When I tried to add the add_network_direction processor, it didn't work until I change the JS function to fld_set so its just a single value. My thought is that if it is an array/list, the processor should just use the first value. Thoughts??

@legoguy1000 legoguy1000 force-pushed the add-network-directions branch 2 times, most recently from 75e604e to 961dc62 Compare April 2, 2021 12:21
@legoguy1000 legoguy1000 marked this pull request as ready for review April 2, 2021 12:21
@legoguy1000 legoguy1000 changed the title [Filebeat] Add network direction processor to Zeek and Suricata modules [Filebeat] Add network direction processor to Zeek, Suricata, Snort, Sonicwall modules Apr 2, 2021
@mergify
Copy link
Contributor

mergify bot commented Apr 7, 2021

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b add-network-directions upstream/add-network-directions
git merge upstream/master
git push upstream add-network-directions

@mergify
Copy link
Contributor

mergify bot commented Apr 8, 2021

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b add-network-directions upstream/add-network-directions
git merge upstream/master
git push upstream add-network-directions

@legoguy1000 legoguy1000 marked this pull request as draft April 28, 2021 15:57
@legoguy1000 legoguy1000 force-pushed the add-network-directions branch 3 times, most recently from 6ceaf3e to 1c5722d Compare April 28, 2021 20:10
@legoguy1000 legoguy1000 marked this pull request as ready for review April 28, 2021 20:10
This reverts commit 1c5722d9f1f6cd5370ef3f3d7a882e49731a4e3d.
@P1llus
Copy link
Member

P1llus commented May 19, 2021

run tests

@mergify
Copy link
Contributor

mergify bot commented May 19, 2021

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b add-network-directions upstream/add-network-directions
git merge upstream/master
git push upstream add-network-directions

@P1llus
Copy link
Member

P1llus commented Jun 18, 2021

/test

@mergify
Copy link
Contributor

mergify bot commented Jun 23, 2021

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b add-network-directions upstream/add-network-directions
git merge upstream/master
git push upstream add-network-directions

@P1llus
Copy link
Member

P1llus commented Jun 24, 2021

/test

@P1llus
Copy link
Member

P1llus commented Jun 24, 2021

Is this okay to merge now @andrewkroh? We moved everything out of ingest pipelines and into beats, so that they offer backwards compatibility as well.

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.

Modifying snort and sonicwall will cause issues in the future since those are generated modules from https://github.com/adriansr/nwdevice2filebeat. This feature could be added to the generator then all of the RSA would get this processor. IMO I think we should remove the changes from those two modules and proceed with this PR.

@@ -1007,8 +1007,8 @@ var ecs_mappings = {
"child_process": {to:[{field: "process.name", setter: fld_prio, prio: 1}]},
"city.dst": {to:[{field: "destination.geo.city_name", setter: fld_set}]},
"city.src": {to:[{field: "source.geo.city_name", setter: fld_set}]},
"daddr": {convert: to_ip, to:[{field: "destination.ip", setter: fld_append},{field: "related.ip", setter: fld_append}]},
"daddr_v6": {convert: to_ip, to:[{field: "destination.ip", setter: fld_append},{field: "related.ip", setter: fld_append}]},
"daddr": {convert: to_ip, to:[{field: "destination.ip", setter: fld_set},{field: "related.ip", setter: fld_append}]},
Copy link
Member

Choose a reason for hiding this comment

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

Why set and not append? Normally related.ip is an array. This seems unrelated to adding network direction.

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 don't recall removing "daddr_v6": {convert: to_ip, to:[{field: "destination.ip", setter: fld_append},{field: "related.ip", setter: fld_append}]},. That is probably an accident. As for the change from {field: "destination.ip", setter: fld_append} to {field: "destination.ip", setter: fld_set}, is because the network_direction processor requires a string field not an array.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The append for related.ip is unchanged.

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 am reverting the changes for Snort and Sonicwall.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for pointing out at source.ip and destination.ip should be scalars. I think we should make that change just not in this PR. Would you please open an issue for this? It does look like you reverted most of the snort/sonicwall changes, but a small part of snort is still changed.

@legoguy1000 legoguy1000 changed the title [Filebeat] Add network direction processor to Zeek, Suricata, Snort, Sonicwall modules [Filebeat] Add network direction processor to Zeek and Suricata modules Jun 25, 2021
@P1llus
Copy link
Member

P1llus commented Jun 29, 2021

/test

@P1llus P1llus merged commit 9e670f7 into elastic:master Jun 29, 2021
@P1llus P1llus added the backport-v7.14.0 Automated backport with mergify label Jun 29, 2021
mergify bot pushed a commit that referenced this pull request Jun 29, 2021
…es (#24620)

* Add network direction processor to zeek and suricata module

* Add Snort & Sonicwall

* update changelog

* use ES network_direction processor

* Revert "use ES network_direction processor"

This reverts commit 1c5722d9f1f6cd5370ef3f3d7a882e49731a4e3d.

* update docs with new variable

* Removed Snort and Sonicwall

* update docs

* missed one

Co-authored-by: Marius Iversen <marius.iversen@elastic.co>
(cherry picked from commit 9e670f7)
@legoguy1000 legoguy1000 deleted the add-network-directions branch June 29, 2021 14:06
v1v added a commit to v1v/beats that referenced this pull request Jun 29, 2021
…arwin-arm64

* upstream/master: (295 commits)
  Update urllib to 1.26.5. (elastic#26380)
  Update golang.org/x/crypto (elastic#26448)
  [Filebeat] Update Fortinet Ingest Pipeline (elastic#24816)
  Move parsers outside of filestream input so others can use them as well (elastic#26541)
  [Filebeat] Fix `threatintel.indicator.url.full` field not populating (elastic#26508)
  [Filebeat] Add network direction processor to Zeek and Suricata modules (elastic#24620)
  Logging code cleanup related to Nomad auto-discovery (elastic#26498)
  [Metricbeat] Add Couchbase's Sync Gateway module (elastic#25599)
  Refactor add_cloud_metadata to handle ECS fields easier (elastic#26438)
  [Elastic Agent] Improper casting of int64 (elastic#26520)
  [Elastic Agent] Enable configuring monitoring namespace (elastic#26439)
  [Heartbeat] configure permissions for synthetics config (elastic#26393)
  Osquerybeat: set the raw index name to supress the timestamp suffix (elastic#26545)
  [Heartbeat] add screenshots config to synthetics (elastic#26455)
  [Elastic Agent] Use http2 to connect to Fleet Server. (elastic#26474)
  Remove all docs about  Beats central management (elastic#26399)
  update data.json for gcp billing (elastic#26506)
  Skip x-pack metricbeat tests (elastic#26537)
  [Elastic Agent] Fix issue with FLEET_CA not being used with Fleet Server in container (elastic#26529)
  Add changelog entry for  elastic#26224 (elastic#26531)
  ...
P1llus pushed a commit that referenced this pull request Jun 29, 2021
…es (#24620) (#26568)

* Add network direction processor to zeek and suricata module

* Add Snort & Sonicwall

* update changelog

* use ES network_direction processor

* Revert "use ES network_direction processor"

This reverts commit 1c5722d9f1f6cd5370ef3f3d7a882e49731a4e3d.

* update docs with new variable

* Removed Snort and Sonicwall

* update docs

* missed one

Co-authored-by: Marius Iversen <marius.iversen@elastic.co>
(cherry picked from commit 9e670f7)

Co-authored-by: Alex Resnick <adr8292@gmail.com>
mdelapenya added a commit to mdelapenya/beats that referenced this pull request Jun 30, 2021
* master: (25 commits)
  fix: Force PLATFORMS environment variable when we build Elastic Agent dependencies on arm64 (elastic#26415)
  macos for metricbeat to run in the extended meta-stage (elastic#26573)
  Packaging: add arm7 platform in the main pipeline (elastic#26575)
  [Heartbeat] Skip flakey timer queue test (elastic#26592)
  Update to "read_pipeline" permission (elastic#26465) (elastic#26580)
  API keys do not reflect the need for read_pipeline (elastic#26466) (elastic#26582)
  Add Fleet agent.id to Agent monitoring data (elastic#26548)
  Add kinesis metricset (elastic#25989)
  Refactor of system/memory metricset (elastic#26334)
  Introduce httpcommon package in libbeat (add support for Proxy) (elastic#25219)
  [Filebeat] change multiline configuration in awss3 input to parsers (elastic#25873)
  docs: Hint for the error "Error extracting container id" (elastic#25824)
  [Docs] Fixed metricbeat redis exported field CPU descriptions (elastic#25846) (elastic#26496)
  Update urllib to 1.26.5. (elastic#26380)
  Update golang.org/x/crypto (elastic#26448)
  [Filebeat] Update Fortinet Ingest Pipeline (elastic#24816)
  Move parsers outside of filestream input so others can use them as well (elastic#26541)
  [Filebeat] Fix `threatintel.indicator.url.full` field not populating (elastic#26508)
  [Filebeat] Add network direction processor to Zeek and Suricata modules (elastic#24620)
  Logging code cleanup related to Nomad auto-discovery (elastic#26498)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.14.0 Automated backport with mergify
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants