Skip to content

Commit

Permalink
[WIP] password-hash: v0.2 breaking changes
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
tarcieri committed Apr 28, 2021
1 parent b96df69 commit 784fa01
Show file tree
Hide file tree
Showing 8 changed files with 185 additions and 402 deletions.
260 changes: 50 additions & 210 deletions password-hash/src/errors.rs
Expand Up @@ -4,59 +4,12 @@ pub use base64ct::Error as B64Error;

use core::fmt;

#[cfg(docsrs)]
use crate::PasswordHasher;
/// Result type.
pub type Result<T> = core::result::Result<T, Error>;

/// Password hash errors.
/// Password hashing errors.
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum HashError {
/// Hash output error.
Hash(OutputError),

/// Params error.
Params(ParamsError),

/// Parse error.
Parse(ParseError),
}

impl fmt::Display for HashError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
match self {
Self::Hash(err) => write!(f, "invalid password hash: {}", err),
Self::Params(err) => write!(f, "invalid params: {}", err),
Self::Parse(err) => write!(f, "parse error: {}", err),
}
}
}

impl From<OutputError> for HashError {
fn from(err: OutputError) -> HashError {
HashError::Hash(err)
}
}

impl From<ParamsError> for HashError {
fn from(err: ParamsError) -> HashError {
match err {
ParamsError::Parse(e) => e.into(),
_ => HashError::Params(err),
}
}
}

impl From<ParseError> for HashError {
fn from(err: ParseError) -> HashError {
HashError::Parse(err)
}
}

#[cfg(feature = "std")]
impl std::error::Error for HashError {}

/// Errors generating password hashes using a [`PasswordHasher`].
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum HasherError {
pub enum Error {
/// Unsupported algorithm.
Algorithm,

Expand All @@ -66,193 +19,80 @@ pub enum HasherError {
/// Cryptographic error.
Crypto,

/// Error generating output.
Output(OutputError),

/// Invalid parameter.
Params(ParamsError),

/// Parse error.
Parse(ParseError),

/// Invalid password.
Password,

/// Invalid algorithm version.
Version,
}

impl fmt::Display for HasherError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
match self {
Self::Algorithm => write!(f, "unsupported algorithm"),
Self::B64(err) => write!(f, "{}", err),
Self::Crypto => write!(f, "cryptographic error"),
Self::Output(err) => write!(f, "PHF output error: {}", err),
Self::Params(err) => write!(f, "{}", err),
Self::Parse(err) => write!(f, "{}", err),
Self::Password => write!(f, "invalid password"),
Self::Version => write!(f, "invalid algorithm version"),
}
}
}

impl From<B64Error> for HasherError {
fn from(err: B64Error) -> HasherError {
HasherError::B64(err)
}
}

impl From<base64ct::InvalidLengthError> for HasherError {
fn from(_: base64ct::InvalidLengthError) -> HasherError {
HasherError::B64(B64Error::InvalidLength)
}
}

impl From<OutputError> for HasherError {
fn from(err: OutputError) -> HasherError {
HasherError::Output(err)
}
}

impl From<ParamsError> for HasherError {
fn from(err: ParamsError) -> HasherError {
HasherError::Params(err)
}
}

impl From<ParseError> for HasherError {
fn from(err: ParseError) -> HasherError {
HasherError::Parse(err)
}
}
/// Output too short (min 10-bytes).
OutputTooShort,

#[cfg(feature = "std")]
impl std::error::Error for HasherError {}
/// Output too long (max 64-bytes).
OutputTooLong,

/// Parameter-related errors.
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum ParamsError {
/// Duplicate parameter name encountered.
DuplicateName,
ParamNameDuplicated,

/// Invalid parameter name.
InvalidName,
ParamNameInvalid,

/// Invalid parameter value.
InvalidValue,
ParamValueInvalid,

/// Maximum number of parameters exceeded.
MaxExceeded,

/// Parse errors.
Parse(ParseError),
}

impl fmt::Display for ParamsError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
match self {
Self::DuplicateName => f.write_str("duplicate parameter"),
Self::InvalidName => f.write_str("invalid parameter name"),
Self::InvalidValue => f.write_str("invalid parameter value"),
Self::MaxExceeded => f.write_str("maximum number of parameters reached"),
Self::Parse(err) => write!(f, "{}", err),
}
}
}

impl From<ParseError> for ParamsError {
fn from(err: ParseError) -> ParamsError {
Self::Parse(err)
}
}

#[cfg(feature = "std")]
impl std::error::Error for ParamsError {}
ParamsMaxExceeded,

/// Parse errors.
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum ParseError {
/// Invalid empty input.
Empty,

/// Input contains invalid character.
InvalidChar(char),

/// Input too short.
TooShort,
/// Invalid password.
Password,

/// Input too long.
TooLong,
}
/// Password hash string contains invalid characters.
PhcStringInvalid,

impl fmt::Display for ParseError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
match self {
Self::Empty => f.write_str("invalid empty input"),
Self::InvalidChar(char) => write!(f, "invalid character '{}'", char),
Self::TooShort => f.write_str("too short"),
Self::TooLong => f.write_str("too long"),
}
}
}
/// Password hash string too short.
PhcStringTooShort,

#[cfg(feature = "std")]
impl std::error::Error for ParseError {}
/// Password hash string too long.
PhcStringTooLong,

/// Password hash function output (i.e. hash/digest) errors.
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum OutputError {
/// "B64" encoding error.
B64(B64Error),
/// Salt too short.
SaltTooShort,

/// Output too short (min 10-bytes).
TooShort,
/// Salt too long.
SaltTooLong,

/// Output too long (max 64-bytes).
TooLong,
/// Invalid algorithm version.
Version,
}

impl fmt::Display for OutputError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> core::result::Result<(), fmt::Error> {
match self {
Self::Algorithm => write!(f, "unsupported algorithm"),
Self::B64(err) => write!(f, "{}", err),
Self::TooShort => f.write_str("PHF output too short (min 10-bytes)"),
Self::TooLong => f.write_str("PHF output too long (max 64-bytes)"),
Self::Crypto => write!(f, "cryptographic error"),
Self::OutputTooShort => f.write_str("PHF output too short (min 10-bytes)"),
Self::OutputTooLong => f.write_str("PHF output too long (max 64-bytes)"),
Self::ParamNameDuplicated => f.write_str("duplicate parameter"),
Self::ParamNameInvalid => f.write_str("invalid parameter name"),
Self::ParamValueInvalid => f.write_str("invalid parameter value"),
Self::ParamsMaxExceeded => f.write_str("maximum number of parameters reached"),
Self::Password => write!(f, "invalid password"),
Self::PhcStringInvalid => write!(f, "password hash string invalid"),
Self::PhcStringTooShort => write!(f, "password hash string too short"),
Self::PhcStringTooLong => write!(f, "password hash string too long"),
Self::SaltTooShort => write!(f, "salt too short"),
Self::SaltTooLong => write!(f, "salt too long"),
Self::Version => write!(f, "invalid algorithm version"),
}
}
}

impl From<B64Error> for OutputError {
fn from(err: B64Error) -> OutputError {
OutputError::B64(err)
}
}

impl From<base64ct::InvalidLengthError> for OutputError {
fn from(_: base64ct::InvalidLengthError) -> OutputError {
OutputError::B64(B64Error::InvalidLength)
}
}

#[cfg(feature = "std")]
impl std::error::Error for OutputError {}

/// Password verification errors.
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub struct VerifyError;
impl std::error::Error for Error {}

impl fmt::Display for VerifyError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
f.write_str("password verification error")
impl From<B64Error> for Error {
fn from(err: B64Error) -> Error {
Error::B64(err)
}
}

impl From<HasherError> for VerifyError {
fn from(_: HasherError) -> VerifyError {
VerifyError
impl From<base64ct::InvalidLengthError> for Error {
fn from(_: base64ct::InvalidLengthError) -> Error {
Error::B64(B64Error::InvalidLength)
}
}

#[cfg(feature = "std")]
impl std::error::Error for VerifyError {}
28 changes: 11 additions & 17 deletions password-hash/src/ident.rs
Expand Up @@ -16,7 +16,7 @@
//!
//! [1]: https://github.com/P-H-C/phc-string-format/blob/master/phc-sf-spec.md

use crate::errors::ParseError;
use crate::{Error, Result};
use core::{convert::TryFrom, fmt, ops::Deref, str};

/// Algorithm or parameter identifier.
Expand Down Expand Up @@ -45,12 +45,6 @@ impl<'a> Ident<'a> {
/// and parameter names according to the PHC string format.
const MAX_LENGTH: usize = 32;

/// Maximum length of an [`Ident`] - 32 ASCII characters (i.e. 32-bytes).
#[deprecated(since = "0.1.4", note = "use Ident::MAX_LENGTH instead")]
pub const fn max_len() -> usize {
Self::MAX_LENGTH
}

/// Parse an [`Ident`] from a string.
///
/// # Panics
Expand Down Expand Up @@ -122,24 +116,24 @@ impl<'a> Deref for Ident<'a> {
// Note: this uses `TryFrom` instead of `FromStr` to support a lifetime on
// the `str` the value is being parsed from.
impl<'a> TryFrom<&'a str> for Ident<'a> {
type Error = ParseError;
type Error = Error;

fn try_from(s: &'a str) -> Result<Self, ParseError> {
fn try_from(s: &'a str) -> Result<Self> {
if s.is_empty() {
return Err(ParseError::Empty);
return Err(Error::ParamNameInvalid);
}

let bytes = s.as_bytes();
let too_long = bytes.len() > Self::MAX_LENGTH;

for &c in bytes {
if !is_char_valid(c) {
return Err(ParseError::InvalidChar(c.into()));
return Err(Error::ParamNameInvalid);
}
}

if too_long {
return Err(ParseError::TooLong);
return Err(Error::ParamNameInvalid);
}

Ok(Self::new(s))
Expand All @@ -165,7 +159,7 @@ const fn is_char_valid(c: u8) -> bool {

#[cfg(test)]
mod tests {
use super::{Ident, ParseError};
use super::{Error, Ident};
use core::convert::TryFrom;

// Invalid ident examples
Expand Down Expand Up @@ -195,7 +189,7 @@ mod tests {
#[test]
fn reject_empty_fallible() {
let err = Ident::try_from(INVALID_EMPTY).err().unwrap();
assert_eq!(err, ParseError::Empty);
assert_eq!(err, Error::ParamNameInvalid);
}

#[test]
Expand All @@ -207,7 +201,7 @@ mod tests {
#[test]
fn reject_invalid_char_fallible() {
let err = Ident::try_from(INVALID_CHAR).err().unwrap();
assert_eq!(err, ParseError::InvalidChar(';'));
assert_eq!(err, Error::ParamNameInvalid);
}

#[test]
Expand All @@ -219,7 +213,7 @@ mod tests {
#[test]
fn reject_too_long_fallible() {
let err = Ident::try_from(INVALID_TOO_LONG).err().unwrap();
assert_eq!(err, ParseError::TooLong);
assert_eq!(err, Error::ParamNameInvalid);
}

#[test]
Expand All @@ -231,6 +225,6 @@ mod tests {
#[test]
fn reject_invalid_char_and_too_long_fallible() {
let err = Ident::try_from(INVALID_CHAR_AND_TOO_LONG).err().unwrap();
assert_eq!(err, ParseError::InvalidChar('!'));
assert_eq!(err, Error::ParamNameInvalid);
}
}

0 comments on commit 784fa01

Please sign in to comment.