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/Cargo.lock b/Cargo.lock index ee5f69c17..72ab8b986 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -920,6 +920,7 @@ dependencies = [ "schemars", "sea-query", "sea-query-binder", + "securer-string", "serde", "serde_json", "serde_urlencoded", @@ -3657,6 +3658,17 @@ dependencies = [ "sqlx", ] +[[package]] +name = "securer-string" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c796cd4a543edb87ad599e495048c5c7f433614c19ae009f5050b24381715efb" +dependencies = [ + "libc", + "subtle", + "zeroize", +] + [[package]] name = "security-framework" version = "3.7.0" diff --git a/Cargo.toml b/Cargo.toml index 7ef4382ea..cba458726 100644 --- a/Cargo.toml +++ b/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" diff --git a/cot/Cargo.toml b/cot/Cargo.toml index 75e9548d8..6a9cc298b 100644 --- a/cot/Cargo.toml +++ b/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"] } diff --git a/cot/src/common_types.rs b/cot/src/common_types.rs index 095885de5..774612e2d 100644 --- a/cot/src/common_types.rs +++ b/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,12 +51,14 @@ 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 /// @@ -63,16 +66,13 @@ const MAX_EMAIL_LENGTH: u32 = 254; /// 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>(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] diff --git a/cot/src/config.rs b/cot/src/config.rs index bf5cad46f..c4f5b42c5 100644 --- a/cot/src/config.rs +++ b/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(&self, serializer: S) -> Result + 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() {