diff --git a/docs/src/contributing_federation_development.md b/docs/src/contributing_federation_development.md index 143ae9f8..8af38a07 100644 --- a/docs/src/contributing_federation_development.md +++ b/docs/src/contributing_federation_development.md @@ -68,3 +68,16 @@ cd /lemmy/ sudo docker-compose pull sudo docker-compose up -d ``` + +## Security Model + +- HTTP signature verify: This ensures that activity really comes from the activity that it claims +- check_is_apub_valid : Makes sure its in our allowed instances list +- Lower level checks: To make sure that the user that creates/updates/removes a post is actually on the same instance as that post + +For the last point, note that we are *not* checking whether the actor that sends the create activity for a post is +actually identical to the post's creator, or that the user that removes a post is a mod/admin. These things are checked +by the API code, and its the responsibility of each instance to check user permissions. This does not leave any attack +vector, as a normal instance user cant do actions that violate the API rules. The only one who could do that is the +admin (and the software deployed by the admin). But the admin can do anything on the instance, including send activities +from other user accounts. So we wouldnt actually gain any security by checking mod permissions or similar. \ No newline at end of file diff --git a/server/src/api/mod.rs b/server/src/api/mod.rs index a9aae823..8124cd4a 100644 --- a/server/src/api/mod.rs +++ b/server/src/api/mod.rs @@ -55,7 +55,7 @@ pub trait Perform { ) -> Result; } -pub async fn is_mod_or_admin( +pub(in crate::api) async fn is_mod_or_admin( pool: &DbPool, user_id: i32, community_id: i32, @@ -65,7 +65,7 @@ pub async fn is_mod_or_admin( }) .await?; if !is_mod_or_admin { - return Err(APIError::err("not_an_admin").into()); + return Err(APIError::err("not_a_mod_or_admin").into()); } Ok(()) } @@ -104,14 +104,14 @@ pub(in crate::api) async fn get_user_from_jwt_opt( } } -pub(in crate::api) fn check_slurs(text: &str) -> Result<(), APIError> { +pub(in crate) fn check_slurs(text: &str) -> Result<(), APIError> { if let Err(slurs) = slur_check(text) { Err(APIError::err(&slurs_vec_to_str(slurs))) } else { Ok(()) } } -pub(in crate::api) fn check_slurs_opt(text: &Option) -> Result<(), APIError> { +pub(in crate) fn check_slurs_opt(text: &Option) -> Result<(), APIError> { match text { Some(t) => check_slurs(t), None => Ok(()), diff --git a/server/src/apub/comment.rs b/server/src/apub/comment.rs index 05b40dbe..fbec5905 100644 --- a/server/src/apub/comment.rs +++ b/server/src/apub/comment.rs @@ -1,6 +1,7 @@ use crate::{ apub::{ activities::{generate_activity_id, send_activity_to_community}, + check_actor_domain, create_apub_response, create_apub_tombstone_response, create_tombstone, @@ -48,7 +49,7 @@ use lemmy_db::{ user::User_, Crud, }; -use lemmy_utils::{convert_datetime, scrape_text_for_mentions, MentionData}; +use lemmy_utils::{convert_datetime, remove_slurs, scrape_text_for_mentions, MentionData}; use log::debug; use serde::Deserialize; use serde_json::Error; @@ -131,6 +132,7 @@ impl FromApub for CommentForm { note: &Note, client: &Client, pool: &DbPool, + expected_domain: Option, ) -> Result { let creator_actor_id = ¬e .attributed_to() @@ -165,23 +167,25 @@ impl FromApub for CommentForm { } None => None, }; + let content = note + .content() + .unwrap() + .as_single_xsd_string() + .unwrap() + .to_string(); + let content_slurs_removed = remove_slurs(&content); Ok(CommentForm { creator_id: creator.id, post_id: post.id, parent_id, - content: note - .content() - .unwrap() - .as_single_xsd_string() - .unwrap() - .to_string(), + content: content_slurs_removed, removed: None, read: None, published: note.published().map(|u| u.to_owned().naive_local()), updated: note.updated().map(|u| u.to_owned().naive_local()), deleted: None, - ap_id: note.id_unchecked().unwrap().to_string(), + ap_id: check_actor_domain(note, expected_domain)?, local: false, }) } diff --git a/server/src/apub/community.rs b/server/src/apub/community.rs index 549d9349..8b522b44 100644 --- a/server/src/apub/community.rs +++ b/server/src/apub/community.rs @@ -1,6 +1,8 @@ use crate::{ + api::{check_slurs, check_slurs_opt}, apub::{ activities::{generate_activity_id, send_activity}, + check_actor_domain, create_apub_response, create_apub_tombstone_response, create_tombstone, @@ -322,7 +324,12 @@ impl FromApub for CommunityForm { type ApubType = GroupExt; /// Parse an ActivityPub group received from another instance into a Lemmy community. - async fn from_apub(group: &GroupExt, client: &Client, pool: &DbPool) -> Result { + async fn from_apub( + group: &GroupExt, + client: &Client, + pool: &DbPool, + expected_domain: Option, + ) -> Result { let creator_and_moderator_uris = group.inner.attributed_to().unwrap(); let creator_uri = creator_and_moderator_uris .as_many() @@ -334,6 +341,25 @@ impl FromApub for CommunityForm { .unwrap(); let creator = get_or_fetch_and_upsert_user(creator_uri, client, pool).await?; + let name = group + .inner + .name() + .unwrap() + .as_one() + .unwrap() + .as_xsd_string() + .unwrap() + .to_string(); + let title = group.inner.preferred_username().unwrap().to_string(); + // TODO: should be parsed as html and tags like