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

[PAN-OS] Add Ipv4/IPv6 sinkhole arguments #27622

Merged
merged 7 commits into from
Jun 28, 2023

Conversation

samuelFain
Copy link
Contributor

@samuelFain samuelFain commented Jun 21, 2023

Contributing to Cortex XSOAR Content

Make sure to register your contribution by filling the contribution registration form

The Pull Request will be reviewed only after the contribution registration form is filled.

Status

  • In Progress
  • Ready
  • In Hold - (Reason for hold)

Related Issues

fixes: CIAC-7081

Description

PAN-OS's pan-os-apply-dns-signature-policy command returns error when sinkhole action is used, but not currently set for a given policy.
This PR adds the missing default IPv4/IPv6 sinkhole to http request parameters, and the option to set non-default IPv4/IPv6 sinkhole addresses with matching command arguments.

Screenshots

Paste here any images that will help the reviewer

Minimum version of Cortex XSOAR

  • 6.0.0
  • 6.1.0
  • 6.2.0
  • 6.5.0

Does it break backward compatibility?

  • Yes
    • Further details:
  • No

Must have

  • Tests
  • Documentation

@samuelFain samuelFain changed the title Add Ipv4/IPv6 arguments [PAN-OS] Add Ipv4/IPv6 sinkhole arguments Jun 22, 2023
@samuelFain samuelFain marked this pull request as ready for review June 22, 2023 13:00
@samuelFain samuelFain requested a review from GuyAfik as a code owner June 22, 2023 13:00
@samuelFain samuelFain self-assigned this Jun 22, 2023
@samuelFain samuelFain requested a review from ShacharKidor June 22, 2023 13:00
@@ -7275,6 +7277,7 @@ def apply_dns_signature_policy_command(args: dict) -> CommandResults:
f'<entry name="{edl}"><packet-capture>{packet_capture}</packet-capture>'
f'<action><{action}/></action></entry>'
f'</lists>'
f'<sinkhole><ipv4-address>{ipv4_address}</ipv4-address><ipv6-address>{ipv6_adderss}</ipv6-address></sinkhole>'
Copy link
Contributor

Choose a reason for hiding this comment

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

@samuelFain don't you need this only in case the action is sinkhole? and not in every case?

Copy link
Contributor Author

@samuelFain samuelFain Jun 22, 2023

Choose a reason for hiding this comment

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

Using the PAN-OS debugger, it seems that the default IPv4/IPv6 sinkhole values are sent even when a different action is used, not just sinkhole.
My guess is that these values are always sent by default, but only used when action=sinkhole.
It does not seem to interfere with any working pan-os-apply-dns-signature-policy command variations.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a TPB to make sure that all other commands pass successfully with the additional parameters' default values?

Copy link
Contributor Author

@samuelFain samuelFain Jun 26, 2023

Choose a reason for hiding this comment

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

@ShacharKidor
There is a TPB that executing this command, but it is not among the TPBs that run during the build. I ran it separately and it passed successfully.
Also, it's important to note that these default values are sent with every command use-case, and they do not impact any other use-case of this command except when action=sinkhole is used. We have already tested this specific case to ensure its functionality.

@samuelFain samuelFain requested a review from GuyAfik June 25, 2023 12:47
Copy link
Contributor

@ShacharKidor ShacharKidor left a comment

Choose a reason for hiding this comment

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

Nice!
Let's wait for Guy's approval.


##### Palo Alto Networks PAN-OS

- Fixed an issue where ***pan-os-apply-dns-signature-policy*** command failed when used the sinkhole action due to lack of default IPv4/Ipv6 sinkhole IP addresses.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Fixed an issue where ***pan-os-apply-dns-signature-policy*** command failed when used the sinkhole action due to lack of default IPv4/Ipv6 sinkhole IP addresses.
- Fixed an issue where the ***pan-os-apply-dns-signature-policy*** command failed when using the sinkhole action due to lack of default IPv4/Ipv6 sinkhole IP addresses.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented suggestion.

@@ -7275,6 +7277,7 @@ def apply_dns_signature_policy_command(args: dict) -> CommandResults:
f'<entry name="{edl}"><packet-capture>{packet_capture}</packet-capture>'
f'<action><{action}/></action></entry>'
f'</lists>'
f'<sinkhole><ipv4-address>{ipv4_address}</ipv4-address><ipv6-address>{ipv6_adderss}</ipv6-address></sinkhole>'
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a TPB to make sure that all other commands pass successfully with the additional parameters' default values?

Copy link
Contributor

@GuyAfik GuyAfik left a comment

Choose a reason for hiding this comment

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

@samuelFain nice!

@samuelFain samuelFain merged commit 3d0c10f into master Jun 28, 2023
@samuelFain samuelFain deleted the bugfix/CIAC-7081/pan-os-apply-dns-signature-policy branch June 28, 2023 11:15
MosheEichler pushed a commit that referenced this pull request Jul 2, 2023
* Add Ipv4/IPv6 arguments

* Update ipv4/ipv6 yml description

* Update release notes

* Add IPv4/IPv6 to panorama_apply_dns_command UT

* Implemented RN suggestion; Fix linting issue
xsoar-bot pushed a commit to xsoar-contrib/content that referenced this pull request Jul 26, 2023
* Add Ipv4/IPv6 arguments

* Update ipv4/ipv6 yml description

* Update release notes

* Add IPv4/IPv6 to panorama_apply_dns_command UT

* Implemented RN suggestion; Fix linting issue
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.

3 participants