Skip to content

PANW | Fix discrepancies in user fields and conform to ECS #4751

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
Merged
merged 9 commits into from
Dec 15, 2022
Merged

PANW | Fix discrepancies in user fields and conform to ECS #4751

merged 9 commits into from
Dec 15, 2022

Conversation

@kcreddy
Copy link
Contributor

@kcreddy kcreddy commented Dec 1, 2022 •
edited
Loading

What does this PR do?

  1. Currently, the traffic and threat pipeline are missing source.user.domain unlike other PANW pipelines. This PR parses and adds source.user.domain just like other pipelines in PANW. Similar to source.user field, this PR also adds destination.user fields which were absent.
  2. Conforming to ECS standards, adds user fields from source.user based on doc here
  3. Add destination.domain by parsing url.original field as url.domain is not present.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.
@kcreddy kcreddy added bug Something isn't working, use only for issues Integration:panw Palo Alto Next-Gen Firewall labels Dec 1, 2022
@elasticmachine
Copy link

elasticmachine commented Dec 1, 2022 •
edited
Loading

💚 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 preview

Expand to view the summary

Build stats

  • Start Time: 2022-12-15T07:53:13.158+0000

  • Duration: 17 min 0 sec

Test stats 🧪

Test Results
Failed 0
Passed 51
Skipped 0
Total 51

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.
@elasticmachine
Copy link

elasticmachine commented Dec 1, 2022 •
edited
Loading

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (1/1) 💚
Files 100.0% (15/15) 💚
Classes 100.0% (15/15) 💚
Methods 98.551% (68/69) 👎 -1.449
Lines 94.339% (3266/3462) 👎 -3.3
Conditionals 100.0% (0/0) 💚
@kcreddy kcreddy marked this pull request as ready for review December 5, 2022 11:33
@kcreddy kcreddy requested a review from a team as a code owner December 5, 2022 11:33
@elasticmachine
Copy link

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)
efd6
efd6 reviewed Dec 5, 2022
View reviewed changes
packages/panw/data_stream/panos/elasticsearch/ingest_pipeline/threat.yml Outdated
Comment on lines 291 to 296
- script:
if: "ctx.panw?.panos?.sub_type == 'url' && (ctx.panw?.panos?.misc instanceof String) && (ctx.panw.panos.misc.contains('/'))"
lang: painless
source: |-
String url_original = ctx.panw.panos.misc;
ctx.destination.domain = url_original.splitOnToken("/")[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this. We have the uri_parts processor above, but I did notice that that was failing because (presumably) the URL is missing a scheme in the inputs that we have here.
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 I have also seen when I was making changes that there might be a bug in uri_parts processor, or that scheme needs to be added to the logs for the processor to work properly.

But the user in SDH would like to either see this field destination.domain or remove it from the documentation (fields.yml) entirely. Splitting on panw.panos.misc when sub_type == 'url' condition is met, is one way to populate this field, which I have done here.

Should I be removing it from the documentation instead?
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably worth filing an issue against ES for the processor if there is a bug there. Can the correct behaviour of the uri_parts processor for the data that we see here be implemented in painless reasonably easily?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me give that a try in painless parsing some of the url fields that uri_parts was supposed to output correctly
Copy link
Contributor Author

@kcreddy kcreddy Dec 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Hey @efd6 , sorry I lost this while I was sick. I replaced uri_parts with script processor which can extract url fields for threat.yml pipeline.
@kcreddy kcreddy merged commit ec69a41 into elastic:main Dec 15, 2022
@elasticmachine
Copy link

Package panw - 3.4.2 containing this change is available at https://epr.elastic.co/search?package=panw
@kcreddy kcreddy deleted the panw_fix_ecs branch February 7, 2025 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working, use only for issues Integration:panw Palo Alto Next-Gen Firewall

3 participants

@kcreddy @elasticmachine @efd6