Merge pull request 'Federation: do not deliver to instances rejected by SimplePolicy' (#8021) from lambadalambda/pleroma:fix/federation-reject-instances into develop

Reviewed-on: https://git.pleroma.social/pleroma/pleroma/pulls/8021

changelog.d/federation-reject-instances.security

@@ -0,0 +1 @@
+Do not deliver activities, including public ones and deletes, to instances rejected by SimplePolicy (or not on its accept list), and refuse their signed fetches in authorized fetch mode

docs/configuration/cheatsheet.md

@@ -177,8 +177,8 @@ To add configuration to your config file, you can copy it from the base config.
 * `media_removal`: List of instances to strip media attachments from and the reason for doing so.
 * `media_nsfw`: List of instances to tag all media as NSFW (sensitive) from and the reason for doing so.
 * `federated_timeline_removal`: List of instances to remove from the Federated Timeline (aka The Whole Known Network) and the reason for doing so.
-* `reject`: List of instances to reject activities (except deletes) from and the reason for doing so.
-* `accept`: List of instances to only accept activities (except deletes) from and the reason for doing so.
+* `reject`: List of instances to reject activities (except deletes) from and the reason for doing so. Activities are also not delivered to these instances, and signed fetches from them are refused in `authorized_fetch_mode`.
+* `accept`: List of instances to only accept activities (except deletes) from and the reason for doing so. If set, activities are also only delivered to these instances, and signed fetches from others are refused in `authorized_fetch_mode`.
 * `followers_only`: Force posts from the given instances to be visible by followers only and the reason for doing so.
 * `report_removal`: List of instances to reject reports from and the reason for doing so.
 * `avatar_removal`: List of instances to strip avatars from and the reason for doing so.

lib/pleroma/web/activity_pub/mrf.ex

@@ -164,6 +164,39 @@ defmodule Pleroma.Web.ActivityPub.MRF do
     Enum.any?(domains, fn domain -> Regex.match?(domain, host) end)
   end
 
+  @doc """
+  Returns a function that is false for a host if SimplePolicy is enabled and
+  rejects it, or has an accept list without it. Used for deliveries and signed
+  fetches, not only for incoming activities. Build it once and reuse it: it
+  compiles the instance lists.
+  """
+  @spec instance_filter() :: (String.t() | nil -> boolean())
+  def instance_filter do
+    if Pleroma.Web.ActivityPub.MRF.SimplePolicy in get_policies() do
+      reject = simple_policy_domains(:reject)
+      accept = simple_policy_domains(:accept)
+
+      fn
+        host when is_binary(host) ->
+          not subdomain_match?(reject, host) and (accept == [] or subdomain_match?(accept, host))
+
+        _ ->
+          false
+      end
+    else
+      &is_binary/1
+    end
+  end
+
+  @spec instance_allowed?(String.t() | nil) :: boolean()
+  def instance_allowed?(host), do: instance_filter().(host)
+
+  defp simple_policy_domains(key) do
+    Pleroma.Config.get([:mrf_simple, key], [])
+    |> instance_list_from_tuples()
+    |> subdomains_regex()
+  end
+
   @spec instance_list_from_tuples([{String.t(), String.t()}]) :: [String.t()]
   def instance_list_from_tuples(list) do
     Enum.map(list, fn {instance, _} -> instance end)

lib/pleroma/web/activity_pub/mrf/simple_policy.ex

@@ -333,12 +333,12 @@ defmodule Pleroma.Web.ActivityPub.MRF.SimplePolicy do
           %{
             key: :reject,
             description:
-              "List of instances to reject activities from (except deletes) and the reason for doing so"
+              "List of instances to reject activities from (except deletes) and the reason for doing so. We also do not deliver to them, and refuse their signed fetches in authorized fetch mode"
           },
           %{
             key: :accept,
             description:
-              "List of instances to only accept activities from (except deletes) and the reason for doing so"
+              "List of instances to only accept activities from (except deletes) and the reason for doing so. If set, we also only deliver to them, and refuse signed fetches from others in authorized fetch mode"
           },
           %{
             key: :followers_only,

lib/pleroma/web/activity_pub/publisher.ex

@@ -11,6 +11,7 @@ defmodule Pleroma.Web.ActivityPub.Publisher do
   alias Pleroma.Object
   alias Pleroma.Repo
   alias Pleroma.User
+  alias Pleroma.Web.ActivityPub.MRF
   alias Pleroma.Web.ActivityPub.Publisher.Prepared
   alias Pleroma.Web.ActivityPub.Relay
   alias Pleroma.Workers.PublisherWorker
@@ -221,18 +222,25 @@ defmodule Pleroma.Web.ActivityPub.Publisher do
     end
   end
 
-  def should_federate?(nil, _), do: false
-  def should_federate?(_, true), do: true
+  def should_federate?(inbox, public), do: federation_filter(public).(inbox)
 
-  def should_federate?(inbox, _) do
-    %{host: host} = URI.parse(inbox)
+  # Compiles the instance lists once, for filtering many inboxes.
+  defp federation_filter(public) do
+    allowed? = MRF.instance_filter()
 
-    quarantined_instances =
+    quarantined =
       Config.get([:instance, :quarantined_instances], [])
-      |> Pleroma.Web.ActivityPub.MRF.instance_list_from_tuples()
-      |> Pleroma.Web.ActivityPub.MRF.subdomains_regex()
+      |> MRF.instance_list_from_tuples()
+      |> MRF.subdomains_regex()
 
-    !Pleroma.Web.ActivityPub.MRF.subdomain_match?(quarantined_instances, host)
+    fn
+      nil ->
+        false
+
+      inbox ->
+        %{host: host} = URI.parse(inbox)
+        allowed?.(host) and (public or not MRF.subdomain_match?(quarantined, host))
+    end
   end
 
   @spec recipients(User.t(), Activity.t()) :: [[User.t()]]
@@ -312,7 +320,7 @@ defmodule Pleroma.Web.ActivityPub.Publisher do
 
   def publish(%User{} = actor, %{data: %{"bcc" => bcc}} = activity)
       when is_list(bcc) and bcc != [] do
-    public = public?(activity)
+    should_federate? = activity |> public?() |> federation_filter()
 
     [priority_recipients, recipients] = recipients(actor, activity)
 
@@ -324,7 +332,7 @@ defmodule Pleroma.Web.ActivityPub.Publisher do
           determine_inbox(activity, user)
         end)
         |> Enum.uniq()
-        |> Enum.filter(fn inbox -> should_federate?(inbox, public) end)
+        |> Enum.filter(should_federate?)
         |> Instances.filter_reachable()
       end)
 
@@ -360,6 +368,8 @@ defmodule Pleroma.Web.ActivityPub.Publisher do
       Relay.publish(activity)
     end
 
+    should_federate? = federation_filter(public)
+
     [priority_inboxes, inboxes] =
       recipients(actor, activity)
       |> Enum.map(fn recipients ->
@@ -368,7 +378,7 @@ defmodule Pleroma.Web.ActivityPub.Publisher do
           determine_inbox(activity, user)
         end)
         |> Enum.uniq()
-        |> Enum.filter(fn inbox -> should_federate?(inbox, public) end)
+        |> Enum.filter(should_federate?)
       end)
 
     inboxes = inboxes -- priority_inboxes

lib/pleroma/web/plugs/http_signature_plug.ex

@@ -101,7 +101,7 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do
          conn.assigns[:actor_id] do
       %{host: host} = URI.parse(conn.assigns.actor_id)
 
-      if MRF.subdomain_match?(rejected_domains(), host) do
+      if MRF.subdomain_match?(rejected_domains(), host) or not MRF.instance_allowed?(host) do
         conn
         |> put_status(:unauthorized)
         |> halt()

lib/pleroma/workers/publisher_worker.ex

@@ -6,6 +6,8 @@ defmodule Pleroma.Workers.PublisherWorker do
   alias Pleroma.Activity
   alias Pleroma.Config
   alias Pleroma.Instances
+  alias Pleroma.Web.ActivityPub.Publisher
+  alias Pleroma.Web.ActivityPub.Visibility
   alias Pleroma.Web.Federator
 
   use Oban.Worker, queue: :federator_outgoing, max_attempts: 13
@@ -32,26 +34,39 @@ defmodule Pleroma.Workers.PublisherWorker do
         end
       end)
 
-    # Cancel / skip the job if this server believed to be unreachable now
-    if not Instances.reachable?(params.inbox) do
-      {:cancel, :unreachable}
-    else
-      case federator().perform(:publish_one, params) do
-        {:ok, _} ->
-          :ok
+    cond do
+      # Cancel / skip the job if this server believed to be unreachable now
+      not Instances.reachable?(params.inbox) ->
+        {:cancel, :unreachable}
 
-        {:error, _} = error ->
-          # Only mark as unreachable on final failure
-          if job.attempt == job.max_attempts do
-            Instances.set_unreachable(params.inbox)
-          end
+      # The instance may have been blocked after the job was queued
+      not federate?(params) ->
+        {:cancel, :rejected}
 
-          error
+      true ->
+        case federator().perform(:publish_one, params) do
+          {:ok, _} ->
+            :ok
 
-        error ->
-          # Unexpected error, may have been client side
-          error
-      end
+          {:error, _} = error ->
+            # Only mark as unreachable on final failure
+            if job.attempt == job.max_attempts do
+              Instances.set_unreachable(params.inbox)
+            end
+
+            error
+
+          error ->
+            # Unexpected error, may have been client side
+            error
+        end
+    end
+  end
+
+  defp federate?(%{inbox: inbox, activity_id: activity_id}) do
+    case Activity.get_by_id(activity_id) do
+      %Activity{} = activity -> Publisher.should_federate?(inbox, Visibility.public?(activity))
+      nil -> true
     end
   end
 

test/pleroma/web/activity_pub/mrf_test.exs

@@ -139,4 +139,37 @@ defmodule Pleroma.Web.ActivityPub.MRFTest do
              type: :group
            }
   end
+
+  describe "instance_allowed?/1" do
+    setup do: clear_config([:mrf, :policies], [Pleroma.Web.ActivityPub.MRF.SimplePolicy])
+    setup do: clear_config([:mrf_simple, :reject], [{"rejected.example", "spam"}])
+    setup do: clear_config([:mrf_simple, :accept], [])
+
+    test "is false for hosts rejected by SimplePolicy, including subdomains" do
+      clear_config([:mrf_simple, :reject], [{"*.rejected.example", "spam"}])
+
+      refute MRF.instance_allowed?("rejected.example")
+      refute MRF.instance_allowed?("sub.rejected.example")
+      assert MRF.instance_allowed?("fine.example")
+    end
+
+    test "only allows accepted hosts when SimplePolicy has an accept list" do
+      clear_config([:mrf_simple, :accept], [{"friend.example", ""}])
+
+      assert MRF.instance_allowed?("friend.example")
+      refute MRF.instance_allowed?("stranger.example")
+    end
+
+    test "ignores the SimplePolicy lists when the policy is not enabled" do
+      clear_config([:mrf, :policies], [])
+      clear_config([:mrf_simple, :accept], [{"friend.example", ""}])
+
+      assert MRF.instance_allowed?("rejected.example")
+      assert MRF.instance_allowed?("stranger.example")
+    end
+
+    test "is false without a host" do
+      refute MRF.instance_allowed?(nil)
+    end
+  end
 end

test/pleroma/web/activity_pub/publisher_test.exs

@@ -32,8 +32,26 @@ defmodule Pleroma.Web.ActivityPub.PublisherTest do
       refute Publisher.should_federate?(nil, true)
     end
 
-    test "it returns true when public is true" do
-      assert Publisher.should_federate?(false, true)
+    test "it returns true for public activities to quarantined instances" do
+      clear_config([:instance, :quarantined_instances], [{"quarantined.example", ""}])
+
+      assert Publisher.should_federate?("https://quarantined.example/inbox", true)
+      refute Publisher.should_federate?("https://quarantined.example/inbox", false)
+    end
+
+    test "it returns false for instances rejected by SimplePolicy, even for public activities" do
+      clear_config([:mrf, :policies], [Pleroma.Web.ActivityPub.MRF.SimplePolicy])
+      clear_config([:mrf_simple, :reject], [{"rejected.example", ""}])
+
+      refute Publisher.should_federate?("https://rejected.example/inbox", true)
+      refute Publisher.should_federate?("https://rejected.example/inbox", false)
+      assert Publisher.should_federate?("https://fine.example/inbox", true)
+    end
+
+    test "it returns false for inboxes without a host" do
+      clear_config([:instance, :quarantined_instances], [{"quarantined.example", ""}])
+
+      refute Publisher.should_federate?("inbox", false)
     end
   end
 
@@ -204,6 +222,32 @@ defmodule Pleroma.Web.ActivityPub.PublisherTest do
       )
     end
 
+    test "doesn't publish a public activity to instances rejected by SimplePolicy" do
+      clear_config([:mrf, :policies], [Pleroma.Web.ActivityPub.MRF.SimplePolicy])
+      clear_config([:mrf_simple, :reject], [{"rejected.example", "spam"}])
+
+      actor = insert(:user)
+
+      for host <- ["rejected.example", "fine.example"] do
+        follower = insert(:user, local: false, inbox: "https://#{host}/users/nick/inbox")
+        {:ok, _, _} = Pleroma.User.follow(follower, actor)
+      end
+
+      actor = refresh_record(actor)
+
+      note_activity =
+        insert(:note_activity, user: actor, recipients: [@as_public, actor.follower_address])
+
+      assert :ok == Publisher.publish(actor, note_activity)
+
+      inboxes =
+        all_enqueued(worker: "Pleroma.Workers.PublisherWorker")
+        |> Enum.map(& &1.args["params"]["inbox"])
+
+      assert "https://fine.example/users/nick/inbox" in inboxes
+      refute "https://rejected.example/users/nick/inbox" in inboxes
+    end
+
     test "Publishes a non-public activity to non-quarantined instances." do
       Config.put([:instance, :quarantined_instances], [{"somedomain.com", "some reason"}])
 

test/pleroma/web/plugs/http_signature_plug_mrf_test.exs

@@ -0,0 +1,38 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Web.Plugs.HTTPSignaturePlugMRFTest do
+  # Not async: the MRF settings are global config.
+  use Pleroma.Web.ConnCase
+
+  alias Pleroma.StaticStubbedConfigMock, as: ConfigMock
+  alias Pleroma.StubbedHTTPSignaturesMock, as: HTTPSignaturesMock
+  alias Pleroma.Web.Plugs.HTTPSignaturePlug
+
+  import Mox
+  import Phoenix.Controller, only: [put_format: 2]
+  import Plug.Conn
+
+  setup do: clear_config([:mrf, :policies], [Pleroma.Web.ActivityPub.MRF.SimplePolicy])
+  setup do: clear_config([:mrf_simple, :reject], [{"mastodon.example.org", "no reason"}])
+
+  defp signed_get(actor) do
+    ConfigMock
+    |> expect(:get, fn [:activitypub, :authorized_fetch_mode], false -> true end)
+    |> expect(:get, fn [:instance, :rejected_instances] -> [] end)
+
+    HTTPSignaturesMock
+    |> expect(:validate_conn, fn _ -> true end)
+
+    build_conn(:get, "/doesntmattter", %{"actor" => actor})
+    |> put_req_header("signature", "keyId=\"#{actor}#main-key")
+    |> put_format("activity+json")
+    |> HTTPSignaturePlug.call(%{})
+  end
+
+  test "rejects signed requests from instances rejected by SimplePolicy" do
+    assert signed_get("http://mastodon.example.org/users/admin").halted
+    refute signed_get("http://allowed.example.org/users/admin").halted
+  end
+end

test/pleroma/workers/publisher_worker_test.exs

@@ -153,4 +153,38 @@ defmodule Pleroma.Workers.PublisherWorkerTest do
       assert {:cancel, :unreachable} = PublisherWorker.perform(job)
     end
   end
+
+  describe "Blocked instances:" do
+    defp publish_one_job(inbox, activity) do
+      %Oban.Job{
+        args: %{
+          "op" => "publish_one",
+          "params" => %{"inbox" => inbox, "activity_id" => activity.id}
+        },
+        attempt: 1,
+        max_attempts: 5
+      }
+    end
+
+    test "cancels queued jobs to instances rejected by SimplePolicy" do
+      clear_config([:mrf, :policies], [Pleroma.Web.ActivityPub.MRF.SimplePolicy])
+      clear_config([:mrf_simple, :reject], [{"rejected.example", ""}])
+      {:ok, activity} = CommonAPI.post(insert(:user), %{status: "public"})
+
+      assert {:cancel, :rejected} =
+               PublisherWorker.perform(
+                 publish_one_job("https://rejected.example/inbox", activity)
+               )
+    end
+
+    test "cancels queued private jobs to quarantined instances" do
+      clear_config([:instance, :quarantined_instances], [{"quarantined.example", ""}])
+      {:ok, activity} = CommonAPI.post(insert(:user), %{status: "hi", visibility: "private"})
+
+      assert {:cancel, :rejected} =
+               PublisherWorker.perform(
+                 publish_one_job("https://quarantined.example/inbox", activity)
+               )
+    end
+  end
 end