Merge remote-tracking branch 'origin/develop' into shigusegubu

* origin/develop: Add tlsv1.3 to suggestions hackney adapter helper & reverse proxy client: enable TLSv1.3 StealEmojiPolicy: fix String rejected_shortcodes Instruct users to run 'git pull' as the pleroma user Also use actor_type to determine if an account is a bot in antiFollowbotPolicy mix: Bump to 2.4.52 for 2.4.3 mergeback Skip cache when /objects or /activities is authenticated Allow to skip cache in Cache plug update sweet_xml [Security]
2022-06-07 16:02:14 +03:00 · 2022-06-07 16:02:14 +03:00 · 83e4a112b9
commit 83e4a112b9
parent 46c4d3b80f 75f912c63f
18 changed files with 175 additions and 36 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -48,6 +48,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

 ### Removed

+## 2.4.3 - 2022-05-06
+
+### Security
+- Private `/objects/` and `/activities/` leaking if cached by authenticated user
+- SweetXML library DTD bomb
+
 ## 2.4.2 - 2022-01-10

 ### Fixed
@ -91,6 +97,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Improved Twittercard and OpenGraph meta tag generation including thumbnails and image dimension metadata when available.
 - AdminAPI: sort users so the newest are at the top.
 - ActivityPub Client-to-Server(C2S): Limitation on the type of Activity/Object are lifted as they are now passed through ObjectValidators
+- MRF (`AntiFollowbotPolicy`): Bot accounts are now also considered followbots. Users can still allow bots to follow them by first following the bot.

 ### Added

--- a/config/description.exs
+++ b/config/description.exs
@ -2726,7 +2726,7 @@ config :pleroma, :config_description, [
                key: :versions,
                type: {:list, :atom},
                description: "List of TLS version to use",
-                suggestions: [:tlsv1, ":tlsv1.1", ":tlsv1.2"]
+                suggestions: [:tlsv1, ":tlsv1.1", ":tlsv1.2", ":tlsv1.3"]
              }
            ]
          }
--- a/docs/administration/updating.md
+++ b/docs/administration/updating.md
@ -17,11 +17,11 @@ su pleroma -s $SHELL -lc "./bin/pleroma_ctl migrate"
 ## For from source installations (using git)

 1. Go to the working directory of Pleroma (default is `/opt/pleroma`)
-2. Run `git pull`. This pulls the latest changes from upstream.
+2. Run `git pull` [^1]. This pulls the latest changes from upstream.
 3. Run `mix deps.get` [^1]. This pulls in any new dependencies.
 4. Stop the Pleroma service.
 5. Run `mix ecto.migrate` [^1] [^2]. This task performs database migrations, if there were any.
 6. Start the Pleroma service.

-[^1]: Depending on which install guide you followed (for example on Debian/Ubuntu), you want to run `mix` tasks as `pleroma` user by adding `sudo -Hu pleroma` before the command.
+[^1]: Depending on which install guide you followed (for example on Debian/Ubuntu), you want to run `git` and `mix` tasks as `pleroma` user by adding `sudo -Hu pleroma` before the command.
 [^2]: Prefix with `MIX_ENV=prod` to run it using the production config file.
--- a/docs/configuration/cheatsheet.md
+++ b/docs/configuration/cheatsheet.md
@ -125,6 +125,7 @@ To add configuration to your config file, you can copy it from the base config.
    * `Pleroma.Web.ActivityPub.MRF.ActivityExpirationPolicy`: Sets a default expiration on all posts made by users of the local instance. Requires `Pleroma.Workers.PurgeExpiredActivity` to be enabled for processing the scheduled delections.
    * `Pleroma.Web.ActivityPub.MRF.ForceBotUnlistedPolicy`: Makes all bot posts to disappear from public timelines.
    * `Pleroma.Web.ActivityPub.MRF.FollowBotPolicy`: Automatically follows newly discovered users from the specified bot account. Local accounts, locked accounts, and users with "#nobot" in their bio are respected and excluded from being followed.
+    * `Pleroma.Web.ActivityPub.MRF.AntiFollowbotPolicy`: Drops follow requests from followbots. Users can still allow bots to follow them by first following the bot.
    * `Pleroma.Web.ActivityPub.MRF.KeywordPolicy`: Rejects or removes from the federated timeline or replaces keywords. (See [`:mrf_keyword`](#mrf_keyword)).
    * `Pleroma.Web.ActivityPub.MRF.ForceMentionsInContent`: Forces every mentioned user to be reflected in the post content.
 * `transparency`: Make the content of your Message Rewrite Facility settings public (via nodeinfo).
--- a/lib/pleroma/http/adapter_helper/hackney.ex
+++ b/lib/pleroma/http/adapter_helper/hackney.ex
@ -24,10 +24,6 @@ defmodule Pleroma.HTTP.AdapterHelper.Hackney do
    |> Pleroma.HTTP.AdapterHelper.maybe_add_proxy(proxy)
  end

-  defp add_scheme_opts(opts, %URI{scheme: "https"}) do
-    Keyword.put(opts, :ssl_options, versions: [:"tlsv1.2", :"tlsv1.1", :tlsv1])
-  end
-
  defp add_scheme_opts(opts, _), do: opts

  defp maybe_add_with_body(opts) do
--- a/lib/pleroma/reverse_proxy/client/hackney.ex
+++ b/lib/pleroma/reverse_proxy/client/hackney.ex
@ -7,7 +7,6 @@ defmodule Pleroma.ReverseProxy.Client.Hackney do

  @impl true
  def request(method, url, headers, body, opts \\ []) do
-    opts = Keyword.put(opts, :ssl_options, versions: [:"tlsv1.2", :"tlsv1.1", :tlsv1])
    :hackney.request(method, url, headers, body, opts)
  end

--- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex
+++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex
@ -84,6 +84,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
         user <- Map.get(assigns, :user, nil),
         {_, true} <- {:visible?, Visibility.visible_for_user?(object, user)} do
      conn
+      |> maybe_skip_cache(user)
      |> assign(:tracking_fun_data, object.id)
      |> set_cache_ttl_for(object)
      |> put_resp_content_type("application/activity+json")
@ -112,6 +113,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
         user <- Map.get(assigns, :user, nil),
         {_, true} <- {:visible?, Visibility.visible_for_user?(activity, user)} do
      conn
+      |> maybe_skip_cache(user)
      |> maybe_set_tracking_data(activity)
      |> set_cache_ttl_for(activity)
      |> put_resp_content_type("application/activity+json")
@ -151,6 +153,15 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
    assign(conn, :cache_ttl, ttl)
  end

+  def maybe_skip_cache(conn, user) do
+    if user do
+      conn
+      |> assign(:skip_cache, true)
+    else
+      conn
+    end
+  end
+
  # GET /relay/following
  def relay_following(conn, _params) do
    with %{halted: false} = conn <- FederatingPlug.call(conn, []) do
--- a/lib/pleroma/web/activity_pub/mrf/anti_followbot_policy.ex
+++ b/lib/pleroma/web/activity_pub/mrf/anti_followbot_policy.ex
@ -24,7 +24,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.AntiFollowbotPolicy do
  defp score_displayname("fedibot"), do: 1.0
  defp score_displayname(_), do: 0.0

-  defp determine_if_followbot(%User{nickname: nickname, name: displayname}) do
+  defp determine_if_followbot(%User{nickname: nickname, name: displayname, actor_type: actor_type}) do
    # nickname will be a binary string except when following a relay
    nick_score =
      if is_binary(nickname) do
@ -45,19 +45,32 @@ defmodule Pleroma.Web.ActivityPub.MRF.AntiFollowbotPolicy do
        0.0
      end

-    nick_score + name_score
+    # actor_type "Service" is a Bot account
+    actor_type_score =
+      if actor_type == "Service" do
+        1.0
+      else
+        0.0
+      end
+
+    nick_score + name_score + actor_type_score
  end

  defp determine_if_followbot(_), do: 0.0

+  defp bot_allowed?(%{"object" => target}, bot_actor) do
+    %User{} = user = normalize_by_ap_id(target)
+
+    User.following?(user, bot_actor)
+  end
+
  @impl true
  def filter(%{"type" => "Follow", "actor" => actor_id} = message) do
    %User{} = actor = normalize_by_ap_id(actor_id)

    score = determine_if_followbot(actor)

-    # TODO: scan biography data for keywords and score it somehow.
-    if score < 0.8 do
+    if score < 0.8 || bot_allowed?(message, actor) do
      {:ok, message}
    else
      {:reject, "[AntiFollowbotPolicy] Scored #{actor_id} as #{score}"}
--- a/lib/pleroma/web/activity_pub/mrf/steal_emoji_policy.ex
+++ b/lib/pleroma/web/activity_pub/mrf/steal_emoji_policy.ex
@ -12,6 +12,14 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicy do

  defp accept_host?(host), do: host in Config.get([:mrf_steal_emoji, :hosts], [])

+  defp shortcode_matches?(shortcode, pattern) when is_binary(pattern) do
+    shortcode == pattern
+  end
+
+  defp shortcode_matches?(shortcode, pattern) do
+    String.match?(shortcode, pattern)
+  end
+
  defp steal_emoji({shortcode, url}, emoji_dir_path) do
    url = Pleroma.Web.MediaProxy.url(url)

@ -72,7 +80,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicy do
          reject_emoji? =
            [:mrf_steal_emoji, :rejected_shortcodes]
            |> Config.get([])
-            |> Enum.find(false, fn regex -> String.match?(shortcode, regex) end)
+            |> Enum.find(false, fn pattern -> shortcode_matches?(shortcode, pattern) end)

          !reject_emoji?
        end)
@ -122,8 +130,12 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicy do
        %{
          key: :rejected_shortcodes,
          type: {:list, :string},
-          description: "Regex-list of shortcodes to reject",
-          suggestions: [""]
+          description: """
+            A list of patterns or matches to reject shortcodes with.
+
+            Each pattern can be a string or [Regex](https://hexdocs.pm/elixir/Regex.html) in the format of `~r/PATTERN/`.
+          """,
+          suggestions: ["foo", ~r/foo/]
        },
        %{
          key: :size_limit,
--- a/lib/pleroma/web/plugs/cache.ex
+++ b/lib/pleroma/web/plugs/cache.ex
@ -97,13 +97,21 @@ defmodule Pleroma.Web.Plugs.Cache do
        key = cache_key(conn, opts)
        content_type = content_type(conn)

+        should_cache = not Map.get(conn.assigns, :skip_cache, false)
+
        conn =
          unless opts[:tracking_fun] do
+            if should_cache do
              @cachex.put(:web_resp_cache, key, {content_type, body}, ttl: ttl)
+            end
+
            conn
          else
            tracking_fun_data = Map.get(conn.assigns, :tracking_fun_data, nil)
+
+            if should_cache do
              @cachex.put(:web_resp_cache, key, {content_type, body, tracking_fun_data}, ttl: ttl)
+            end

            opts.tracking_fun.(conn, tracking_fun_data)
          end
--- a/mix.exs
+++ b/mix.exs
@ -4,7 +4,7 @@ defmodule Pleroma.Mixfile do
  def project do
    [
      app: :pleroma,
-      version: version("2.4.51"),
+      version: version("2.4.52"),
      elixir: "~> 1.9",
      elixirc_paths: elixirc_paths(Mix.env()),
      compilers: [:phoenix, :gettext] ++ Mix.compilers(),
@ -145,7 +145,7 @@ defmodule Pleroma.Mixfile do
      {:mogrify, "~> 0.9.1"},
      {:ex_aws, "~> 2.1.6"},
      {:ex_aws_s3, "~> 2.0"},
-      {:sweet_xml, "~> 0.6.6"},
+      {:sweet_xml, "~> 0.7.2"},
      {:earmark, "~> 1.4.15"},
      {:bbcode_pleroma, "~> 0.2.0"},
      {:crypt,
--- a/mix.lock
+++ b/mix.lock
@ -120,7 +120,7 @@
  "remote_ip": {:git, "https://git.pleroma.social/pleroma/remote_ip.git", "b647d0deecaa3acb140854fe4bda5b7e1dc6d1c8", [ref: "b647d0deecaa3acb140854fe4bda5b7e1dc6d1c8"]},
  "sleeplocks": {:hex, :sleeplocks, "1.1.1", "3d462a0639a6ef36cc75d6038b7393ae537ab394641beb59830a1b8271faeed3", [:rebar3], [], "hexpm", "84ee37aeff4d0d92b290fff986d6a95ac5eedf9b383fadfd1d88e9b84a1c02e1"},
  "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.6", "cf344f5692c82d2cd7554f5ec8fd961548d4fd09e7d22f5b62482e5aeaebd4b0", [:make, :mix, :rebar3], [], "hexpm", "bdb0d2471f453c88ff3908e7686f86f9be327d065cc1ec16fa4540197ea04680"},
-  "sweet_xml": {:hex, :sweet_xml, "0.6.6", "fc3e91ec5dd7c787b6195757fbcf0abc670cee1e4172687b45183032221b66b8", [:mix], [], "hexpm", "2e1ec458f892ffa81f9f8386e3f35a1af6db7a7a37748a64478f13163a1f3573"},
+  "sweet_xml": {:hex, :sweet_xml, "0.7.2", "4729f997286811fabdd8288f8474e0840a76573051062f066c4b597e76f14f9f", [:mix], [], "hexpm", "6894e68a120f454534d99045ea3325f7740ea71260bc315f82e29731d570a6e8"},
  "swoosh": {:hex, :swoosh, "1.3.11", "34f79c57f19892b43bd2168de9ff5de478a721a26328ef59567aad4243e7a77b", [:mix], [{:cowboy, "~> 1.1 or ~> 2.4", [hex: :cowboy, repo: "hexpm", optional: true]}, {:finch, "~> 0.6", [hex: :finch, repo: "hexpm", optional: true]}, {:gen_smtp, "~> 0.13 or ~> 1.0", [hex: :gen_smtp, repo: "hexpm", optional: true]}, {:hackney, "~> 1.9", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mail, "~> 0.2", [hex: :mail, repo: "hexpm", optional: true]}, {:mime, "~> 1.1", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_cowboy, ">= 1.0.0", [hex: :plug_cowboy, repo: "hexpm", optional: true]}], "hexpm", "f1e2a048db454f9982b9cf840f75e7399dd48be31ecc2a7dc10012a803b913af"},
  "syslog": {:hex, :syslog, "1.1.0", "6419a232bea84f07b56dc575225007ffe34d9fdc91abe6f1b2f254fd71d8efc2", [:rebar3], [], "hexpm", "4c6a41373c7e20587be33ef841d3de6f3beba08519809329ecc4d27b15b659e1"},
  "table_rex": {:hex, :table_rex, "3.1.1", "0c67164d1714b5e806d5067c1e96ff098ba7ae79413cc075973e17c38a587caa", [:mix], [], "hexpm", "678a23aba4d670419c23c17790f9dcd635a4a89022040df7d5d772cb21012490"},
--- a/test/pleroma/config_db_test.exs
+++ b/test/pleroma/config_db_test.exs
@ -238,10 +238,11 @@ defmodule Pleroma.ConfigDBTest do
    end

    test "ssl options" do
-      assert ConfigDB.to_elixir_types([":tlsv1", ":tlsv1.1", ":tlsv1.2"]) == [
+      assert ConfigDB.to_elixir_types([":tlsv1", ":tlsv1.1", ":tlsv1.2", ":tlsv1.3"]) == [
               :tlsv1,
               :"tlsv1.1",
-               :"tlsv1.2"
+               :"tlsv1.2",
+               :"tlsv1.3"
             ]
    end

--- a/test/pleroma/docs/generator_test.exs
+++ b/test/pleroma/docs/generator_test.exs
@ -86,7 +86,7 @@ defmodule Pleroma.Docs.GeneratorTest do
          key: :versions,
          type: {:list, :atom},
          description: "List of TLS version to use",
-          suggestions: [:tlsv1, ":tlsv1.1", ":tlsv1.2"]
+          suggestions: [:tlsv1, ":tlsv1.1", ":tlsv1.2", ":tlsv1.3"]
        }
      ]
    },
@ -213,7 +213,7 @@ defmodule Pleroma.Docs.GeneratorTest do
    test "suggestion for tls versions" do
      [%{children: children} | _] = Generator.convert_to_strings(@descriptions)
      child = Enum.at(children, 8)
-      assert child[:suggestions] == [":tlsv1", ":tlsv1.1", ":tlsv1.2"]
+      assert child[:suggestions] == [":tlsv1", ":tlsv1.1", ":tlsv1.2", ":tlsv1.3"]
    end

    test "subgroup with module name" do
--- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs
+++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs
@ -291,6 +291,30 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
      assert json_response(conn, 200) == ObjectView.render("object.json", %{object: note})
    end

+    test "does not cache authenticated response", %{conn: conn} do
+      user = insert(:user)
+      reader = insert(:user)
+
+      {:ok, post} =
+        CommonAPI.post(user, %{status: "test @#{reader.nickname}", visibility: "local"})
+
+      object = Object.normalize(post, fetch: false)
+      uuid = String.split(object.data["id"], "/") |> List.last()
+
+      assert response =
+               conn
+               |> assign(:user, reader)
+               |> put_req_header("accept", "application/activity+json")
+               |> get("/objects/#{uuid}")
+
+      json_response(response, 200)
+
+      conn
+      |> put_req_header("accept", "application/activity+json")
+      |> get("/objects/#{uuid}")
+      |> json_response(404)
+    end
+
    test "it returns 404 for non-public messages", %{conn: conn} do
      note = insert(:direct_note)
      uuid = String.split(note.data["id"], "/") |> List.last()
--- a/test/pleroma/web/activity_pub/mrf/anti_followbot_policy_test.exs
+++ b/test/pleroma/web/activity_pub/mrf/anti_followbot_policy_test.exs
@ -6,6 +6,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.AntiFollowbotPolicyTest do
  use Pleroma.DataCase, async: true
  import Pleroma.Factory

+  alias Pleroma.User
  alias Pleroma.Web.ActivityPub.MRF.AntiFollowbotPolicy

  describe "blocking based on attributes" do
@ -38,9 +39,25 @@ defmodule Pleroma.Web.ActivityPub.MRF.AntiFollowbotPolicyTest do

      assert {:reject, "[AntiFollowbotPolicy]" <> _} = AntiFollowbotPolicy.filter(message)
    end
+
+    test "matches followbots by actor_type" do
+      actor = insert(:user, %{actor_type: "Service"})
+      target = insert(:user)
+
+      message = %{
+        "@context" => "https://www.w3.org/ns/activitystreams",
+        "type" => "Follow",
+        "actor" => actor.ap_id,
+        "object" => target.ap_id,
+        "id" => "https://example.com/activities/1234"
+      }
+
+      assert {:reject, "[AntiFollowbotPolicy]" <> _} = AntiFollowbotPolicy.filter(message)
+    end
  end

-  test "it allows non-followbots" do
+  describe "it allows" do
+    test "non-followbots" do
      actor = insert(:user)
      target = insert(:user)

@ -55,6 +72,24 @@ defmodule Pleroma.Web.ActivityPub.MRF.AntiFollowbotPolicyTest do
      {:ok, _} = AntiFollowbotPolicy.filter(message)
    end

+    test "bots if the target follows the bots" do
+      actor = insert(:user, %{actor_type: "Service"})
+      target = insert(:user)
+
+      User.follow(target, actor)
+
+      message = %{
+        "@context" => "https://www.w3.org/ns/activitystreams",
+        "type" => "Follow",
+        "actor" => actor.ap_id,
+        "object" => target.ap_id,
+        "id" => "https://example.com/activities/1234"
+      }
+
+      {:ok, _} = AntiFollowbotPolicy.filter(message)
+    end
+  end
+
  test "it gracefully handles nil display names" do
    actor = insert(:user, %{name: nil})
    target = insert(:user)
--- a/test/pleroma/web/activity_pub/mrf/steal_emoji_policy_test.exs
+++ b/test/pleroma/web/activity_pub/mrf/steal_emoji_policy_test.exs
@ -60,7 +60,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicyTest do
           |> File.exists?()
  end

-  test "reject shortcode", %{message: message} do
+  test "reject regex shortcode", %{message: message} do
    refute "firedfox" in installed()

    clear_config(:mrf_steal_emoji,
@ -74,6 +74,20 @@ defmodule Pleroma.Web.ActivityPub.MRF.StealEmojiPolicyTest do
    refute "firedfox" in installed()
  end

+  test "reject string shortcode", %{message: message} do
+    refute "firedfox" in installed()
+
+    clear_config(:mrf_steal_emoji,
+      hosts: ["example.org"],
+      size_limit: 284_468,
+      rejected_shortcodes: ["firedfox"]
+    )
+
+    assert {:ok, _message} = StealEmojiPolicy.filter(message)
+
+    refute "firedfox" in installed()
+  end
+
  test "reject if size is above the limit", %{message: message} do
    refute "firedfox" in installed()

--- a/test/pleroma/web/plugs/cache_test.exs
+++ b/test/pleroma/web/plugs/cache_test.exs
@ -179,4 +179,22 @@ defmodule Pleroma.Web.Plugs.CacheTest do
             |> send_resp(:im_a_teapot, "🥤")
             |> sent_resp()
  end
+
+  test "ignores if skip_cache is assigned" do
+    assert @miss_resp ==
+             conn(:get, "/")
+             |> assign(:skip_cache, true)
+             |> Cache.call(%{query_params: false, ttl: nil})
+             |> put_resp_content_type("cofe/hot")
+             |> send_resp(:ok, "cofe")
+             |> sent_resp()
+
+    assert @miss_resp ==
+             conn(:get, "/")
+             |> assign(:skip_cache, true)
+             |> Cache.call(%{query_params: false, ttl: nil})
+             |> put_resp_content_type("cofe/hot")
+             |> send_resp(:ok, "cofe")
+             |> sent_resp()
+  end
 end