fix(federation): reject actors that point at other hosts

A remote actor could set its inbox or shared inbox to a third-party
host, so our signed deliveries went there. It could also claim
another actor's followers collection, which the visibility checks
trust.

Validate fetched actors and incoming actor Updates: inbox,
endpoints.sharedInbox, followers, following, featured and the
publicKey id must be on the same host as the actor id.

Based on Akkoma f07eb4cb5. The Bridgy fixture gets the key id that
current Bridgy Fed serves.

changelog.d/actor-containment.security

@@ -0,0 +1 @@
+Reject remote actors whose inbox, shared inbox, followers, following or featured collection, or key id is on another host

lib/pleroma/object/containment.ex

@@ -112,4 +112,14 @@ defmodule Pleroma.Object.Containment do
     do: contain_origin(id, object)
 
   def contain_child(_), do: :ok
+
+  @doc "Checks that two URIs are on the same host. URIs without a host never match."
+  def same_origin(id, other_id) when is_binary(id) and is_binary(other_id) do
+    case {URI.parse(id), URI.parse(other_id)} do
+      {%URI{host: host}, %URI{host: host}} when is_binary(host) and host != "" -> :ok
+      _ -> :error
+    end
+  end
+
+  def same_origin(_id, _other_id), do: :error
 end

lib/pleroma/web/activity_pub/activity_pub.ex

@@ -21,6 +21,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
   alias Pleroma.Upload
   alias Pleroma.User
   alias Pleroma.Web.ActivityPub.MRF
+  alias Pleroma.Web.ActivityPub.ObjectValidators.UserValidator
   alias Pleroma.Web.ActivityPub.Transmogrifier
   alias Pleroma.Web.Streamer
   alias Pleroma.Web.WebFinger
@@ -1924,6 +1925,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
 
   def user_data_from_user_object(data, additional \\ []) do
     with {:ok, data} <- MRF.filter(data),
+         {:ok, data, _} <- UserValidator.validate(data, []),
          {:ok, data} <- object_to_user_data(data, additional) do
       {:ok, data}
     else

lib/pleroma/web/activity_pub/object_validator.ex

@@ -11,7 +11,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do
 
   @behaviour Pleroma.Web.ActivityPub.ObjectValidator.Validating
 
-  import Pleroma.Constants, only: [activity_types: 0, object_types: 0]
+  import Pleroma.Constants, only: [activity_types: 0, actor_types: 0, object_types: 0]
 
   alias Pleroma.Activity
   alias Pleroma.EctoType.ActivityPub.ObjectValidators
@@ -37,6 +37,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do
   alias Pleroma.Web.ActivityPub.ObjectValidators.QuestionValidator
   alias Pleroma.Web.ActivityPub.ObjectValidators.UndoValidator
   alias Pleroma.Web.ActivityPub.ObjectValidators.UpdateValidator
+  alias Pleroma.Web.ActivityPub.ObjectValidators.UserValidator
 
   @impl true
   def validate(object, meta)
@@ -226,7 +227,8 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do
   end
 
   def validate(%{"type" => type} = object, meta) when type == "Update" do
-    with {:ok, object} <-
+    with :ok <- validate_updated_actor(object, meta),
+         {:ok, object} <-
            object
            |> UpdateValidator.cast_and_validate(meta)
            |> Ecto.Changeset.apply_action(:insert) do
@@ -247,6 +249,19 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do
 
   def validate(o, m), do: {:error, {:validator_not_set, {o, m}}}
 
+  defp validate_updated_actor(%{"object" => %{"type" => type} = actor}, meta)
+       when type in actor_types() do
+    with {_, false} <- {:local, Access.get(meta, :local, false)},
+         {:ok, _, _} <- UserValidator.validate(actor, meta) do
+      :ok
+    else
+      {:local, true} -> :ok
+      error -> error
+    end
+  end
+
+  defp validate_updated_actor(_, _), do: :ok
+
   def cast_and_apply_and_stringify_with_history(object) do
     do_separate_with_history(object, fn object ->
       with {:ok, object_data} <- cast_and_apply(object),

lib/pleroma/web/activity_pub/object_validators/user_validator.ex

@@ -0,0 +1,52 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Web.ActivityPub.ObjectValidators.UserValidator do
+  @moduledoc """
+  Checks that a remote actor document only points at its own host.
+
+  We deliver to the inbox and shared inbox, use the collections for
+  addressing and visibility checks, and map signature key ids back to
+  the actor. If an actor could point any of these at another host, it
+  could redirect our signed deliveries or claim another actor's
+  collections. Optional fields are only checked when present.
+
+  Based on Akkoma's UserValidator.
+  """
+
+  @behaviour Pleroma.Web.ActivityPub.ObjectValidator.Validating
+
+  alias Pleroma.Object.Containment
+
+  @collections ["inbox", "followers", "following", "featured"]
+
+  @impl true
+  def validate(%{"id" => id} = data, meta) when is_binary(id) do
+    fields =
+      Enum.map(@collections, &{&1, data[&1]}) ++
+        [
+          {"sharedInbox", get_in_map(data, ["endpoints", "sharedInbox"])},
+          {"publicKey", get_in_map(data, ["publicKey", "id"])}
+        ]
+
+    case Enum.find(fields, fn {_, value} -> not same_origin?(id, value) end) do
+      nil -> {:ok, data, meta}
+      {field, _} -> {:error, "#{field} is not on the same host as the actor"}
+    end
+  end
+
+  def validate(_data, _meta), do: {:error, "Actor has no id"}
+
+  defp same_origin?(_id, nil), do: true
+  defp same_origin?(id, %{"id" => value}), do: same_origin?(id, value)
+  defp same_origin?(id, value), do: Containment.same_origin(id, value) == :ok
+
+  defp get_in_map(data, [key | rest]) do
+    case data do
+      %{^key => value} when rest == [] -> value
+      %{^key => value} -> get_in_map(value, rest)
+      _ -> nil
+    end
+  end
+end

test/fixtures/bridgy/actor.json

@@ -70,7 +70,7 @@
   "preferredUsername": "jk.nipponalba.scot",
   "summary": "",
   "publicKey": {
-    "id": "jk.nipponalba.scot",
+    "id": "https://fed.brid.gy/jk.nipponalba.scot#key",
     "publicKeyPem": "-----BEGIN PUBLIC KEY-----\nMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDdarxwzxnNbJ2hneWOYHkYJowk\npyigQtxlUd0VjgSQHwxU9kWqfbrHBVADyTtcqi/4dAzQd3UnCI1TPNnn4LPZY9PW\noiWd3Zl1/EfLFxO7LU9GS7fcSLQkyj5JNhSlN3I8QPudZbybrgRDVZYooDe1D+52\n5KLGqC2ajrIVOiDRTQIDAQAB\n-----END PUBLIC KEY-----"
   },
   "inbox": "https://fed.brid.gy/jk.nipponalba.scot/inbox",

test/pleroma/web/activity_pub/activity_pub_test.exs

@@ -244,6 +244,31 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
              }
     end
 
+    test "rejects actors whose inbox is on another host" do
+      ap_id = "https://example.com/users/evil"
+
+      user_data =
+        "test/fixtures/users_mock/user.json"
+        |> File.read!()
+        |> String.replace("{{nickname}}", "evil")
+        |> Jason.decode!()
+        |> Map.put("inbox", "https://victim.example/inbox")
+        |> Map.put("featured", nil)
+        |> Jason.encode!()
+
+      Tesla.Mock.mock(fn
+        %{method: :get, url: ^ap_id} ->
+          %Tesla.Env{
+            status: 200,
+            body: user_data,
+            headers: [{"content-type", "application/activity+json"}]
+          }
+      end)
+
+      assert {:error, _} = ActivityPub.make_user_from_ap_id(ap_id)
+      refute User.get_by_ap_id(ap_id)
+    end
+
     test "fetches user featured collection" do
       ap_id = "https://example.com/users/lain"
 

test/pleroma/web/activity_pub/object_validators/user_validator_test.exs

@@ -0,0 +1,65 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Web.ActivityPub.ObjectValidators.UserValidatorTest do
+  use ExUnit.Case, async: true
+
+  alias Pleroma.Web.ActivityPub.ObjectValidators.UserValidator
+
+  defp actor do
+    "test/fixtures/users_mock/user.json"
+    |> File.read!()
+    |> String.replace("{{nickname}}", "lain")
+    |> Jason.decode!()
+  end
+
+  test "accepts an actor whose endpoints and collections are on its own host" do
+    data = actor()
+    assert {:ok, ^data, []} = UserValidator.validate(data, [])
+  end
+
+  test "accepts an actor without optional fields" do
+    data = Map.drop(actor(), ["endpoints", "featured", "followers", "following", "publicKey"])
+    assert {:ok, _, _} = UserValidator.validate(data, [])
+  end
+
+  test "accepts all actor fixtures" do
+    for path <- Path.wildcard("test/fixtures/**/*.json"),
+        {:ok, %{"inbox" => _, "id" => id} = data} when is_binary(id) <-
+          [Jason.decode(File.read!(path))] do
+      assert {{:ok, _, _}, ^path} = {UserValidator.validate(data, []), path}
+    end
+  end
+
+  for field <- ["inbox", "followers", "following", "featured"] do
+    test "rejects an actor whose #{field} is on another host" do
+      data = Map.put(actor(), unquote(field), "https://victim.example/users/lain/x")
+      assert {:error, _} = UserValidator.validate(data, [])
+    end
+  end
+
+  test "rejects an actor whose sharedInbox is on another host" do
+    data = put_in(actor(), ["endpoints", "sharedInbox"], "https://victim.example/inbox")
+    assert {:error, _} = UserValidator.validate(data, [])
+  end
+
+  test "rejects an actor whose collection is an object on another host" do
+    data = Map.put(actor(), "followers", %{"id" => "https://victim.example/users/v/followers"})
+    assert {:error, _} = UserValidator.validate(data, [])
+  end
+
+  test "rejects an actor whose key id is on another host" do
+    data = put_in(actor(), ["publicKey", "id"], "https://victim.example/users/lain#main-key")
+    assert {:error, _} = UserValidator.validate(data, [])
+  end
+
+  test "rejects an actor whose key id has no host" do
+    data = put_in(actor(), ["publicKey", "id"], "lain")
+    assert {:error, _} = UserValidator.validate(data, [])
+  end
+
+  test "rejects an actor without an id" do
+    assert {:error, _} = UserValidator.validate(Map.delete(actor(), "id"), [])
+  end
+end

test/pleroma/web/activity_pub/transmogrifier/user_update_handling_test.exs

@@ -11,8 +11,11 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.UserUpdateHandlingTest do
 
   import Pleroma.Factory
 
+  # matches the host of the actor in mastodon-update.json
+  @gargron "http://mastodon.example.org/users/gargron"
+
   test "it works for incoming update activities" do
-    user = insert(:user, local: false)
+    user = insert(:user, local: false, ap_id: @gargron)
 
     update_data = File.read!("test/fixtures/mastodon-update.json") |> Jason.decode!()
 
@@ -50,8 +53,28 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.UserUpdateHandlingTest do
     assert user.bio == "<p>Some bio</p>"
   end
 
+  test "it rejects updates that move the shared inbox to another host" do
+    user = insert(:user, local: false, ap_id: @gargron)
+
+    update_data = File.read!("test/fixtures/mastodon-update.json") |> Jason.decode!()
+
+    object =
+      update_data["object"]
+      |> Map.put("actor", user.ap_id)
+      |> Map.put("id", user.ap_id)
+      |> put_in(["endpoints", "sharedInbox"], "https://victim.example/inbox")
+
+    update_data =
+      update_data
+      |> Map.put("actor", user.ap_id)
+      |> Map.put("object", object)
+
+    assert {:error, _} = Transmogrifier.handle_incoming(update_data)
+    assert User.get_cached_by_ap_id(user.ap_id).shared_inbox == user.shared_inbox
+  end
+
   test "it selects an actor type after an extension type" do
-    user = insert(:user, local: false)
+    user = insert(:user, local: false, ap_id: @gargron)
     update_data = File.read!("test/fixtures/mastodon-update.json") |> Jason.decode!()
 
     object =
@@ -70,7 +93,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.UserUpdateHandlingTest do
   end
 
   test "it works with alsoKnownAs" do
-    %{ap_id: actor} = insert(:user, local: false)
+    %{ap_id: actor} = insert(:user, local: false, ap_id: @gargron)
 
     assert User.get_cached_by_ap_id(actor).also_known_as == []
 
@@ -97,7 +120,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.UserUpdateHandlingTest do
   end
 
   test "it works with custom profile fields" do
-    user = insert(:user, local: false)
+    user = insert(:user, local: false, ap_id: @gargron)
 
     assert user.fields == []
 
@@ -155,7 +178,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.UserUpdateHandlingTest do
   end
 
   test "it works for incoming update activities which lock the account" do
-    user = insert(:user, local: false)
+    user = insert(:user, local: false, ap_id: @gargron)
 
     update_data = File.read!("test/fixtures/mastodon-update.json") |> Jason.decode!()