From 03662501c3d1aea06526d76177c002e6b8c72766 Mon Sep 17 00:00:00 2001 From: FloatingGhost Date: Fri, 14 Oct 2022 11:48:32 +0100 Subject: [PATCH] Check that the signature matches the creator --- lib/pleroma/web/activity_pub/publisher.ex | 4 +- .../mapped_signature_to_identity_plug.ex | 54 +++++++++++++++++-- .../activity_pub_controller_test.exs | 35 ++++++++++++ ...mapped_signature_to_identity_plug_test.exs | 22 ++++++++ 4 files changed, 110 insertions(+), 5 deletions(-) diff --git a/lib/pleroma/web/activity_pub/publisher.ex b/lib/pleroma/web/activity_pub/publisher.ex index ed67a060d..b187d3a48 100644 --- a/lib/pleroma/web/activity_pub/publisher.ex +++ b/lib/pleroma/web/activity_pub/publisher.ex @@ -108,8 +108,8 @@ defp blocked_instances do Config.get([:mrf_simple, :reject], []) end - defp should_federate?(inbox) do - %{host: host} = URI.parse(inbox) + def should_federate?(url) do + %{host: host} = URI.parse(url) quarantined_instances = blocked_instances() diff --git a/lib/pleroma/web/plugs/mapped_signature_to_identity_plug.ex b/lib/pleroma/web/plugs/mapped_signature_to_identity_plug.ex index 58cb0316a..a73def682 100644 --- a/lib/pleroma/web/plugs/mapped_signature_to_identity_plug.ex +++ b/lib/pleroma/web/plugs/mapped_signature_to_identity_plug.ex @@ -19,6 +19,7 @@ def call(%{assigns: %{user: %User{}}} = conn, _opts), do: conn def call(%{assigns: %{valid_signature: true}, params: %{"actor" => actor}} = conn, _opts) do with actor_id <- Utils.get_ap_id(actor), {:user, %User{} = user} <- {:user, user_from_key_id(conn)}, + {:federate, true} <- {:federate, should_federate?(user)}, {:user_match, true} <- {:user_match, user.ap_id == actor_id} do conn |> assign(:user, user) @@ -27,33 +28,70 @@ def call(%{assigns: %{valid_signature: true}, params: %{"actor" => actor}} = con {:user_match, false} -> Logger.debug("Failed to map identity from signature (payload actor mismatch)") Logger.debug("key_id=#{inspect(key_id_from_conn(conn))}, actor=#{inspect(actor)}") - assign(conn, :valid_signature, false) + + conn + |> assign(:valid_signature, false) # remove me once testsuite uses mapped capabilities instead of what we do now {:user, nil} -> Logger.debug("Failed to map identity from signature (lookup failure)") Logger.debug("key_id=#{inspect(key_id_from_conn(conn))}, actor=#{actor}") + conn + |> assign(:valid_signature, false) + + {:federate, false} -> + Logger.debug("Identity from signature is instance blocked") + Logger.debug("key_id=#{inspect(key_id_from_conn(conn))}, actor=#{actor}") + + conn + |> assign(:valid_signature, false) end end # no payload, probably a signed fetch def call(%{assigns: %{valid_signature: true}} = conn, _opts) do - with %User{} = user <- user_from_key_id(conn) do + with %User{} = user <- user_from_key_id(conn), + {:federate, true} <- {:federate, should_federate?(user)} do conn |> assign(:user, user) |> AuthHelper.skip_oauth() else + {:federate, false} -> + Logger.debug("Identity from signature is instance blocked") + Logger.debug("key_id=#{inspect(key_id_from_conn(conn))}") + + conn + |> assign(:valid_signature, false) + + nil -> + Logger.debug("Failed to map identity from signature (lookup failure)") + Logger.debug("key_id=#{inspect(key_id_from_conn(conn))}") + + only_permit_user_routes(conn) + _ -> Logger.debug("Failed to map identity from signature (no payload actor mismatch)") Logger.debug("key_id=#{inspect(key_id_from_conn(conn))}") - assign(conn, :valid_signature, false) + + conn + |> assign(:valid_signature, false) end end # no signature at all def call(conn, _opts), do: conn + defp only_permit_user_routes(%{path_info: ["users", _]} = conn) do + conn + |> assign(:limited_ap, true) + end + + defp only_permit_user_routes(conn) do + conn + |> assign(:valid_signature, false) + end + defp key_id_from_conn(conn) do with %{"keyId" => key_id} <- HTTPSignatures.signature_for_conn(conn), {:ok, ap_id} <- Signature.key_id_to_actor_id(key_id) do @@ -73,4 +111,14 @@ defp user_from_key_id(conn) do nil end end + + defp should_federate?(%User{ap_id: ap_id}), do: should_federate?(ap_id) + + defp should_federate?(ap_id) do + if Pleroma.Config.get([:activitypub, :authorized_fetch_mode], false) do + Pleroma.Web.ActivityPub.Publisher.should_federate?(ap_id) + else + true + end + end end diff --git a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs index e209bb46b..424b87b20 100644 --- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs @@ -559,6 +559,10 @@ test "it inserts an incoming activity into the database", %{conn: conn} do conn = conn |> assign(:valid_signature, true) + |> put_req_header( + "signature", + "keyId=\"http://mastodon.example.org/users/admin/main-key\"" + ) |> put_req_header("content-type", "application/activity+json") |> post("/inbox", data) @@ -589,6 +593,7 @@ test "it inserts an incoming activity into the database" <> conn = conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{user.ap_id}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/inbox", data) @@ -602,12 +607,15 @@ test "it clears `unreachable` federation status of the sender", %{conn: conn} do data = File.read!("test/fixtures/mastodon-post-activity.json") |> Jason.decode!() sender_url = data["actor"] + sender = insert(:user, ap_id: data["actor"]) + Instances.set_consistently_unreachable(sender_url) refute Instances.reachable?(sender_url) conn = conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{sender.ap_id}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/inbox", data) @@ -632,6 +640,7 @@ test "accept follow activity", %{conn: conn} do assert "ok" == conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{followed_relay.ap_id}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/inbox", accept) |> json_response(200) @@ -698,6 +707,11 @@ test "accepts Add/Remove activities", %{conn: conn} do actor = "https://example.com/users/lain" + insert(:user, + ap_id: actor, + featured_address: "https://example.com/users/lain/collections/featured" + ) + Tesla.Mock.mock(fn %{ method: :get, @@ -743,6 +757,7 @@ test "accepts Add/Remove activities", %{conn: conn} do assert "ok" == conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{actor}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/inbox", data) |> json_response(200) @@ -750,6 +765,7 @@ test "accepts Add/Remove activities", %{conn: conn} do ObanHelpers.perform(all_enqueued(worker: ReceiverWorker)) assert Activity.get_by_ap_id(data["id"]) user = User.get_cached_by_ap_id(data["actor"]) + assert user.pinned_objects[data["object"]] data = %{ @@ -764,6 +780,7 @@ test "accepts Add/Remove activities", %{conn: conn} do assert "ok" == conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{actor}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/inbox", data) |> json_response(200) @@ -790,6 +807,12 @@ test "mastodon pin/unpin", %{conn: conn} do actor = "https://example.com/users/lain" + sender = + insert(:user, + ap_id: actor, + featured_address: "https://example.com/users/lain/collections/featured" + ) + Tesla.Mock.mock(fn %{ method: :get, @@ -844,6 +867,7 @@ test "mastodon pin/unpin", %{conn: conn} do assert "ok" == conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{sender.ap_id}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/inbox", data) |> json_response(200) @@ -863,6 +887,7 @@ test "mastodon pin/unpin", %{conn: conn} do assert "ok" == conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{actor}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/inbox", data) |> json_response(200) @@ -894,6 +919,7 @@ test "it inserts an incoming activity into the database", %{conn: conn, data: da conn = conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{data["actor"]}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/users/#{user.nickname}/inbox", data) @@ -915,6 +941,7 @@ test "it accepts messages with to as string instead of array", %{conn: conn, dat conn = conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{data["actor"]}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/users/#{user.nickname}/inbox", data) @@ -936,6 +963,7 @@ test "it accepts messages with cc as string instead of array", %{conn: conn, dat conn = conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{data["actor"]}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/users/#{user.nickname}/inbox", data) @@ -960,6 +988,7 @@ test "it accepts messages with bcc as string instead of array", %{conn: conn, da conn = conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{data["actor"]}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/users/#{user.nickname}/inbox", data) @@ -987,6 +1016,7 @@ test "it accepts announces with to as string instead of array", %{conn: conn} do conn = conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{announcer.ap_id}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/users/#{user.nickname}/inbox", data) @@ -1017,6 +1047,7 @@ test "it accepts messages from actors that are followed by the user", %{ conn = conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{actor.ap_id}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/users/#{recipient.nickname}/inbox", data) @@ -1063,6 +1094,7 @@ test "it clears `unreachable` federation status of the sender", %{conn: conn, da conn = conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{data["actor"]}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/users/#{user.nickname}/inbox", data) @@ -1101,6 +1133,7 @@ test "it removes all follower collections but actor's", %{conn: conn} do conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{actor.ap_id}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/users/#{recipient.nickname}/inbox", data) |> json_response(200) @@ -1193,6 +1226,7 @@ test "forwarded report", %{conn: conn} do conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{actor.ap_id}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/users/#{reported_user.nickname}/inbox", data) |> json_response(200) @@ -1248,6 +1282,7 @@ test "forwarded report from mastodon", %{conn: conn} do conn |> assign(:valid_signature, true) + |> put_req_header("signature", "keyId=\"#{remote_actor}/main-key\"") |> put_req_header("content-type", "application/activity+json") |> post("/users/#{reported_user.nickname}/inbox", data) |> json_response(200) diff --git a/test/pleroma/web/plugs/mapped_signature_to_identity_plug_test.exs b/test/pleroma/web/plugs/mapped_signature_to_identity_plug_test.exs index 00ce6492d..21c574ba3 100644 --- a/test/pleroma/web/plugs/mapped_signature_to_identity_plug_test.exs +++ b/test/pleroma/web/plugs/mapped_signature_to_identity_plug_test.exs @@ -9,6 +9,8 @@ defmodule Pleroma.Web.Plugs.MappedSignatureToIdentityPlugTest do import Tesla.Mock import Plug.Conn + import Pleroma.Tests.Helpers, only: [clear_config: 2] + setup do mock(fn env -> apply(HttpRequestMock, :request, [env]) end) :ok @@ -47,6 +49,26 @@ test "it considers a mapped identity to be invalid when it mismatches a payload" assert %{valid_signature: false} == conn.assigns end + test "it considers a mapped identity to be invalid when the associated instance is blocked" do + clear_config([:activitypub, :authorized_fetch_mode], true) + + clear_config([:mrf_simple, :reject], [ + {"mastodon.example.org", "anime is banned"} + ]) + + on_exit(fn -> + Pleroma.Config.put([:activitypub, :authorized_fetch_mode], false) + Pleroma.Config.put([:mrf_simple, :reject], []) + end) + + conn = + build_conn(:post, "/doesntmattter", %{"actor" => "http://mastodon.example.org/users/admin"}) + |> set_signature("http://mastodon.example.org/users/admin") + |> MappedSignatureToIdentityPlug.call(%{}) + + assert %{valid_signature: false} == conn.assigns + end + @tag skip: "known breakage; the testsuite presently depends on it" test "it considers a mapped identity to be invalid when the identity cannot be found" do conn =