diff --git a/changelog.d/actor-containment.security b/changelog.d/actor-containment.security new file mode 100644 index 000000000..58dbca4bc --- /dev/null +++ b/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 diff --git a/lib/pleroma/object/containment.ex b/lib/pleroma/object/containment.ex index c4cc1f014..b50d560d1 100644 --- a/lib/pleroma/object/containment.ex +++ b/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 diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 1279e0cb4..18cb2cadc 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/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 diff --git a/lib/pleroma/web/activity_pub/object_validator.ex b/lib/pleroma/web/activity_pub/object_validator.ex index 0254d6dc1..468298a4d 100644 --- a/lib/pleroma/web/activity_pub/object_validator.ex +++ b/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), diff --git a/lib/pleroma/web/activity_pub/object_validators/user_validator.ex b/lib/pleroma/web/activity_pub/object_validators/user_validator.ex new file mode 100644 index 000000000..183be4940 --- /dev/null +++ b/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 +# 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 diff --git a/test/fixtures/bridgy/actor.json b/test/fixtures/bridgy/actor.json index 5b2d8982b..b4e859a82 100644 --- a/test/fixtures/bridgy/actor.json +++ b/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", diff --git a/test/pleroma/web/activity_pub/activity_pub_test.exs b/test/pleroma/web/activity_pub/activity_pub_test.exs index ed6db558b..125a2b4f9 100644 --- a/test/pleroma/web/activity_pub/activity_pub_test.exs +++ b/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" diff --git a/test/pleroma/web/activity_pub/object_validators/user_validator_test.exs b/test/pleroma/web/activity_pub/object_validators/user_validator_test.exs new file mode 100644 index 000000000..91b33b9e2 --- /dev/null +++ b/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 +# 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 diff --git a/test/pleroma/web/activity_pub/transmogrifier/user_update_handling_test.exs b/test/pleroma/web/activity_pub/transmogrifier/user_update_handling_test.exs index cf995e143..b39bc8780 100644 --- a/test/pleroma/web/activity_pub/transmogrifier/user_update_handling_test.exs +++ b/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 == "

Some bio

" 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!()