No OneTemporary
Actions

Authored By

Unknown

Size

14 KB

Referenced Files

None

Subscribers

None

View Options

	diff --git a/changelog.d/federation-reject-instances.security b/changelog.d/federation-reject-instances.security
	new file mode 100644
	index 000000000..d79327317
	--- /dev/null
	+++ b/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
	diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md
	index 0460caf26..4865ff0aa 100644
	--- a/docs/configuration/cheatsheet.md
	+++ b/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.
	diff --git a/lib/pleroma/web/activity_pub/mrf.ex b/lib/pleroma/web/activity_pub/mrf.ex
	index 51ab476b7..2496d1858 100644
	--- a/lib/pleroma/web/activity_pub/mrf.ex
	+++ b/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)
	diff --git a/lib/pleroma/web/activity_pub/mrf/simple_policy.ex b/lib/pleroma/web/activity_pub/mrf/simple_policy.ex
	index 7045fe90f..b58d1c75a 100644
	--- a/lib/pleroma/web/activity_pub/mrf/simple_policy.ex
	+++ b/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,
	diff --git a/lib/pleroma/web/activity_pub/publisher.ex b/lib/pleroma/web/activity_pub/publisher.ex
	index e3c4e01a4..17fae2a86 100644
	--- a/lib/pleroma/web/activity_pub/publisher.ex
	+++ b/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
	diff --git a/lib/pleroma/web/plugs/http_signature_plug.ex b/lib/pleroma/web/plugs/http_signature_plug.ex
	index 2e16212ce..1f7cb51f3 100644
	--- a/lib/pleroma/web/plugs/http_signature_plug.ex
	+++ b/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()
	diff --git a/test/pleroma/web/activity_pub/mrf_test.exs b/test/pleroma/web/activity_pub/mrf_test.exs
	index 2d0f3b317..d897e529d 100644
	--- a/test/pleroma/web/activity_pub/mrf_test.exs
	+++ b/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
	diff --git a/test/pleroma/web/activity_pub/publisher_test.exs b/test/pleroma/web/activity_pub/publisher_test.exs
	index 447ffda1b..e8b6ecf38 100644
	--- a/test/pleroma/web/activity_pub/publisher_test.exs
	+++ b/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"}])

	diff --git a/test/pleroma/web/plugs/http_signature_plug_mrf_test.exs b/test/pleroma/web/plugs/http_signature_plug_mrf_test.exs
	new file mode 100644
	index 000000000..1ee81b1a0
	--- /dev/null
	+++ b/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

File Metadata

Mime Type: text/x-diff
Expires: Thu, Oct 29, 6:20 AM (7 h, 58 m)
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 1797644
Default Alt Text: (14 KB)

No OneTemporaryActions

View Options

File Metadata

Event Timeline

No OneTemporary
Actions