-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Enhance ASFF parser to more resemble the SecurityHub parser #10176
base: dev
Are you sure you want to change the base?
Enhance ASFF parser to more resemble the SecurityHub parser #10176
Conversation
Hi there 👋, @DryRunSecurity here, below is a summary of our analysis and 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 change in the provided patch is related to the While the changes are not directly related to security vulnerabilities or risks, there are a few points worth considering. The code handles the case where the "Resources" field is missing, which is a good practice to ensure the parser can handle a variety of ASFF data formats. Additionally, the code already includes logic to extract endpoint information (EC2 instance IP addresses) from the "Resources" field and associate them with the Files Changed:
Powered by DryRun Security |
89459e9
to
f3b5770
Compare
worker_processes defaulting to 1 leads to sub-optimal performance. Changing this to 'auto' allows for more performance out-of-the-box at no detriment to anything else. This will help newer users from experiencing performance limitations due to the 1 worker_process, when most modern systems have multiple cores to take advantage of.
My goal is to make the ASFF findings more actionable than what is currently being added into DefectDojo fields. In the other SecurityHub parser it was enhanced to add ResourceId into the description. I want to also do this with the ASFF findings, in a similar way. Also, in my opinion (and this change can be removed no worries) but - vuln_id_from_tool is more appropriate for matching to the security hub finding Id/Arn than the 'unique_id_from_tool'. unique_id_from_tool shouldn't match a vuln_id (which is the finding_id from AWS) - but instead maybe could be matched to a ResourceId. Ideally this would be a host or an endpoint, but not always will a Finding have a host associated with it. HOWEVER, a finding will >always< have a ResourceId and ARN from AWS. So other than adding into the Description, I'm not sure which other field makes sense to add this to. I can remove the unique_id_from_tool and vuln_id_from_tool changes if those are not ideal.
Remove accidental merge of different branch
f3b5770
to
4bf505e
Compare
My goal is to make the ASFF findings more actionable than what is currently being added into DefectDojo fields. In the other SecurityHub parser it was enhanced to add ResourceId into the description. I want to also do this with the ASFF findings, in a similar way.
Also, in my opinion (and this change can be removed no worries) but - vuln_id_from_tool is more appropriate for matching to the security hub finding Id/Arn than the 'unique_id_from_tool'.
unique_id_from_tool shouldn't match a vuln_id (which is the finding_id from AWS) - but instead maybe could be matched to a ResourceId. Ideally this would be a host or an endpoint, but not always will a Finding have a host associated with it. HOWEVER, a finding will >always< have a ResourceId and ARN from AWS. So other than adding into the Description, I'm not sure which other field makes sense to add this to. I can remove the unique_id_from_tool and vuln_id_from_tool changes if those are not ideal.
We are narrowing the scope of acceptable enhancements to DefectDojo in preparation for v3. Learn more here:
https://github.com/DefectDojo/django-DefectDojo/blob/master/readme-docs/CONTRIBUTING.md
Description
Describe the feature / bug fix implemented by this PR.
If this is a new parser, the parser guide may be worth (re)reading.
Test results
Ideally you extend the test suite in
tests/
anddojo/unittests
to cover the changed in this PR.Alternatively, describe what you have and haven't tested.
Documentation
Please update any documentation when needed in the documentation folder)
Checklist
This checklist is for your information.
dev
.dev
.bugfix
branch.Extra information
Please clear everything below when submitting your pull request, it's here purely for your information.
Moderators: Labels currently accepted for PRs:
Contributors: Git Tips
Rebase on dev branch
If the dev branch has changed since you started working on it, please rebase your work after the current dev.
On your working branch
mybranch
:In case of conflict:
When everything's fine on your local branch, force push to your
myOrigin
remote:To cancel everything:
Squashing commits
pick
byfixup
on the commits you want squashed outpick
byreword
on the first commit if you want to change the commit messageForce push to your
myOrigin
remote: