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/changelog.d/fetch-id-containment.security b/changelog.d/fetch-id-containment.security new file mode 100644 index 000000000..aee5eca95 --- /dev/null +++ b/changelog.d/fetch-id-containment.security @@ -0,0 +1 @@ +Only accept fetched objects whose id is the URL they were fetched from; for other ids, refetch once from the canonical id on the same host diff --git a/changelog.d/get-actor-crash.fix b/changelog.d/get-actor-crash.fix new file mode 100644 index 000000000..591ac3ba2 --- /dev/null +++ b/changelog.d/get-actor-crash.fix @@ -0,0 +1 @@ +Reject fetched objects whose author has no id instead of crashing diff --git a/changelog.d/refetch-containment.security b/changelog.d/refetch-containment.security new file mode 100644 index 000000000..bfd118ab1 --- /dev/null +++ b/changelog.d/refetch-containment.security @@ -0,0 +1 @@ +Do not let a refetched object (such as a poll) change its id or author diff --git a/changelog.d/user-fetch-display-url.fix b/changelog.d/user-fetch-display-url.fix new file mode 100644 index 000000000..fcde4ecf4 --- /dev/null +++ b/changelog.d/user-fetch-display-url.fix @@ -0,0 +1 @@ +Update the known user instead of failing when an actor is fetched from a display URL diff --git a/lib/pleroma/object/containment.ex b/lib/pleroma/object/containment.ex index c4cc1f014..531100cbb 100644 --- a/lib/pleroma/object/containment.ex +++ b/lib/pleroma/object/containment.ex @@ -44,6 +44,9 @@ defmodule Pleroma.Object.Containment do id end + # remote data can carry any shape here; callers treat nil as "no actor" + def get_actor(_), do: nil + def get_object(%{"object" => id}) when is_binary(id) do id end @@ -108,8 +111,46 @@ defmodule Pleroma.Object.Containment do def contain_origin_from_id(_id, _data), do: :error + @doc """ + Checks that a fetched document's id is the URL it was fetched from + (after redirects). + + Fragments, a trailing slash, the case of the host and differences + that our URL encoding removes before a request are ignored. + Documents without an id never match. + + Based on Akkoma's `contain_id_to_fetch/2`. + """ + def contain_id_to_fetch(url, %{"id" => id}) when is_binary(url) and is_binary(id) do + if normalize_uri(url) == normalize_uri(id), do: :ok, else: :error + end + + def contain_id_to_fetch(_url, _data), do: :error + + defp normalize_uri(uri) do + uri = uri |> Pleroma.Utils.URIEncoding.encode_url() |> URI.parse() + + %URI{ + uri + | fragment: nil, + host: uri.host && String.downcase(uri.host, :ascii), + path: uri.path && String.replace_suffix(uri.path, "/", "") + } + |> URI.to_string() + end + def contain_child(%{"object" => %{"id" => id, "attributedTo" => _} = object}), 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/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 084d44d5e..706ee317c 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -11,6 +11,7 @@ defmodule Pleroma.Object.Fetcher do alias Pleroma.Web.ActivityPub.InternalFetchActor alias Pleroma.Web.ActivityPub.MRF alias Pleroma.Web.ActivityPub.ObjectValidator + alias Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes alias Pleroma.Web.ActivityPub.Pipeline alias Pleroma.Web.ActivityPub.Transmogrifier alias Pleroma.Web.Federator @@ -46,9 +47,14 @@ defmodule Pleroma.Object.Fetcher do end end - def refetch_object(%Object{data: %{"id" => id}} = object) do + # The stored id is canonical, so the refetch must not redirect, and the + # remote must not swap in another object or author. + # Based on Akkoma 9061d148b and fee57eb37. + def refetch_object(%Object{data: %{"id" => id} = data} = object) do with {:local, false} <- {:local, Object.local?(object)}, - {:ok, new_data} <- fetch_and_contain_remote_object_from_id(id), + {:ok, new_data} <- fetch_and_contain(id, true), + {:id, true} <- {:id, new_data["id"] == id}, + {:actor, true} <- {:actor, actor_of(new_data) == actor_of(data)}, {:ok, object} <- reinject_object(object, new_data) do {:ok, object} else @@ -57,6 +63,8 @@ defmodule Pleroma.Object.Fetcher do end end + defp actor_of(data), do: CommonFixes.fix_actor(data)["actor"] + @typep fetcher_errors :: :error | :reject | :allowed_depth | :fetch | :containment | :transmogrifier @@ -140,17 +148,39 @@ defmodule Pleroma.Object.Fetcher do def fetch_and_contain_remote_object_from_id(%{"id" => id}), do: fetch_and_contain_remote_object_from_id(id) - def fetch_and_contain_remote_object_from_id(id) when is_binary(id) do + def fetch_and_contain_remote_object_from_id(id) when is_binary(id), + do: fetch_and_contain(id, false) + + def fetch_and_contain_remote_object_from_id(_id), + do: {:error, "id must be a string"} + + # The fetched document's id must be the URL it came from. Display URLs + # (e.g. Mastodon's /@user/123) serve the object under another id, so + # if the id differs we refetch once from that id, which has to be on the + # same host, must not redirect, and must then match exactly. + # Based on Akkoma 8684964c5 and 940792f8b. + defp fetch_and_contain(id, canonical?) do Logger.debug("Fetching object #{id} via AP") with {:scheme, true} <- {:scheme, String.starts_with?(id, "http")}, {_, true} <- {:mrf, MRF.id_filter(id)}, {_, :ok} <- {:local_fetch, Containment.contain_local_fetch(id)}, - {:ok, body} <- get_object(id), + {:ok, final_url, body} <- get_object(id), + {_, true} <- {:redirect, not canonical? or same_url?(final_url, id)}, {:ok, data} <- safe_json_decode(body), - :ok <- Containment.contain_origin_from_id(id, data) do + :ok <- Containment.contain_origin_from_id(id, data), + {_, _, :ok} <- {:id, data, Containment.contain_id_to_fetch(final_url, data)} do {:ok, data} else + {:id, %{"id" => canonical_id}, :error} when is_binary(canonical_id) and not canonical? -> + fetch_and_contain(canonical_id, true) + + {:id, _, :error} -> + {:error, :id_mismatch} + + {:redirect, false} -> + {:error, :canonical_id_redirect} + {:scheme, _} -> {:error, "Unsupported URI scheme"} @@ -168,8 +198,7 @@ defmodule Pleroma.Object.Fetcher do end end - def fetch_and_contain_remote_object_from_id(_id), - do: {:error, "id must be a string"} + defp same_url?(url, other), do: Containment.contain_id_to_fetch(url, %{"id" => other}) == :ok defp check_crossdomain_redirect(final_host, _original_url) when is_nil(final_host) do {:cross_domain_redirect, false} @@ -195,7 +224,7 @@ defmodule Pleroma.Object.Fetcher do with {:cross_domain_redirect, false} <- check_crossdomain_redirect(remote_host, id), {_, content_type} <- List.keyfind(headers, "content-type", 0), {:ok, _media_type} <- verify_content_type(content_type) do - {:ok, body} + {:ok, if(final_url in [nil, ""], do: id, else: final_url), body} else {:cross_domain_redirect, true} -> {:error, {:cross_domain_redirect, true}} @@ -209,7 +238,7 @@ defmodule Pleroma.Object.Fetcher do case List.keyfind(headers, "content-type", 0) do {_, content_type} -> case verify_content_type(content_type) do - {:ok, _} -> {:ok, body} + {:ok, _} -> {:ok, id, body} error -> error end diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 1279e0cb4..b997e7831 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 @@ -2053,11 +2055,12 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do def enqueue_pin_fetches(_), do: nil def make_user_from_ap_id(ap_id, additional \\ []) do - user = User.get_cached_by_ap_id(ap_id) - with {:ok, data} <- fetch_and_prepare_user_from_ap_id(ap_id, additional) do enqueue_pin_fetches(data) + # the fetched actor's id differs from ap_id when it was fetched from a display URL + user = User.get_cached_by_ap_id(data.ap_id) + if user do user |> User.remote_user_changeset(data) 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..a368ccc16 --- /dev/null +++ b/lib/pleroma/web/activity_pub/object_validators/user_validator.ex @@ -0,0 +1,53 @@ +# 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; null and + empty strings count as missing. + + 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, value) when value in [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/fixtures/tesla_mock/relay@mastdon.example.org.json b/test/fixtures/tesla_mock/relay@mastdon.example.org.json index c1fab7d3b..21dd405c8 100644 --- a/test/fixtures/tesla_mock/relay@mastdon.example.org.json +++ b/test/fixtures/tesla_mock/relay@mastdon.example.org.json @@ -11,7 +11,7 @@ "toot": "http://joinmastodon.org/ns#", "Emoji": "toot:Emoji" }], - "id": "http://mastodon.example.org/users/admin", + "id": "http://mastodon.example.org/users/relay", "type": "Application", "invisible": true, "following": "http://mastodon.example.org/users/admin/following", @@ -24,8 +24,8 @@ "url": "http://mastodon.example.org/@admin", "manuallyApprovesFollowers": false, "publicKey": { - "id": "http://mastodon.example.org/users/admin#main-key", - "owner": "http://mastodon.example.org/users/admin", + "id": "http://mastodon.example.org/users/relay#main-key", + "owner": "http://mastodon.example.org/users/relay", "publicKeyPem": "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAtc4Tir+3ADhSNF6VKrtW\nOU32T01w7V0yshmQei38YyiVwVvFu8XOP6ACchkdxbJ+C9mZud8qWaRJKVbFTMUG\nNX4+6Q+FobyuKrwN7CEwhDALZtaN2IPbaPd6uG1B7QhWorrY+yFa8f2TBM3BxnUy\nI4T+bMIZIEYG7KtljCBoQXuTQmGtuffO0UwJksidg2ffCF5Q+K//JfQagJ3UzrR+\nZXbKMJdAw4bCVJYs4Z5EhHYBwQWiXCyMGTd7BGlmMkY6Av7ZqHKC/owp3/0EWDNz\nNqF09Wcpr3y3e8nA10X40MJqp/wR+1xtxp+YGbq/Cj5hZGBG7etFOmIpVBrDOhry\nBwIDAQAB\n-----END PUBLIC KEY-----\n" }, "attachment": [{ diff --git a/test/pleroma/object/containment_test.exs b/test/pleroma/object/containment_test.exs index 5560dfce5..2fdc83605 100644 --- a/test/pleroma/object/containment_test.exs +++ b/test/pleroma/object/containment_test.exs @@ -127,4 +127,56 @@ defmodule Pleroma.Object.ContainmentTest do :ok = Containment.contain_child(data) end end + + describe "get_actor/1" do + test "returns nil for actor shapes without an id" do + for actor <- [%{"type" => "Person"}, %{"id" => 1}, 1, true] do + assert nil == Containment.get_actor(%{"actor" => actor}) + assert nil == Containment.get_actor(%{"actor" => nil, "attributedTo" => actor}) + end + + assert nil == Containment.get_actor(%{}) + end + + test "contain_origin/2 rejects actor shapes without an id" do + assert :error == + Containment.contain_origin("https://a.test/o/1", %{ + "attributedTo" => %{"type" => "Person"} + }) + end + end + + describe "contain_id_to_fetch/2" do + test "matches the exact id" do + assert :ok = + Containment.contain_id_to_fetch("https://a.test/o/1", %{ + "id" => "https://a.test/o/1" + }) + end + + test "ignores fragments, a trailing slash and the host case" do + assert :ok = + Containment.contain_id_to_fetch("https://A.test/o/1/", %{ + "id" => "https://a.test/o/1#x" + }) + end + + test "rejects another id on the same host" do + assert :error = + Containment.contain_id_to_fetch("https://a.test/o/1", %{ + "id" => "https://a.test/o/2" + }) + end + + test "rejects a different query" do + assert :error = + Containment.contain_id_to_fetch("https://a.test/o?page=1", %{ + "id" => "https://a.test/o?page=2" + }) + end + + test "rejects documents without an id" do + assert :error = Containment.contain_id_to_fetch("https://a.test/o/1", %{}) + end + end end diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index 9afa34fa2..61a6fb517 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -48,6 +48,8 @@ defmodule Pleroma.Object.FetcherTest do %{method: :get, url: "https://social.sakamoto.gq/notice/9wTkLEnuq47B25EehM"} -> %Tesla.Env{ status: 200, + # Pleroma redirects /notice/:id to the object id + url: "https://social.sakamoto.gq/objects/f20f2497-66d9-4a52-a2e1-1be2a39c32c1", body: File.read!("test/fixtures/fetch_mocks/9wTkLEnuq47B25EehM.json"), headers: HttpRequestMock.activitypub_object_headers() } @@ -463,6 +465,78 @@ defmodule Pleroma.Object.FetcherTest do %{object1: local_object1, object2: local_object2} end + test "it rejects a refetched object whose actor changed", %{object1: object1} do + insert(:user, ap_id: "https://mastodon.social/users/someone_else") + {:ok, o} = Object.create(object1) + + mock(fn %{method: :get, url: "https://mastodon.social/1"} -> + %Tesla.Env{ + status: 200, + headers: [{"content-type", "application/activity+json"}], + body: + object1 + |> Map.merge(%{ + "actor" => "https://mastodon.social/users/someone_else", + "attributedTo" => "https://mastodon.social/users/someone_else", + "content" => "hijacked" + }) + |> Jason.encode!() + } + end) + + assert {:error, _} = Fetcher.refetch_object(o) + assert Object.get_by_ap_id("https://mastodon.social/1").data["content"] == "test 1" + end + + test "it rejects a refetched object whose author has no id", %{object1: object1} do + {:ok, o} = Object.create(object1) + + mock(fn %{method: :get, url: "https://mastodon.social/1"} -> + %Tesla.Env{ + status: 200, + headers: [{"content-type", "application/activity+json"}], + body: + object1 + |> Map.delete("actor") + |> Map.put("attributedTo", %{"type" => "Person"}) + |> Jason.encode!() + } + end) + + assert {:error, _} = Fetcher.refetch_object(o) + end + + test "it rejects a refetched object whose id changed", %{object1: object1} do + {:ok, o} = Object.create(object1) + + mock(fn %{method: :get, url: "https://mastodon.social/1"} -> + %Tesla.Env{ + status: 200, + url: "https://mastodon.social/2", + headers: [{"content-type", "application/activity+json"}], + body: object1 |> Map.put("id", "https://mastodon.social/2") |> Jason.encode!() + } + end) + + assert {:error, _} = Fetcher.refetch_object(o) + end + + test "it rejects a refetched object whose id differs only by a trailing slash", %{ + object1: object1 + } do + {:ok, o} = Object.create(object1) + + mock(fn %{method: :get, url: "https://mastodon.social/1"} -> + %Tesla.Env{ + status: 200, + headers: [{"content-type", "application/activity+json"}], + body: object1 |> Map.put("id", "https://mastodon.social/1/") |> Jason.encode!() + } + end) + + assert {:error, _} = Fetcher.refetch_object(o) + end + test "it keeps formerRepresentations if remote does not have this attr", %{object1: object1} do full_object1 = object1 @@ -642,7 +716,23 @@ defmodule Pleroma.Object.FetcherTest do }) } - # No redirect - same domain + # Same-domain redirect to the object id + %{method: :get, url: "https://original.test/@actor/redirect"} -> + %Tesla.Env{ + status: 200, + url: "https://original.test/objects/redirect-target", + headers: [{"content-type", "application/activity+json"}], + body: + Jason.encode!(%{ + "id" => "https://original.test/objects/redirect-target", + "type" => "Note", + "content" => "This has a same-domain redirect", + "actor" => "https://original.test/users/actor", + "attributedTo" => "https://original.test/users/actor" + }) + } + + # Same-domain redirect to a document claiming the original URL as id %{method: :get, url: "https://original.test/objects/same-domain-redirect"} -> %Tesla.Env{ status: 200, @@ -695,12 +785,19 @@ defmodule Pleroma.Object.FetcherTest do test "it accepts objects with same-domain redirects" do assert {:ok, data} = Fetcher.fetch_and_contain_remote_object_from_id( - "https://original.test/objects/same-domain-redirect" + "https://original.test/@actor/redirect" ) assert data["content"] == "This has a same-domain redirect" end + test "it rejects redirect targets that claim the original URL as id" do + assert {:error, _} = + Fetcher.fetch_and_contain_remote_object_from_id( + "https://original.test/objects/same-domain-redirect" + ) + end + test "it handles responses without URL field (common in tests)" do assert {:ok, data} = Fetcher.fetch_and_contain_remote_object_from_id( @@ -711,6 +808,150 @@ defmodule Pleroma.Object.FetcherTest do end end + describe "id containment" do + setup do + note = fn id, content -> + %Tesla.Env{ + status: 200, + headers: [{"content-type", "application/activity+json"}], + body: + Jason.encode!(%{ + "id" => id, + "type" => "Note", + "content" => content, + "actor" => "https://host.test/users/actor", + "attributedTo" => "https://host.test/users/actor" + }) + } + end + + mock(fn + # display URL serving the canonical object + %{method: :get, url: "https://host.test/@actor/1"} -> + note.("https://host.test/objects/1", "display") + + %{method: :get, url: "https://host.test/objects/1"} -> + note.("https://host.test/objects/1", "canonical") + + # uploaded file claiming to be another object on the same host + %{method: :get, url: "https://host.test/media/evil.json"} -> + note.("https://host.test/objects/2", "spoofed") + + %{method: :get, url: "https://host.test/objects/2"} -> + note.("https://host.test/objects/2", "real") + + # canonical id does not serve itself + %{method: :get, url: "https://host.test/media/loop.json"} -> + note.("https://host.test/objects/3", "loop") + + %{method: :get, url: "https://host.test/objects/3"} -> + note.("https://host.test/objects/4", "other") + + # canonical id redirects + %{method: :get, url: "https://host.test/media/redirect.json"} -> + note.("https://host.test/objects/5", "redirect") + + %{method: :get, url: "https://host.test/objects/5"} -> + %{note.("https://host.test/objects/5", "moved") | url: "https://host.test/media/x"} + + %{method: :get, url: "https://host.test/objects/slash"} -> + note.("https://host.test/objects/slash/", "slash") + + %{method: :get, url: "https://host.test/objects/no-id"} -> + note.(nil, "no id") + end) + + :ok + end + + test "it refetches display URLs from the canonical id" do + assert {:ok, %{"id" => "https://host.test/objects/1", "content" => "canonical"}} = + Fetcher.fetch_and_contain_remote_object_from_id("https://host.test/@actor/1") + end + + test "it does not trust a document claiming another id on the same host" do + assert {:ok, %{"id" => "https://host.test/objects/2", "content" => "real"}} = + Fetcher.fetch_and_contain_remote_object_from_id( + "https://host.test/media/evil.json" + ) + end + + test "it refetches at most once" do + assert {:error, :id_mismatch} = + Fetcher.fetch_and_contain_remote_object_from_id( + "https://host.test/media/loop.json" + ) + end + + test "it does not follow redirects when refetching from the canonical id" do + assert {:error, _} = + Fetcher.fetch_and_contain_remote_object_from_id( + "https://host.test/media/redirect.json" + ) + end + + test "it ignores a trailing slash" do + assert {:ok, %{"content" => "slash"}} = + Fetcher.fetch_and_contain_remote_object_from_id("https://host.test/objects/slash") + end + + test "it accepts ids that only differ by URL encoding" do + clear_config(:test_url_encoding, true) + + mock(fn %{method: :get, url: url} -> + # the encoder sends uppercase escapes + assert url in [ + "https://host.test/users/%E3%81%82", + "https://host.test/notes/%E3%81%82" + ] + + id = + if url =~ "users", + do: "https://host.test/users/あ", + else: "https://host.test/notes/%e3%81%82" + + %Tesla.Env{ + status: 200, + url: url, + headers: [{"content-type", "application/activity+json"}], + body: Jason.encode!(%{"id" => id, "type" => "Note"}) + } + end) + + assert {:ok, _} = + Fetcher.fetch_and_contain_remote_object_from_id("https://host.test/users/あ") + + assert {:ok, _} = + Fetcher.fetch_and_contain_remote_object_from_id( + "https://host.test/notes/%e3%81%82" + ) + end + + test "it rejects objects whose author has no id" do + mock(fn %{method: :get, url: "https://host.test/objects/anon" = url} -> + %Tesla.Env{ + status: 200, + headers: [{"content-type", "application/activity+json"}], + body: + Jason.encode!(%{ + "id" => url, + "type" => "Note", + "content" => "anon", + "attributedTo" => %{"type" => "Person", "name" => "nobody"} + }) + } + end) + + assert {:containment, :error} = + Fetcher.fetch_object_from_id("https://host.test/objects/anon") + end + + test "it rejects documents without an id" do + assert {:error, _} = + Fetcher.fetch_and_contain_remote_object_from_id("https://host.test/objects/no-id") + end + end + describe "fetch with history" do setup do object2 = %{ diff --git a/test/pleroma/web/activity_pub/activity_pub_test.exs b/test/pleroma/web/activity_pub/activity_pub_test.exs index ed6db558b..c118cd8f1 100644 --- a/test/pleroma/web/activity_pub/activity_pub_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_test.exs @@ -244,6 +244,58 @@ 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 "updates the known user when fetched from a display URL" do + ap_id = "https://example.com/users/lain" + display_url = "https://example.com/@lain" + user = insert(:user, local: false, ap_id: ap_id, nickname: "lain@example.com") + + user_data = + "test/fixtures/users_mock/user.json" + |> File.read!() + |> String.replace("{{nickname}}", "lain") + |> Jason.decode!() + |> Map.merge(%{"featured" => nil, "name" => "new name"}) + |> Jason.encode!() + + Tesla.Mock.mock(fn %{method: :get, url: url} when url in [ap_id, display_url] -> + %Tesla.Env{ + status: 200, + body: user_data, + headers: [{"content-type", "application/activity+json"}] + } + end) + + assert {:ok, %User{id: id, name: "new name"}} = + ActivityPub.make_user_from_ap_id(display_url) + + assert id == user.id + end + test "fetches user featured collection" do ap_id = "https://example.com/users/lain" @@ -516,7 +568,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do end test "fetches user birthday information from misskey" do - user_id = "https://misskey.io/@mkljczk" + user_id = "https://misskey.io/users/8dhi2ne167" Tesla.Mock.mock(fn %{ @@ -2033,7 +2085,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do json( %{ "@context" => "https://www.w3.org/ns/activitystreams", - "id" => "https://remote.org/users/masto_hidden_counters/followers" + "id" => "https://remote.org/users/masto_hidden_counters/following" }, headers: HttpRequestMock.activitypub_object_headers() ) @@ -2045,7 +2097,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do json( %{ "@context" => "https://www.w3.org/ns/activitystreams", - "id" => "https://remote.org/users/masto_hidden_counters/following" + "id" => "https://remote.org/users/masto_hidden_counters/followers" }, headers: HttpRequestMock.activitypub_object_headers() ) 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..fbc89462e --- /dev/null +++ b/test/pleroma/web/activity_pub/object_validators/user_validator_test.exs @@ -0,0 +1,75 @@ +# 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 + + test "treats empty and null fields as missing" do + data = + actor() + |> Map.merge(%{"inbox" => "", "followers" => nil, "following" => "", "featured" => ""}) + |> put_in(["endpoints", "sharedInbox"], "") + |> put_in(["publicKey", "id"], "") + + assert {:ok, _, _} = UserValidator.validate(data, []) + 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!() diff --git a/test/support/http_request_mock.ex b/test/support/http_request_mock.ex index 82affe7ac..94be59536 100644 --- a/test/support/http_request_mock.ex +++ b/test/support/http_request_mock.ex @@ -577,6 +577,16 @@ defmodule HttpRequestMock do }} end + # the same status via its canonical id + def get("http://mastodon.example.org/users/admin/statuses/99541947525187367", _, _, _) do + {:ok, + %Tesla.Env{ + status: 200, + body: File.read!("test/fixtures/mastodon-note-object.json"), + headers: activitypub_object_headers() + }} + end + def get("http://mastodon.example.org/@admin/99541947525187368", _, _, _) do {:ok, %Tesla.Env{