Skip to content

feat: Add password-hash feature#97

Open
sorairolake wants to merge 3 commits intoKeats:masterfrom
sorairolake:feature/rust-crypto-traits
Open

feat: Add password-hash feature#97
sorairolake wants to merge 3 commits intoKeats:masterfrom
sorairolake:feature/rust-crypto-traits

Conversation

@sorairolake
Copy link
Copy Markdown
Contributor

This implements traits from the kdf crate and the password-hash crate, and the PasswordHash struct and the PasswordHashRef struct from the mcf crate.

These are implemented in crates such as argon2 and scrypt on https://github.com/RustCrypto/password-hashes, and this pull request will allow this crate to use the same API as these crates.

@sorairolake sorairolake force-pushed the feature/rust-crypto-traits branch from 5621739 to f918a0b Compare March 17, 2026 16:31
@Keats
Copy link
Copy Markdown
Owner

Keats commented Mar 18, 2026

Why do we need more than https://crates.io/crates/password-hash?

@tarcieri
Copy link
Copy Markdown

As the author of both crates, I would suggest password-hash over kdf, as bcrypt isn't a proper KDF due to its input and output size limitations

@sorairolake sorairolake force-pushed the feature/rust-crypto-traits branch from f918a0b to b29b9d1 Compare April 10, 2026 15:08
@sorairolake sorairolake changed the title feat: Implement traits from Rust Crypto feat: Add password-hash feature Apr 10, 2026
@sorairolake
Copy link
Copy Markdown
Contributor Author

Sorry for the delay 🙇‍♂️

Based on #97 (comment), I removed the kdf feature. This also removes the kdf crate from dependencies.

Comment thread Cargo.toml Outdated

[features]
default = ["std", "zeroize"]
default = ["std", "zeroize", "password-hash"]
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

it probably shouldn't be part of the default feature

Comment thread src/bcrypt.rs Outdated
#[cfg(feature = "password-hash")]
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
/// bcrypt context.
pub struct Bcrypt {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

can that be used internally? a bit weird to have that just for the password-hash feature and this mean we duplicate the cost check

Comment thread src/mcf.rs Outdated
salt: &[u8],
alg_id: Option<&str>,
version: Option<password_hash::Version>,
params: Self::Params,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

is the params the cost?

@sorairolake sorairolake requested a review from Keats April 13, 2026 05:58
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.

3 participants