Page MenuHomePhorge
Files F85825372

No OneTemporary
Actions

Authored By
Unknown
Size
13 KB
Referenced Files
None
Subscribers
None

View Options

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

File Metadata

Mime Type
text/x-diff
Expires
Fri, Oct 30, 12:19 AM (1 d, 13 h)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1798977
Default Alt Text
(13 KB)

Event Timeline