]> Untitled Git - lemmy.git/commitdiff
Add option to disable strict allowlist (fixes #1486) (#1581)
authorNutomic <me@nutomic.com>
Wed, 21 Apr 2021 13:36:07 +0000 (13:36 +0000)
committerGitHub <noreply@github.com>
Wed, 21 Apr 2021 13:36:07 +0000 (09:36 -0400)
* Add option to disable strict allowlist (fixes #1486)

* adjust docs

16 files changed:
config/config.hjson
crates/apub/src/activities/send/community.rs
crates/apub/src/activity_queue.rs
crates/apub/src/fetcher/fetch.rs
crates/apub/src/lib.rs
crates/apub/src/objects/comment.rs
crates/apub/src/objects/community.rs
crates/apub/src/objects/mod.rs
crates/apub/src/objects/person.rs
crates/apub/src/objects/post.rs
crates/apub/src/objects/private_message.rs
crates/apub_receive/src/activities/receive/private_message.rs
crates/apub_receive/src/inbox/mod.rs
crates/apub_receive/src/inbox/person_inbox.rs
crates/utils/src/settings/defaults.rs
crates/utils/src/settings/structs.rs

index 1ab231c2ff1288c2ff120333407d4da95d6b2f7c..c532428c0e12447c23820eb2ec0ce297eea95989 100644 (file)
     # Allows and blocks are described here:
     # https://join.lemmy.ml/docs/en/federation/administration.html#instance-allowlist-and-blocklist
     #
-    # comma separated list of instances with which federation is allowed
-    # Only one of these blocks should be uncommented
+    # list of instances with which federation is allowed
     # allowed_instances: ["instance1.tld","instance2.tld"]
-    # comma separated list of instances which are blocked from federating
+    # instances which we never federate anything with (but previously federated objects are unaffected)
     # blocked_instances: []
+    # If true, only federate with instances on the allowlist and block everything else. If false,
+    # use allowlist only for remote communities, and posts/comments in local communities.
+    # strict_allowlist: true
   }
   captcha: {
     enabled: true
index f5d32142821c1e0ee93d0133a9b9c3967ac2e242..c5112f58cac0229645e9ffaabd988964a91e87d4 100644 (file)
@@ -290,7 +290,7 @@ impl CommunityType for Community {
       .map(|i| i.into_inner())
       .unique()
       // Don't send to blocked instances
-      .filter(|inbox| check_is_apub_id_valid(inbox).is_ok())
+      .filter(|inbox| check_is_apub_id_valid(inbox, false).is_ok())
       .collect();
 
     Ok(inboxes)
index 924b3dd61546ab3bb3414d78ad4f9e82b59e7ca0..7016e0ca51b579a21feb73c81058e8e8a0988b99 100644 (file)
@@ -46,7 +46,7 @@ where
   Kind: Serialize,
   <T as Extends<Kind>>::Error: From<serde_json::Error> + Send + Sync + 'static,
 {
-  if check_is_apub_id_valid(&inbox).is_ok() {
+  if check_is_apub_id_valid(&inbox, false).is_ok() {
     debug!(
       "Sending activity {:?} to {}",
       &activity.id_unchecked(),
@@ -83,7 +83,7 @@ where
   .flatten()
   .unique()
   .filter(|inbox| inbox.host_str() != Some(&Settings::get().hostname()))
-  .filter(|inbox| check_is_apub_id_valid(inbox).is_ok())
+  .filter(|inbox| check_is_apub_id_valid(inbox, false).is_ok())
   .map(|inbox| inbox.to_owned())
   .collect();
   debug!(
@@ -124,7 +124,7 @@ where
       .await?;
   } else {
     let inbox = community.get_shared_inbox_or_inbox_url();
-    check_is_apub_id_valid(&inbox)?;
+    check_is_apub_id_valid(&inbox, false)?;
     debug!(
       "Sending activity {:?} to community {}",
       &activity.id_unchecked(),
@@ -160,7 +160,7 @@ where
   );
   let mentions = mentions
     .iter()
-    .filter(|inbox| check_is_apub_id_valid(inbox).is_ok())
+    .filter(|inbox| check_is_apub_id_valid(inbox, false).is_ok())
     .map(|i| i.to_owned())
     .collect();
   send_activity_internal(
index 304d1cd1418462d4277eb48eff6cdef515c58b85..955fc3165bf3fcf7cdca6e1cc14b632bdee2af35 100644 (file)
@@ -59,7 +59,7 @@ where
   if *recursion_counter > MAX_REQUEST_NUMBER {
     return Err(LemmyError::from(anyhow!("Maximum recursion depth reached")).into());
   }
-  check_is_apub_id_valid(&url)?;
+  check_is_apub_id_valid(&url, false)?;
 
   let timeout = Duration::from_secs(60);
 
index 89109318a8b4b02bce46734bfeeb86d131b9e10d..394de73bbc943de32e0caa91243f3ca1414e9a40 100644 (file)
@@ -65,8 +65,7 @@ pub static APUB_JSON_CONTENT_TYPE: &str = "application/activity+json";
 /// - URL being in the allowlist (if it is active)
 /// - URL not being in the blocklist (if it is active)
 ///
-/// Note that only one of allowlist and blacklist can be enabled, not both.
-pub fn check_is_apub_id_valid(apub_id: &Url) -> Result<(), LemmyError> {
+pub fn check_is_apub_id_valid(apub_id: &Url, use_strict_allowlist: bool) -> Result<(), LemmyError> {
   let settings = Settings::get();
   let domain = apub_id.domain().context(location_info!())?.to_string();
   let local_instance = settings.get_hostname_without_port()?;
@@ -95,30 +94,33 @@ pub fn check_is_apub_id_valid(apub_id: &Url) -> Result<(), LemmyError> {
     return Err(anyhow!("invalid apub id scheme {}: {}", apub_id.scheme(), apub_id).into());
   }
 
-  let allowed_instances = Settings::get().get_allowed_instances();
-  let blocked_instances = Settings::get().get_blocked_instances();
-
-  if allowed_instances.is_none() && blocked_instances.is_none() {
-    Ok(())
-  } else if let Some(mut allowed) = allowed_instances {
-    // need to allow this explicitly because apub receive might contain objects from our local
-    // instance. split is needed to remove the port in our federation test setup.
-    allowed.push(local_instance);
-
-    if allowed.contains(&domain) {
-      Ok(())
-    } else {
-      Err(anyhow!("{} not in federation allowlist", domain).into())
-    }
-  } else if let Some(blocked) = blocked_instances {
+  // TODO: might be good to put the part above in one method, and below in another
+  //       (which only gets called in apub::objects)
+  //        -> no that doesnt make sense, we still need the code below for blocklist and strict allowlist
+  if let Some(blocked) = Settings::get().get_blocked_instances() {
     if blocked.contains(&domain) {
-      Err(anyhow!("{} is in federation blocklist", domain).into())
-    } else {
-      Ok(())
+      return Err(anyhow!("{} is in federation blocklist", domain).into());
     }
-  } else {
-    panic!("Invalid config, both allowed_instances and blocked_instances are specified");
   }
+
+  if let Some(mut allowed) = Settings::get().get_allowed_instances() {
+    // Only check allowlist if this is a community, or strict allowlist is enabled.
+    let strict_allowlist = Settings::get()
+      .federation()
+      .strict_allowlist
+      .unwrap_or(true);
+    if use_strict_allowlist || strict_allowlist {
+      // need to allow this explicitly because apub receive might contain objects from our local
+      // instance.
+      allowed.push(local_instance);
+
+      if !allowed.contains(&domain) {
+        return Err(anyhow!("{} not in federation allowlist", domain).into());
+      }
+    }
+  }
+
+  Ok(())
 }
 
 /// Common functions for ActivityPub objects, which are implemented by most (but not all) objects
index 1480e5e9535947cf8bbbedeef98eadb65671d733..ae16b76fe0f85bf188234e9e7abe7cb52c64e64d 100644 (file)
@@ -1,6 +1,7 @@
 use crate::{
   extensions::context::lemmy_context,
   fetcher::objects::{get_or_fetch_and_insert_comment, get_or_fetch_and_insert_post},
+  get_community_from_to_or_cc,
   objects::{
     check_object_domain,
     check_object_for_community_or_site_ban,
@@ -145,6 +146,8 @@ impl FromApubToForm<NoteExt> for CommentForm {
     request_counter: &mut i32,
     _mod_action_allowed: bool,
   ) -> Result<CommentForm, LemmyError> {
+    let community = get_community_from_to_or_cc(note, context, request_counter).await?;
+    let ap_id = Some(check_object_domain(note, expected_domain, community.local)?);
     let creator_actor_id = &note
       .attributed_to()
       .context(location_info!())?
@@ -202,7 +205,7 @@ impl FromApubToForm<NoteExt> for CommentForm {
       published: note.published().map(|u| u.to_owned().naive_local()),
       updated: note.updated().map(|u| u.to_owned().naive_local()),
       deleted: None,
-      ap_id: Some(check_object_domain(note, expected_domain)?),
+      ap_id,
       local: Some(false),
     })
   }
index f886fd1b00a08b39e5ac4592d4edd9df248a3c26..a72575d079363e34400722b1b7b266237b5f4640 100644 (file)
@@ -203,7 +203,7 @@ impl FromApubToForm<GroupExt> for CommunityForm {
       updated: group.inner.updated().map(|u| u.to_owned().naive_local()),
       deleted: None,
       nsfw: Some(group.ext_one.sensitive.unwrap_or(false)),
-      actor_id: Some(check_object_domain(group, expected_domain)?),
+      actor_id: Some(check_object_domain(group, expected_domain, true)?),
       local: Some(false),
       private_key: None,
       public_key: Some(group.ext_two.to_owned().public_key.public_key_pem),
index 83c78282cf3928e29b0ebe464940f04b92b8c4f8..351ece1fcd35c1e73402173739c3ba0988470570 100644 (file)
@@ -98,13 +98,14 @@ where
 pub(in crate::objects) fn check_object_domain<T, Kind>(
   apub: &T,
   expected_domain: Url,
+  use_strict_allowlist: bool,
 ) -> Result<DbUrl, LemmyError>
 where
   T: Base + AsBase<Kind>,
 {
   let domain = expected_domain.domain().context(location_info!())?;
   let object_id = apub.id(domain)?.context(location_info!())?;
-  check_is_apub_id_valid(object_id)?;
+  check_is_apub_id_valid(object_id, use_strict_allowlist)?;
   Ok(object_id.to_owned().into())
 }
 
index 3e468d5c81bc7991b78b8013114f262a057f2d53..34146dc25ea97bcc87a685b3dcf62b7bf4fa2b57 100644 (file)
@@ -189,7 +189,7 @@ impl FromApubToForm<PersonExt> for PersonForm {
       banner: banner.map(|o| o.map(|i| i.into())),
       published: person.inner.published().map(|u| u.to_owned().naive_local()),
       updated: person.updated().map(|u| u.to_owned().naive_local()),
-      actor_id: Some(check_object_domain(person, expected_domain)?),
+      actor_id: Some(check_object_domain(person, expected_domain, false)?),
       bio: Some(bio),
       local: Some(false),
       admin: Some(false),
index 57cb76524c569bf240448e61a720735062226193..8bbc07605acfe6f2c6465ae26d9e907cecd59c2e 100644 (file)
@@ -143,12 +143,13 @@ impl FromApubToForm<PageExt> for PostForm {
     request_counter: &mut i32,
     mod_action_allowed: bool,
   ) -> Result<PostForm, LemmyError> {
+    let community = get_community_from_to_or_cc(page, context, request_counter).await?;
     let ap_id = if mod_action_allowed {
       let id = page.id_unchecked().context(location_info!())?;
-      check_is_apub_id_valid(id)?;
+      check_is_apub_id_valid(id, community.local)?;
       id.to_owned().into()
     } else {
-      check_object_domain(page, expected_domain)?
+      check_object_domain(page, expected_domain, community.local)?
     };
     let ext = &page.ext_one;
     let creator_actor_id = page
@@ -162,8 +163,6 @@ impl FromApubToForm<PageExt> for PostForm {
     let creator =
       get_or_fetch_and_upsert_person(creator_actor_id, context, request_counter).await?;
 
-    let community = get_community_from_to_or_cc(page, context, request_counter).await?;
-
     let thumbnail_url: Option<Url> = match &page.inner.image() {
       Some(any_image) => Image::from_any_base(
         any_image
index 5502fc355e2363c2c24cd73df397c2971789c9fa..6bb4820f4cabcde25d4dce59ecd85e5ec6c81d25 100644 (file)
@@ -1,5 +1,4 @@
 use crate::{
-  check_is_apub_id_valid,
   extensions::context::lemmy_context,
   fetcher::person::get_or_fetch_and_upsert_person,
   objects::{
@@ -116,8 +115,7 @@ impl FromApubToForm<NoteExt> for PrivateMessageForm {
       .context(location_info!())?;
     let recipient =
       get_or_fetch_and_upsert_person(&recipient_actor_id, context, request_counter).await?;
-    let ap_id = note.id_unchecked().context(location_info!())?.to_string();
-    check_is_apub_id_valid(&Url::parse(&ap_id)?)?;
+    let ap_id = Some(check_object_domain(note, expected_domain, false)?);
 
     let content = get_source_markdown_value(note)?.context(location_info!())?;
 
@@ -129,7 +127,7 @@ impl FromApubToForm<NoteExt> for PrivateMessageForm {
       updated: note.updated().map(|u| u.to_owned().naive_local()),
       deleted: None,
       read: None,
-      ap_id: Some(check_object_domain(note, expected_domain)?),
+      ap_id,
       local: Some(false),
     })
   }
index 6be1a0415ced0b9ce65e338caf05083a421ec7fe..a0dcb81ea518e4aacfc3a542b48737b2cff29f5f 100644 (file)
@@ -220,7 +220,7 @@ where
     .to_owned()
     .single_xsd_any_uri()
     .context(location_info!())?;
-  check_is_apub_id_valid(&person_id)?;
+  check_is_apub_id_valid(&person_id, false)?;
   // check that the sender is a person, not a community
   get_or_fetch_and_upsert_person(&person_id, &context, request_counter).await?;
 
index 453781fde8072300831fd55d7e4c423702620652..dce2c279433dee806fad12cfbfaec5468de68d1d 100644 (file)
@@ -85,7 +85,7 @@ where
     .to_owned()
     .single_xsd_any_uri()
     .context(location_info!())?;
-  check_is_apub_id_valid(&actor_id)?;
+  check_is_apub_id_valid(&actor_id, false)?;
   let actor = get_or_fetch_and_upsert_actor(&actor_id, &context, request_counter).await?;
   verify_signature(&request, actor.as_ref())?;
   Ok(actor)
index 66b6c95ac03407a73723c90b0699629fa7e67991..c9b9df4233e973bf3932d4af25ace68d21072b9d 100644 (file)
@@ -302,7 +302,7 @@ pub async fn receive_announce(
     .context(location_info!())?;
 
   let inner_id = inner_activity.id().context(location_info!())?.to_owned();
-  check_is_apub_id_valid(&inner_id)?;
+  check_is_apub_id_valid(&inner_id, false)?;
   if is_activity_already_known(context.pool(), &inner_id).await? {
     return Ok(());
   }
index 3e7b344f23a86e26a892ff11171d4d36b16c3119..2883536ab32f69ce4cacc82fe8721c7be742e7b7 100644 (file)
@@ -49,6 +49,7 @@ impl Default for FederationConfig {
       enabled: false,
       allowed_instances: None,
       blocked_instances: None,
+      strict_allowlist: Some(true),
     }
   }
 }
index 3863f6e3c2ad3312dbdaffa551dd759895558f53..7aad404b30f4b98c7839ddd419a3ddb0831a8cc4 100644 (file)
@@ -77,6 +77,7 @@ pub struct FederationConfig {
   pub enabled: bool,
   pub allowed_instances: Option<Vec<String>>,
   pub blocked_instances: Option<Vec<String>>,
+  pub strict_allowlist: Option<bool>,
 }
 
 #[derive(Debug, Deserialize, Clone)]