From 99aac07714ceb8926d153d301f62de5c577f1ad5 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Fri, 17 May 2024 02:41:57 +0200 Subject: [PATCH] Mark database fields as sensitive so they dont show up in logs (#4720) * Mark database fields as sensitive so they dont show up in logs * add file * fix test * Update crates/apub/src/objects/person.rs Co-authored-by: SleeplessOne1917 <28871516+SleeplessOne1917@users.noreply.github.com> * Update crates/apub/src/objects/community.rs Co-authored-by: SleeplessOne1917 <28871516+SleeplessOne1917@users.noreply.github.com> * Update crates/apub/src/objects/instance.rs Co-authored-by: SleeplessOne1917 <28871516+SleeplessOne1917@users.noreply.github.com> --------- Co-authored-by: SleeplessOne1917 <28871516+SleeplessOne1917@users.noreply.github.com> Co-authored-by: Dessalines --- .../src/local_user/generate_totp_secret.rs | 8 +- crates/api/src/local_user/save_settings.rs | 3 +- crates/api_common/src/claims.rs | 9 +- crates/api_common/src/context.rs | 2 +- crates/api_common/src/lib.rs | 1 - crates/api_common/src/person.rs | 34 ++--- crates/api_common/src/sensitive.rs | 116 ------------------ crates/apub/src/objects/community.rs | 3 +- crates/apub/src/objects/instance.rs | 3 +- crates/apub/src/objects/person.rs | 3 +- .../src/impls/password_reset_request.rs | 4 +- crates/db_schema/src/lib.rs | 1 + crates/db_schema/src/sensitive.rs | 57 +++++++++ crates/db_schema/src/source/community.rs | 3 +- crates/db_schema/src/source/local_user.rs | 7 +- crates/db_schema/src/source/login_token.rs | 6 +- .../src/source/password_reset_request.rs | 6 +- crates/db_schema/src/source/person.rs | 3 +- crates/db_schema/src/source/secret.rs | 3 +- crates/db_schema/src/source/site.rs | 7 +- 20 files changed, 114 insertions(+), 165 deletions(-) delete mode 100644 crates/api_common/src/sensitive.rs create mode 100644 crates/db_schema/src/sensitive.rs diff --git a/crates/api/src/local_user/generate_totp_secret.rs b/crates/api/src/local_user/generate_totp_secret.rs index e8bb0284..f2bfe7f9 100644 --- a/crates/api/src/local_user/generate_totp_secret.rs +++ b/crates/api/src/local_user/generate_totp_secret.rs @@ -1,11 +1,7 @@ use crate::{build_totp_2fa, generate_totp_2fa_secret}; use activitypub_federation::config::Data; use actix_web::web::Json; -use lemmy_api_common::{ - context::LemmyContext, - person::GenerateTotpSecretResponse, - sensitive::Sensitive, -}; +use lemmy_api_common::{context::LemmyContext, person::GenerateTotpSecretResponse}; use lemmy_db_schema::source::local_user::{LocalUser, LocalUserUpdateForm}; use lemmy_db_views::structs::{LocalUserView, SiteView}; use lemmy_utils::error::{LemmyErrorType, LemmyResult}; @@ -41,6 +37,6 @@ pub async fn generate_totp_secret( .await?; Ok(Json(GenerateTotpSecretResponse { - totp_secret_url: Sensitive::new(secret_url), + totp_secret_url: secret_url.into(), })) } diff --git a/crates/api/src/local_user/save_settings.rs b/crates/api/src/local_user/save_settings.rs index 02b17305..609d5274 100644 --- a/crates/api/src/local_user/save_settings.rs +++ b/crates/api/src/local_user/save_settings.rs @@ -28,6 +28,7 @@ use lemmy_utils::{ error::{LemmyErrorType, LemmyResult}, utils::validation::{is_valid_bio_field, is_valid_display_name, is_valid_matrix_id}, }; +use std::ops::Deref; #[tracing::instrument(skip(context))] pub async fn save_user_settings( @@ -57,7 +58,7 @@ pub async fn save_user_settings( if let Some(Some(email)) = &email { let previous_email = local_user_view.local_user.email.clone().unwrap_or_default(); // if email was changed, check that it is not taken and send verification mail - if &previous_email != email { + if previous_email.deref() != email { if LocalUser::is_email_taken(&mut context.pool(), email).await? { return Err(LemmyErrorType::EmailAlreadyExists)?; } diff --git a/crates/api_common/src/claims.rs b/crates/api_common/src/claims.rs index a926cb0b..160c581c 100644 --- a/crates/api_common/src/claims.rs +++ b/crates/api_common/src/claims.rs @@ -1,9 +1,10 @@ -use crate::{context::LemmyContext, sensitive::Sensitive}; +use crate::context::LemmyContext; use actix_web::{http::header::USER_AGENT, HttpRequest}; use chrono::Utc; use jsonwebtoken::{decode, encode, DecodingKey, EncodingKey, Header, Validation}; use lemmy_db_schema::{ newtypes::LocalUserId, + sensitive::SensitiveString, source::login_token::{LoginToken, LoginTokenCreateForm}, }; use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult}; @@ -40,7 +41,7 @@ impl Claims { user_id: LocalUserId, req: HttpRequest, context: &LemmyContext, - ) -> LemmyResult> { + ) -> LemmyResult { let hostname = context.settings().hostname.clone(); let my_claims = Claims { sub: user_id.0.to_string(), @@ -50,7 +51,7 @@ impl Claims { let secret = &context.secret().jwt_secret; let key = EncodingKey::from_secret(secret.as_ref()); - let token = encode(&Header::default(), &my_claims, &key)?; + let token: SensitiveString = encode(&Header::default(), &my_claims, &key)?.into(); let ip = req .connection_info() .realip_remote_addr() @@ -67,7 +68,7 @@ impl Claims { user_agent, }; LoginToken::create(&mut context.pool(), form).await?; - Ok(Sensitive::new(token)) + Ok(token) } } diff --git a/crates/api_common/src/context.rs b/crates/api_common/src/context.rs index ba9eb407..f4ac41db 100644 --- a/crates/api_common/src/context.rs +++ b/crates/api_common/src/context.rs @@ -64,7 +64,7 @@ impl LemmyContext { let client = ClientBuilder::new(client).build(); let secret = Secret { id: 0, - jwt_secret: String::new(), + jwt_secret: String::new().into(), }; let rate_limit_cell = RateLimitCell::with_test_config(); diff --git a/crates/api_common/src/lib.rs b/crates/api_common/src/lib.rs index 4bb7ada5..9d12d2e1 100644 --- a/crates/api_common/src/lib.rs +++ b/crates/api_common/src/lib.rs @@ -14,7 +14,6 @@ pub mod private_message; pub mod request; #[cfg(feature = "full")] pub mod send_activity; -pub mod sensitive; pub mod site; #[cfg(feature = "full")] pub mod utils; diff --git a/crates/api_common/src/person.rs b/crates/api_common/src/person.rs index 6f020df6..cad30ca2 100644 --- a/crates/api_common/src/person.rs +++ b/crates/api_common/src/person.rs @@ -1,6 +1,6 @@ -use crate::sensitive::Sensitive; use lemmy_db_schema::{ newtypes::{CommentReplyId, CommunityId, LanguageId, PersonId, PersonMentionId}, + sensitive::SensitiveString, source::site::Site, CommentSortType, ListingType, @@ -25,8 +25,8 @@ use ts_rs::TS; #[cfg_attr(feature = "full", ts(export))] /// Logging into lemmy. pub struct Login { - pub username_or_email: Sensitive, - pub password: Sensitive, + pub username_or_email: SensitiveString, + pub password: SensitiveString, /// May be required, if totp is enabled for their account. pub totp_2fa_token: Option, } @@ -38,11 +38,11 @@ pub struct Login { /// Register / Sign up to lemmy. pub struct Register { pub username: String, - pub password: Sensitive, - pub password_verify: Sensitive, + pub password: SensitiveString, + pub password_verify: SensitiveString, pub show_nsfw: Option, /// email is mandatory if email verification is enabled on the server - pub email: Option>, + pub email: Option, /// The UUID of the captcha item. pub captcha_uuid: Option, /// Your captcha answer. @@ -99,7 +99,7 @@ pub struct SaveUserSettings { /// Your display name, which can contain strange characters, and does not need to be unique. pub display_name: Option, /// Your email. - pub email: Option>, + pub email: Option, /// Your bio / info, in markdown. pub bio: Option, /// Your matrix user id. Ex: @my_user:matrix.org @@ -140,9 +140,9 @@ pub struct SaveUserSettings { #[cfg_attr(feature = "full", ts(export))] /// Changes your account password. pub struct ChangePassword { - pub new_password: Sensitive, - pub new_password_verify: Sensitive, - pub old_password: Sensitive, + pub new_password: SensitiveString, + pub new_password_verify: SensitiveString, + pub old_password: SensitiveString, } #[skip_serializing_none] @@ -152,7 +152,7 @@ pub struct ChangePassword { /// A response for your login. pub struct LoginResponse { /// This is None in response to `Register` if email verification is enabled, or the server requires registration applications. - pub jwt: Option>, + pub jwt: Option, /// If registration applications are required, this will return true for a signup response. pub registration_created: bool, /// If email verifications are required, this will return true for a signup response. @@ -340,7 +340,7 @@ pub struct CommentReplyResponse { #[cfg_attr(feature = "full", ts(export))] /// Delete your account. pub struct DeleteAccount { - pub password: Sensitive, + pub password: SensitiveString, pub delete_content: bool, } @@ -349,7 +349,7 @@ pub struct DeleteAccount { #[cfg_attr(feature = "full", ts(export))] /// Reset your password via email. pub struct PasswordReset { - pub email: Sensitive, + pub email: SensitiveString, } #[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq, Hash)] @@ -357,9 +357,9 @@ pub struct PasswordReset { #[cfg_attr(feature = "full", ts(export))] /// Change your password after receiving a reset request. pub struct PasswordChangeAfterReset { - pub token: Sensitive, - pub password: Sensitive, - pub password_verify: Sensitive, + pub token: SensitiveString, + pub password: SensitiveString, + pub password_verify: SensitiveString, } #[skip_serializing_none] @@ -405,7 +405,7 @@ pub struct VerifyEmail { #[cfg_attr(feature = "full", derive(TS))] #[cfg_attr(feature = "full", ts(export))] pub struct GenerateTotpSecretResponse { - pub totp_secret_url: Sensitive, + pub totp_secret_url: SensitiveString, } #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Hash)] diff --git a/crates/api_common/src/sensitive.rs b/crates/api_common/src/sensitive.rs deleted file mode 100644 index 4dd12080..00000000 --- a/crates/api_common/src/sensitive.rs +++ /dev/null @@ -1,116 +0,0 @@ -use serde::{Deserialize, Serialize}; -use std::{ - borrow::Borrow, - ops::{Deref, DerefMut}, -}; -#[cfg(feature = "full")] -use ts_rs::TS; - -#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Deserialize, Serialize, Default)] -#[serde(transparent)] -pub struct Sensitive(T); - -impl Sensitive { - pub fn new(item: T) -> Self { - Sensitive(item) - } - pub fn into_inner(self) -> T { - self.0 - } -} - -impl std::fmt::Debug for Sensitive { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("Sensitive").finish() - } -} - -impl AsRef for Sensitive { - fn as_ref(&self) -> &T { - &self.0 - } -} - -impl AsRef for Sensitive { - fn as_ref(&self) -> &str { - &self.0 - } -} - -impl AsRef<[u8]> for Sensitive { - fn as_ref(&self) -> &[u8] { - self.0.as_ref() - } -} - -impl AsRef<[u8]> for Sensitive> { - fn as_ref(&self) -> &[u8] { - self.0.as_ref() - } -} - -impl AsMut for Sensitive { - fn as_mut(&mut self) -> &mut T { - &mut self.0 - } -} - -impl AsMut for Sensitive { - fn as_mut(&mut self) -> &mut str { - &mut self.0 - } -} - -impl Deref for Sensitive { - type Target = str; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl DerefMut for Sensitive { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.0 - } -} - -impl From for Sensitive { - fn from(t: T) -> Self { - Sensitive(t) - } -} - -impl From<&str> for Sensitive { - fn from(s: &str) -> Self { - Sensitive(s.into()) - } -} - -impl Borrow for Sensitive { - fn borrow(&self) -> &T { - &self.0 - } -} - -impl Borrow for Sensitive { - fn borrow(&self) -> &str { - &self.0 - } -} - -#[cfg(feature = "full")] -impl TS for Sensitive { - fn name() -> String { - "string".to_string() - } - fn name_with_type_args(_args: Vec) -> String { - "string".to_string() - } - fn dependencies() -> Vec { - Vec::new() - } - fn transparent() -> bool { - true - } -} diff --git a/crates/apub/src/objects/community.rs b/crates/apub/src/objects/community.rs index 4e719895..93c2c83a 100644 --- a/crates/apub/src/objects/community.rs +++ b/crates/apub/src/objects/community.rs @@ -28,6 +28,7 @@ use lemmy_api_common::{ }, }; use lemmy_db_schema::{ + sensitive::SensitiveString, source::{ activity::ActorType, actor_language::CommunityLanguage, @@ -213,7 +214,7 @@ impl Actor for ApubCommunity { } fn private_key_pem(&self) -> Option { - self.private_key.clone() + self.private_key.clone().map(SensitiveString::into_inner) } fn inbox(&self) -> Url { diff --git a/crates/apub/src/objects/instance.rs b/crates/apub/src/objects/instance.rs index 2f01609a..24bb63b4 100644 --- a/crates/apub/src/objects/instance.rs +++ b/crates/apub/src/objects/instance.rs @@ -29,6 +29,7 @@ use lemmy_api_common::{ }; use lemmy_db_schema::{ newtypes::InstanceId, + sensitive::SensitiveString, source::{ activity::ActorType, actor_language::SiteLanguage, @@ -187,7 +188,7 @@ impl Actor for ApubSite { } fn private_key_pem(&self) -> Option { - self.private_key.clone() + self.private_key.clone().map(SensitiveString::into_inner) } fn inbox(&self) -> Url { diff --git a/crates/apub/src/objects/person.rs b/crates/apub/src/objects/person.rs index 1aac170d..61ff0462 100644 --- a/crates/apub/src/objects/person.rs +++ b/crates/apub/src/objects/person.rs @@ -30,6 +30,7 @@ use lemmy_api_common::{ }, }; use lemmy_db_schema::{ + sensitive::SensitiveString, source::{ activity::ActorType, local_site::LocalSite, @@ -200,7 +201,7 @@ impl Actor for ApubPerson { } fn private_key_pem(&self) -> Option { - self.private_key.clone() + self.private_key.clone().map(SensitiveString::into_inner) } fn inbox(&self) -> Url { diff --git a/crates/db_schema/src/impls/password_reset_request.rs b/crates/db_schema/src/impls/password_reset_request.rs index 762aa465..4fad632f 100644 --- a/crates/db_schema/src/impls/password_reset_request.rs +++ b/crates/db_schema/src/impls/password_reset_request.rs @@ -50,7 +50,7 @@ impl PasswordResetRequest { ) -> Result { let form = PasswordResetRequestForm { local_user_id: from_local_user_id, - token: token_, + token: token_.into(), }; Self::create(pool, &form).await @@ -134,7 +134,7 @@ mod tests { let expected_password_reset_request = PasswordResetRequest { id: inserted_password_reset_request.id, local_user_id: inserted_local_user.id, - token: token.to_string(), + token: token.to_string().into(), published: inserted_password_reset_request.published, }; diff --git a/crates/db_schema/src/lib.rs b/crates/db_schema/src/lib.rs index 37c44e26..43124322 100644 --- a/crates/db_schema/src/lib.rs +++ b/crates/db_schema/src/lib.rs @@ -24,6 +24,7 @@ pub mod aggregates; #[cfg(feature = "full")] pub mod impls; pub mod newtypes; +pub mod sensitive; #[cfg(feature = "full")] #[rustfmt::skip] #[allow(clippy::wildcard_imports)] diff --git a/crates/db_schema/src/sensitive.rs b/crates/db_schema/src/sensitive.rs new file mode 100644 index 00000000..340679e2 --- /dev/null +++ b/crates/db_schema/src/sensitive.rs @@ -0,0 +1,57 @@ +use serde::{Deserialize, Serialize}; +use std::{fmt::Debug, ops::Deref}; +#[cfg(feature = "full")] +use ts_rs::TS; + +#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Deserialize, Serialize, Default)] +#[cfg_attr(feature = "full", derive(DieselNewType))] +#[serde(transparent)] +pub struct SensitiveString(String); + +impl SensitiveString { + pub fn into_inner(self) -> String { + self.0 + } +} + +impl Debug for SensitiveString { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Sensitive").finish() + } +} + +impl AsRef<[u8]> for SensitiveString { + fn as_ref(&self) -> &[u8] { + self.0.as_ref() + } +} + +impl Deref for SensitiveString { + type Target = str; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl From for SensitiveString { + fn from(t: String) -> Self { + SensitiveString(t) + } +} + +#[cfg(feature = "full")] +impl TS for SensitiveString { + fn name() -> String { + "string".to_string() + } + fn name_with_type_args(_args: Vec) -> String { + "string".to_string() + } + fn dependencies() -> Vec { + Vec::new() + } + fn transparent() -> bool { + true + } +} diff --git a/crates/db_schema/src/source/community.rs b/crates/db_schema/src/source/community.rs index 16a17819..fe7f120e 100644 --- a/crates/db_schema/src/source/community.rs +++ b/crates/db_schema/src/source/community.rs @@ -2,6 +2,7 @@ use crate::schema::{community, community_follower, community_moderator, community_person_ban}; use crate::{ newtypes::{CommunityId, DbUrl, InstanceId, PersonId}, + sensitive::SensitiveString, source::placeholder_apub_url, CommunityVisibility, }; @@ -39,7 +40,7 @@ pub struct Community { /// Whether the community is local. pub local: bool, #[serde(skip)] - pub private_key: Option, + pub private_key: Option, #[serde(skip)] pub public_key: String, #[serde(skip)] diff --git a/crates/db_schema/src/source/local_user.rs b/crates/db_schema/src/source/local_user.rs index f159232f..8af893b1 100644 --- a/crates/db_schema/src/source/local_user.rs +++ b/crates/db_schema/src/source/local_user.rs @@ -2,6 +2,7 @@ use crate::schema::local_user; use crate::{ newtypes::{LocalUserId, PersonId}, + sensitive::SensitiveString, ListingType, PostListingMode, SortType, @@ -24,8 +25,8 @@ pub struct LocalUser { /// The person_id for the local user. pub person_id: PersonId, #[serde(skip)] - pub password_encrypted: String, - pub email: Option, + pub password_encrypted: SensitiveString, + pub email: Option, /// Whether to show NSFW content. pub show_nsfw: bool, pub theme: String, @@ -47,7 +48,7 @@ pub struct LocalUser { /// Whether their registration application has been accepted. pub accepted_application: bool, #[serde(skip)] - pub totp_2fa_secret: Option, + pub totp_2fa_secret: Option, /// Open links in a new tab. pub open_links_in_new_tab: bool, pub blur_nsfw: bool, diff --git a/crates/db_schema/src/source/login_token.rs b/crates/db_schema/src/source/login_token.rs index e5d61f7c..38aac33e 100644 --- a/crates/db_schema/src/source/login_token.rs +++ b/crates/db_schema/src/source/login_token.rs @@ -1,6 +1,6 @@ -use crate::newtypes::LocalUserId; #[cfg(feature = "full")] use crate::schema::login_token; +use crate::{newtypes::LocalUserId, sensitive::SensitiveString}; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use serde_with::skip_serializing_none; @@ -18,7 +18,7 @@ use ts_rs::TS; pub struct LoginToken { /// Jwt token for this login #[serde(skip)] - pub token: String, + pub token: SensitiveString, pub user_id: LocalUserId, /// Time of login pub published: DateTime, @@ -31,7 +31,7 @@ pub struct LoginToken { #[cfg_attr(feature = "full", derive(Insertable, AsChangeset))] #[cfg_attr(feature = "full", diesel(table_name = login_token))] pub struct LoginTokenCreateForm { - pub token: String, + pub token: SensitiveString, pub user_id: LocalUserId, pub ip: Option, pub user_agent: Option, diff --git a/crates/db_schema/src/source/password_reset_request.rs b/crates/db_schema/src/source/password_reset_request.rs index edc030e9..dbc930b8 100644 --- a/crates/db_schema/src/source/password_reset_request.rs +++ b/crates/db_schema/src/source/password_reset_request.rs @@ -1,6 +1,6 @@ -use crate::newtypes::LocalUserId; #[cfg(feature = "full")] use crate::schema::password_reset_request; +use crate::{newtypes::LocalUserId, sensitive::SensitiveString}; use chrono::{DateTime, Utc}; #[derive(PartialEq, Eq, Debug)] @@ -9,7 +9,7 @@ use chrono::{DateTime, Utc}; #[cfg_attr(feature = "full", diesel(check_for_backend(diesel::pg::Pg)))] pub struct PasswordResetRequest { pub id: i32, - pub token: String, + pub token: SensitiveString, pub published: DateTime, pub local_user_id: LocalUserId, } @@ -18,5 +18,5 @@ pub struct PasswordResetRequest { #[cfg_attr(feature = "full", diesel(table_name = password_reset_request))] pub struct PasswordResetRequestForm { pub local_user_id: LocalUserId, - pub token: String, + pub token: SensitiveString, } diff --git a/crates/db_schema/src/source/person.rs b/crates/db_schema/src/source/person.rs index 25e65ae7..45eacd4f 100644 --- a/crates/db_schema/src/source/person.rs +++ b/crates/db_schema/src/source/person.rs @@ -2,6 +2,7 @@ use crate::schema::{person, person_follower}; use crate::{ newtypes::{DbUrl, InstanceId, PersonId}, + sensitive::SensitiveString, source::placeholder_apub_url, }; use chrono::{DateTime, Utc}; @@ -36,7 +37,7 @@ pub struct Person { /// Whether the person is local to our site. pub local: bool, #[serde(skip)] - pub private_key: Option, + pub private_key: Option, #[serde(skip)] pub public_key: String, #[serde(skip)] diff --git a/crates/db_schema/src/source/secret.rs b/crates/db_schema/src/source/secret.rs index 164670a2..36c0b691 100644 --- a/crates/db_schema/src/source/secret.rs +++ b/crates/db_schema/src/source/secret.rs @@ -1,5 +1,6 @@ #[cfg(feature = "full")] use crate::schema::secret; +use crate::sensitive::SensitiveString; #[derive(Clone)] #[cfg_attr(feature = "full", derive(Queryable, Selectable, Identifiable))] @@ -7,5 +8,5 @@ use crate::schema::secret; #[cfg_attr(feature = "full", diesel(check_for_backend(diesel::pg::Pg)))] pub struct Secret { pub id: i32, - pub jwt_secret: String, + pub jwt_secret: SensitiveString, } diff --git a/crates/db_schema/src/source/site.rs b/crates/db_schema/src/source/site.rs index 83d23c77..04c219f2 100644 --- a/crates/db_schema/src/source/site.rs +++ b/crates/db_schema/src/source/site.rs @@ -1,6 +1,9 @@ -use crate::newtypes::{DbUrl, InstanceId, SiteId}; #[cfg(feature = "full")] use crate::schema::site; +use crate::{ + newtypes::{DbUrl, InstanceId, SiteId}, + sensitive::SensitiveString, +}; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use serde_with::skip_serializing_none; @@ -35,7 +38,7 @@ pub struct Site { /// The site inbox pub inbox_url: DbUrl, #[serde(skip)] - pub private_key: Option, + pub private_key: Option, // TODO: mark as `serde(skip)` in next major release as its not needed for api pub public_key: String, pub instance_id: InstanceId,