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

USTA v4 Threat Stream API Full Pack [Partner Contributer] #37475

Open
wants to merge 27 commits into
base: contrib/mdisec_ustav4
Choose a base branch
from

Conversation

mdisec
Copy link

@mdisec mdisec commented Nov 28, 2024

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: link to the issue

Description

Hello,

Prodaft's threat intelligence solution, named USTA, already has a "Pack" developed by a community contributor. However, this Pack is quite outdated and utilizes only a limited number of APIs.

Recently, we released a new series of APIs as part of an ongoing integration project with our clients who are using XSOAR. To provide a more comprehensive and robust solution, we decided to develop our own official Pack. This Pack offers significantly enhanced features and also includes all the existing functionalities provided in the USTA Pack created by the community member.

We have already completed the process of becoming a technology partner successfully. However, due to persistent technical issues with the partner portal, we have been unable to proceed with the official steps required to publish our Pack. Unfortunately, all the support tickets we submitted regarding these issues have ended without a resolution.

We started the development of the Pack while we were trying to tackle the issue on the partnerpurtal. Today, I believe the Pack is ready for your review and hopefully the release it on marketplace.

We've been told that there will be technical point of contact is going to be assigned us but as I described above, we are kind a lost. So I am reaching out to seek your guidance/assistance would be greatly appreciated.

Thank you in advance for your support.

Must have

  • Tests
  • Documentation

@Shellyber Shellyber added Partner-Approved Contribution Form Filled Whether contribution form filled or not. Partner labels Nov 29, 2024
@RotemAmit RotemAmit added Partner Support Level Indicates that the contribution is for Partner supported pack Contribution Thank you! Contributions are always welcome! labels Dec 1, 2024
@RotemAmit RotemAmit requested a review from idovandijk December 1, 2024 11:46
@MLainer1 MLainer1 self-requested a review December 1, 2024 12:08
@kobymeir kobymeir changed the base branch from master to contrib/mdisec_ustav4 December 2, 2024 09:51
@CLAassistant
Copy link

CLAassistant commented Dec 3, 2024

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Contributor

@MLainer1 MLainer1 left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution.
I've added my initial notes, make sure to go through them.

Can you add a bit about the feed? is it a full-fetch feed or incremental? it is not obvious from the code

Comment on lines +233 to +234
demisto.setLastRun(next_run)
demisto.incidents(incidents)
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
demisto.setLastRun(next_run)
demisto.incidents(incidents)
demisto.incidents(incidents)
demisto.setLastRun(next_run)

arg_name='First fetch time',
required=True
)
assert first_fetch_time
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
assert first_fetch_time

The arg_to_datetime function will fail if there is no value in the 'first_fetch' parameter

if last_fetch := last_run.get('last_fetch', None):
first_fetch_time = last_fetch

assert first_fetch_time
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
assert first_fetch_time

client.check_auth()
except DemistoException as e:
if 'Connection Timeout Error' in str(e):
return 'Connection error. Unable to connect to the USTA API! Make sure that your IP is whitelisted in the USTA.'
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
return 'Connection error. Unable to connect to the USTA API! Make sure that your IP is whitelisted in the USTA.'
raise ValueError('Connection error. Unable to connect to the USTA API! Make sure that your IP is whitelisted in the USTA.')

def stolen_credit_cards_search_api_request(self, **kwargs) -> dict:
params = assign_params(**kwargs)
headers = self._headers
demisto.debug(f'stolen_credit_cards_search_api_request: {params}')
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
demisto.debug(f'stolen_credit_cards_search_api_request: {params}')

Please remove before merging, as it can expose sensitive data


incidents: list[dict[str, Any]] = []

alerts = client.stolen_credit_cards_incidents(status=status, start=first_fetch_time, size=max_results)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use the last_fetch time as the start time for the fetch? this way we will not need to store the incidents ids in the last run

return 'ok'


def parse_malware_hashes(indicator: dict) -> dict:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it malware or file?

@samuelFain
Copy link
Contributor

samuelFain commented Dec 5, 2024

Hi @mdisec, thank you for your contribution.
We are working to publish the review on your PR, in the mean time, can you look at the pre-commit errors and fix those issues?
A few more questions:
Is the USTA Threat Stream IOC Feed an incremental or a full fetch feed? it is not very clear from the code.
Is it necessary to have 2 integrations USTA Account Takeover Prevention and USTA Stolen Credit Card? the code looks the same and I think you can merge both of the commands to one integration. Is there a reason from your side to create 2 different integrations?
Thank you, and let me know if you have any questions.

@mdisec
Copy link
Author

mdisec commented Dec 5, 2024

Hey @samuelFain ,

Thanks for the response. I believe that I've managed to sort out the pre-commit issue.

Our clients who requested us to bring the USTA Pack to Cortex also asked for the ability to manage the "Account Takeover Prevention" and "Stolen Credit Card" integrations separately. For instance, Client A only has access to the ATP module, while Client B uses only the Stolen Credit Card module. Meanwhile, Client C wants to manage these two modules individually, using separate configurations.

Additionally, we are working on a major feature release for the "Account Takeover Prevention" module of USTA, scheduled for the end of Q1. This release will require implementing new integrations and commands within the "Account Takeover Prevention" integration.

Given these requirements, I believe splitting the modules would provide better clarity and flexibility for our clients as well as us (pack maintainers)

@MLainer1 MLainer1 self-assigned this Dec 8, 2024
@MLainer1
Copy link
Contributor

Hi @mdisec, make sure to look at the review notes, and fix the pre-commit errors.
Let me know if you need any help.

Copy link
Contributor

@idovandijk idovandijk left a comment

Choose a reason for hiding this comment

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

Hi @mdisec, I appreciate your patience with the security review if this PR.
I'm happy to hear your story ends in this amazing contribution. I can see it going into the Marketplace with some tweaks!

I'm numbering my comments below for convenience purposes:

  1. Some incident fields are not using Title Case as per the naming convention - please change the names of the fields (the display name is different than the CLI name)
image Remember to also modify them in the mapper if they're there, and the layout. I think it's not very readable at the moment image
  1. Please also change the input names to use PascalCase (they should not have spaces in them). You may want to give a read of this page for general conventions https://xsoar.pan.dev/docs/playbooks/playbook-conventions
image Remember to also modify the affected tasks that use the input.
  1. Let's give a more descriptive description to the input - minimum password for what?

  2. There is a missing Else path for this task. Is it because it always must be "yes"? If so, why use a condition task?

image
  1. Please rename tasks to be more indicative of their logic. For example, Incident Password Length can be Check incident password length, and Check Password Length can be Check if the password meets the minimum length.

  2. Have you considered automating the password reset in the playbook? Since we're in XSIAM/XSOAR territory 😎

  3. Some of your files have a fromversion of 6.0.0, but the majority if your content is intended for 6.10. Is there a reason for the different fromversions? If not, please use the default one of the Demisto SDK.

Let me know if you're having any issues or need clarifications. Feel free to give me a ping on the Slack DFIR if needed too. Once these are implemented, let me know if you can do a short demo for the playbook, mapper, layout.

Thanks again, and I'm hoping to see this join the Marketplace quickly!

@MLainer1
Copy link
Contributor

Hi @mdisec, let me know if you need assistance completing the review notes.

@mdisec
Copy link
Author

mdisec commented Dec 31, 2024

Hi @mdisec, let me know if you need assistance completing the review notes.

Hey,

Thanks for all the update and reviews. Sorry for the late response due to my vacation and Christmast holiday.

I will fix everything within two weeks (which is our next sprint starting 2nd jan).

Happy new year to you all team 🎄♥️🙏

@mdisec
Copy link
Author

mdisec commented Jan 10, 2025

Hi @mdisec, let me know if you need assistance completing the review notes.

Hey @MLainer1 , I would like to ask you a question about an issue I've been trying to solve over the past two days. But I'm having an issue to submit the form at https://start.paloaltonetworks.com/join-our-slack-community to join a Slack channel. Form doesn't do anything even if I fill all the infos. Can you help me to have an invitation ? my email is mehmetprodaft[.]com

For some reason, I can't get pre-commit errors from CI/CD on my local site... Any idea ?

demisto-sdk pre-commit --no-docker --no-validate --input Packs/USTAv4

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contribution Form Filled Whether contribution form filled or not. Contribution Thank you! Contributions are always welcome! External PR Partner Support Level Indicates that the contribution is for Partner supported pack Partner Partner-Approved Security Review TIM Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants