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

password-hash: error handling problems #547

Closed
tarcieri opened this issue Feb 13, 2021 · 1 comment
Closed

password-hash: error handling problems #547

tarcieri opened this issue Feb 13, 2021 · 1 comment

Comments

@tarcieri
Copy link
Member

tarcieri commented Feb 13, 2021

The current error handling situation in the password-hash crate seems both overcomplicated and has other issues which limit the ability to easily map between error types in important scenarios.

This actually ended up indirectly leading to a panic in the argon2 crate (RustCrypto/password-hashes#135) due to an unimplemented!() macro triggered by an unexpected error when an appropriate conversion wasn't possible.

Looking at that issue and the comments I brought up, here are some high-level concerns it raised:

  • Large number of error types, and sometimes it's unclear which to use
  • Important cases not handled: HasherError does not have a way to provide salt-related errors (this is a result of some refactoring work where salt-related error types were lost)
  • The password_hash::Output::init_with API is problematic because it uses a closure and mandates the use of OutputError from that closure. It could perhaps be improved by being generic over the error type, since in practical contexts a user will almost always want HasherError, but in other cases may want a concrete crate-level error type. Another solution might be to find a way to avoid using a closure entirely.
tarcieri added a commit that referenced this issue Apr 25, 2021
This commit contains a set of breaking changes intended to be released
as part of `password-hash` v0.2, addressing a number of currently open
issues about the crate discovered when integrating it into the crates in
the https://github.com/RustCrypto/password-hashes repo.

This includes:

- Allow passing `&str`, `&Salt`, or `&SaltString` as salt (#529)
- Simplify error handling (#547)
- TBD
tarcieri added a commit that referenced this issue Apr 25, 2021
This commit contains a set of breaking changes intended to be released
as part of `password-hash` v0.2, addressing a number of currently open
issues about the crate discovered when integrating it into the crates in
the https://github.com/RustCrypto/password-hashes repo.

This includes:

- Allow passing `&str`, `&Salt`, or `&SaltString` as salt (#529)
- Simplify error handling (#547)
- TBD
tarcieri added a commit that referenced this issue Apr 28, 2021
This commit contains a set of breaking changes intended to be released
as part of `password-hash` v0.2, addressing a number of currently open
issues about the crate discovered when integrating it into the crates in
the https://github.com/RustCrypto/password-hashes repo.

This includes:

- Allow specifying output length and version with params (#505)
- Allow passing `&str`, `&Salt`, or `&SaltString` as salt (#529)
- Simplify error handling (#547)
tarcieri added a commit that referenced this issue Apr 28, 2021
This commit contains a set of breaking changes intended to be released
as part of `password-hash` v0.2, addressing a number of currently open
issues about the crate discovered when integrating it into the crates in
the https://github.com/RustCrypto/password-hashes repo.

This includes:

- Allow specifying output length and version with params (#505)
- Allow passing `&str`, `&Salt`, or `&SaltString` as salt (#529)
- Simplify error handling (#547)
tarcieri added a commit that referenced this issue Apr 28, 2021
This commit contains a set of breaking changes intended to be released
as part of `password-hash` v0.2, addressing a number of currently open
issues about the crate discovered when integrating it into the crates in
the https://github.com/RustCrypto/password-hashes repo.

This includes:

- Allow specifying output length and version with params (#505)
- Allow passing `&str`, `&Salt`, or `&SaltString` as salt (#529)
- Simplify error handling (#547)
tarcieri added a commit that referenced this issue Apr 28, 2021
This commit contains a set of breaking changes intended to be released
as part of `password-hash` v0.2, addressing a number of currently open
issues about the crate discovered when integrating it into the crates in
the https://github.com/RustCrypto/password-hashes repo.

This includes:

- Allow specifying output length and version with params (#505)
- Allow passing `&str`, `&Salt`, or `&SaltString` as salt (#529)
- Simplify error handling (#547)
tarcieri added a commit that referenced this issue Apr 28, 2021
This commit contains a set of breaking changes intended to be released
as part of `password-hash` v0.2, addressing a number of currently open
issues about the crate discovered when integrating it into the crates in
the https://github.com/RustCrypto/password-hashes repo.

This includes:

- Allow specifying output length and version with params (#505)
- Allow passing `&str`, `&Salt`, or `&SaltString` as salt (#529)
- Simplify error handling (#547)
@tarcieri
Copy link
Member Author

Fixed in #615

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant