From a39623744a597556666366e1aaff451ff4b97a31 Mon Sep 17 00:00:00 2001 From: dAxpeDDa Date: Thu, 16 Jun 2022 23:19:22 +0200 Subject: [PATCH 1/2] Add `password_hash::Error::OutputSize` --- password-hash/src/errors.rs | 54 +++++++++++++++++++++++++------------ password-hash/src/lib.rs | 8 +++--- password-hash/src/output.rs | 34 ++++++++++++++++++----- 3 files changed, 69 insertions(+), 27 deletions(-) diff --git a/password-hash/src/errors.rs b/password-hash/src/errors.rs index 48ff66b0b..40f696719 100644 --- a/password-hash/src/errors.rs +++ b/password-hash/src/errors.rs @@ -2,6 +2,7 @@ pub use base64ct::Error as B64Error; +use core::cmp::Ordering; use core::fmt; /// Result type. @@ -20,11 +21,21 @@ pub enum Error { /// Cryptographic error. Crypto, - /// Output too short (min 10-bytes). - OutputTooShort, - - /// Output too long (max 64-bytes). - OutputTooLong, + /// Output size unexpected. + OutputSize { + /// Indicates why the output size is unexpected. + /// + /// - [`Ordering::Less`]: Size is too small. + /// - [`Ordering::Equal`]: Size is not exactly as `expected`. + /// - [`Ordering::Greater`]: Size is too long. + provided: Ordering, + /// Expected output size in relation to `provided`. + /// + /// - [`Ordering::Less`]: Minimum size. + /// - [`Ordering::Equal`]: Expecrted size. + /// - [`Ordering::Greater`]: Maximum size. + expected: usize, + }, /// Duplicate parameter name encountered. ParamNameDuplicated, @@ -41,14 +52,11 @@ pub enum Error { /// Invalid password. Password, - /// Password hash string contains invalid characters. - PhcStringInvalid, - - /// Password hash string too short. - PhcStringTooShort, + /// Password hash string invalid. + PhcStringField, - /// Password hash string too long. - PhcStringTooLong, + /// Password hash string contains leftover characters. + PhcStringLeftover, /// Salt invalid. SaltInvalid(InvalidValue), @@ -63,16 +71,28 @@ impl fmt::Display for Error { Self::Algorithm => write!(f, "unsupported algorithm"), Self::B64Encoding(err) => write!(f, "{}", err), 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::OutputSize { provided, expected } => match provided { + Ordering::Less => write!( + f, + "output size too short, expected at least {} bytes", + expected + ), + Ordering::Equal => write!(f, "output size unexpected, expected {} bytes", expected), + Ordering::Greater => write!( + f, + "output size too long, expected at most {} bytes", + expected + ), + }, Self::ParamNameDuplicated => f.write_str("duplicate parameter"), Self::ParamNameInvalid => f.write_str("invalid parameter name"), Self::ParamValueInvalid(val_err) => write!(f, "invalid parameter value: {}", val_err), 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::PhcStringField => write!(f, "password hash string missing field"), + Self::PhcStringLeftover => { + write!(f, "password hash string contains leftover characters") + } Self::SaltInvalid(val_err) => write!(f, "salt invalid: {}", val_err), Self::Version => write!(f, "invalid algorithm version"), } diff --git a/password-hash/src/lib.rs b/password-hash/src/lib.rs index f451fb0a8..c0059facf 100644 --- a/password-hash/src/lib.rs +++ b/password-hash/src/lib.rs @@ -132,19 +132,19 @@ impl<'a> PasswordHash<'a> { /// Parse a password hash from the given [`Encoding`]. pub fn parse(s: &'a str, encoding: Encoding) -> Result { if s.is_empty() { - return Err(Error::PhcStringTooShort); + return Err(Error::PhcStringField); } let mut fields = s.split(PASSWORD_HASH_SEPARATOR); let beginning = fields.next().expect("no first field"); if beginning.chars().next().is_some() { - return Err(Error::PhcStringInvalid); + return Err(Error::PhcStringField); } let algorithm = fields .next() - .ok_or(Error::PhcStringTooShort) + .ok_or(Error::PhcStringField) .and_then(Ident::try_from)?; let mut version = None; @@ -187,7 +187,7 @@ impl<'a> PasswordHash<'a> { } if fields.next().is_some() { - return Err(Error::PhcStringTooLong); + return Err(Error::PhcStringLeftover); } Ok(Self { diff --git a/password-hash/src/output.rs b/password-hash/src/output.rs index c2560ce12..d988b55d8 100644 --- a/password-hash/src/output.rs +++ b/password-hash/src/output.rs @@ -1,7 +1,11 @@ //! Outputs from password hashing functions. use crate::{Encoding, Error, Result}; -use core::{cmp::PartialEq, fmt, str::FromStr}; +use core::{ + cmp::{Ordering, PartialEq}, + fmt, + str::FromStr, +}; use subtle::{Choice, ConstantTimeEq}; /// Output from password hashing functions, i.e. the "hash" or "digest" @@ -149,11 +153,17 @@ impl Output { F: FnOnce(&mut [u8]) -> Result<()>, { if output_size < Self::MIN_LENGTH { - return Err(Error::OutputTooShort); + return Err(Error::OutputSize { + provided: Ordering::Less, + expected: Self::MIN_LENGTH, + }); } if output_size > Self::MAX_LENGTH { - return Err(Error::OutputTooLong); + return Err(Error::OutputSize { + provided: Ordering::Greater, + expected: Self::MAX_LENGTH, + }); } let mut bytes = [0u8; Self::MAX_LENGTH]; @@ -266,7 +276,7 @@ impl fmt::Debug for Output { #[cfg(test)] mod tests { - use super::{Error, Output}; + use super::{Error, Ordering, Output}; #[test] fn new_with_valid_min_length_input() { @@ -286,14 +296,26 @@ mod tests { fn reject_new_too_short() { let bytes = [9u8; 9]; let err = Output::new(&bytes).err().unwrap(); - assert_eq!(err, Error::OutputTooShort); + assert_eq!( + err, + Error::OutputSize { + provided: Ordering::Less, + expected: Output::MIN_LENGTH + } + ); } #[test] fn reject_new_too_long() { let bytes = [65u8; 65]; let err = Output::new(&bytes).err().unwrap(); - assert_eq!(err, Error::OutputTooLong); + assert_eq!( + err, + Error::OutputSize { + provided: Ordering::Greater, + expected: Output::MAX_LENGTH + } + ); } #[test] From 91e204976017fce59e2c45ed69cc1e1ec1cd8027 Mon Sep 17 00:00:00 2001 From: daxpedda Date: Mon, 9 Jan 2023 17:11:32 +0100 Subject: [PATCH 2/2] Change `PhcStringLeftover` to `PhcStringTrailingData` Co-authored-by: Tony Arcieri --- password-hash/src/errors.rs | 8 ++++---- password-hash/src/lib.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/password-hash/src/errors.rs b/password-hash/src/errors.rs index 40f696719..f4719af31 100644 --- a/password-hash/src/errors.rs +++ b/password-hash/src/errors.rs @@ -55,8 +55,8 @@ pub enum Error { /// Password hash string invalid. PhcStringField, - /// Password hash string contains leftover characters. - PhcStringLeftover, + /// Password hash string contains trailing data. + PhcStringTrailingData, /// Salt invalid. SaltInvalid(InvalidValue), @@ -90,8 +90,8 @@ impl fmt::Display for Error { Self::ParamsMaxExceeded => f.write_str("maximum number of parameters reached"), Self::Password => write!(f, "invalid password"), Self::PhcStringField => write!(f, "password hash string missing field"), - Self::PhcStringLeftover => { - write!(f, "password hash string contains leftover characters") + Self::PhcStringTrailingData => { + write!(f, "password hash string contains trailing characters") } Self::SaltInvalid(val_err) => write!(f, "salt invalid: {}", val_err), Self::Version => write!(f, "invalid algorithm version"), diff --git a/password-hash/src/lib.rs b/password-hash/src/lib.rs index c0059facf..a7e2b1c96 100644 --- a/password-hash/src/lib.rs +++ b/password-hash/src/lib.rs @@ -187,7 +187,7 @@ impl<'a> PasswordHash<'a> { } if fields.next().is_some() { - return Err(Error::PhcStringLeftover); + return Err(Error::PhcStringTrailingData); } Ok(Self {