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

DHS Feed v2 #19760

Merged
merged 132 commits into from
Dec 15, 2022
Merged

DHS Feed v2 #19760

merged 132 commits into from
Dec 15, 2022

Conversation

BEAdi
Copy link
Contributor

@BEAdi BEAdi commented Jun 26, 2022

Status

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

Related Issues

fixes: https://jira-hq.paloaltonetworks.local/browse/CIAC-4405, https://jira-hq.paloaltonetworks.local/browse/CIAC-4465

Description

Enhancement to support TAXII v2 for DHS feed.
Currently we don't have an instance (requested one here), so it is not tested yet.

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

@BEAdi BEAdi requested a review from yuvalbenshalom December 7, 2022 11:34
BEAdi added 5 commits December 7, 2022 13:59
# Conflicts:
#	Packs/ApiModules/ReleaseNotes/2_2_11.md
#	Packs/ApiModules/Scripts/TAXII2ApiModule/TAXII2ApiModule.py
#	Packs/ApiModules/Scripts/TAXII2ApiModule/TAXII2ApiModule_test.py
#	Packs/FeedTAXII/ReleaseNotes/1_1_16.md
Copy link
Contributor

@yuvalbenshalom yuvalbenshalom left a comment

Choose a reason for hiding this comment

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

Good job.
See my comments.

example, 1 minute, 12 hour, 24 days. Limited to 48 hours.
name: added_after
description: Allows you to test your feed and to make sure you can fetch indicators
successfully. Due to API limitations this command may take a long time to run, make sure 'execution-timeout' argument is increased.
Copy link
Contributor

Choose a reason for hiding this comment

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

we should direct to an explanation in the readme to how to configure "execution-timeout".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

- defaultValue: '10'
description: Maximum number of indicators to return. Default is 10. If you are increasing this value, make sure 'execution-timeout' argument is also increased.
name: limit
- defaultValue: '24 hours'
Copy link
Contributor

Choose a reason for hiding this comment

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

check that yo return in the command the same error of the test module:

  1. the value is not parsed correctly
  2. the value is more than 48 hours, and the error should explain it cannot be more than 48 hour due to DHS limitations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

type: 19
- additionalinfo: 'The time interval for the first fetch (retroactive) in the following format: <number> <time
unit> of type minute/hour/day. For example, 1 minute, 12 hour. Limited to 48 hours.'
defaultvalue: 24 hours
Copy link
Contributor

Choose a reason for hiding this comment

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

this value should be checked in the test module and should return areadeble rare in the cases:

  1. the value is not parsed correctly
  2. the value is more than 48 hours, and the error should explain it cannot be more than 48 hours due to DHS limitations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

''' COMMAND FUNCTIONS '''


def command_test_module(client: Taxii2FeedClient):
Copy link
Contributor

Choose a reason for hiding this comment

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

checks we should have in they order:

  1. check the "added after" parameter(see my comment in thee yml.)
  2. get collections(already exist)
  3. fetch indicators with added after 6 hours OR the added after param, the shorter of the 2)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed

@@ -93,43 +93,21 @@ configuration:
required: false
type: 19
- additionalinfo: 'The time interval for the first fetch (retroactive) in the following format: <number> <time
unit> of type minute/hour/day/year. For example, 1 minute, 12 hour.'
unit> of type minute/hour/day. For example, 1 minute, 12 hour. Limited to 48 hours.'
Copy link
Contributor

Choose a reason for hiding this comment

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

If it is more than 1 (hour, minute, day) will it be hours/minutes/days? For example 3 minutes. 12 hours.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, both singular and plural are valid. If the value will be more than 48 hours (or 2 days), an error will be raised.

@ShirleyDenkberg
Copy link
Contributor

@yuvalbenshalom @DeanArbel @Ni-Knight Doc review completed.

@xsoar-bot
Copy link
Contributor

@BEAdi BEAdi merged commit fde5d66 into master Dec 15, 2022
@BEAdi BEAdi deleted the DHS_FeedV2 branch December 15, 2022 14:35
efelmandar pushed a commit that referenced this pull request Jan 4, 2023
xsoar-bot pushed a commit to xsoar-contrib/content that referenced this pull request Jan 11, 2023
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.

7 participants