Skip to content

[flake8-bandit] Update insecure hash functions (S324)#16580

Closed
VascoSch92 wants to merge 2 commits intoastral-sh:mainfrom
VascoSch92:S324
Closed

[flake8-bandit] Update insecure hash functions (S324)#16580
VascoSch92 wants to merge 2 commits intoastral-sh:mainfrom
VascoSch92:S324

Conversation

@VascoSch92
Copy link
Contributor

This PR solves issue #16572

Could be an idea to define a HashSet containing the names of the insicure hash functions? In this way would be easier to update in case other insicure hash functions are added to the list.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2025

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+2 -0 violations, +0 -0 fixes in 1 projects; 54 projects unchanged)

apache/airflow (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --no-preview --select ALL

+ dev/breeze/src/airflow_breeze/utils/path_utils.py:107:32: S324 Probable use of insecure hash functions in `hashlib`: `blake2b`
+ scripts/ci/pre_commit/update_breeze_config_hash.py:44:32: S324 Probable use of insecure hash functions in `hashlib`: `blake2b`

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
S324 2 2 0 0 0

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+2 -0 violations, +0 -0 fixes in 1 projects; 54 projects unchanged)

apache/airflow (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ dev/breeze/src/airflow_breeze/utils/path_utils.py:107:32: S324 Probable use of insecure hash functions in `hashlib`: `blake2b`
+ scripts/ci/pre_commit/update_breeze_config_hash.py:44:32: S324 Probable use of insecure hash functions in `hashlib`: `blake2b`

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
S324 2 2 0 0 0

@AlexWaygood AlexWaygood changed the title [flake8-bandit] Update insicure hash functions (S324) [flake8-bandit] Update insecure hash functions (S324) Mar 9, 2025
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

See inline comment

Copy link
Contributor

@Skylion007 Skylion007 left a comment

Choose a reason for hiding this comment

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

Aren't we doing this the wrong way? We should be maintaining a whitelist instead of a blacklist of FIPS supported hashlib functions. Maybe with a way to specify additional safe ones through the pyproject.toml? As far I understand it, FIPS specification is a whitelist of supported functions, not a blacklist anyway.

@VascoSch92
Copy link
Contributor Author

Aren't we doing this the wrong way? We should be maintaining a whitelist instead of a blacklist of FIPS supported hashlib functions. Maybe with a way to specify additional safe ones through the pyproject.toml? As far I understand it, FIPS specification is a whitelist of supported functions, not a blacklist anyway.

Should I update the rule with respect to that?

@VascoSch92
Copy link
Contributor Author

Hey @MichaReiser @Skylion007

I update the rule to have a whitelist instead of a blacklist.

For instance, we have just two elements in the whitelist, namely, sha256, and sha512.

What do you think about that?

Should also update the description to mention these two hashlib?

@mikeshardmind
Copy link

mikeshardmind commented Apr 19, 2025

The FIPS-approved hashes are not the only secure hash functions in hashlib. FIPS is govermental compliance, and not an exclusive set of "Good" hashes. While it may make sense to have a lint rule for people who need FIPS compliance, that's not what this lint rule is documented for.

An example of this would be blake2. While blake3 is faster without becoming insecure, blake2 is not a weak, broken, or otherwise unfit for security related use as a hash function

@ntBre
Copy link
Contributor

ntBre commented Sep 24, 2025

Thanks for your work on this! I'll close it for now since we updated the documentation instead in #20534.

@ntBre ntBre closed this Sep 24, 2025
@VascoSch92 VascoSch92 deleted the S324 branch September 25, 2025 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Comments