From ceffb8a8918b83d482e9c1da64fec22b428a61f3 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 23 Aug 2024 13:52:19 -0400 Subject: Drop incoming Delete activities from unknown actors --- lib/pleroma/workers/receiver_worker.ex | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/workers/receiver_worker.ex b/lib/pleroma/workers/receiver_worker.ex index d4db97b63..ea86a3a12 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 {_, false} <- {:unknown_delete, unknown_delete?(params)}, + 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 @@ -58,6 +59,7 @@ defmodule Pleroma.Workers.ReceiverWorker do defp process_errors(errors) do case errors do + {:unknown_delete, true} -> {:cancel, "Delete from unknown actor"} {:error, :origin_containment_failed} -> {:cancel, :origin_containment_failed} {:error, :already_present} -> {:cancel, :already_present} {:error, {:validate_object, _} = reason} -> {:cancel, reason} @@ -71,4 +73,16 @@ defmodule Pleroma.Workers.ReceiverWorker do e -> {:error, e} end end + + defp unknown_delete?(%{ + "type" => "Delete", + "actor" => actor + }) do + case User.get_cached_by_ap_id(actor) do + %User{} -> false + _ -> true + end + end + + defp unknown_delete?(_), do: false end -- cgit v1.2.3 From 4bc6f334f49c27e1f466052fee6fa2f3d5d2ee74 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 23 Aug 2024 14:18:04 -0400 Subject: Revert unintentional change --- lib/pleroma/workers/receiver_worker.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/workers/receiver_worker.ex b/lib/pleroma/workers/receiver_worker.ex index ea86a3a12..4033fec0f 100644 --- a/lib/pleroma/workers/receiver_worker.ex +++ b/lib/pleroma/workers/receiver_worker.ex @@ -34,7 +34,7 @@ defmodule Pleroma.Workers.ReceiverWorker do } with {_, false} <- {:unknown_delete, unknown_delete?(params)}, - User.get_or_fetch_by_ap_id(conn_data.params["actor"]), + {:ok, %User{} = _actor} <- 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 -- cgit v1.2.3 From 1c394dd18c5d61dc716a9b9cda981a359de32456 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 23 Aug 2024 14:24:04 -0400 Subject: Move the check to the inbox --- .../web/activity_pub/activity_pub_controller.ex | 32 +++++++++++++++++----- lib/pleroma/workers/receiver_worker.ex | 15 +--------- 2 files changed, 26 insertions(+), 21 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index cdd054e1a..a24dcfc9c 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -294,13 +294,19 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do end def inbox(%{assigns: %{valid_signature: false}} = conn, params) do - Federator.incoming_ap_doc(%{ - method: conn.method, - req_headers: conn.req_headers, - request_path: conn.request_path, - params: params, - query_string: conn.query_string - }) + case unknown_delete?(params) do + true -> + :ok + + false -> + Federator.incoming_ap_doc(%{ + method: conn.method, + req_headers: conn.req_headers, + request_path: conn.request_path, + params: params, + query_string: conn.query_string + }) + end json(conn, "ok") end @@ -558,4 +564,16 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do |> json(UserView.render("featured.json", %{user: user})) end end + + defp unknown_delete?(%{ + "type" => "Delete", + "actor" => actor + }) do + case User.get_cached_by_ap_id(actor) do + %User{} -> false + _ -> true + end + end + + defp unknown_delete?(_), do: false end diff --git a/lib/pleroma/workers/receiver_worker.ex b/lib/pleroma/workers/receiver_worker.ex index 4033fec0f..ce92ef9dd 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 {_, false} <- {:unknown_delete, unknown_delete?(params)}, - {: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"]), {: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 @@ -73,16 +72,4 @@ defmodule Pleroma.Workers.ReceiverWorker do e -> {:error, e} end end - - defp unknown_delete?(%{ - "type" => "Delete", - "actor" => actor - }) do - case User.get_cached_by_ap_id(actor) do - %User{} -> false - _ -> true - end - end - - defp unknown_delete?(_), do: false end -- cgit v1.2.3 From 27fcc421719062d5de9bf4dc90f3349595eb278d Mon Sep 17 00:00:00 2001 From: feld Date: Sat, 24 Aug 2024 16:53:22 +0000 Subject: Use Pleroma.Object.Containment.get_actor/1 to reliably find the actor of an incoming activity or object --- lib/pleroma/web/activity_pub/activity_pub_controller.ex | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index a24dcfc9c..77ec26f14 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -567,9 +567,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do defp unknown_delete?(%{ "type" => "Delete", - "actor" => actor - }) do - case User.get_cached_by_ap_id(actor) do + } = data) do + case data |> Pleroma.Object.Containment.get_actor() |> User.get_cached_by_ap_id() do %User{} -> false _ -> true end -- cgit v1.2.3 From 06deacd58e6aa676847530f24c6799162ed06777 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Sat, 24 Aug 2024 20:03:50 -0400 Subject: Formatting --- lib/pleroma/web/activity_pub/activity_pub_controller.ex | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index 77ec26f14..4c83d1927 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -565,9 +565,11 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do end end - defp unknown_delete?(%{ - "type" => "Delete", - } = data) do + defp unknown_delete?( + %{ + "type" => "Delete" + } = data + ) do case data |> Pleroma.Object.Containment.get_actor() |> User.get_cached_by_ap_id() do %User{} -> false _ -> true -- cgit v1.2.3 From 16a9b34876f3a9289c02253e802bffdac4901ac0 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 29 Aug 2024 14:16:59 -0400 Subject: Convert to an Plug called InboxGuard --- lib/pleroma/constants.ex | 12 ++++ .../web/activity_pub/activity_pub_controller.ex | 33 +++-------- lib/pleroma/web/plugs/inbox_guard_plug.ex | 66 ++++++++++++++++++++++ lib/pleroma/web/router.ex | 6 +- 4 files changed, 90 insertions(+), 27 deletions(-) create mode 100644 lib/pleroma/web/plugs/inbox_guard_plug.ex (limited to 'lib') diff --git a/lib/pleroma/constants.ex b/lib/pleroma/constants.ex index 3a5e35301..f969bfdbb 100644 --- a/lib/pleroma/constants.ex +++ b/lib/pleroma/constants.ex @@ -85,6 +85,18 @@ defmodule Pleroma.Constants do ] ) + const(allowed_activity_types_from_strangers, + do: [ + "Block", + "Create", + "Flag", + "Follow", + "Like", + "Move", + "React" + ] + ) + # basic regex, just there to weed out potential mistakes # https://datatracker.ietf.org/doc/html/rfc2045#section-5.1 const(mime_regex, diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index 4c83d1927..cdd054e1a 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -294,19 +294,13 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do end def inbox(%{assigns: %{valid_signature: false}} = conn, params) do - case unknown_delete?(params) do - true -> - :ok - - false -> - Federator.incoming_ap_doc(%{ - method: conn.method, - req_headers: conn.req_headers, - request_path: conn.request_path, - params: params, - query_string: conn.query_string - }) - end + Federator.incoming_ap_doc(%{ + method: conn.method, + req_headers: conn.req_headers, + request_path: conn.request_path, + params: params, + query_string: conn.query_string + }) json(conn, "ok") end @@ -564,17 +558,4 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do |> json(UserView.render("featured.json", %{user: user})) end end - - defp unknown_delete?( - %{ - "type" => "Delete" - } = data - ) do - case data |> Pleroma.Object.Containment.get_actor() |> User.get_cached_by_ap_id() do - %User{} -> false - _ -> true - end - end - - defp unknown_delete?(_), do: false end diff --git a/lib/pleroma/web/plugs/inbox_guard_plug.ex b/lib/pleroma/web/plugs/inbox_guard_plug.ex new file mode 100644 index 000000000..643b586d4 --- /dev/null +++ b/lib/pleroma/web/plugs/inbox_guard_plug.ex @@ -0,0 +1,66 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2022 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.Plugs.InboxGuardPlug do + import Plug.Conn + import Pleroma.Constants, only: [allowed_activity_types_from_strangers: 0] + + alias Pleroma.Config + alias Pleroma.User + + def init(options) do + options + end + + def call(%{assigns: %{valid_signature: true}} = conn, _opts) do + conn + end + + def call(conn, _opts) do + with {_, true} <- {:federating, Config.get!([:instance, :federating])}, + true <- known_actor?(conn) do + conn + else + {:federating, false} -> + conn + |> json(403, "Not federating") + + _ -> + conn + |> filter_from_strangers() + end + end + + # If signature failed but we know this actor we should + # accept it as we may only need to refetch their public key + # during processing + defp known_actor?(%{body_params: data}) do + case Pleroma.Object.Containment.get_actor(data) |> User.get_cached_by_ap_id() do + %User{} -> true + _ -> false + end + end + + # Only permit a subset of activity types from strangers + # or else it will add actors you've never interacted with + # to the database + defp filter_from_strangers(%{body_params: %{"type" => type}} = conn) do + with true <- type in allowed_activity_types_from_strangers() do + conn + else + _ -> + conn + |> json(400, "Invalid activity type for an unknown actor") + end + end + + defp json(conn, status, resp) do + json_resp = Jason.encode!(resp) + + conn + |> put_resp_content_type("application/json") + |> resp(status, json_resp) + |> halt() + end +end diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index 6492e3861..9b9ee421c 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -217,6 +217,10 @@ defmodule Pleroma.Web.Router do plug(Pleroma.Web.Plugs.MappedSignatureToIdentityPlug) end + pipeline :inbox_guard do + plug(Pleroma.Web.Plugs.InboxGuardPlug) + end + pipeline :static_fe do plug(Pleroma.Web.Plugs.StaticFEPlug) end @@ -920,7 +924,7 @@ defmodule Pleroma.Web.Router do end scope "/", Pleroma.Web.ActivityPub do - pipe_through(:activitypub) + pipe_through([:activitypub, :inbox_guard]) post("/inbox", ActivityPubController, :inbox) post("/users/:nickname/inbox", ActivityPubController, :inbox) end -- cgit v1.2.3 From e2cdae2c88eb22588209923d83c2a9c52d16c48c Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 29 Aug 2024 14:23:07 -0400 Subject: Change relay inbox response when not federating to a 403 for consistency --- lib/pleroma/web/activity_pub/activity_pub_controller.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index cdd054e1a..a08eda5f4 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -311,7 +311,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do post_inbox_relayed_create(conn, params) else conn - |> put_status(:bad_request) + |> put_status(403) |> json("Not federating") end end -- cgit v1.2.3 From 990b2058df11cc32f260d0ffcf7dd0f342d435b4 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 29 Aug 2024 14:30:23 -0400 Subject: Remove unnecessary error match in ReceiverWorker --- lib/pleroma/workers/receiver_worker.ex | 1 - 1 file changed, 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/workers/receiver_worker.ex b/lib/pleroma/workers/receiver_worker.ex index ce92ef9dd..d4db97b63 100644 --- a/lib/pleroma/workers/receiver_worker.ex +++ b/lib/pleroma/workers/receiver_worker.ex @@ -58,7 +58,6 @@ defmodule Pleroma.Workers.ReceiverWorker do defp process_errors(errors) do case errors do - {:unknown_delete, true} -> {:cancel, "Delete from unknown actor"} {:error, :origin_containment_failed} -> {:cancel, :origin_containment_failed} {:error, :already_present} -> {:cancel, :already_present} {:error, {:validate_object, _} = reason} -> {:cancel, reason} -- cgit v1.2.3 From 5205e846eb5cfbd0adfa5031ad75e96fccbc86d8 Mon Sep 17 00:00:00 2001 From: feld Date: Fri, 30 Aug 2024 13:14:05 +0000 Subject: Update allowed activity types from strangers Move is emitted from the old account EmojiReact is ~ Like Announced TBD --- lib/pleroma/constants.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/constants.ex b/lib/pleroma/constants.ex index f969bfdbb..bbd183683 100644 --- a/lib/pleroma/constants.ex +++ b/lib/pleroma/constants.ex @@ -92,8 +92,8 @@ defmodule Pleroma.Constants do "Flag", "Follow", "Like", - "Move", - "React" + "EmojiReact", + "Announce" ] ) -- cgit v1.2.3 From e38f5f1a817d6da30e9a128ec74a2a7c78faf174 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 30 Aug 2024 09:35:04 -0400 Subject: Add recognized activity types to a constant and use it in the test --- lib/pleroma/constants.ex | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'lib') diff --git a/lib/pleroma/constants.ex b/lib/pleroma/constants.ex index bbd183683..5268ebe7a 100644 --- a/lib/pleroma/constants.ex +++ b/lib/pleroma/constants.ex @@ -85,6 +85,24 @@ defmodule Pleroma.Constants do ] ) + const(activity_types, + do: [ + "Create", + "Update", + "Delete", + "Follow", + "Accept", + "Reject", + "Add", + "Remove", + "Like", + "Announce", + "Undo", + "Flag", + "EmojiReact" + ] + ) + const(allowed_activity_types_from_strangers, do: [ "Block", -- cgit v1.2.3 From 11ee94ae17094a2bc33505a31671b8c705f768a4 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 30 Aug 2024 09:46:10 -0400 Subject: InboxGuardPlug: Add early rejection of unknown activity types --- lib/pleroma/web/plugs/inbox_guard_plug.ex | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/web/plugs/inbox_guard_plug.ex b/lib/pleroma/web/plugs/inbox_guard_plug.ex index 643b586d4..0064cce76 100644 --- a/lib/pleroma/web/plugs/inbox_guard_plug.ex +++ b/lib/pleroma/web/plugs/inbox_guard_plug.ex @@ -4,7 +4,7 @@ defmodule Pleroma.Web.Plugs.InboxGuardPlug do import Plug.Conn - import Pleroma.Constants, only: [allowed_activity_types_from_strangers: 0] + import Pleroma.Constants, only: [activity_types: 0, allowed_activity_types_from_strangers: 0] alias Pleroma.Config alias Pleroma.User @@ -14,24 +14,46 @@ defmodule Pleroma.Web.Plugs.InboxGuardPlug do end def call(%{assigns: %{valid_signature: true}} = conn, _opts) do - conn + with {_, true} <- {:federating, Config.get!([:instance, :federating])} do + conn + |> filter_activity_types() + else + {:federating, false} -> + conn + |> json(403, "Not federating") + |> halt() + end end def call(conn, _opts) do with {_, true} <- {:federating, Config.get!([:instance, :federating])}, - true <- known_actor?(conn) do + conn = filter_activity_types(conn), + {:known, true} <- {:known, known_actor?(conn)} do conn else {:federating, false} -> conn |> json(403, "Not federating") + |> halt() - _ -> + {:known, false} -> conn |> filter_from_strangers() end end + # Early rejection of unrecognized types + defp filter_activity_types(%{body_params: %{"type" => type}} = conn) do + with true <- type in activity_types() do + conn + else + _ -> + conn + |> json(400, "Invalid activity type") + |> halt() + end + end + # If signature failed but we know this actor we should # accept it as we may only need to refetch their public key # during processing @@ -52,6 +74,7 @@ defmodule Pleroma.Web.Plugs.InboxGuardPlug do _ -> conn |> json(400, "Invalid activity type for an unknown actor") + |> halt() end end -- cgit v1.2.3