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

AWS Security Hub parser add unique_id_from_tool and deduplication #4376

Merged
merged 30 commits into from
Jun 18, 2021

Conversation

nlandais
Copy link
Contributor

@nlandais nlandais commented Apr 27, 2021

  • Add name of the AWS resource in the title for ease of use and to help in the remediation process
  • Dedupe finding on unique_id_from_tool: use the SecHub finding id (value after the last / from the finding's ARN)

Nicolas Landais and others added 6 commits February 1, 2021 14:19
…ing DefectDojo in debug mode"

This reverts commit 081fefe.
- Add name of the AWS resource in the title for ease of use and to help 
in the remediation process
- Dedupe on unique_id_from_tool: use the SecHub finding id (value after 
the last / from the finding's ARN)
- Remove logiv aimed at closing the findings because it does actually 
work and competes for the close_old_findings logic
dojo/urls.py Outdated Show resolved Hide resolved
Copy link
Member

@valentijnscholten valentijnscholten left a comment

Choose a reason for hiding this comment

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

comments

dojo/tools/awssecurityhub/parser.py Outdated Show resolved Hide resolved
dojo/tools/awssecurityhub/parser.py Outdated Show resolved Hide resolved
@madchap madchap closed this May 3, 2021
@madchap madchap reopened this May 3, 2021
Copy link
Contributor

@damiencarol damiencarol left a comment

Choose a reason for hiding this comment

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

Could you add some checks in the unit tests that show the new data in the findings?

.gitmodules Outdated Show resolved Hide resolved
@nlandais nlandais requested a review from damiencarol May 4, 2021 17:51
@nlandais
Copy link
Contributor Author

Could you add some checks in the unit tests that show the new data in the findings?
@damiencarol Bonjour Damien, pourrais-tu elaborer sur la requete consistant a modifier les unittests pour le filtre d'ingestion des donnees provenant de AWS SecurityHub ?
The parser does not change much in functionality, the big change in this PR is the fact that the deduplication and filtering was not working properly when ingesting one report at a time from AWS SecurityHub.
The behavior observed prior to this change request was as follows:

  • When drilling down into to a finding , the "imilar findings" section showed all the other AWS Security Hub findings regardless of the AWS resource associated with the finding.
  • Each new imported finding marked the previous one as duplicate, which is consistent with the behavior reported in the first bullet point.
    In conclusion, there isn't any new data in the findings, the proposed change implements the behavior we come to expect with DefectDojo when it comes to aggregating similar findings, managing duplicate findings and closing stale/old findings.
    Feel free to DM me on the OWASP defectodjo Slack channel (handle: Nicolas Landais) to discuss further.
    -Merci

@nlandais nlandais changed the title Simplifying the parser for AWS Security Hub findings: AWS Security Hub parser setting changes May 20, 2021
@damiencarol damiencarol changed the title AWS Security Hub parser setting changes AWS Security Hub parser add unique_id_from_tool and deduplication May 22, 2021
Copy link
Contributor

@damiencarol damiencarol left a comment

Choose a reason for hiding this comment

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

As you are adding modifications to unique_id_from_tool attribute, could you add some checks in the unit tests that checks these new data?

dojo/tools/awssecurityhub/parser.py Outdated Show resolved Hide resolved
@damiencarol
Copy link
Contributor

@nlandais there is something out of scope in the PR. it seems that one sub-module for documentation is updated.

Nicolas Landais and others added 8 commits May 26, 2021 14:34
@nlandais nlandais requested a review from damiencarol June 2, 2021 02:14
dojo/tools/awssecurityhub/parser.py Outdated Show resolved Hide resolved
Re-instating the mitigated=mitigated, in the finding properties
Add ing boolean to finding property to indicate the mitigation status (when mitigation date is not NULL, mitigation status gets set to TRUE)
@damiencarol
Copy link
Contributor

@valentijnscholten could you take another look, @nlandais made a lot of modifications since the first reviews.

@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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

@valentijnscholten valentijnscholten merged commit de9eb94 into DefectDojo:dev Jun 18, 2021
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.

None yet

5 participants