password-hash: remove phc::Ident lifetime (#2107)

`Ident` is a short string with a max length of 32-bytes. Instead of
having a lifetime, we can simply store the 32-bytes as an owned type.

This is an incremental step towards removing the lifetime from the
`phc::PasswordHash` type.

The implementation makes the previous `Buffer` (now `StringBuf`) type
used for `Params` const generic around a particular size, and adds some
basic const init functionality, as well as checks for arithmetic
overflow.

Author: tarcieri

password-hash/src/errors.rs

@@ -21,6 +21,9 @@ pub enum Error {
     /// Cryptographic error.
     Crypto,
 
+    /// Out of memory (heap allocation failure).
+    OutOfMemory,
+
     /// Output size unexpected.
     OutputSize {
         /// Indicates why the output size is unexpected.
@@ -61,11 +64,14 @@ pub enum Error {
     /// Salt invalid.
     SaltInvalid(InvalidValue),
 
+    /// Value exceeds the maximum allowed length.
+    TooLong,
+
+    /// Value does not satisfy the minimum length.
+    TooShort,
+
     /// Invalid algorithm version.
     Version,
-
-    /// Out of memory (heap allocation failure).
-    OutOfMemory,
 }
 
 impl fmt::Display for Error {
@@ -74,6 +80,7 @@ impl fmt::Display for Error {
             Self::Algorithm => write!(f, "unsupported algorithm"),
             Self::B64Encoding(err) => write!(f, "{err}"),
             Self::Crypto => write!(f, "cryptographic error"),
+            Self::OutOfMemory => write!(f, "out of memory"),
             Self::OutputSize { provided, expected } => match provided {
                 Ordering::Less => write!(
                     f,
@@ -94,8 +101,9 @@ impl fmt::Display for Error {
                 write!(f, "password hash string contains trailing characters")
             }
             Self::SaltInvalid(val_err) => write!(f, "salt invalid: {val_err}"),
+            Self::TooLong => f.write_str("value to long"),
+            Self::TooShort => f.write_str("value to short"),
             Self::Version => write!(f, "invalid algorithm version"),
-            Self::OutOfMemory => write!(f, "out of memory"),
         }
     }
 }

password-hash/src/phc.rs

@@ -4,22 +4,21 @@ mod ident;
 mod output;
 mod params;
 mod salt;
+mod string_buf;
 mod value;
 
-use crate::{Error, PasswordHasher, PasswordVerifier};
 pub use ident::Ident;
 pub use output::Output;
 pub use params::ParamsString;
 pub use salt::{Salt, SaltString};
 pub use value::{Decimal, Value};
 
-use core::fmt;
+use crate::{Error, PasswordHasher, PasswordVerifier};
+use core::{fmt, str::FromStr};
+use string_buf::StringBuf;
 
 #[cfg(feature = "alloc")]
-use alloc::{
-    str::FromStr,
-    string::{String, ToString},
-};
+use alloc::string::{String, ToString};
 
 /// Separator character used in password hashes (e.g. `$6$...`).
 const PASSWORD_HASH_SEPARATOR: char = '$';
@@ -62,7 +61,7 @@ pub struct PasswordHash<'a> {
     ///
     /// This corresponds to the `<id>` field in a PHC string, a.k.a. the
     /// symbolic name for the function.
-    pub algorithm: Ident<'a>,
+    pub algorithm: Ident,
 
     /// Optional version field.
     ///
@@ -103,7 +102,7 @@ impl<'a> PasswordHash<'a> {
         let algorithm = fields
             .next()
             .ok_or(Error::PhcStringField)
-            .and_then(Ident::try_from)?;
+            .and_then(Ident::from_str)?;
 
         let mut version = None;
         let mut params = ParamsString::new();
@@ -264,7 +263,7 @@ impl PasswordHashString {
     }
 
     /// Password hashing algorithm identifier.
-    pub fn algorithm(&self) -> Ident<'_> {
+    pub fn algorithm(&self) -> Ident {
         self.password_hash().algorithm
     }
 

password-hash/src/phc/ident.rs

@@ -16,8 +16,13 @@
 //!
 //! [1]: https://github.com/P-H-C/phc-string-format/blob/master/phc-sf-spec.md
 
+use super::StringBuf;
 use crate::{Error, Result};
-use core::{fmt, ops::Deref, str};
+use core::{
+    fmt,
+    ops::Deref,
+    str::{self, FromStr},
+};
 
 /// Algorithm or parameter identifier.
 ///
@@ -32,9 +37,9 @@ use core::{fmt, ops::Deref, str};
 ///
 /// [1]: https://github.com/P-H-C/phc-string-format/blob/master/phc-sf-spec.md
 #[derive(Copy, Clone, Eq, Hash, PartialEq, PartialOrd, Ord)]
-pub struct Ident<'a>(&'a str);
+pub struct Ident(StringBuf<{ Ident::MAX_LENGTH }>);
 
-impl<'a> Ident<'a> {
+impl Ident {
     /// Maximum length of an [`Ident`] - 32 ASCII characters (i.e. 32-bytes).
     ///
     /// This value corresponds to the maximum size of a function symbolic names
@@ -49,7 +54,7 @@ impl<'a> Ident<'a> {
     ///
     /// String must conform to the constraints given in the type-level
     /// documentation.
-    pub const fn new(s: &'a str) -> Result<Self> {
+    pub const fn new(s: &str) -> Result<Self> {
         let input = s.as_bytes();
 
         match input.len() {
@@ -64,7 +69,10 @@ impl<'a> Ident<'a> {
                     i += 1;
                 }
 
-                Ok(Self(s))
+                match StringBuf::new(s) {
+                    Ok(buf) => Ok(Self(buf)),
+                    Err(e) => Err(e),
+                }
             }
             _ => Err(Error::ParamNameInvalid),
         }
@@ -75,7 +83,7 @@ impl<'a> Ident<'a> {
     /// This function exists as a workaround for `unwrap` not yet being
     /// stable in `const fn` contexts, and is intended to allow the result to
     /// be bound to a constant value.
-    pub const fn new_unwrap(s: &'a str) -> Self {
+    pub const fn new_unwrap(s: &str) -> Self {
         assert!(!s.is_empty(), "PHC ident string can't be empty");
         assert!(s.len() <= Self::MAX_LENGTH, "PHC ident string too long");
 
@@ -86,42 +94,48 @@ impl<'a> Ident<'a> {
     }
 
     /// Borrow this ident as a `str`
-    pub fn as_str(&self) -> &'a str {
-        self.0
+    pub fn as_str(&self) -> &str {
+        &self.0
     }
 }
 
-impl AsRef<str> for Ident<'_> {
+impl AsRef<str> for Ident {
     fn as_ref(&self) -> &str {
         self.as_str()
     }
 }
 
-impl Deref for Ident<'_> {
+impl Deref for Ident {
     type Target = str;
 
     fn deref(&self) -> &str {
         self.as_str()
     }
 }
 
-// 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> {
+impl FromStr for Ident {
+    type Err = Error;
+
+    fn from_str(s: &str) -> Result<Self> {
+        Self::new(s)
+    }
+}
+
+impl TryFrom<&str> for Ident {
     type Error = Error;
 
-    fn try_from(s: &'a str) -> Result<Self> {
+    fn try_from(s: &str) -> Result<Self> {
         Self::new(s)
     }
 }
 
-impl fmt::Display for Ident<'_> {
+impl fmt::Display for Ident {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.write_str(self)
     }
 }
 
-impl fmt::Debug for Ident<'_> {
+impl fmt::Debug for Ident {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_tuple("Ident").field(&self.as_ref()).finish()
     }

password-hash/src/phc/params.rs

@@ -1,23 +1,23 @@
 //! Algorithm parameters.
 
-use crate::phc::value::{Decimal, Value};
-use crate::{Error, Result, errors::InvalidValue, phc::Ident};
+use super::{Decimal, Ident, StringBuf, Value};
+use crate::{Error, Result, errors::InvalidValue};
 use base64ct::{Base64Unpadded as B64, Encoding};
 use core::{
-    fmt::{self, Debug, Write},
+    fmt::{self, Debug, Write as _},
     str::{self, FromStr},
 };
 
 /// Individual parameter name/value pair.
-pub type Pair<'a> = (Ident<'a>, Value<'a>);
+pub type Pair<'a> = (Ident, Value<'a>);
 
 /// Delimiter character between name/value pairs.
 pub(crate) const PAIR_DELIMITER: char = '=';
 
 /// Delimiter character between parameters.
 pub(crate) const PARAMS_DELIMITER: char = ',';
 
-/// Maximum number of supported parameters.
+/// Maximum serialized length of parameters.
 const MAX_LENGTH: usize = 127;
 
 /// Error message used with `expect` for when internal invariants are violated
@@ -38,7 +38,7 @@ const INVARIANT_VIOLATED_MSG: &str = "PHC params invariant violated";
 ///
 /// [1]: https://github.com/P-H-C/phc-string-format/blob/master/phc-sf-spec.md#specification
 #[derive(Clone, Default, Eq, PartialEq)]
-pub struct ParamsString(Buffer);
+pub struct ParamsString(StringBuf<MAX_LENGTH>);
 
 impl ParamsString {
     /// Create new empty [`ParamsString`].
@@ -47,7 +47,7 @@ impl ParamsString {
     }
 
     /// Add the given byte value to the [`ParamsString`], encoding it as "B64".
-    pub fn add_b64_bytes<'a>(&mut self, name: impl TryInto<Ident<'a>>, bytes: &[u8]) -> Result<()> {
+    pub fn add_b64_bytes(&mut self, name: impl TryInto<Ident>, bytes: &[u8]) -> Result<()> {
         if !self.is_empty() {
             self.0
                 .write_char(PARAMS_DELIMITER)
@@ -72,15 +72,15 @@ impl ParamsString {
     }
 
     /// Add a key/value pair with a decimal value to the [`ParamsString`].
-    pub fn add_decimal<'a>(&mut self, name: impl TryInto<Ident<'a>>, value: Decimal) -> Result<()> {
+    pub fn add_decimal(&mut self, name: impl TryInto<Ident>, value: Decimal) -> Result<()> {
         let name = name.try_into().map_err(|_| Error::ParamNameInvalid)?;
         self.add(name, value)
     }
 
     /// Add a key/value pair with a string value to the [`ParamsString`].
     pub fn add_str<'a>(
         &mut self,
-        name: impl TryInto<Ident<'a>>,
+        name: impl TryInto<Ident>,
         value: impl TryInto<Value<'a>>,
     ) -> Result<()> {
         let name = name.try_into().map_err(|_| Error::ParamNameInvalid)?;
@@ -118,7 +118,7 @@ impl ParamsString {
     }
 
     /// Get a parameter [`Value`] by name.
-    pub fn get<'a>(&self, name: impl TryInto<Ident<'a>>) -> Option<Value<'_>> {
+    pub fn get(&self, name: impl TryInto<Ident>) -> Option<Value<'_>> {
         let name = name.try_into().ok()?;
 
         for (n, v) in self.iter() {
@@ -131,19 +131,19 @@ impl ParamsString {
     }
 
     /// Get a parameter as a `str`.
-    pub fn get_str<'a>(&self, name: impl TryInto<Ident<'a>>) -> Option<&str> {
+    pub fn get_str(&self, name: impl TryInto<Ident>) -> Option<&str> {
         self.get(name).map(|value| value.as_str())
     }
 
     /// Get a parameter as a [`Decimal`].
     ///
     /// See [`Value::decimal`] for format information.
-    pub fn get_decimal<'a>(&self, name: impl TryInto<Ident<'a>>) -> Option<Decimal> {
+    pub fn get_decimal(&self, name: impl TryInto<Ident>) -> Option<Decimal> {
         self.get(name).and_then(|value| value.decimal().ok())
     }
 
     /// Add a value to this [`ParamsString`] using the provided callback.
-    fn add(&mut self, name: Ident<'_>, value: impl fmt::Display) -> Result<()> {
+    fn add(&mut self, name: Ident, value: impl fmt::Display) -> Result<()> {
         if self.get(name).is_some() {
             return Err(Error::ParamNameDuplicated);
         }
@@ -183,7 +183,7 @@ impl FromStr for ParamsString {
             param
                 .next()
                 .ok_or(Error::ParamNameInvalid)
-                .and_then(Ident::try_from)?;
+                .and_then(Ident::from_str)?;
 
             // Validate value
             param
@@ -199,7 +199,7 @@ impl FromStr for ParamsString {
         let mut bytes = [0u8; MAX_LENGTH];
         bytes[..s.len()].copy_from_slice(s.as_bytes());
 
-        Ok(Self(Buffer {
+        Ok(Self(StringBuf {
             bytes,
             length: s.len() as u8,
         }))
@@ -260,7 +260,7 @@ impl<'a> Iterator for Iter<'a> {
 
         let name = param
             .next()
-            .and_then(|id| Ident::try_from(id).ok())
+            .and_then(|id| Ident::from_str(id).ok())
             .expect(INVARIANT_VIOLATED_MSG);
 
         let value = param
@@ -273,54 +273,6 @@ impl<'a> Iterator for Iter<'a> {
     }
 }
 
-/// Parameter buffer.
-#[derive(Clone, Debug, Eq)]
-struct Buffer {
-    /// Byte array containing an ASCII-encoded string.
-    bytes: [u8; MAX_LENGTH],
-
-    /// Length of the string in ASCII characters (i.e. bytes).
-    length: u8,
-}
-
-impl AsRef<str> for Buffer {
-    fn as_ref(&self) -> &str {
-        str::from_utf8(&self.bytes[..(self.length as usize)]).expect(INVARIANT_VIOLATED_MSG)
-    }
-}
-
-impl Default for Buffer {
-    fn default() -> Buffer {
-        Buffer {
-            bytes: [0u8; MAX_LENGTH],
-            length: 0,
-        }
-    }
-}
-
-impl PartialEq for Buffer {
-    fn eq(&self, other: &Self) -> bool {
-        // Ensure comparisons always honor the initialized portion of the buffer
-        self.as_ref().eq(other.as_ref())
-    }
-}
-
-impl Write for Buffer {
-    fn write_str(&mut self, input: &str) -> fmt::Result {
-        let bytes = input.as_bytes();
-        let length = self.length as usize;
-
-        if length + bytes.len() > MAX_LENGTH {
-            return Err(fmt::Error);
-        }
-
-        self.bytes[length..(length + bytes.len())].copy_from_slice(bytes);
-        self.length += bytes.len() as u8;
-
-        Ok(())
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::{Error, Ident, ParamsString, Value};