Skip to content

pbkdf2: remove sha1 feature#853

Merged
tarcieri merged 2 commits intomasterfrom
pbkdf2/remove-sha1-feature
Mar 19, 2026
Merged

pbkdf2: remove sha1 feature#853
tarcieri merged 2 commits intomasterfrom
pbkdf2/remove-sha1-feature

Conversation

@tarcieri
Copy link
Member

@tarcieri tarcieri commented Mar 19, 2026

This came up in some discussions about hypothetical FIPS support, noting NIST already recommends against SHA-1 and plans on complete phase-out by December 31st, 2030:

https://www.nist.gov/news-events/news/2022/12/nist-retires-sha-1-cryptographic-algorithm

That article notes on that date FIPS 180-5 will be published which removes SHA-1, and a revision to SP 800-131A will reflect that SHA-1 is unsuitable for use in FIPS modules:

“Modules that still use SHA-1 after 2030 will not be permitted for purchase by the federal government,” Celi said. “Companies have eight years to submit updated modules that no longer use SHA-1. Because there is often a backlog of submissions before a deadline, we recommend that developers submit their updated modules well in advance, so that CMVP has time to respond.”

Though we're not quite to that deadline, this PR proposes to remove direct support for SHA-1 in anticipitation of these changes.

This came up in some discussions about hypothetical FIPS support, noting
NIST already recommends against SHA-1 and plans on complete phase-out by
December 31st, 2030:

https://www.nist.gov/news-events/news/2022/12/nist-retires-sha-1-cryptographic-algorithm

That article notes on that date FIPS 180-5 will be published which
removes SHA-1, and a revision to SP 800-131A will reflect that SHA-1 is
unsuitable for use in FIPS modules.

Though we're not quite to that deadline, this PR proposes to remove
direct support for SHA-1 in anticipitation of these changes.
@tarcieri tarcieri requested a review from newpavlov March 19, 2026 00:03
Copy link
Member

@newpavlov newpavlov left a comment

Choose a reason for hiding this comment

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

IMO pbkdf2 should not depend on any hash crates (including sha2) outside of dev dependencies in the first place, but it's a separate issue.

/// Test vectors from RFC 6070:
/// https://www.rfc-editor.org/rfc/rfc6070
#[test]
fn pbkdf2_rfc6070() {
Copy link
Member

Choose a reason for hiding this comment

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

I think we can keep this test.

@newpavlov
Copy link
Member

Also, do not forget to update the changelog.

@tarcieri
Copy link
Member Author

The algorithm-specific support is needed/useful for implementing MCF and PHC support, which is only defined for PBKDF2 when used with HMAC-SHA1 and HMAC-SHA2

@newpavlov
Copy link
Member

Yes, but it does not mean that it should be part of pbkdf2. One potential solution could be to move it into a separate "composite" crate (e.g. pbkdf2-sha2 or such), it also would help with the compatible versions selection issue.

@tarcieri tarcieri merged commit 969ac83 into master Mar 19, 2026
15 checks passed
@tarcieri tarcieri deleted the pbkdf2/remove-sha1-feature branch March 19, 2026 00:19
@tarcieri
Copy link
Member Author

I think it would be weird if someone had to discover and reach for a completely separate crate to get basic PBKDF2 functionality

@newpavlov
Copy link
Member

newpavlov commented Mar 19, 2026

PBKDF2 is not a NIST-only algorithm (unlike you argued with GCM) and it's defined generically. Sure, SHA-2 it the most popular choice, but the same could be said about AES with the CTR mode. And I am surely do not think we should include aes as an optional dependency of ctr.

@tarcieri
Copy link
Member Author

tarcieri commented Mar 19, 2026

Those other algorithms aren’t supported in MCF/PHC contexts. It has nothing to do with NIST.

@newpavlov
Copy link
Member

newpavlov commented Mar 19, 2026

What makes you say that someone would not like to use PBKDF2 with Streebog or BLAKE2/3 in MCF/PHC contexts? IIUC there is no OID-like centralized registry for algorithm IDs used in MCF/PHC, names like pbkdf2-sha256 are mostly historic convention. Theoretically an app could use a completely different name if it wants. The spec does not say anything about how <id> should be assigned.

@tarcieri
Copy link
Member Author

If there’s any actual precedent for that we can support it, but without that I don’t think it makes sense to support bespoke combinations

@newpavlov
Copy link
Member

newpavlov commented Mar 19, 2026

PKDF2+Streebog is literally a national standard! Same for BelT.

@tarcieri
Copy link
Member Author

tarcieri commented Mar 19, 2026

…as used in MCF/PHC? Again it has nothing to do with national standards

@newpavlov
Copy link
Member

newpavlov commented Mar 19, 2026

How else would you store a hashed password in a database?

Again it has nothing to do with national standards

Yes, it does. It means that the combination can be legitimately used in practice (or in some cases has to) and it's weird to declare that everything except PBKDF2-HMAC-SHA2 has a second-class support.

@tarcieri
Copy link
Member Author

All I’m saying is that we should implement MCF/PHC for algorithms which are already in use with those formats (e.g. implemented by passlib), as opposed to inventing bespoke algorithm identifiers that are literally not supported by any software on earth.

PBKDF2 is already a legacy algorithm IMO. We don’t need to encourage the proliferation of more variants for the sake of encouraging more variants, especially ones no one has even asked for.

@newpavlov
Copy link
Member

newpavlov commented Mar 19, 2026

I am not saying that we should "invent" algorithm identifiers. I am saying that IMO they should not be part of the pbkdf2 crate and we should make it easy to include custom algorithms with "invented" identifiers if necessary.

Imagine a hypothetical PHC2 with numeric IDs for algorithms instead of strings. Would we include these IDs in every implementation crate or should we keep it elsewhere?

I will try to draft an alternative proposal with PRs later when I have some free time.

@tarcieri
Copy link
Member Author

tarcieri commented Mar 19, 2026

I think such needless extensibility is an undesirable anti-feature which will add undue complexity. These decisions should be motivated by real-world interoperability requirements.

IMO the algorithmic usages for PBKDF2 with MCF/PHC are for already-defined fixed sets of algorithms.

I think you should justify additional algorithm support by first finding precedent for its use. Where is literally any other software that implements MCF/PHC in such a way? If you can’t point to that, your proposal has literally no justification for existing.

Otherwise we should just implement the standard algorithms that are in e.g. passlib and ubiquitous on *IX systems and call it a day.

@newpavlov
Copy link
Member

newpavlov commented Mar 19, 2026

After a cursory search: https://wiki.astralinux.ru/pages/viewpage.action?pageId=216540367

And I am sure that PBKDF2+Streebog is used with PHC in proprietary projects which have to follow GOSTs. I can not provide links to those for obvious reasons.

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.

2 participants