From: Felix Ableitner Date: Fri, 12 Mar 2021 13:47:55 +0000 (+0100) Subject: WIP: check that modifications are made by same user, add docs X-Git-Url: http://these/git/%22https:/nerdica.net/photo/contact/80/%24%7BappleTouchIcon%7D?a=commitdiff_plain;h=de39d575923205432c1aca235550d659deb26bfd;p=lemmy.git WIP: check that modifications are made by same user, add docs --- diff --git a/crates/apub/src/inbox/community_inbox.rs b/crates/apub/src/inbox/community_inbox.rs index de44ef6f..1495c4bb 100644 --- a/crates/apub/src/inbox/community_inbox.rs +++ b/crates/apub/src/inbox/community_inbox.rs @@ -161,7 +161,7 @@ pub(crate) async fn community_receive_message( true } CommunityValidTypes::Delete => { - receive_delete_for_community(context, any_base.clone(), &actor_url).await?; + receive_delete_for_community(context, any_base.clone(), None, &actor_url).await?; true } CommunityValidTypes::Add => { @@ -228,7 +228,7 @@ async fn handle_undo( handle_undo_follow(any_base, actor_url, to_community, &context).await?; Ok(false) } else { - receive_undo_for_community(context, any_base, &actor_url, request_counter).await?; + receive_undo_for_community(context, any_base, None, &actor_url, request_counter).await?; Ok(true) } } diff --git a/crates/apub/src/inbox/receive_for_community.rs b/crates/apub/src/inbox/receive_for_community.rs index 2a1427f6..fed0c462 100644 --- a/crates/apub/src/inbox/receive_for_community.rs +++ b/crates/apub/src/inbox/receive_for_community.rs @@ -119,28 +119,7 @@ pub(in crate::inbox) async fn receive_update_for_community( let update = Update::from_any_base(activity)?.context(location_info!())?; verify_activity_domains_valid(&update, &expected_domain, true)?; verify_is_addressed_to_public(&update)?; - - // Check that actor is the creator (or a mod) - let actor = update - .actor()? - .to_owned() - .single_xsd_any_uri() - .context(location_info!())?; - let actor = get_or_fetch_and_upsert_user(&actor, context, request_counter).await?; - let object_id = update - .object() - .as_one() - .map(|o| o.id()) - .flatten() - .context(location_info!())?; - let original_author = match find_post_or_comment_by_id(context, object_id.to_owned()).await? { - PostOrComment::Post(p) => p.creator_id, - PostOrComment::Comment(c) => c.creator_id, - }; - if actor.id != original_author { - let community = extract_community_from_cc(&update, context).await?; - verify_mod_activity(&update, announce.to_owned(), &community, context).await?; - } + verify_modification_actor_instance(&update, &announce, context).await?; let kind = update .object() @@ -213,11 +192,13 @@ pub(in crate::inbox) async fn receive_dislike_for_community( pub(in crate::inbox) async fn receive_delete_for_community( context: &LemmyContext, activity: AnyBase, + announce: Option, expected_domain: &Url, ) -> Result<(), LemmyError> { let delete = Delete::from_any_base(activity)?.context(location_info!())?; verify_activity_domains_valid(&delete, &expected_domain, true)?; verify_is_addressed_to_public(&delete)?; + verify_modification_actor_instance(&delete, &announce, context).await?; let object = delete .object() @@ -245,7 +226,6 @@ pub(in crate::inbox) async fn receive_remove_for_community( verify_mod_activity(&remove, announce, &community, context).await?; verify_is_addressed_to_public(&remove)?; - verify_actor_is_community_mod(&remove, &community, context).await?; if remove.target().is_some() { let remove_mod = remove @@ -294,12 +274,14 @@ enum UndoableActivities { pub(in crate::inbox) async fn receive_undo_for_community( context: &LemmyContext, activity: AnyBase, + announce: Option, expected_domain: &Url, request_counter: &mut i32, ) -> Result<(), LemmyError> { let undo = Undo::from_any_base(activity)?.context(location_info!())?; verify_activity_domains_valid(&undo, &expected_domain.to_owned(), true)?; verify_is_addressed_to_public(&undo)?; + verify_modification_actor_instance(&undo, &announce, context).await?; use UndoableActivities::*; match undo @@ -405,7 +387,6 @@ pub(in crate::inbox) async fn receive_add_for_community( verify_mod_activity(&add, announce, &community, context).await?; verify_is_addressed_to_public(&add)?; - verify_actor_is_community_mod(&add, &community, context).await?; verify_add_remove_moderator_target(&add, &community)?; let new_mod = add @@ -480,6 +461,7 @@ async fn fetch_post_or_comment_by_id( Err(NotFound.into()) } +/// Searches the activity's cc field for a Community ID, and returns the community. async fn extract_community_from_cc( activity: &T, context: &LemmyContext, @@ -505,6 +487,12 @@ where Ok(community) } +/// Checks that a moderation activity was sent by a user who is listed as mod for the community. +/// This is only used in the case of remote mods, as local mod actions don't go through the +/// community inbox. +/// +/// This method should only be used for activities received by the community, not for activities +/// used by community followers. async fn verify_actor_is_community_mod( activity: &T, community: &Community, @@ -538,6 +526,16 @@ where Ok(()) } +/// This method behaves differently, depending if it is called via community inbox (activity +/// received by community from a remote user), or via user inbox (activity received by user from +/// community). We distinguish the cases by checking if the activity is wrapper in an announce +/// (only true when sent from user to community). +/// +/// In the first case, we check that the actor is listed as community mod. In the second case, we +/// only check that the announce comes from the same domain as the activity. We trust the +/// community's instance to have validated the inner activity correctly. We can't do this validation +/// here, because we don't know who the instance admins are. Plus this allows for compatibility with +/// software that uses different rules for mod actions. pub(crate) async fn verify_mod_activity( mod_action: &T, announce: Option, @@ -547,18 +545,16 @@ pub(crate) async fn verify_mod_activity( where T: ActorAndObjectRef + BaseExt, { - // Remove was sent by community to user, we just check that it came from the right domain - if let Some(announce) = announce { - verify_activity_domains_valid(&announce, &community.actor_id.to_owned().into(), false)?; - } - // Remove was sent by a remote mod to community, check that they are actually mod - else { - verify_actor_is_community_mod(mod_action, community, context).await?; + match announce { + None => verify_actor_is_community_mod(mod_action, community, context).await?, + Some(a) => verify_activity_domains_valid(&a, &community.actor_id.to_owned().into(), false)?, } Ok(()) } +/// For Add/Remove community moderator activities, check that the target field actually contains +/// /c/community/moderators. Any different values are unsupported. fn verify_add_remove_moderator_target( activity: &T, community: &Community, @@ -576,3 +572,36 @@ where } Ok(()) } + +/// For activities like Update, Delete or Undo, check that the actor is from the same instance +/// as the original object itself (or is a remote mod). +async fn verify_modification_actor_instance( + activity: &T, + announce: &Option, + context: &LemmyContext, +) -> Result<(), LemmyError> +where + T: ActorAndObjectRef + BaseExt + AsObject, +{ + let actor_id = activity + .actor()? + .to_owned() + .single_xsd_any_uri() + .context(location_info!())?; + let object_id = activity + .object() + .as_one() + .map(|o| o.id()) + .flatten() + .context(location_info!())?; + let original_id = match find_post_or_comment_by_id(context, object_id.to_owned()).await? { + PostOrComment::Post(p) => p.ap_id.into_inner(), + PostOrComment::Comment(c) => c.ap_id.into_inner(), + }; + if actor_id.domain() != original_id.domain() { + let community = extract_community_from_cc(activity, context).await?; + verify_mod_activity(activity, announce.to_owned(), &community, context).await?; + } + + Ok(()) +} diff --git a/crates/apub/src/inbox/user_inbox.rs b/crates/apub/src/inbox/user_inbox.rs index 571e3329..691a5d41 100644 --- a/crates/apub/src/inbox/user_inbox.rs +++ b/crates/apub/src/inbox/user_inbox.rs @@ -304,12 +304,21 @@ pub async fn receive_announce( Some(Dislike) => { receive_dislike_for_community(context, inner_activity, &inner_id, request_counter).await } - Some(Delete) => receive_delete_for_community(context, inner_activity, &inner_id).await, + Some(Delete) => { + receive_delete_for_community(context, inner_activity, Some(announce), &inner_id).await + } Some(Remove) => { receive_remove_for_community(context, inner_activity, Some(announce), request_counter).await } Some(Undo) => { - receive_undo_for_community(context, inner_activity, &inner_id, request_counter).await + receive_undo_for_community( + context, + inner_activity, + Some(announce), + &inner_id, + request_counter, + ) + .await } Some(Add) => { receive_add_for_community(context, inner_activity, Some(announce), request_counter).await