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

extend AWS prowler v3 parser #10372

Merged
merged 30 commits into from
Jul 3, 2024

Conversation

kagahd
Copy link
Contributor

@kagahd kagahd commented Jun 10, 2024

Instead to add a new prowler v4 parser as suggested in PR #10338 this PR extends the existing prowler v3 parser to be able to import also prowler v4 reports in json-ocsf format.
This approach solves the deduplication problems between prowler v3 and v4 reports, described in PR #10338.

Copy link

dryrunsecurity bot commented Jun 10, 2024

Hi there 👋, @DryRunSecurity here, below is a summary of our analysis and findings.

DryRun Security Status Findings
Server-Side Request Forgery Analyzer 0 findings
Configured Codepaths Analyzer 0 findings
IDOR Analyzer 0 findings
Sensitive Files Analyzer 0 findings
SQL Injection Analyzer 0 findings
Authn/Authz Analyzer 0 findings
Secrets Analyzer 0 findings

Note

🟢 Risk threshold not exceeded.

Change Summary (click to expand)

The following is a summary of changes in this pull request made by me, your security buddy 🤖. Note that this summary is auto-generated and not meant to be a definitive list of security issues but rather a helpful summary from a security perspective.

Summary:

The code changes in this pull request are focused on enhancing the integration between the DefectDojo application security tool and the AWS Prowler security assessment tool. The changes include the following key aspects:

  1. AWS Prowler V3 and V4 Parser Support: The code introduces a new parser class, AWSProwlerV3plusParser, that can handle both Prowler V3 JSON and Prowler V4 JSON-OCSF file formats. This ensures that DefectDojo can support the latest versions of the Prowler tool and the security findings they produce.

  2. Parsing Logic and Error Handling: The AWSProwlerV3plusParser class delegates the parsing to specialized parsers for Prowler V3 and V4 formats, ensuring a modular and extensible design. The code also includes error handling to handle unknown file formats.

  3. Security Best Practices: The code changes highlight the importance of following security best practices, such as the principle of least privilege, when configuring IAM roles and policies in AWS environments. The documentation and sample files provide guidance on identifying and addressing issues like overly permissive trust relationships and the use of highly privileged IAM policies.

  4. Unit Testing: The pull request includes the addition of several unit tests to verify the behavior of the AWS Prowler V3+ parser in different scenarios, including handling files with no vulnerabilities, a single vulnerability, and multiple vulnerabilities. This helps to ensure the reliability and robustness of the parser implementation.

Overall, these changes enhance the security assessment capabilities of the DefectDojo platform by providing seamless integration with the AWS Prowler tool and promoting the adoption of security best practices in AWS environments. As an application security engineer, I would recommend thoroughly reviewing the changes and ensuring that the parser implementation is secure and maintains the integrity of the security data it processes.

Files Changed:

  1. unittests/scans/aws_prowler_v3plus/no_vuln.ocsf.json: This file is an empty JSON array, which appears to be a sample or test file for the AWS Prowler V3+ functionality. The creation of this file does not introduce any obvious security vulnerabilities.

  2. docs/content/en/integrations/parsers/file/aws_prowler_v3plus.md: This file updates the documentation for the AWS Prowler V3 parser in the DefectDojo application. It provides information on the expected input file formats (Prowler V3 JSON and Prowler V4 JSON-OCSF) and the security best practices related to IAM roles and policies.

  3. dojo/tools/aws_prowler_v3plus/parser.py: This file introduces the AWSProwlerV3plusParser class, which can handle both Prowler V3 and V4 input file formats. The changes focus on adding support for the different file formats and delegating the parsing logic to specialized parsers.

  4. dojo/tools/aws_prowler_v3plus/prowler_v3.py: This file contains the implementation of the AWSProwlerV3Parser class, which is responsible for processing the Prowler V3 JSON data and creating Finding objects.

  5. unittests/scans/aws_prowler_v3plus/one_vuln.ocsf.json and unittests/scans/aws_prowler_v3plus/many_vuln.ocsf.json: These files contain sample Prowler V4 JSON-OCSF data with one and multiple security vulnerabilities, respectively. They are used for testing the parser's functionality.

  6. dojo/tools/aws_prowler_v3plus/prowler_v4.py: This file contains the implementation of the AWSProwlerV4Parser class, which is responsible for processing the Prowler V4 JSON-OCSF data and creating Finding objects.

  7. unittests/tools/test_aws_prowler_v3plus_parser.py: This file contains the unit tests for the AWSProwlerV3plusParser class, ensuring the parser's behavior in different scenarios, including handling files with no vulnerabilities, a single vulnerability, and multiple vulnerabilities.

Powered by DryRun Security

dojo/tools/aws_prowler_v3/parser.py Outdated Show resolved Hide resolved
dojo/tools/aws_prowler_v3/parser.py Outdated Show resolved Hide resolved
@mtesauro
Copy link
Contributor

@kagahd Once the files are separated, this looks ready to approve. Nice work so far 👍

@kagahd
Copy link
Contributor Author

kagahd commented Jun 11, 2024

As happened also in PR #10372, only 1 unit test in alpine and also in debian failed, but this is not due to my code contribution.
Related logs:

2024-06-11T08:19:36.4619889Z uwsgi-1  | FAIL: test_delete_preview (unittests.test_rest_framework.TestsTest.test_delete_preview)
2024-06-11T08:19:36.4620600Z uwsgi-1  | ----------------------------------------------------------------------
2024-06-11T08:19:36.4621097Z uwsgi-1  | Traceback (most recent call last):
2024-06-11T08:19:36.4621664Z uwsgi-1  |   File "/app/unittests/test_rest_framework.py", line 173, in wrapper
2024-06-11T08:19:36.4622208Z uwsgi-1  |     f(self, *args, **kwargs)
2024-06-11T08:19:36.4622976Z uwsgi-1  |   File "/app/unittests/test_rest_framework.py", line 544, in test_delete_preview
2024-06-11T08:19:36.4623853Z uwsgi-1  |     self.assertEqual(self.deleted_objects, len(response.data['results']), response.content[:1000])
2024-06-11T08:19:36.4628382Z uwsgi-1  | AssertionError: 5 != 18 : b'{"count":18,"next":null,"previous":null,"results":[{"model":"Test","id":3,"name":"ZAP Scan"},{"model":"Finding","id":3,"name":"High Impact Test Finding"},{"model":"Finding_found_by","id":3500,"name":"Finding_found_by object (3500)"},{"model":"Finding","id":2,"name":"High Impact Test Finding"},{"model":"Endpoint_Status","id":1,"name":"\'High Impact Test Finding\' on \'ftp://localhost\'"},{"model":"Finding_found_by","id":3499,"name":"Finding_found_by object (3499)"},{"model":"Finding","id":4,"name":"High Impact Test Finding"},{"model":"Finding_found_by","id":3501,"name":"Finding_found_by object (3501)"},{"model":"Finding","id":5,"name":"High Impact Test Finding"},{"model":"Finding_found_by","id":3502,"name":"Finding_found_by object (3502)"},{"model":"JIRA_Issue","id":2,"name":"Security How-to | Finding: High Impact Test Finding, ID: 5 | Jira Key: 222"},{"model":"Finding","id":6,"name":"High Impact Test Finding"},{"model":"Finding_found_by","id":3503,"name":"Finding_found_by object (3503)"},{'
2024-06-11T08:19:36.4632367Z uwsgi-1  | 
2024-06-11T08:19:36.4632761Z uwsgi-1  | ----------------------------------------------------------------------
2024-06-11T08:19:36.4633235Z uwsgi-1  | Ran 2693 tests in 338.437s
2024-06-11T08:19:36.4633558Z uwsgi-1  | 
2024-06-11T08:19:36.4633852Z uwsgi-1  | FAILED (failures=1, skipped=472)

@kagahd
Copy link
Contributor Author

kagahd commented Jun 11, 2024

@kagahd Once the files are separated, this looks ready to approve. Nice work so far 👍

Thanks @mtesauro! The files are separated now.

@mtesauro
Copy link
Contributor

@kagahd
Just re-kicked the REST tests - we've had issues with them lately. There's a PR to fix this that you probably should wait till it's merged and rebase based on the dev branch after that PR is merged.

Those rest tests (prior to that PR) are super flaky and fail intermittently after moving them to Postgres in preparation for deprecating MySQL.

The PR that fixes it and to watch is #10387

Signed-off-by: DefectDojo <[email protected]>
@kagahd
Copy link
Contributor Author

kagahd commented Jun 13, 2024

you probably should wait till it's merged and rebase based on the dev branch after that PR is merged.

Hi @mtesauro , I followed your suggestion but now, the Configured Codepaths Analyzer is failing which was run successfully before the rebase on the dev branch.

@mtesauro
Copy link
Contributor

About the Code Path Analyzer see:
#10384 (comment)

Copy link
Contributor

@mtesauro mtesauro left a comment

Choose a reason for hiding this comment

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

Approved

Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@github-actions github-actions bot removed the helm label Jun 18, 2024
Copy link
Contributor

github-actions bot commented Jul 2, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@kagahd
Copy link
Contributor Author

kagahd commented Jul 2, 2024

It's more than 3 weeks ago that I opened this pull request. If you don't merge it, merge conflicts will accumulate and will be harder to resolve.
DefectDojo team, are you really interested in contributions or not?

@mtesauro
Copy link
Contributor

mtesauro commented Jul 2, 2024

@kagahd

It's more than 3 weeks ago that I opened this pull request. If you don't merge it, merge conflicts will accumulate and will be harder to resolve. DefectDojo team, are you really interested in contributions or not?

Yes, we're always happy to have PRs from the community. It's summer time (at least in the northern hemisphere) and the combination of:

  • several maintainers going on holiday recently
  • several large PRs that were queued up for 2.36.0 release (which happened yesterday)
  • preparation for deprecation of MySQL and RabbitMQ (see here especially this one)
  • something strange happening with Github caches for GH Actions not being correctly evicted in a timely manner and slowing down our GH Action runs
  • I was at OWASP's AppSec Lisbon presenting on DefectDojo last week so am just now catching up on PRs myself

has kept the maintainers busier that normal in June.

You've got the needed 4 approvals, if you can fix this one last merge conflict, I'm ready and very willing to merge this PR.

# Conflicts:
#	unittests/tools/test_aws_prowler_v3_parser.py
Copy link
Contributor

github-actions bot commented Jul 3, 2024

Conflicts have been resolved. A maintainer will review the pull request shortly.

@kagahd
Copy link
Contributor Author

kagahd commented Jul 3, 2024

Thanks for your explanations and willingness to merge the PR @mtesauro!
It seems that all is green now, so it's ready to be merged 🎉

@Maffooch Maffooch merged commit 8b9f9a4 into DefectDojo:dev Jul 3, 2024
125 checks passed
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.

6 participants