From fc450fdefc2df2bbec20a79fb2c60a95e7f41833 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 28 Aug 2024 15:45:13 -0400 Subject: ReceiverWorker: cancel job if user fetch is forbidden An instance block with authenticated fetch being required can cause this as we couldn't get the user to find their public key to verify the signature. Commonly observed if someone boosts/Announces a post from an instance that blocked you. --- lib/pleroma/workers/receiver_worker.ex | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/workers/receiver_worker.ex b/lib/pleroma/workers/receiver_worker.ex index d4db97b63..7dce02a5f 100644 --- a/lib/pleroma/workers/receiver_worker.ex +++ b/lib/pleroma/workers/receiver_worker.ex @@ -56,17 +56,20 @@ defmodule Pleroma.Workers.ReceiverWorker do def timeout(_job), do: :timer.seconds(5) + defp process_errors({:error, {:error, _} = error}), do: process_errors(error) + defp process_errors(errors) do case errors do {:error, :origin_containment_failed} -> {:cancel, :origin_containment_failed} {:error, :already_present} -> {:cancel, :already_present} {:error, {:validate_object, _} = reason} -> {:cancel, reason} - {:error, {:error, {:validate, {:error, _changeset} = reason}}} -> {:cancel, reason} + {:error, {:validate, {:error, _changeset} = reason}} -> {:cancel, reason} {:error, {:reject, _} = reason} -> {:cancel, reason} {:signature, false} -> {:cancel, :invalid_signature} {:error, "Object has been deleted"} = reason -> {:cancel, reason} {:error, {:side_effects, {:error, :no_object_actor}} = reason} -> {:cancel, reason} {:error, :not_found} = reason -> {:cancel, reason} + {:error, :forbidden} = reason -> {:cancel, reason} {:error, _} = e -> e e -> {:error, e} end -- cgit v1.2.3 From 6ae629cfe072d236453d256017618fe9a8c44755 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 28 Aug 2024 17:24:59 -0400 Subject: Cancel ReceiverWorker jobs if the user account has been disabled / deactivated --- lib/pleroma/workers/receiver_worker.ex | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/workers/receiver_worker.ex b/lib/pleroma/workers/receiver_worker.ex index 7dce02a5f..80518f6fd 100644 --- a/lib/pleroma/workers/receiver_worker.ex +++ b/lib/pleroma/workers/receiver_worker.ex @@ -33,7 +33,8 @@ defmodule Pleroma.Workers.ReceiverWorker do query_string: query_string } - with {:ok, %User{} = _actor} <- User.get_or_fetch_by_ap_id(conn_data.params["actor"]), + with {:ok, %User{} = actor} <- User.get_or_fetch_by_ap_id(conn_data.params["actor"]), + {:user_active, true} <- {:user_active, match?(true, actor.is_active)}, {:ok, _public_key} <- Signature.refetch_public_key(conn_data), {:signature, true} <- {:signature, Signature.validate_signature(conn_data)}, {:ok, res} <- Federator.perform(:incoming_ap_doc, params) do @@ -70,6 +71,7 @@ defmodule Pleroma.Workers.ReceiverWorker do {:error, {:side_effects, {:error, :no_object_actor}} = reason} -> {:cancel, reason} {:error, :not_found} = reason -> {:cancel, reason} {:error, :forbidden} = reason -> {:cancel, reason} + {:user_active, false} = reason -> {:cancel, reason} {:error, _} = e -> e e -> {:error, e} end -- cgit v1.2.3 From 2346807ac93d5acb9901823cceaffe5c305c1e20 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 28 Aug 2024 17:44:33 -0400 Subject: Annotate error cases --- lib/pleroma/workers/receiver_worker.ex | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/workers/receiver_worker.ex b/lib/pleroma/workers/receiver_worker.ex index 80518f6fd..4b1f74a27 100644 --- a/lib/pleroma/workers/receiver_worker.ex +++ b/lib/pleroma/workers/receiver_worker.ex @@ -61,17 +61,21 @@ defmodule Pleroma.Workers.ReceiverWorker do defp process_errors(errors) do case errors do - {:error, :origin_containment_failed} -> {:cancel, :origin_containment_failed} - {:error, :already_present} -> {:cancel, :already_present} - {:error, {:validate_object, _} = reason} -> {:cancel, reason} + # User fetch failures + {:error, :not_found} = reason -> {:cancel, reason} + {:error, :forbidden} = reason -> {:cancel, reason} + # Inactive user + {:user_active, false} = reason -> {:cancel, reason} + # Validator will error and return a changeset error + # e.g., duplicate activities or if the object was deleted {:error, {:validate, {:error, _changeset} = reason}} -> {:cancel, reason} + # MRFs will return a reject {:error, {:reject, _} = reason} -> {:cancel, reason} + # HTTP Sigs {:signature, false} -> {:cancel, :invalid_signature} - {:error, "Object has been deleted"} = reason -> {:cancel, reason} + {:error, :origin_containment_failed} -> {:cancel, :origin_containment_failed} + {:error, {:validate_object, _} = reason} -> {:cancel, reason} {:error, {:side_effects, {:error, :no_object_actor}} = reason} -> {:cancel, reason} - {:error, :not_found} = reason -> {:cancel, reason} - {:error, :forbidden} = reason -> {:cancel, reason} - {:user_active, false} = reason -> {:cancel, reason} {:error, _} = e -> e e -> {:error, e} end -- cgit v1.2.3 From 380a6a6df31a16a89f5c5cc497ddc1360cea3854 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 28 Aug 2024 17:45:31 -0400 Subject: :validate_object is not a real error returned from anywhere --- lib/pleroma/web/federator.ex | 5 ----- lib/pleroma/workers/receiver_worker.ex | 1 - 2 files changed, 6 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/web/federator.ex b/lib/pleroma/web/federator.ex index 2df716556..e812b1a46 100644 --- a/lib/pleroma/web/federator.ex +++ b/lib/pleroma/web/federator.ex @@ -121,11 +121,6 @@ defmodule Pleroma.Web.Federator do Logger.debug("Unhandled actor #{actor}, #{inspect(e)}") {:error, e} - {:error, {:validate_object, _}} = e -> - Logger.error("Incoming AP doc validation error: #{inspect(e)}") - Logger.debug(Jason.encode!(params, pretty: true)) - e - e -> # Just drop those for now Logger.debug(fn -> "Unhandled activity\n" <> Jason.encode!(params, pretty: true) end) diff --git a/lib/pleroma/workers/receiver_worker.ex b/lib/pleroma/workers/receiver_worker.ex index 4b1f74a27..c7e6bc5ea 100644 --- a/lib/pleroma/workers/receiver_worker.ex +++ b/lib/pleroma/workers/receiver_worker.ex @@ -74,7 +74,6 @@ defmodule Pleroma.Workers.ReceiverWorker do # HTTP Sigs {:signature, false} -> {:cancel, :invalid_signature} {:error, :origin_containment_failed} -> {:cancel, :origin_containment_failed} - {:error, {:validate_object, _} = reason} -> {:cancel, reason} {:error, {:side_effects, {:error, :no_object_actor}} = reason} -> {:cancel, reason} {:error, _} = e -> e e -> {:error, e} -- cgit v1.2.3 From c5ca806aa0023e25755947a3bf0d54242e45f65a Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 28 Aug 2024 17:57:34 -0400 Subject: Add back one of the duplicate checks to fix a test, document where it comes from --- lib/pleroma/workers/receiver_worker.ex | 2 ++ 1 file changed, 2 insertions(+) (limited to 'lib') diff --git a/lib/pleroma/workers/receiver_worker.ex b/lib/pleroma/workers/receiver_worker.ex index c7e6bc5ea..810fda67c 100644 --- a/lib/pleroma/workers/receiver_worker.ex +++ b/lib/pleroma/workers/receiver_worker.ex @@ -69,6 +69,8 @@ defmodule Pleroma.Workers.ReceiverWorker do # Validator will error and return a changeset error # e.g., duplicate activities or if the object was deleted {:error, {:validate, {:error, _changeset} = reason}} -> {:cancel, reason} + # Duplicate detection during Normalization + {:error, :already_present} -> {:cancel, :already_present} # MRFs will return a reject {:error, {:reject, _} = reason} -> {:cancel, reason} # HTTP Sigs -- cgit v1.2.3 From 8a3efa7152488460934c1fadc8ab86efd7d47c04 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 28 Aug 2024 18:02:35 -0400 Subject: More error annotations --- lib/pleroma/workers/receiver_worker.ex | 3 +++ 1 file changed, 3 insertions(+) (limited to 'lib') diff --git a/lib/pleroma/workers/receiver_worker.ex b/lib/pleroma/workers/receiver_worker.ex index 810fda67c..6787a59ef 100644 --- a/lib/pleroma/workers/receiver_worker.ex +++ b/lib/pleroma/workers/receiver_worker.ex @@ -75,8 +75,11 @@ defmodule Pleroma.Workers.ReceiverWorker do {:error, {:reject, _} = reason} -> {:cancel, reason} # HTTP Sigs {:signature, false} -> {:cancel, :invalid_signature} + # Origin / URL validation failed somewhere possibly due to spoofing {:error, :origin_containment_failed} -> {:cancel, :origin_containment_failed} + # Unclear if this can be reached {:error, {:side_effects, {:error, :no_object_actor}} = reason} -> {:cancel, reason} + # Catchall {:error, _} = e -> e e -> {:error, e} end -- cgit v1.2.3 From 1821ef4f157980bdf64f7540ee5aa8e26fa3102e Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 28 Aug 2024 18:35:01 -0400 Subject: Move user active check into Federator.perform/1 --- lib/pleroma/web/federator.ex | 3 ++- lib/pleroma/workers/receiver_worker.ex | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/web/federator.ex b/lib/pleroma/web/federator.ex index e812b1a46..58260afa8 100644 --- a/lib/pleroma/web/federator.ex +++ b/lib/pleroma/web/federator.ex @@ -102,7 +102,8 @@ defmodule Pleroma.Web.Federator do # NOTE: we use the actor ID to do the containment, this is fine because an # actor shouldn't be acting on objects outside their own AP server. - with {_, {:ok, _user}} <- {:actor, User.get_or_fetch_by_ap_id(actor)}, + with {_, {:ok, user}} <- {:actor, User.get_or_fetch_by_ap_id(actor)}, + {:user_active, true} <- {:user_active, match?(true, user.is_active)}, nil <- Activity.normalize(params["id"]), {_, :ok} <- {:correct_origin?, Containment.contain_origin_from_id(actor, params)}, diff --git a/lib/pleroma/workers/receiver_worker.ex b/lib/pleroma/workers/receiver_worker.ex index 6787a59ef..0373ec15f 100644 --- a/lib/pleroma/workers/receiver_worker.ex +++ b/lib/pleroma/workers/receiver_worker.ex @@ -33,8 +33,7 @@ defmodule Pleroma.Workers.ReceiverWorker do query_string: query_string } - with {:ok, %User{} = actor} <- User.get_or_fetch_by_ap_id(conn_data.params["actor"]), - {:user_active, true} <- {:user_active, match?(true, actor.is_active)}, + with {:ok, %User{}} <- User.get_or_fetch_by_ap_id(conn_data.params["actor"]), {:ok, _public_key} <- Signature.refetch_public_key(conn_data), {:signature, true} <- {:signature, Signature.validate_signature(conn_data)}, {:ok, res} <- Federator.perform(:incoming_ap_doc, params) do @@ -65,7 +64,7 @@ defmodule Pleroma.Workers.ReceiverWorker do {:error, :not_found} = reason -> {:cancel, reason} {:error, :forbidden} = reason -> {:cancel, reason} # Inactive user - {:user_active, false} = reason -> {:cancel, reason} + {:error, {:user_active, false} = reason} -> {:cancel, reason} # Validator will error and return a changeset error # e.g., duplicate activities or if the object was deleted {:error, {:validate, {:error, _changeset} = reason}} -> {:cancel, reason} -- cgit v1.2.3