From 2cdfd65acc4b92e5b74b9a9eaf559a9afc63873b Mon Sep 17 00:00:00 2001 From: "Ibrahim E.Gad" Date: Thu, 15 May 2025 21:42:40 +0300 Subject: [PATCH 01/11] feat: add secure-string dependency and integrate into Password struct - Updated struct to use SecureString for enhanced security. - Implemented PartialEq and Eq to be able to compare 2 Password instances - Added tests for equality comparison of instances. --- Cargo.lock | 11 +++++++++ Cargo.toml | 1 + cot/Cargo.toml | 1 + cot/src/common_types.rs | 54 +++++++++++++++++++++++++++++------------ 4 files changed, 51 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ebce5601e..1fdecb927 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -617,6 +617,7 @@ dependencies = [ "schemars", "sea-query", "sea-query-binder", + "secure-string", "serde", "serde_html_form", "serde_json", @@ -2537,6 +2538,16 @@ dependencies = [ "sqlx", ] +[[package]] +name = "secure-string" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "548ba8c9ff631f7bb3a64de1e8ad73fe20f6d04090724f2b496ed45314ad7488" +dependencies = [ + "libc", + "zeroize", +] + [[package]] name = "security-framework" version = "2.11.1" diff --git a/Cargo.toml b/Cargo.toml index bb0b081e0..53f8311b7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -94,6 +94,7 @@ insta-cmd = "0.6" mime_guess = { version = "2", default-features = false } mockall = "0.13" password-auth = { version = "1", default-features = false } +secure-string = "0.3.0" petgraph = { version = "0.8", default-features = false } pin-project-lite = "0.2" prettyplease = "0.2" diff --git a/cot/Cargo.toml b/cot/Cargo.toml index 26004189b..20b30d1fe 100644 --- a/cot/Cargo.toml +++ b/cot/Cargo.toml @@ -42,6 +42,7 @@ humantime.workspace = true indexmap.workspace = true mime_guess.workspace = true password-auth = { workspace = true, features = ["std", "argon2"] } +secure-string.workspace = true pin-project-lite.workspace = true schemars = { workspace = true, optional = true } sea-query = { workspace = true, optional = true } diff --git a/cot/src/common_types.rs b/cot/src/common_types.rs index 6de40eb6a..128bb3ea5 100644 --- a/cot/src/common_types.rs +++ b/cot/src/common_types.rs @@ -5,6 +5,7 @@ //! general-purpose newtype wrappers and associated trait implementations to //! ensure consistent and safe processing of form data. +use secure_string::SecureString; use std::fmt::Debug; use std::str::FromStr; @@ -45,12 +46,13 @@ const MAX_EMAIL_LENGTH: u32 = 254; /// compare it 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. You can also +/// use the [`Password::as_str`] method to 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. /// /// # Examples /// @@ -58,17 +60,23 @@ const MAX_EMAIL_LENGTH: u32 = 254; /// use cot::auth::Password; /// /// let password = Password::new("pass"); -/// assert_eq!(&format!("{:?}", password), "Password(\"**********\")"); +/// assert_eq!(&format!("{:?}", password), "Password(***SECRET***)"); +/// +/// let another_password = Password::new("pass"); +/// assert_eq!(password, another_password); +/// /// ``` -#[derive(Clone)] -pub struct Password(String); +#[derive(Clone, Debug)] +pub struct Password(SecureString); -impl Debug for Password { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_tuple("Password").field(&"**********").finish() +impl PartialEq for Password { + fn eq(&self, other: &Self) -> bool { + self.0 == other.0 } } +impl Eq for Password {} + impl Password { /// Creates a new password object. /// @@ -80,7 +88,7 @@ impl Password { /// let password = Password::new("password"); /// ``` #[must_use] - pub fn new>(password: T) -> Self { + pub fn new>(password: T) -> Self { Self(password.into()) } @@ -96,7 +104,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. @@ -111,7 +119,7 @@ impl Password { /// ``` #[must_use] pub fn into_string(self) -> String { - self.0 + self.0.into_unsecure() } } @@ -422,7 +430,21 @@ mod tests { #[test] fn password_debug() { let password = Password::new("password"); - assert_eq!(format!("{password:?}"), "Password(\"**********\")"); + assert_eq!(format!("{password:?}"), "Password(***SECRET***)"); + } + + #[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] From 8ee2f46afcbfde54e190e828f46cb59d817184e6 Mon Sep 17 00:00:00 2001 From: "Ibrahim E.Gad" Date: Thu, 15 May 2025 22:54:51 +0300 Subject: [PATCH 02/11] feat: replace Box<[u8]> with SecureBytes in SecretKey for enhanced security - Updated SecretKey struct to use SecureBytes for improved security handling. - Adjusted implementations of PartialEq and Debug to align with SecureBytes. - Implemented serialization for SecretKey since we cannot derive Serialize since SecureBytes doesn't implement it . - Included tests for JSON serialization of SecretKey. --- cot/src/config.rs | 48 +++++++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/cot/src/config.rs b/cot/src/config.rs index 37024064b..22880af50 100644 --- a/cot/src/config.rs +++ b/cot/src/config.rs @@ -19,8 +19,8 @@ use std::time::Duration; use derive_builder::Builder; use derive_more::with_trait::{Debug, From}; +use secure_string::SecureBytes; use serde::{Deserialize, Serialize}; -use subtle::ConstantTimeEq; /// The configuration for a project. /// @@ -819,11 +819,12 @@ impl SessionMiddlewareConfigBuilder { /// /// # 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 is inherited from +/// [`SecureBytes`], which is constant-time 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 /// @@ -834,9 +835,18 @@ impl SessionMiddlewareConfigBuilder { /// assert_eq!(key.as_bytes(), &[1, 2, 3]); /// ``` #[repr(transparent)] -#[derive(Clone, Serialize, Deserialize)] +#[derive(Clone, Deserialize, Debug)] #[serde(from = "String")] -pub struct SecretKey(Box<[u8]>); +pub struct SecretKey(SecureBytes); + +impl Serialize for SecretKey { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + serializer.serialize_bytes(self.as_bytes()) + } +} impl SecretKey { /// Create a new [`SecretKey`] from a byte array. @@ -851,7 +861,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`]. @@ -866,7 +876,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. @@ -881,7 +891,7 @@ impl SecretKey { /// ``` #[must_use] pub fn into_bytes(self) -> Box<[u8]> { - self.0 + self.0.unsecure().to_vec().into_boxed_slice() } } @@ -905,19 +915,12 @@ impl From<&str> for SecretKey { impl PartialEq for SecretKey { fn eq(&self, other: &Self) -> bool { - self.0.ct_eq(&other.0).into() + self.0 == other.0 } } 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(&[]) @@ -1005,6 +1008,7 @@ impl Debug for DatabaseUrl { #[cfg(test)] mod tests { use super::*; + use serde_json; #[test] fn from_toml_valid() { @@ -1078,4 +1082,12 @@ 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]"); + } } From c7dc276039e4435888bea5a1390f72c78d1226a9 Mon Sep 17 00:00:00 2001 From: "Ibrahim E.Gad" Date: Tue, 27 May 2025 22:59:22 +0300 Subject: [PATCH 03/11] chore: Refactor Password and SecretKey structs for Miri compatibility - Refactored Password and SecretKey structs to provide Miri-compatible versions, using String and Box<[u8]> respectively. - Adjusted methods for creating and accessing passwords and secret keys to ensure compatibility with Miri's limitations. --- .gitignore | 3 ++ cot/src/common_types.rs | 61 +++++++++++++++++++++++++++++++++-------- cot/src/config.rs | 47 ++++++++++++++++++++++++++++--- 3 files changed, 96 insertions(+), 15 deletions(-) diff --git a/.gitignore b/.gitignore index a611abcd7..a07843edf 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,6 @@ target/ *.db *.sqlite3 *.sqlite3-journal + +# VSCode +.vscode/ diff --git a/cot/src/common_types.rs b/cot/src/common_types.rs index 128bb3ea5..dec1ef5fc 100644 --- a/cot/src/common_types.rs +++ b/cot/src/common_types.rs @@ -5,7 +5,6 @@ //! general-purpose newtype wrappers and associated trait implementations to //! ensure consistent and safe processing of form data. -use secure_string::SecureString; use std::fmt::Debug; use std::str::FromStr; @@ -16,6 +15,8 @@ use cot::db::impl_postgres::PostgresValueRef; #[cfg(feature = "sqlite")] use cot::db::impl_sqlite::SqliteValueRef; use email_address::EmailAddress; +#[cfg(not(miri))] +use secure_string::SecureString; #[cfg(feature = "db")] use crate::db::{ColumnType, DatabaseField, DbValue, FromDbValue, SqlxValueRef, ToDbValue}; @@ -46,13 +47,14 @@ const MAX_EMAIL_LENGTH: u32 = 254; /// compare it with the other password. This method uses constant-time /// equality comparison, which protects against timing attacks. /// -/// 2. An alternative is to compare 2 instances of the [`Password`] type directly -/// because this password struct implements the [`PartialEq`] trait. You can also -/// use the [`Password::as_str`] method to 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. +/// You can also use the [`Password::as_str`] method to 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. /// /// # Examples /// @@ -64,9 +66,9 @@ const MAX_EMAIL_LENGTH: u32 = 254; /// /// let another_password = Password::new("pass"); /// assert_eq!(password, another_password); -/// /// ``` #[derive(Clone, Debug)] +#[cfg(not(miri))] pub struct Password(SecureString); impl PartialEq for Password { @@ -77,6 +79,15 @@ impl PartialEq for Password { impl Eq for Password {} +/// A password - simplified version for Miri testing. +/// +/// When running under Miri, we use a simple String wrapper instead of +/// SecureString to avoid the mlock system call that Miri doesn't support. +#[derive(Clone)] +#[cfg(miri)] +pub struct Password(String); + +#[cfg(not(miri))] impl Password { /// Creates a new password object. /// @@ -88,8 +99,8 @@ impl Password { /// let password = Password::new("password"); /// ``` #[must_use] - pub fn new>(password: T) -> Self { - Self(password.into()) + pub fn new>(password: T) -> Self { + Self(SecureString::from(password.into())) } /// Returns the password as a string. @@ -123,6 +134,27 @@ impl Password { } } +#[cfg(miri)] +impl Password { + /// Creates a new password object - Miri version. + #[must_use] + pub fn new>(password: T) -> Self { + Self(password.into()) + } + + /// Returns the password as a string - Miri version. + #[must_use] + pub fn as_str(&self) -> &str { + &self.0 + } + + /// Consumes the object and returns the password as a string - Miri version. + #[must_use] + pub fn into_string(self) -> String { + self.0 + } +} + impl From<&Password> for Password { fn from(password: &Password) -> Self { password.clone() @@ -141,6 +173,13 @@ impl From for Password { } } +#[cfg(miri)] +impl Debug for Password { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Password(***SECRET***)") + } +} + /// A validated email address. /// /// This is a newtype wrapper around diff --git a/cot/src/config.rs b/cot/src/config.rs index 22880af50..9cb4c433f 100644 --- a/cot/src/config.rs +++ b/cot/src/config.rs @@ -19,8 +19,10 @@ use std::time::Duration; use derive_builder::Builder; use derive_more::with_trait::{Debug, From}; +#[cfg(not(miri))] use secure_string::SecureBytes; use serde::{Deserialize, Serialize}; +use subtle::ConstantTimeEq; /// The configuration for a project. /// @@ -819,8 +821,8 @@ impl SessionMiddlewareConfigBuilder { /// /// # Security /// -/// The implementation of the [`PartialEq`] trait for this type is inherited from -/// [`SecureBytes`], which is constant-time to prevent timing attacks. +/// The implementation of the [`PartialEq`] trait for this type is inherited +/// from [`SecureBytes`], which is constant-time to prevent timing attacks. /// /// The implementation of the [`Debug`] trait for this type is inherited from /// [`SecureBytes`], which hides the secret key to prevent it from being leaked @@ -837,8 +839,19 @@ impl SessionMiddlewareConfigBuilder { #[repr(transparent)] #[derive(Clone, Deserialize, Debug)] #[serde(from = "String")] +#[cfg(not(miri))] pub struct SecretKey(SecureBytes); +/// A secret key - simplified version for Miri testing. +/// +/// When running under Miri, we use a simple Box<[u8]> wrapper instead of +/// SecureBytes to avoid the mlock system call that Miri doesn't support. +#[repr(transparent)] +#[derive(Clone, Deserialize, Debug)] +#[serde(from = "String")] +#[cfg(miri)] +pub struct SecretKey(Box<[u8]>); + impl Serialize for SecretKey { fn serialize(&self, serializer: S) -> Result where @@ -860,10 +873,18 @@ impl SecretKey { /// assert_eq!(key.as_bytes(), &[1, 2, 3]); /// ``` #[must_use] + #[cfg(not(miri))] pub fn new(hash: &[u8]) -> Self { Self(SecureBytes::new(hash.to_vec())) } + /// Create a new [`SecretKey`] from a byte array - Miri version. + #[must_use] + #[cfg(miri)] + pub fn new(hash: &[u8]) -> Self { + Self(Box::from(hash)) + } + /// Get the byte array stored in the [`SecretKey`]. /// /// # Examples @@ -875,10 +896,18 @@ impl SecretKey { /// assert_eq!(key.as_bytes(), &[1, 2, 3]); /// ``` #[must_use] + #[cfg(not(miri))] pub fn as_bytes(&self) -> &[u8] { self.0.unsecure() } + /// Get the byte array stored in the [`SecretKey`] - Miri version. + #[must_use] + #[cfg(miri)] + pub fn as_bytes(&self) -> &[u8] { + &self.0 + } + /// Consume the [`SecretKey`] and return the byte array stored in it. /// /// # Examples @@ -890,9 +919,18 @@ impl SecretKey { /// assert_eq!(key.into_bytes(), Box::from([1, 2, 3])); /// ``` #[must_use] + #[cfg(not(miri))] pub fn into_bytes(self) -> Box<[u8]> { self.0.unsecure().to_vec().into_boxed_slice() } + + /// Consume the [`SecretKey`] and return the byte array stored in it - Miri + /// version. + #[must_use] + #[cfg(miri)] + pub fn into_bytes(self) -> Box<[u8]> { + self.0 + } } impl From<&[u8]> for SecretKey { @@ -915,7 +953,7 @@ impl From<&str> for SecretKey { impl PartialEq for SecretKey { fn eq(&self, other: &Self) -> bool { - self.0 == other.0 + self.as_bytes().ct_eq(other.as_bytes()).into() } } @@ -1007,9 +1045,10 @@ impl Debug for DatabaseUrl { #[cfg(test)] mod tests { - use super::*; use serde_json; + use super::*; + #[test] fn from_toml_valid() { let toml_content = r#" From e255fd17a6bc642421c8d479035d7c66cf3aba4c Mon Sep 17 00:00:00 2001 From: "Ibrahim E.Gad" Date: Tue, 27 May 2025 23:06:53 +0300 Subject: [PATCH 04/11] Fix inconsistency in docs about the implementation of PartialEq --- cot/src/config.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cot/src/config.rs b/cot/src/config.rs index 9cb4c433f..6a6bf726a 100644 --- a/cot/src/config.rs +++ b/cot/src/config.rs @@ -821,8 +821,8 @@ impl SessionMiddlewareConfigBuilder { /// /// # Security /// -/// The implementation of the [`PartialEq`] trait for this type is inherited -/// from [`SecureBytes`], which 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 is inherited from /// [`SecureBytes`], which hides the secret key to prevent it from being leaked From c2d06c0440acbecf43fc2b9e124171783c90323c Mon Sep 17 00:00:00 2001 From: "Ibrahim E.Gad" Date: Sat, 31 May 2025 13:20:28 +0300 Subject: [PATCH 05/11] Add Unlicense to cargo-deny's allowed list of licenses --- deny.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/deny.toml b/deny.toml index 6c253cebc..33a87555b 100644 --- a/deny.toml +++ b/deny.toml @@ -20,4 +20,5 @@ allow = [ "0BSD", "BSD-3-Clause", "Zlib", + "Unlicense", ] From 767a1f6dc8189fb47698156b9645a67c81c3d230 Mon Sep 17 00:00:00 2001 From: "Ibrahim E.Gad" Date: Sun, 1 Jun 2025 13:47:40 +0300 Subject: [PATCH 06/11] Use constant time comparison for Password comparison --- cot/src/common_types.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/cot/src/common_types.rs b/cot/src/common_types.rs index dec1ef5fc..be60fb106 100644 --- a/cot/src/common_types.rs +++ b/cot/src/common_types.rs @@ -17,6 +17,7 @@ use cot::db::impl_sqlite::SqliteValueRef; use email_address::EmailAddress; #[cfg(not(miri))] use secure_string::SecureString; +use subtle::ConstantTimeEq; #[cfg(feature = "db")] use crate::db::{ColumnType, DatabaseField, DbValue, FromDbValue, SqlxValueRef, ToDbValue}; @@ -48,13 +49,13 @@ const MAX_EMAIL_LENGTH: u32 = 254; /// equality comparison, which protects against timing attacks. /// /// 2. An alternative is to compare 2 instances of the [`Password`] type -/// directly because this password struct implements the [`PartialEq`] trait. -/// You can also use the [`Password::as_str`] method to 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. +/// 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 /// @@ -73,7 +74,10 @@ pub struct Password(SecureString); impl PartialEq for Password { fn eq(&self, other: &Self) -> bool { - self.0 == other.0 + self.as_str() + .as_bytes() + .ct_eq(other.as_str().as_bytes()) + .into() } } From 8f5aa2c66cf1e4fd26bd55991f4d15b2c9d5d74b Mon Sep 17 00:00:00 2001 From: Marek 'seqre' Grzelak Date: Mon, 9 Mar 2026 20:22:03 +0100 Subject: [PATCH 07/11] fix merge conflicts --- cot/src/config.rs | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/cot/src/config.rs b/cot/src/config.rs index 3f903a5d7..bab6795a8 100644 --- a/cot/src/config.rs +++ b/cot/src/config.rs @@ -2099,7 +2099,7 @@ impl Default for EmailConfig { /// assert_eq!(key.as_bytes(), &[1, 2, 3]); /// ``` #[repr(transparent)] -#[derive(Clone, Deserialize, Debug)] +#[derive(Clone, Deserialize)] #[serde(from = "String")] #[cfg(not(miri))] pub struct SecretKey(SecureBytes); @@ -2213,12 +2213,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 { @@ -2228,6 +2222,12 @@ impl Debug for SecretKey { } } +impl PartialEq for SecretKey { + fn eq(&self, other: &Self) -> bool { + self.as_bytes().ct_eq(other.as_bytes()).into() + } +} + impl Default for SecretKey { fn default() -> Self { Self::new(&[]) @@ -2417,17 +2417,6 @@ impl From<&str> for CacheUrl { } } -impl PartialEq for SecretKey { - fn eq(&self, other: &Self) -> bool { - self.as_bytes().ct_eq(other.as_bytes()).into() - } -} - -impl Eq for SecretKey {} - -impl Default for SecretKey { - fn default() -> Self { - Self::new(&[]) #[cfg(feature = "cache")] impl TryFrom for CacheType { type Error = ParseCacheTypeError; @@ -2796,6 +2785,7 @@ mod tests { 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() { From 477ee40d034141c9ee3440318932128f8219fd03 Mon Sep 17 00:00:00 2001 From: Marek 'seqre' Grzelak Date: Fri, 22 May 2026 19:32:28 +0200 Subject: [PATCH 08/11] use securer-string --- .cargo/deny.toml | 1 - Cargo.lock | 9 +++++---- Cargo.toml | 2 +- cot/Cargo.toml | 2 +- cot/src/common_types.rs | 2 +- cot/src/config.rs | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.cargo/deny.toml b/.cargo/deny.toml index d4bfe0cc1..44e84bbbd 100644 --- a/.cargo/deny.toml +++ b/.cargo/deny.toml @@ -25,5 +25,4 @@ allow = [ "MIT", "Unicode-3.0", "Zlib", - "Unlicense", ] diff --git a/Cargo.lock b/Cargo.lock index 232373a7b..af56e9779 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -920,7 +920,7 @@ dependencies = [ "schemars", "sea-query", "sea-query-binder", - "secure-string", + "securer-string", "serde", "serde_json", "serde_urlencoded", @@ -3659,12 +3659,13 @@ dependencies = [ ] [[package]] -name = "secure-string" -version = "0.3.0" +name = "securer-string" +version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "548ba8c9ff631f7bb3a64de1e8ad73fe20f6d04090724f2b496ed45314ad7488" +checksum = "4edeb1ced12a195136c31341b32b0734e7f77d004cc4ad260053c4d4bb50a4be" dependencies = [ "libc", + "subtle", "zeroize", ] diff --git a/Cargo.toml b/Cargo.toml index 5a8057808..41b7209e5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -115,7 +115,7 @@ mime_guess = { version = "2", default-features = false } mockall = "0.14" multer = "3" password-auth = { version = "1", default-features = false } -secure-string = "0.3.0" +securer-string = "0.1" petgraph = { version = "0.8", default-features = false } pin-project-lite = "0.2" prettyplease = "0.2" diff --git a/cot/Cargo.toml b/cot/Cargo.toml index 3d4619062..6a9cc298b 100644 --- a/cot/Cargo.toml +++ b/cot/Cargo.toml @@ -46,7 +46,7 @@ mime.workspace = true mime_guess.workspace = true multer.workspace = true password-auth = { workspace = true, features = ["std", "argon2"] } -secure-string.workspace = true +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"] } diff --git a/cot/src/common_types.rs b/cot/src/common_types.rs index 219607f76..270d17bb8 100644 --- a/cot/src/common_types.rs +++ b/cot/src/common_types.rs @@ -17,7 +17,7 @@ use cot::db::impl_sqlite::SqliteValueRef; use cot::form::FormFieldValidationError; use email_address::EmailAddress; #[cfg(not(miri))] -use secure_string::SecureString; +use securer_string::SecureString; use serde::{Deserialize, Serialize}; use subtle::ConstantTimeEq; use thiserror::Error; diff --git a/cot/src/config.rs b/cot/src/config.rs index 180fb1881..8fd74d26c 100644 --- a/cot/src/config.rs +++ b/cot/src/config.rs @@ -23,7 +23,7 @@ use cot_core::error::impl_into_cot_error; use derive_builder::Builder; use derive_more::with_trait::{Debug, From}; #[cfg(not(miri))] -use secure_string::SecureBytes; +use securer_string::SecureBytes; use serde::{Deserialize, Serialize}; use subtle::ConstantTimeEq; use thiserror::Error; From 3e055c47b591e89c089a89074afa4a7f8387a5c3 Mon Sep 17 00:00:00 2001 From: Marek 'seqre' Grzelak Date: Fri, 22 May 2026 19:54:59 +0200 Subject: [PATCH 09/11] clean up password and secret key --- cot/src/common_types.rs | 55 +++----------------------------- cot/src/config.rs | 69 +++++++---------------------------------- 2 files changed, 16 insertions(+), 108 deletions(-) diff --git a/cot/src/common_types.rs b/cot/src/common_types.rs index 270d17bb8..496c78990 100644 --- a/cot/src/common_types.rs +++ b/cot/src/common_types.rs @@ -16,10 +16,8 @@ use cot::db::impl_postgres::PostgresValueRef; use cot::db::impl_sqlite::SqliteValueRef; use cot::form::FormFieldValidationError; use email_address::EmailAddress; -#[cfg(not(miri))] use securer_string::SecureString; use serde::{Deserialize, Serialize}; -use subtle::ConstantTimeEq; use thiserror::Error; #[cfg(feature = "db")] @@ -73,30 +71,9 @@ const MAX_EMAIL_LENGTH: u32 = 254; /// let another_password = Password::new("pass"); /// assert_eq!(password, another_password); /// ``` -#[derive(Clone, Debug)] -#[cfg(not(miri))] +#[derive(Clone, PartialEq, Eq)] pub struct Password(SecureString); -impl PartialEq for Password { - fn eq(&self, other: &Self) -> bool { - self.as_str() - .as_bytes() - .ct_eq(other.as_str().as_bytes()) - .into() - } -} - -impl Eq for Password {} - -/// A password - simplified version for Miri testing. -/// -/// When running under Miri, we use a simple String wrapper instead of -/// SecureString to avoid the mlock system call that Miri doesn't support. -#[derive(Clone)] -#[cfg(miri)] -pub struct Password(String); - -#[cfg(not(miri))] impl Password { /// Creates a new password object. /// @@ -143,24 +120,9 @@ impl Password { } } -#[cfg(miri)] -impl Password { - /// Creates a new password object - Miri version. - #[must_use] - pub fn new>(password: T) -> Self { - Self(password.into()) - } - - /// Returns the password as a string - Miri version. - #[must_use] - pub fn as_str(&self) -> &str { - &self.0 - } - - /// Consumes the object and returns the password as a string - Miri version. - #[must_use] - pub fn into_string(self) -> String { - self.0 +impl Debug for Password { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_tuple("Password").finish_non_exhaustive() } } @@ -182,13 +144,6 @@ impl From for Password { } } -#[cfg(miri)] -impl Debug for Password { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "Password(***SECRET***)") - } -} - /// A validated URL wrapper. /// /// This structure ensures that the contained URL is correctly formatted and @@ -881,7 +836,7 @@ mod tests { #[test] fn password_debug() { let password = Password::new("password"); - assert_eq!(format!("{password:?}"), "Password(***SECRET***)"); + assert_eq!(format!("{password:?}"), "Password(..)"); } #[test] diff --git a/cot/src/config.rs b/cot/src/config.rs index 8fd74d26c..c4f5b42c5 100644 --- a/cot/src/config.rs +++ b/cot/src/config.rs @@ -22,10 +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}; -#[cfg(not(miri))] use securer_string::SecureBytes; use serde::{Deserialize, Serialize}; -use subtle::ConstantTimeEq; use thiserror::Error; #[cfg(feature = "email")] @@ -2134,21 +2132,10 @@ impl Default for EmailConfig { /// assert_eq!(key.as_bytes(), &[1, 2, 3]); /// ``` #[repr(transparent)] -#[derive(Clone, Deserialize)] +#[derive(Clone, Deserialize, PartialEq, Eq)] #[serde(from = "String")] -#[cfg(not(miri))] pub struct SecretKey(SecureBytes); -/// A secret key - simplified version for Miri testing. -/// -/// When running under Miri, we use a simple Box<[u8]> wrapper instead of -/// SecureBytes to avoid the mlock system call that Miri doesn't support. -#[repr(transparent)] -#[derive(Clone, Deserialize, Debug)] -#[serde(from = "String")] -#[cfg(miri)] -pub struct SecretKey(Box<[u8]>); - impl Serialize for SecretKey { fn serialize(&self, serializer: S) -> Result where @@ -2170,18 +2157,10 @@ impl SecretKey { /// assert_eq!(key.as_bytes(), &[1, 2, 3]); /// ``` #[must_use] - #[cfg(not(miri))] pub fn new(hash: &[u8]) -> Self { Self(SecureBytes::new(hash.to_vec())) } - /// Create a new [`SecretKey`] from a byte array - Miri version. - #[must_use] - #[cfg(miri)] - pub fn new(hash: &[u8]) -> Self { - Self(Box::from(hash)) - } - /// Get the byte array stored in the [`SecretKey`]. /// /// # Examples @@ -2193,18 +2172,10 @@ impl SecretKey { /// assert_eq!(key.as_bytes(), &[1, 2, 3]); /// ``` #[must_use] - #[cfg(not(miri))] pub fn as_bytes(&self) -> &[u8] { self.0.unsecure() } - /// Get the byte array stored in the [`SecretKey`] - Miri version. - #[must_use] - #[cfg(miri)] - pub fn as_bytes(&self) -> &[u8] { - &self.0 - } - /// Consume the [`SecretKey`] and return the byte array stored in it. /// /// # Examples @@ -2216,17 +2187,20 @@ impl SecretKey { /// assert_eq!(key.into_bytes(), Box::from([1, 2, 3])); /// ``` #[must_use] - #[cfg(not(miri))] pub fn into_bytes(self) -> Box<[u8]> { self.0.unsecure().to_vec().into_boxed_slice() } +} - /// Consume the [`SecretKey`] and return the byte array stored in it - Miri - /// version. - #[must_use] - #[cfg(miri)] - pub fn into_bytes(self) -> Box<[u8]> { - self.0 +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(&[]) } } @@ -2248,27 +2222,6 @@ impl From<&str> for SecretKey { } } -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 PartialEq for SecretKey { - fn eq(&self, other: &Self) -> bool { - self.as_bytes().ct_eq(other.as_bytes()).into() - } -} - -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 From 517c689ce31dd949447f23f312dbcd3fac73743d Mon Sep 17 00:00:00 2001 From: Marek 'seqre' Grzelak Date: Fri, 22 May 2026 20:04:41 +0200 Subject: [PATCH 10/11] update securer-string to 0.1.2 --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index af56e9779..72ab8b986 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3660,9 +3660,9 @@ dependencies = [ [[package]] name = "securer-string" -version = "0.1.1" +version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4edeb1ced12a195136c31341b32b0734e7f77d004cc4ad260053c4d4bb50a4be" +checksum = "c796cd4a543edb87ad599e495048c5c7f433614c19ae009f5050b24381715efb" dependencies = [ "libc", "subtle", diff --git a/Cargo.toml b/Cargo.toml index 41b7209e5..cba458726 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -115,7 +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" +securer-string = "0.1.2" petgraph = { version = "0.8", default-features = false } pin-project-lite = "0.2" prettyplease = "0.2" From d2d814f8f1bf2d5691f96b86d7fba2950817526c Mon Sep 17 00:00:00 2001 From: Marek 'seqre' Grzelak Date: Sat, 23 May 2026 09:25:58 +0200 Subject: [PATCH 11/11] fix forgotten test --- cot/src/common_types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cot/src/common_types.rs b/cot/src/common_types.rs index 496c78990..774612e2d 100644 --- a/cot/src/common_types.rs +++ b/cot/src/common_types.rs @@ -66,7 +66,7 @@ const MAX_EMAIL_LENGTH: u32 = 254; /// use cot::common_types::Password; /// /// let password = Password::new("pass"); -/// assert_eq!(&format!("{:?}", password), "Password(***SECRET***)"); +/// assert_eq!(&format!("{:?}", password), "Password(..)"); /// /// let another_password = Password::new("pass"); /// assert_eq!(password, another_password);