Use secure_string for more secure Password and SecretKey

.gitignore

@@ -13,3 +13,6 @@ target/
 *.db
 *.sqlite3
 *.sqlite3-journal
+
+# VSCode
+.vscode/

Cargo.toml

@@ -115,6 +115,7 @@ mime_guess = { version = "2", default-features = false }
 mockall = "0.14"
 multer = "3"
 password-auth = { version = "1", default-features = false }
+securer-string = "0.1.2"
 petgraph = { version = "0.8", default-features = false }
 pin-project-lite = "0.2"
 prettyplease = "0.2"

cot/Cargo.toml

@@ -46,6 +46,7 @@ mime.workspace = true
 mime_guess.workspace = true
 multer.workspace = true
 password-auth = { workspace = true, features = ["std", "argon2"] }
+securer-string.workspace = true
 pin-project-lite.workspace = true
 redis = { workspace = true, features = ["aio", "tokio-comp"], optional = true }
 schemars = { workspace = true, optional = true, features = ["derive"] }

cot/src/common_types.rs

@@ -16,6 +16,7 @@ use cot::db::impl_postgres::PostgresValueRef;
 use cot::db::impl_sqlite::SqliteValueRef;
 use cot::form::FormFieldValidationError;
 use email_address::EmailAddress;
+use securer_string::SecureString;
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
 
@@ -50,29 +51,28 @@ const MAX_EMAIL_LENGTH: u32 = 254;
 ///    with the other password. This method uses constant-time equality
 ///    comparison, which protects against timing attacks.
 ///
-/// 2. An alternative is to use the [`Password::as_str`] method and compare the
-///    strings directly. This approach uses non-constant-time comparison, which
-///    is less secure but may be acceptable in certain legitimate use cases
-///    where the security tradeoff is understood, e.g., when you're creating a
-///    user registration form with the "retype your password" field, where both
-///    passwords come from the same source anyway.
+/// 2. An alternative is to compare 2 instances of the [`Password`] type
+///    directly because this password struct implements the [`PartialEq`] trait
+///    which also uses constant-time comparison. Comparing 2 instances of the
+///    [`Password`] type is less secure than using [`PasswordHash::verify`], but
+///    may be acceptable in certain legitimate use cases where the security
+///    tradeoff is understood, e.g., when you're creating a user registration
+///    form with the "retype your password" field, in this case this approach
+///    might save on hashing costs.
 ///
 /// # Examples
 ///
 /// ```
 /// use cot::common_types::Password;
 ///
 /// let password = Password::new("pass");
-/// assert_eq!(&format!("{:?}", password), "Password(\"**********\")");
+/// assert_eq!(&format!("{:?}", password), "Password(..)");
+///
+/// let another_password = Password::new("pass");
+/// assert_eq!(password, another_password);
 /// ```
-#[derive(Clone)]
-pub struct Password(String);
-
-impl Debug for Password {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_tuple("Password").field(&"**********").finish()
-    }
-}
+#[derive(Clone, PartialEq, Eq)]
+pub struct Password(SecureString);
 
 impl Password {
     /// Creates a new password object.
@@ -86,7 +86,7 @@ impl Password {
     /// ```
     #[must_use]
     pub fn new<T: Into<String>>(password: T) -> Self {
-        Self(password.into())
+        Self(SecureString::from(password.into()))
     }
 
     /// Returns the password as a string.
@@ -101,7 +101,7 @@ impl Password {
     /// ```
     #[must_use]
     pub fn as_str(&self) -> &str {
-        &self.0
+        self.0.unsecure()
     }
 
     /// Consumes the object and returns the password as a string.
@@ -116,7 +116,13 @@ impl Password {
     /// ```
     #[must_use]
     pub fn into_string(self) -> String {
-        self.0
+        self.0.into_unsecure()
+    }
+}
+
+impl Debug for Password {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_tuple("Password").finish_non_exhaustive()
     }
 }
 
@@ -830,7 +836,21 @@ mod tests {
     #[test]
     fn password_debug() {
         let password = Password::new("password");
-        assert_eq!(format!("{password:?}"), "Password(\"**********\")");
+        assert_eq!(format!("{password:?}"), "Password(..)");
+    }
+
+    #[test]
+    fn password_eq() {
+        let password1 = Password::new("password");
+        let password2 = Password::new("password");
+        assert_eq!(password1, password2);
+    }
+
+    #[test]
+    fn password_ne() {
+        let password1 = Password::new("password");
+        let password2 = Password::new("password2");
+        assert_ne!(password1, password2);
     }
 
     #[test]

cot/src/config.rs

@@ -22,8 +22,8 @@ use chrono::{DateTime, FixedOffset, Utc};
 use cot_core::error::impl_into_cot_error;
 use derive_builder::Builder;
 use derive_more::with_trait::{Debug, From};
+use securer_string::SecureBytes;
 use serde::{Deserialize, Serialize};
-use subtle::ConstantTimeEq;
 use thiserror::Error;
 
 #[cfg(feature = "email")]
@@ -2116,11 +2116,12 @@ impl Default for EmailConfig {
 ///
 /// # Security
 ///
-/// The implementation of the [`PartialEq`] trait for this type is constant-time
-/// to prevent timing attacks.
+/// The implementation of the [`PartialEq`] trait for this type uses
+/// constant-time comparison to prevent timing attacks.
 ///
-/// The implementation of the [`Debug`] trait for this type hides the secret key
-/// to prevent it from being leaked in logs or other debug output.
+/// The implementation of the [`Debug`] trait for this type is inherited from
+/// [`SecureBytes`], which hides the secret key to prevent it from being leaked
+/// in logs or other debug output.
 ///
 /// # Examples
 ///
@@ -2131,9 +2132,18 @@ impl Default for EmailConfig {
 /// assert_eq!(key.as_bytes(), &[1, 2, 3]);
 /// ```
 #[repr(transparent)]
-#[derive(Clone, Serialize, Deserialize)]
+#[derive(Clone, Deserialize, PartialEq, Eq)]
 #[serde(from = "String")]
-pub struct SecretKey(Box<[u8]>);
+pub struct SecretKey(SecureBytes);
+
+impl Serialize for SecretKey {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        serializer.serialize_bytes(self.as_bytes())
+    }
+}
 
 impl SecretKey {
     /// Create a new [`SecretKey`] from a byte array.
@@ -2148,7 +2158,7 @@ impl SecretKey {
     /// ```
     #[must_use]
     pub fn new(hash: &[u8]) -> Self {
-        Self(Box::from(hash))
+        Self(SecureBytes::new(hash.to_vec()))
     }
 
     /// Get the byte array stored in the [`SecretKey`].
@@ -2163,7 +2173,7 @@ impl SecretKey {
     /// ```
     #[must_use]
     pub fn as_bytes(&self) -> &[u8] {
-        &self.0
+        self.0.unsecure()
     }
 
     /// Consume the [`SecretKey`] and return the byte array stored in it.
@@ -2178,7 +2188,19 @@ impl SecretKey {
     /// ```
     #[must_use]
     pub fn into_bytes(self) -> Box<[u8]> {
-        self.0
+        self.0.unsecure().to_vec().into_boxed_slice()
+    }
+}
+
+impl Debug for SecretKey {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_tuple("SecretKey").finish_non_exhaustive()
+    }
+}
+
+impl Default for SecretKey {
+    fn default() -> Self {
+        Self::new(&[])
     }
 }
 
@@ -2200,27 +2222,6 @@ impl From<&str> for SecretKey {
     }
 }
 
-impl PartialEq for SecretKey {
-    fn eq(&self, other: &Self) -> bool {
-        self.0.ct_eq(&other.0).into()
-    }
-}
-
-impl Eq for SecretKey {}
-
-impl Debug for SecretKey {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        // write in single line, regardless whether alternate mode was used or not
-        write!(f, "SecretKey(\"**********\")")
-    }
-}
-
-impl Default for SecretKey {
-    fn default() -> Self {
-        Self::new(&[])
-    }
-}
-
 /// A URL for the database.
 ///
 /// This is a wrapper over the [`url::Url`] type, which is used to store the
@@ -2495,6 +2496,7 @@ impl From<&str> for EmailUrl {
 
 #[cfg(test)]
 mod tests {
+    use serde_json;
     use time::OffsetDateTime;
 
     use super::*;
@@ -2764,6 +2766,14 @@ mod tests {
             StaticFilesPathRewriteMode::QueryParam
         );
     }
+
+    #[test]
+    fn secret_key_serialize_json() {
+        let key = SecretKey::from("abc123");
+        let serialized = serde_json::to_string(&key).unwrap();
+        // Should serialize as a byte array
+        assert_eq!(serialized, "[97,98,99,49,50,51]");
+    }
     #[test]
     #[cfg(feature = "redis")]
     fn cache_type_from_str_redis() {