diff options
| -rw-r--r-- | CHANGELOG.md | 11 | ||||
| -rw-r--r-- | lib/pleroma/web/activity_pub/mrf.ex | 24 | ||||
| -rw-r--r-- | lib/pleroma/web/activity_pub/mrf/keyword_policy.ex | 67 | ||||
| -rw-r--r-- | lib/pleroma/web/activity_pub/mrf/subchain_policy.ex | 3 | ||||
| -rw-r--r-- | lib/pleroma/web/activity_pub/pipeline.ex | 8 | ||||
| -rw-r--r-- | lib/pleroma/web/api_spec/operations/chat_operation.ex | 3 | ||||
| -rw-r--r-- | lib/pleroma/web/api_spec/operations/status_operation.ex | 2 | ||||
| -rw-r--r-- | lib/pleroma/web/common_api/common_api.ex | 3 | ||||
| -rw-r--r-- | lib/pleroma/web/pleroma_api/controllers/chat_controller.ex | 10 | ||||
| -rw-r--r-- | test/web/activity_pub/pipeline_test.exs | 16 | ||||
| -rw-r--r-- | test/web/common_api/common_api_test.exs | 11 | ||||
| -rw-r--r-- | test/web/pleroma_api/controllers/chat_controller_test.exs | 19 | 
12 files changed, 124 insertions, 53 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 92635f6d0..7125e6c1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,17 @@ All notable changes to this project will be documented in this file.  The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## unreleased-patch - ??? + +### Security + +- Fix most MRF rules either crashing or not being applied to objects passed into the Common Pipeline (ChatMessage, Question, Answer, Audio, Event) + +### Fixed + +- Welcome Chat messages preventing user registration with MRF Simple Policy applied to the local instance +- Mastodon API: the public timeline returning an error when the `reply_visibility` parameter is set to `self` for an unauthenticated user +  ## [2.1.1] - 2020-09-08  ### Security diff --git a/lib/pleroma/web/activity_pub/mrf.ex b/lib/pleroma/web/activity_pub/mrf.ex index 206d6af52..5e5361082 100644 --- a/lib/pleroma/web/activity_pub/mrf.ex +++ b/lib/pleroma/web/activity_pub/mrf.ex @@ -5,16 +5,34 @@  defmodule Pleroma.Web.ActivityPub.MRF do    @callback filter(Map.t()) :: {:ok | :reject, Map.t()} -  def filter(policies, %{} = object) do +  def filter(policies, %{} = message) do      policies -    |> Enum.reduce({:ok, object}, fn -      policy, {:ok, object} -> policy.filter(object) +    |> Enum.reduce({:ok, message}, fn +      policy, {:ok, message} -> policy.filter(message)        _, error -> error      end)    end    def filter(%{} = object), do: get_policies() |> filter(object) +  def pipeline_filter(%{} = message, meta) do +    object = meta[:object_data] +    ap_id = message["object"] + +    if object && ap_id do +      with {:ok, message} <- filter(Map.put(message, "object", object)) do +        meta = Keyword.put(meta, :object_data, message["object"]) +        {:ok, Map.put(message, "object", ap_id), meta} +      else +        {err, message} -> {err, message, meta} +      end +    else +      {err, message} = filter(message) + +      {err, message, meta} +    end +  end +    def get_policies do      Pleroma.Config.get([:mrf, :policies], []) |> get_policies()    end diff --git a/lib/pleroma/web/activity_pub/mrf/keyword_policy.ex b/lib/pleroma/web/activity_pub/mrf/keyword_policy.ex index 15e09dcf0..db66cfa3e 100644 --- a/lib/pleroma/web/activity_pub/mrf/keyword_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/keyword_policy.ex @@ -20,9 +20,17 @@ defmodule Pleroma.Web.ActivityPub.MRF.KeywordPolicy do      String.match?(string, pattern)    end -  defp check_reject(%{"object" => %{"content" => content, "summary" => summary}} = message) do +  defp object_payload(%{} = object) do +    [object["content"], object["summary"], object["name"]] +    |> Enum.filter(& &1) +    |> Enum.join("\n") +  end + +  defp check_reject(%{"object" => %{} = object} = message) do +    payload = object_payload(object) +      if Enum.any?(Pleroma.Config.get([:mrf_keyword, :reject]), fn pattern -> -         string_matches?(content, pattern) or string_matches?(summary, pattern) +         string_matches?(payload, pattern)         end) do        {:reject, "[KeywordPolicy] Matches with rejected keyword"}      else @@ -30,12 +38,12 @@ defmodule Pleroma.Web.ActivityPub.MRF.KeywordPolicy do      end    end -  defp check_ftl_removal( -         %{"to" => to, "object" => %{"content" => content, "summary" => summary}} = message -       ) do +  defp check_ftl_removal(%{"to" => to, "object" => %{} = object} = message) do +    payload = object_payload(object) +      if Pleroma.Constants.as_public() in to and           Enum.any?(Pleroma.Config.get([:mrf_keyword, :federated_timeline_removal]), fn pattern -> -           string_matches?(content, pattern) or string_matches?(summary, pattern) +           string_matches?(payload, pattern)           end) do        to = List.delete(to, Pleroma.Constants.as_public())        cc = [Pleroma.Constants.as_public() | message["cc"] || []] @@ -51,35 +59,24 @@ defmodule Pleroma.Web.ActivityPub.MRF.KeywordPolicy do      end    end -  defp check_replace(%{"object" => %{"content" => content, "summary" => summary}} = message) do -    content = -      if is_binary(content) do -        content -      else -        "" -      end - -    summary = -      if is_binary(summary) do -        summary -      else -        "" -      end - -    {content, summary} = -      Enum.reduce( -        Pleroma.Config.get([:mrf_keyword, :replace]), -        {content, summary}, -        fn {pattern, replacement}, {content_acc, summary_acc} -> -          {String.replace(content_acc, pattern, replacement), -           String.replace(summary_acc, pattern, replacement)} -        end -      ) - -    {:ok, -     message -     |> put_in(["object", "content"], content) -     |> put_in(["object", "summary"], summary)} +  defp check_replace(%{"object" => %{} = object} = message) do +    object = +      ["content", "name", "summary"] +      |> Enum.filter(fn field -> Map.has_key?(object, field) && object[field] end) +      |> Enum.reduce(object, fn field, object -> +        data = +          Enum.reduce( +            Pleroma.Config.get([:mrf_keyword, :replace]), +            object[field], +            fn {pat, repl}, acc -> String.replace(acc, pat, repl) end +          ) + +        Map.put(object, field, data) +      end) + +    message = Map.put(message, "object", object) + +    {:ok, message}    end    @impl true diff --git a/lib/pleroma/web/activity_pub/mrf/subchain_policy.ex b/lib/pleroma/web/activity_pub/mrf/subchain_policy.ex index c9f20571f..048052da6 100644 --- a/lib/pleroma/web/activity_pub/mrf/subchain_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/subchain_policy.ex @@ -28,8 +28,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.SubchainPolicy do          }"        ) -      subchain -      |> MRF.filter(message) +      MRF.filter(subchain, message)      else        _e -> {:ok, message}      end diff --git a/lib/pleroma/web/activity_pub/pipeline.ex b/lib/pleroma/web/activity_pub/pipeline.ex index 36e325c37..2db86f116 100644 --- a/lib/pleroma/web/activity_pub/pipeline.ex +++ b/lib/pleroma/web/activity_pub/pipeline.ex @@ -26,13 +26,17 @@ defmodule Pleroma.Web.ActivityPub.Pipeline do        {:error, e} ->          {:error, e} + +      {:reject, e} -> +        {:reject, e}      end    end    def do_common_pipeline(object, meta) do      with {_, {:ok, validated_object, meta}} <-             {:validate_object, ObjectValidator.validate(object, meta)}, -         {_, {:ok, mrfd_object}} <- {:mrf_object, MRF.filter(validated_object)}, +         {_, {:ok, mrfd_object, meta}} <- +           {:mrf_object, MRF.pipeline_filter(validated_object, meta)},           {_, {:ok, activity, meta}} <-             {:persist_object, ActivityPub.persist(mrfd_object, meta)},           {_, {:ok, activity, meta}} <- @@ -40,7 +44,7 @@ defmodule Pleroma.Web.ActivityPub.Pipeline do           {_, {:ok, _}} <- {:federation, maybe_federate(activity, meta)} do        {:ok, activity, meta}      else -      {:mrf_object, {:reject, _}} -> {:ok, nil, meta} +      {:mrf_object, {:reject, message, _}} -> {:reject, message}        e -> {:error, e}      end    end diff --git a/lib/pleroma/web/api_spec/operations/chat_operation.ex b/lib/pleroma/web/api_spec/operations/chat_operation.ex index b1a0d26ab..56554d5b4 100644 --- a/lib/pleroma/web/api_spec/operations/chat_operation.ex +++ b/lib/pleroma/web/api_spec/operations/chat_operation.ex @@ -184,7 +184,8 @@ defmodule Pleroma.Web.ApiSpec.ChatOperation do              "application/json",              ChatMessage            ), -        400 => Operation.response("Bad Request", "application/json", ApiError) +        400 => Operation.response("Bad Request", "application/json", ApiError), +        422 => Operation.response("MRF Rejection", "application/json", ApiError)        },        security: [          %{ diff --git a/lib/pleroma/web/api_spec/operations/status_operation.ex b/lib/pleroma/web/api_spec/operations/status_operation.ex index 5bd4619d5..d7ebde6f6 100644 --- a/lib/pleroma/web/api_spec/operations/status_operation.ex +++ b/lib/pleroma/web/api_spec/operations/status_operation.ex @@ -55,7 +55,7 @@ defmodule Pleroma.Web.ApiSpec.StatusOperation do              "application/json",              %Schema{oneOf: [Status, ScheduledStatus]}            ), -        422 => Operation.response("Bad Request", "application/json", ApiError) +        422 => Operation.response("Bad Request / MRF Rejection", "application/json", ApiError)        }      }    end diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index 5ad2b91c2..3c7b9e794 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -49,6 +49,9 @@ defmodule Pleroma.Web.CommonAPI do                local: true              )} do        {:ok, activity} +    else +      {:common_pipeline, {:reject, _} = e} -> e +      e -> e      end    end diff --git a/lib/pleroma/web/pleroma_api/controllers/chat_controller.ex b/lib/pleroma/web/pleroma_api/controllers/chat_controller.ex index 1f2e953f7..ea0921c77 100644 --- a/lib/pleroma/web/pleroma_api/controllers/chat_controller.ex +++ b/lib/pleroma/web/pleroma_api/controllers/chat_controller.ex @@ -90,6 +90,16 @@ defmodule Pleroma.Web.PleromaAPI.ChatController do        conn        |> put_view(MessageReferenceView)        |> render("show.json", chat_message_reference: cm_ref) +    else +      {:reject, message} -> +        conn +        |> put_status(:unprocessable_entity) +        |> json(%{error: message}) + +      {:error, message} -> +        conn +        |> put_status(:bad_request) +        |> json(%{error: message})      end    end diff --git a/test/web/activity_pub/pipeline_test.exs b/test/web/activity_pub/pipeline_test.exs index f2a231eaf..210a06563 100644 --- a/test/web/activity_pub/pipeline_test.exs +++ b/test/web/activity_pub/pipeline_test.exs @@ -26,7 +26,7 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do          {            Pleroma.Web.ActivityPub.MRF,            [], -          [filter: fn o -> {:ok, o} end] +          [pipeline_filter: fn o, m -> {:ok, o, m} end]          },          {            Pleroma.Web.ActivityPub.ActivityPub, @@ -51,7 +51,7 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do                   Pleroma.Web.ActivityPub.Pipeline.common_pipeline(activity, meta)          assert_called(Pleroma.Web.ActivityPub.ObjectValidator.validate(activity, meta)) -        assert_called(Pleroma.Web.ActivityPub.MRF.filter(activity)) +        assert_called(Pleroma.Web.ActivityPub.MRF.pipeline_filter(activity, meta))          assert_called(Pleroma.Web.ActivityPub.ActivityPub.persist(activity, meta))          assert_called(Pleroma.Web.ActivityPub.SideEffects.handle(activity, meta))          refute called(Pleroma.Web.Federator.publish(activity)) @@ -68,7 +68,7 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do          {            Pleroma.Web.ActivityPub.MRF,            [], -          [filter: fn o -> {:ok, o} end] +          [pipeline_filter: fn o, m -> {:ok, o, m} end]          },          {            Pleroma.Web.ActivityPub.ActivityPub, @@ -93,7 +93,7 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do                   Pleroma.Web.ActivityPub.Pipeline.common_pipeline(activity, meta)          assert_called(Pleroma.Web.ActivityPub.ObjectValidator.validate(activity, meta)) -        assert_called(Pleroma.Web.ActivityPub.MRF.filter(activity)) +        assert_called(Pleroma.Web.ActivityPub.MRF.pipeline_filter(activity, meta))          assert_called(Pleroma.Web.ActivityPub.ActivityPub.persist(activity, meta))          assert_called(Pleroma.Web.ActivityPub.SideEffects.handle(activity, meta))          assert_called(Pleroma.Web.Federator.publish(activity)) @@ -109,7 +109,7 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do          {            Pleroma.Web.ActivityPub.MRF,            [], -          [filter: fn o -> {:ok, o} end] +          [pipeline_filter: fn o, m -> {:ok, o, m} end]          },          {            Pleroma.Web.ActivityPub.ActivityPub, @@ -131,7 +131,7 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do                   Pleroma.Web.ActivityPub.Pipeline.common_pipeline(activity, meta)          assert_called(Pleroma.Web.ActivityPub.ObjectValidator.validate(activity, meta)) -        assert_called(Pleroma.Web.ActivityPub.MRF.filter(activity)) +        assert_called(Pleroma.Web.ActivityPub.MRF.pipeline_filter(activity, meta))          assert_called(Pleroma.Web.ActivityPub.ActivityPub.persist(activity, meta))          assert_called(Pleroma.Web.ActivityPub.SideEffects.handle(activity, meta))        end @@ -148,7 +148,7 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do          {            Pleroma.Web.ActivityPub.MRF,            [], -          [filter: fn o -> {:ok, o} end] +          [pipeline_filter: fn o, m -> {:ok, o, m} end]          },          {            Pleroma.Web.ActivityPub.ActivityPub, @@ -170,7 +170,7 @@ defmodule Pleroma.Web.ActivityPub.PipelineTest do                   Pleroma.Web.ActivityPub.Pipeline.common_pipeline(activity, meta)          assert_called(Pleroma.Web.ActivityPub.ObjectValidator.validate(activity, meta)) -        assert_called(Pleroma.Web.ActivityPub.MRF.filter(activity)) +        assert_called(Pleroma.Web.ActivityPub.MRF.pipeline_filter(activity, meta))          assert_called(Pleroma.Web.ActivityPub.ActivityPub.persist(activity, meta))          assert_called(Pleroma.Web.ActivityPub.SideEffects.handle(activity, meta))        end diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index 4ba6232dc..28bb6db30 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -213,6 +213,17 @@ defmodule Pleroma.Web.CommonAPITest do        assert message == :content_too_long      end + +    test "it reject messages via MRF" do +      clear_config([:mrf_keyword, :reject], ["GNO"]) +      clear_config([:mrf, :policies], [Pleroma.Web.ActivityPub.MRF.KeywordPolicy]) + +      author = insert(:user) +      recipient = insert(:user) + +      assert {:reject, "[KeywordPolicy] Matches with rejected keyword"} == +               CommonAPI.post_chat_message(author, recipient, "GNO/Linux") +    end    end    describe "unblocking" do diff --git a/test/web/pleroma_api/controllers/chat_controller_test.exs b/test/web/pleroma_api/controllers/chat_controller_test.exs index 7be5fe09c..44a78a738 100644 --- a/test/web/pleroma_api/controllers/chat_controller_test.exs +++ b/test/web/pleroma_api/controllers/chat_controller_test.exs @@ -100,7 +100,7 @@ defmodule Pleroma.Web.PleromaAPI.ChatControllerTest do          |> post("/api/v1/pleroma/chats/#{chat.id}/messages")          |> json_response_and_validate_schema(400) -      assert result +      assert %{"error" => "no_content"} == result      end      test "it works with an attachment", %{conn: conn, user: user} do @@ -126,6 +126,23 @@ defmodule Pleroma.Web.PleromaAPI.ChatControllerTest do        assert result["attachment"]      end + +    test "gets MRF reason when rejected", %{conn: conn, user: user} do +      clear_config([:mrf_keyword, :reject], ["GNO"]) +      clear_config([:mrf, :policies], [Pleroma.Web.ActivityPub.MRF.KeywordPolicy]) + +      other_user = insert(:user) + +      {:ok, chat} = Chat.get_or_create(user.id, other_user.ap_id) + +      result = +        conn +        |> put_req_header("content-type", "application/json") +        |> post("/api/v1/pleroma/chats/#{chat.id}/messages", %{"content" => "GNO/Linux"}) +        |> json_response_and_validate_schema(422) + +      assert %{"error" => "[KeywordPolicy] Matches with rejected keyword"} == result +    end    end    describe "DELETE /api/v1/pleroma/chats/:id/messages/:message_id" do  | 
