Skip to content

Store fingerprint from bearer in unique_id_from_tool#12346

Merged
mtesauro merged 3 commits into
DefectDojo:devfrom
wolframite:enhancement/add-fingerprint-to-bearer-parser
May 23, 2025
Merged

Store fingerprint from bearer in unique_id_from_tool#12346
mtesauro merged 3 commits into
DefectDojo:devfrom
wolframite:enhancement/add-fingerprint-to-bearer-parser

Conversation

@wolframite

@wolframite wolframite commented Apr 29, 2025 •
edited by valentijnscholten
Loading

Copy link
Copy Markdown
Contributor

Description

I'm storing the unique fingerprint from bearer which is always present in the scan file in the field unique_id_from_tool. I also changed the deduplication algorithm setting in dojo/settings/settings.dist.py from DEDUPE_ALGO_HASH_CODE to DEDUPE_ALGO_UNIQUE_ID_FROM_TOOL_OR_HASH_CODE

Test results

There is no existing unit test to extend.

Documentation

Updating the documentation should not be necessary for this tiny change

Checklist

This checklist is for your information.

  • Make sure to rebase your PR against the very latest dev.
  • Features/Changes should be submitted against the dev.
  • Bugfixes should be submitted against the bugfix branch.
  • Give a meaningful name to your PR, as it may end up being used in the release notes.
  • Your code is flake8 compliant.
  • Your code is python 3.11 compliant.
  • If this is a new feature and not a bug fix, you've included the proper documentation in the docs at https://github.com/DefectDojo/django-DefectDojo/tree/dev/docs as part of this PR.
  • Model changes must include the necessary migrations in the dojo/db_migrations folder.
  • Add applicable tests to the unit tests.
  • Add the proper label to categorize your PR.

Extra information

This is my first PR for defect dojo, please let me know if I made a mistake. I'll fix it right away.

@github-actions github-actions Bot added settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR parser labels Apr 29, 2025
@dryrunsecurity

dryrunsecurity Bot commented Apr 29, 2025

Copy link
Copy Markdown

DryRun Security

No security concerns detected in this pull request.

All finding details can be found in the DryRun Security Dashboard.

@valentijnscholten

valentijnscholten commented Apr 30, 2025

Copy link
Copy Markdown
Member

Thanks for your PR. Could you add some asserts to the unit tests on this field?

I also notice the old_fingerprint field in the reports. Do you know how that works? Just need to make sure the fingerprint is unique/constant for each finding.

@wolframite

wolframite commented Apr 30, 2025

Copy link
Copy Markdown
Contributor Author

Checking the source where the fingerprints are generated it looks like they're only unique within one scan. If the code changes and a weakness is fixed, then the fingerprints will be re-used.

The fingerprint is the md5 hash of the filename and the rule id suffixed with the index of findings of this specific file. The old fingerprint has the same hash, but is suffixed with the index of all findings. I assume that this was just the old way of calculating the fingerprint.

This is a bug in bearer, because if you ignore the fingerprint with the index 2 and then fix the weakness with the index 0, the weakness which previously had index 3 will be ignored now instead.

This sounds like we should not use the fingerprint to track findings across multiple scans. That means I should revert deduplication setting back to DEDUPE_ALGO_HASH_CODE right?

@Maffooch

Maffooch commented Apr 30, 2025

Copy link
Copy Markdown
Contributor

This sounds like we should not use the fingerprint to track findings across multiple scans. That means I should revert deduplication setting back to DEDUPE_ALGO_HASH_CODE right?

Yep, you've got the right idea 😉 if the current hash code fields of title and severity are fitting the need we may be okay here

@wolframite

wolframite commented May 1, 2025

Copy link
Copy Markdown
Contributor Author

If I remember correctly the title is the rule id + file name + line number, so this would be "uniquer".

@wolframite wolframite force-pushed the enhancement/add-fingerprint-to-bearer-parser branch from b262c3c to 97ff876 Compare May 1, 2025 15:53
@github-actions github-actions Bot removed the settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR label May 1, 2025
Maffooch
Maffooch approved these changes May 8, 2025
View reviewed changes

@Maffooch Maffooch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be strictly additive without change to dedupe settings for existing folks, so this seems safe

valentijnscholten
valentijnscholten reviewed May 8, 2025
View reviewed changes
Comment thread dojo/tools/bearer_cli/parser.py
@wolframite wolframite force-pushed the enhancement/add-fingerprint-to-bearer-parser branch from 97ff876 to 5beb6db Compare May 16, 2025 09:03
@github-actions github-actions Bot added the settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR label May 16, 2025
valentijnscholten
valentijnscholten reviewed May 16, 2025
View reviewed changes
Comment thread dojo/tools/bearer_cli/parser.py
@valentijnscholten valentijnscholten mentioned this pull request May 16, 2025
Merged
@valentijnscholten valentijnscholten added this to the 2.47.0 milestone May 17, 2025
mtesauro
mtesauro approved these changes May 18, 2025
View reviewed changes

@mtesauro mtesauro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved

@mtesauro mtesauro merged commit 24ac437 into DefectDojo:dev May 23, 2025
77 checks passed
xansec pushed a commit to xansec/django-DefectDojo that referenced this pull request Jun 18, 2025
@wolframite @valentijnscholten @xansec
Store fingerprint from bearer in unique_id_from_tool (DefectDojo#12346)
d949ef5 
* Store fingerprint from bearer in unique_id_from_tool

* Update dojo/tools/bearer_cli/parser.py

* Updated bearer unit test to assert on the fingerprint being set into unique_id_from_tool

---------

Co-authored-by: valentijnscholten <valentijnscholten@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Maffooch Maffooch Maffooch approved these changes

@mtesauro mtesauro mtesauro approved these changes

@valentijnscholten valentijnscholten valentijnscholten approved these changes

+1 more reviewer

@hblankenship hblankenship hblankenship approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

parser settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR unittests

Projects

None yet

Milestone

2.47.0

Development

Successfully merging this pull request may close these issues.

5 participants

@wolframite @valentijnscholten @Maffooch @mtesauro @hblankenship