From fe15ff3c5174025299984c220a7cef644c42f7ff Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Thu, 15 Oct 2020 15:38:49 +0200 Subject: [PATCH] Also verify activity domains in shared inbox (fixes #1196) --- lemmy_apub/src/activities/receive/announce.rs | 66 ++++++++++--------- lemmy_apub/src/activities/receive/create.rs | 10 +-- lemmy_apub/src/activities/receive/delete.rs | 9 +-- lemmy_apub/src/activities/receive/dislike.rs | 7 +- lemmy_apub/src/activities/receive/like.rs | 7 +- lemmy_apub/src/activities/receive/remove.rs | 19 ++---- lemmy_apub/src/activities/receive/undo.rs | 65 ++++++++---------- lemmy_apub/src/activities/receive/update.rs | 10 +-- lemmy_apub/src/inbox/shared_inbox.rs | 24 +++---- 9 files changed, 106 insertions(+), 111 deletions(-) diff --git a/lemmy_apub/src/activities/receive/announce.rs b/lemmy_apub/src/activities/receive/announce.rs index 50c2ed9d6..3167d6b90 100644 --- a/lemmy_apub/src/activities/receive/announce.rs +++ b/lemmy_apub/src/activities/receive/announce.rs @@ -1,48 +1,50 @@ -use crate::activities::receive::{ - create::receive_create, - delete::receive_delete, - dislike::receive_dislike, - like::receive_like, - receive_unhandled_activity, - remove::receive_remove, - undo::receive_undo, - update::receive_update, -}; -use activitystreams::{ - activity::*, - base::{AnyBase, BaseExt}, - prelude::ExtendsExt, +use crate::{ + activities::receive::{ + create::receive_create, + delete::receive_delete, + dislike::receive_dislike, + like::receive_like, + receive_unhandled_activity, + remove::receive_remove, + undo::receive_undo, + update::receive_update, + verify_activity_domains_valid, + }, + check_is_apub_id_valid, + ActorType, }; +use activitystreams::{activity::*, base::AnyBase, prelude::ExtendsExt}; use actix_web::HttpResponse; use anyhow::Context; use lemmy_utils::{location_info, LemmyError}; use lemmy_websocket::LemmyContext; pub async fn receive_announce( - activity: AnyBase, context: &LemmyContext, + activity: AnyBase, + actor: &dyn ActorType, ) -> Result { let announce = Announce::from_any_base(activity)?.context(location_info!())?; - - // ensure that announce and community come from the same instance - let community_id = announce - .actor()? - .to_owned() - .single_xsd_any_uri() - .context(location_info!())?; - announce.id(community_id.domain().context(location_info!())?)?; + verify_activity_domains_valid(&announce, actor.actor_id()?, false)?; let kind = announce.object().as_single_kind_str(); - let object = announce.object(); - let object2 = object.clone().one().context(location_info!())?; + let object = announce + .object() + .to_owned() + .one() + .context(location_info!())?; + + let inner_id = object.id().context(location_info!())?.to_owned(); + check_is_apub_id_valid(&inner_id)?; + match kind { - Some("Create") => receive_create(object2, context).await, - Some("Update") => receive_update(object2, context).await, - Some("Like") => receive_like(object2, context).await, - Some("Dislike") => receive_dislike(object2, context).await, - Some("Delete") => receive_delete(object2, context).await, - Some("Remove") => receive_remove(object2, context).await, - Some("Undo") => receive_undo(object2, context).await, + Some("Create") => receive_create(context, object, inner_id).await, + Some("Update") => receive_update(context, object, inner_id).await, + Some("Like") => receive_like(context, object, inner_id).await, + Some("Dislike") => receive_dislike(context, object, inner_id).await, + Some("Delete") => receive_delete(context, object, inner_id).await, + Some("Remove") => receive_remove(context, object, inner_id).await, + Some("Undo") => receive_undo(context, object, inner_id).await, _ => receive_unhandled_activity(announce), } } diff --git a/lemmy_apub/src/activities/receive/create.rs b/lemmy_apub/src/activities/receive/create.rs index 201d620ba..2f796949b 100644 --- a/lemmy_apub/src/activities/receive/create.rs +++ b/lemmy_apub/src/activities/receive/create.rs @@ -3,6 +3,7 @@ use crate::{ announce_if_community_is_local, get_actor_as_user, receive_unhandled_activity, + verify_activity_domains_valid, }, ActorType, FromApub, @@ -24,16 +25,15 @@ use lemmy_websocket::{ LemmyContext, UserOperation, }; +use url::Url; pub async fn receive_create( - activity: AnyBase, context: &LemmyContext, + activity: AnyBase, + expected_domain: Url, ) -> Result { let create = Create::from_any_base(activity)?.context(location_info!())?; - - // ensure that create and actor come from the same instance - let user = get_actor_as_user(&create, context).await?; - create.id(user.actor_id()?.domain().context(location_info!())?)?; + verify_activity_domains_valid(&create, expected_domain, true)?; match create.object().as_single_kind_str() { Some("Page") => receive_create_post(create, context).await, diff --git a/lemmy_apub/src/activities/receive/delete.rs b/lemmy_apub/src/activities/receive/delete.rs index 579c6e7ea..063266cac 100644 --- a/lemmy_apub/src/activities/receive/delete.rs +++ b/lemmy_apub/src/activities/receive/delete.rs @@ -2,6 +2,7 @@ use crate::activities::receive::{ announce_if_community_is_local, find_by_id, get_actor_as_user, + verify_activity_domains_valid, FindResults, }; use activitystreams::{activity::Delete, base::AnyBase, prelude::*}; @@ -27,12 +28,15 @@ use lemmy_websocket::{ LemmyContext, UserOperation, }; +use url::Url; pub async fn receive_delete( - activity: AnyBase, context: &LemmyContext, + activity: AnyBase, + expected_domain: Url, ) -> Result { let delete = Delete::from_any_base(activity)?.context(location_info!())?; + verify_activity_domains_valid(&delete, expected_domain, true)?; let object = delete .object() @@ -40,9 +44,6 @@ pub async fn receive_delete( .single_xsd_any_uri() .context(location_info!())?; - // Ensure that delete activity comes from the same domain as the object - delete.id(object.domain().context(location_info!())?)?; - match find_by_id(context, object).await { Ok(FindResults::Post(p)) => receive_delete_post(context, delete, p).await, Ok(FindResults::Comment(c)) => receive_delete_comment(context, delete, c).await, diff --git a/lemmy_apub/src/activities/receive/dislike.rs b/lemmy_apub/src/activities/receive/dislike.rs index 1007e6154..06cecef83 100644 --- a/lemmy_apub/src/activities/receive/dislike.rs +++ b/lemmy_apub/src/activities/receive/dislike.rs @@ -3,6 +3,7 @@ use crate::{ announce_if_community_is_local, get_actor_as_user, receive_unhandled_activity, + verify_activity_domains_valid, }, fetcher::{get_or_fetch_and_insert_comment, get_or_fetch_and_insert_post}, FromApub, @@ -27,10 +28,12 @@ use lemmy_websocket::{ LemmyContext, UserOperation, }; +use url::Url; pub async fn receive_dislike( - activity: AnyBase, context: &LemmyContext, + activity: AnyBase, + expected_domain: Url, ) -> Result { let enable_downvotes = blocking(context.pool(), move |conn| { Site::read(conn, 1).map(|s| s.enable_downvotes) @@ -41,6 +44,8 @@ pub async fn receive_dislike( } let dislike = Dislike::from_any_base(activity)?.context(location_info!())?; + verify_activity_domains_valid(&dislike, expected_domain, false)?; + match dislike.object().as_single_kind_str() { Some("Page") => receive_dislike_post(dislike, context).await, Some("Note") => receive_dislike_comment(dislike, context).await, diff --git a/lemmy_apub/src/activities/receive/like.rs b/lemmy_apub/src/activities/receive/like.rs index ef53e7918..4010204d5 100644 --- a/lemmy_apub/src/activities/receive/like.rs +++ b/lemmy_apub/src/activities/receive/like.rs @@ -3,6 +3,7 @@ use crate::{ announce_if_community_is_local, get_actor_as_user, receive_unhandled_activity, + verify_activity_domains_valid, }, fetcher::{get_or_fetch_and_insert_comment, get_or_fetch_and_insert_post}, FromApub, @@ -25,12 +26,16 @@ use lemmy_websocket::{ LemmyContext, UserOperation, }; +use url::Url; pub async fn receive_like( - activity: AnyBase, context: &LemmyContext, + activity: AnyBase, + expected_domain: Url, ) -> Result { let like = Like::from_any_base(activity)?.context(location_info!())?; + verify_activity_domains_valid(&like, expected_domain, false)?; + match like.object().as_single_kind_str() { Some("Page") => receive_like_post(like, context).await, Some("Note") => receive_like_comment(like, context).await, diff --git a/lemmy_apub/src/activities/receive/remove.rs b/lemmy_apub/src/activities/receive/remove.rs index c9f248d64..5bdb39bf7 100644 --- a/lemmy_apub/src/activities/receive/remove.rs +++ b/lemmy_apub/src/activities/receive/remove.rs @@ -1,8 +1,5 @@ -use crate::{ - activities::receive::{find_by_id, get_actor_as_user, FindResults}, - ActorType, -}; -use activitystreams::{activity::Remove, base::AnyBase, error::DomainError, prelude::*}; +use crate::activities::receive::{find_by_id, verify_activity_domains_valid, FindResults}; +use activitystreams::{activity::Remove, base::AnyBase, prelude::*}; use actix_web::HttpResponse; use anyhow::Context; use lemmy_db::{ @@ -25,13 +22,16 @@ use lemmy_websocket::{ LemmyContext, UserOperation, }; +use url::Url; pub async fn receive_remove( - activity: AnyBase, context: &LemmyContext, + activity: AnyBase, + expected_domain: Url, ) -> Result { let remove = Remove::from_any_base(activity)?.context(location_info!())?; - let actor = get_actor_as_user(&remove, context).await?; + verify_activity_domains_valid(&remove, expected_domain, false)?; + let cc = remove .cc() .map(|c| c.as_many()) @@ -49,11 +49,6 @@ pub async fn receive_remove( .single_xsd_any_uri() .context(location_info!())?; - // Ensure that remove comes from the same domain as the community - if actor.actor_id()?.domain() != community_id.domain() { - return Err(DomainError.into()); - } - // Ensure that remove activity comes from the same domain as the community remove.id(community_id.domain().context(location_info!())?)?; diff --git a/lemmy_apub/src/activities/receive/undo.rs b/lemmy_apub/src/activities/receive/undo.rs index 9c5dc01a9..5b4b23fae 100644 --- a/lemmy_apub/src/activities/receive/undo.rs +++ b/lemmy_apub/src/activities/receive/undo.rs @@ -5,62 +5,43 @@ use crate::activities::receive::{ receive_unhandled_activity, undo_comment::*, undo_post::*, + verify_activity_domains_valid, FindResults, }; -use activitystreams::{ - activity::*, - base::{AnyBase, AsBase}, - prelude::*, -}; +use activitystreams::{activity::*, base::AnyBase, prelude::*}; use actix_web::HttpResponse; use anyhow::{anyhow, Context}; use lemmy_db::{community::Community, community_view::CommunityView}; use lemmy_structs::{blocking, community::CommunityResponse}; use lemmy_utils::{location_info, LemmyError}; use lemmy_websocket::{messages::SendCommunityRoomMessage, LemmyContext, UserOperation}; +use url::Url; pub async fn receive_undo( - activity: AnyBase, context: &LemmyContext, + activity: AnyBase, + expected_domain: Url, ) -> Result { let undo = Undo::from_any_base(activity)?.context(location_info!())?; + verify_activity_domains_valid(&undo, expected_domain.to_owned(), true)?; + match undo.object().as_single_kind_str() { - Some("Delete") => receive_undo_delete(undo, context).await, - Some("Remove") => receive_undo_remove(undo, context).await, - Some("Like") => receive_undo_like(undo, context).await, - Some("Dislike") => receive_undo_dislike(undo, context).await, + Some("Delete") => receive_undo_delete(context, undo, expected_domain).await, + Some("Remove") => receive_undo_remove(context, undo, expected_domain).await, + Some("Like") => receive_undo_like(context, undo, expected_domain).await, + Some("Dislike") => receive_undo_dislike(context, undo, expected_domain).await, _ => receive_unhandled_activity(undo), } } -fn check_is_undo_valid(outer_activity: &Undo, inner_activity: &T) -> Result<(), LemmyError> -where - T: AsBase + ActorAndObjectRef, -{ - let outer_actor = outer_activity.actor()?; - let outer_actor_uri = outer_actor - .as_single_xsd_any_uri() - .context(location_info!())?; - - let inner_actor = inner_activity.actor()?; - let inner_actor_uri = inner_actor - .as_single_xsd_any_uri() - .context(location_info!())?; - - if outer_actor_uri.domain() != inner_actor_uri.domain() { - Err(anyhow!("Cant undo receive from a different instance").into()) - } else { - Ok(()) - } -} - async fn receive_undo_delete( - undo: Undo, context: &LemmyContext, + undo: Undo, + expected_domain: Url, ) -> Result { let delete = Delete::from_any_base(undo.object().to_owned().one().context(location_info!())?)? .context(location_info!())?; - check_is_undo_valid(&undo, &delete)?; + verify_activity_domains_valid(&delete, expected_domain, true)?; let object = delete .object() @@ -77,12 +58,13 @@ async fn receive_undo_delete( } async fn receive_undo_remove( - undo: Undo, context: &LemmyContext, + undo: Undo, + expected_domain: Url, ) -> Result { let remove = Remove::from_any_base(undo.object().to_owned().one().context(location_info!())?)? .context(location_info!())?; - check_is_undo_valid(&undo, &remove)?; + verify_activity_domains_valid(&remove, expected_domain, false)?; let object = remove .object() @@ -98,10 +80,14 @@ async fn receive_undo_remove( } } -async fn receive_undo_like(undo: Undo, context: &LemmyContext) -> Result { +async fn receive_undo_like( + context: &LemmyContext, + undo: Undo, + expected_domain: Url, +) -> Result { let like = Like::from_any_base(undo.object().to_owned().one().context(location_info!())?)? .context(location_info!())?; - check_is_undo_valid(&undo, &like)?; + verify_activity_domains_valid(&like, expected_domain, false)?; let type_ = like .object() @@ -115,12 +101,13 @@ async fn receive_undo_like(undo: Undo, context: &LemmyContext) -> Result Result { let dislike = Dislike::from_any_base(undo.object().to_owned().one().context(location_info!())?)? .context(location_info!())?; - check_is_undo_valid(&undo, &dislike)?; + verify_activity_domains_valid(&dislike, expected_domain, false)?; let type_ = dislike .object() diff --git a/lemmy_apub/src/activities/receive/update.rs b/lemmy_apub/src/activities/receive/update.rs index 0dd21ef5a..1acef5b59 100644 --- a/lemmy_apub/src/activities/receive/update.rs +++ b/lemmy_apub/src/activities/receive/update.rs @@ -3,6 +3,7 @@ use crate::{ announce_if_community_is_local, get_actor_as_user, receive_unhandled_activity, + verify_activity_domains_valid, }, fetcher::{get_or_fetch_and_insert_comment, get_or_fetch_and_insert_post}, ActorType, @@ -26,16 +27,15 @@ use lemmy_websocket::{ LemmyContext, UserOperation, }; +use url::Url; pub async fn receive_update( - activity: AnyBase, context: &LemmyContext, + activity: AnyBase, + expected_domain: Url, ) -> Result { let update = Update::from_any_base(activity)?.context(location_info!())?; - - // ensure that update and actor come from the same instance - let user = get_actor_as_user(&update, context).await?; - update.id(user.actor_id()?.domain().context(location_info!())?)?; + verify_activity_domains_valid(&update, expected_domain, true)?; match update.object().as_single_kind_str() { Some("Page") => receive_update_post(update, context).await, diff --git a/lemmy_apub/src/inbox/shared_inbox.rs b/lemmy_apub/src/inbox/shared_inbox.rs index cdaa0e8fb..d70568061 100644 --- a/lemmy_apub/src/inbox/shared_inbox.rs +++ b/lemmy_apub/src/inbox/shared_inbox.rs @@ -48,7 +48,7 @@ pub async fn shared_inbox( ) -> Result { let activity = input.into_inner(); - let actor = activity + let actor_id = activity .actor()? .to_owned() .single_xsd_any_uri() @@ -56,25 +56,25 @@ pub async fn shared_inbox( debug!( "Shared inbox received activity {:?} from {}", &activity.id_unchecked(), - &actor + &actor_id ); - check_is_apub_id_valid(&actor)?; + check_is_apub_id_valid(&actor_id)?; - let actor = get_or_fetch_and_upsert_actor(&actor, &context).await?; + let actor = get_or_fetch_and_upsert_actor(&actor_id, &context).await?; verify_signature(&request, actor.as_ref())?; let any_base = activity.clone().into_any_base()?; let kind = activity.kind().context(location_info!())?; let res = match kind { - ValidTypes::Announce => receive_announce(any_base, &context).await, - ValidTypes::Create => receive_create(any_base, &context).await, - ValidTypes::Update => receive_update(any_base, &context).await, - ValidTypes::Like => receive_like(any_base, &context).await, - ValidTypes::Dislike => receive_dislike(any_base, &context).await, - ValidTypes::Remove => receive_remove(any_base, &context).await, - ValidTypes::Delete => receive_delete(any_base, &context).await, - ValidTypes::Undo => receive_undo(any_base, &context).await, + ValidTypes::Announce => receive_announce(&context, any_base, actor.as_ref()).await, + ValidTypes::Create => receive_create(&context, any_base, actor_id).await, + ValidTypes::Update => receive_update(&context, any_base, actor_id).await, + ValidTypes::Like => receive_like(&context, any_base, actor_id).await, + ValidTypes::Dislike => receive_dislike(&context, any_base, actor_id).await, + ValidTypes::Remove => receive_remove(&context, any_base, actor_id).await, + ValidTypes::Delete => receive_delete(&context, any_base, actor_id).await, + ValidTypes::Undo => receive_undo(&context, any_base, actor_id).await, }; insert_activity(actor.user_id(), activity.clone(), false, context.pool()).await?;