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

[Filebeat] panos config option to set internal/external zones #22998

Merged
merged 3 commits into from
Dec 9, 2020
Merged

[Filebeat] panos config option to set internal/external zones #22998

merged 3 commits into from
Dec 9, 2020

Conversation

leehinman
Copy link
Contributor

What does this PR do?

adds configuration option to set internal and external zones.

  • default internal zone is "trust"
  • default external zone is "untrust"

Why is it important?

internal and external zones are used to determine network.direction.
Previously static values of "trust" and "untrust" were used, but the
zone names can be controlled by the user.

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.

How to test this PR locally

TESTING_FILEBEAT_MODULES=panw mage -v pythonIntegTest

Related issues

@leehinman
panos config option to set internal/external zones
7b4fd2f 
- default internal zone is "trust"
- default external zone is "untrust"
- allows for user to define zones for determining network.direction

Relates elastic#21674
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Dec 8, 2020
@leehinman leehinman added enhancement Filebeat Filebeat needs_backport PR is waiting to be backported to other branches. Team:Security-External Integrations labels Dec 8, 2020
@elasticmachine
Copy link
Collaborator

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

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Dec 8, 2020
@leehinman leehinman mentioned this pull request Dec 8, 2020
Closed
andrewkroh
andrewkroh approved these changes Dec 9, 2020
View reviewed changes
CHANGELOG.next.asciidoc Outdated
@@ -738,6 +738,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Add support for UNIX datagram sockets in `unix` input. {issues}18632[18632] {pull}22699[22699]
- Add new httpjson input features and mark old config ones for deprecation {pull}22320[22320]
- Add logic for external network.direction in sophos xg fileset {pull}22973[22973]
- Add configuration option to set external and internal networks for panw panos fileset {pull}XXXXX[XXXXX]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Add configuration option to set external and internal networks for panw panos fileset {pull}XXXXX[XXXXX]
- Add configuration option to set external and internal networks for panw panos fileset {pull}22998[22998]

x-pack/filebeat/module/panw/panos/ingest/pipeline.yml Outdated
@@ -134,24 +134,23 @@ processors:
- set:
field: network.direction
value: inbound
if: 'ctx?.panw?.panos?.type == "TRAFFIC" && ctx?.panw?.panos?.source?.zone == "untrust" && ctx?.panw?.panos?.destination?.zone == "trust"'
if: 'ctx?.panw?.panos?.type == "TRAFFIC" && ctx?._temp_?.external_zones != null && ctx?._temp_?.internal_zones != null && ctx?.observer?.ingress?.zone != null && ctx?.observer?.egress?.zone != null && ctx._temp_.external_zones.contains(ctx.observer.ingress.zone) && ctx._temp_.internal_zones.contains(ctx.observer.egress.zone)'
Copy link
Member

Choose a reason for hiding this comment

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

That's a lot to take in, but it looks right. I found it easier to read like this since I can see all the logic at once.

Suggested change
if: 'ctx?.panw?.panos?.type == "TRAFFIC" && ctx?._temp_?.external_zones != null && ctx?._temp_?.internal_zones != null && ctx?.observer?.ingress?.zone != null && ctx?.observer?.egress?.zone != null && ctx._temp_.external_zones.contains(ctx.observer.ingress.zone) && ctx._temp_.internal_zones.contains(ctx.observer.egress.zone)'
if: >
ctx?.panw?.panos?.type == "TRAFFIC" &&
ctx?._temp_?.external_zones != null &&
ctx?._temp_?.internal_zones != null &&
ctx?.observer?.ingress?.zone != null &&
ctx?.observer?.egress?.zone != null &&
ctx._temp_.external_zones.contains(ctx.observer.ingress.zone) &&
ctx._temp_.internal_zones.contains(ctx.observer.egress.zone)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah those were awful, see if the new formatting is easier on the eyes.

leehinman and others added 2 commits December 8, 2020 20:52
@leehinman
Fixes
a2b82fb 
- changelog pr number
- improve readability of network.direction if statements
@leehinman
Merge branch 'master' into 21674_panw_network_direction
aaa35fc
@elasticmachine
Copy link
Collaborator

💚 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: Pull request #22998 updated

  • Start Time: 2020-12-09T02:56:28.564+0000

  • Duration: 57 min 33 sec

Test stats 🧪

Test Results
Failed 0
Passed 2420
Skipped 259
Total 2679

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 2420
Skipped 259
Total 2679

@leehinman leehinman merged commit 7b7bbe9 into elastic:master Dec 9, 2020
@leehinman leehinman deleted the 21674_panw_network_direction branch December 9, 2020 17:09
@leehinman leehinman mentioned this pull request Dec 9, 2020
Merged
5 tasks
leehinman added a commit to leehinman/beats that referenced this pull request Dec 9, 2020
@leehinman
[Filebeat] panos config option to set internal/external zones (elasti…
1d05ea6 
…c#22998)

* panos config option to set internal/external zones

- default internal zone is "trust"
- default external zone is "untrust"
- allows for user to define zones for determining network.direction

Relates elastic#21674

(cherry picked from commit 7b7bbe9)
@leehinman leehinman added v7.11.0 and removed needs_backport PR is waiting to be backported to other branches. labels Dec 9, 2020
leehinman added a commit that referenced this pull request Dec 9, 2020
@leehinman
[Filebeat] panos config option to set internal/external zones (#22998) (
a5db4ac 
#23037)

* panos config option to set internal/external zones

- default internal zone is "trust"
- default external zone is "untrust"
- allows for user to define zones for determining network.direction

Relates #21674

(cherry picked from commit 7b7bbe9)
This was referenced Dec 10, 2020
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewkroh andrewkroh andrewkroh approved these changes

Assignees
No one assigned
Labels
enhancement Filebeat Filebeat v7.11.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@leehinman @elasticmachine @andrewkroh