federation: strip internal fields from incoming updates and history
When note editing support was added, it was omitted to strip internal fields from edited notes and their history. This was uncovered due to Mastodon inlining the like count as a "likes" collection conflicting with our internal "likes" list causing validation failures. In a spot check with likes/like_count it was not possible to inject those internal fields into the local db via Update, but this was not extensively tested for all fields and avenues. Similarly address normalisation did not normalise addressing in the object history, although this was never at risk of being exploitable. The revision history of the Pleroma MR adding edit support reveals recusrive stripping was intentionally avoided, since it will end up removing e.g. emoji from outgoing activities. This appears to still be true. However, all current internal fields ("pleroma_interal" appears to be unused) contain data already publicised otherwise anyway. In the interest of fixing a federation bug (and at worst potential data injection) quickly outgoing stripping is left non-recursive for now. Of course the ultimate fix here is to not mix remote and internal data into the same map in the first place, but unfortunately having a single map of all truth is a core assumption of *oma's AP doc processing. Changing this is a masive undertaking and not suitable for providing a short-term fix.
This commit is contained in:
parent
11ad4711eb
commit
8243fc0ef4
2 changed files with 219 additions and 12 deletions
|
@ -29,6 +29,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
|
|||
|
||||
@doc """
|
||||
Modifies an incoming AP object (mastodon format) to our internal format.
|
||||
(This only deals with non-activity AP objects)
|
||||
"""
|
||||
def fix_object(object, options \\ []) do
|
||||
object
|
||||
|
@ -44,6 +45,38 @@ def fix_object(object, options \\ []) do
|
|||
|> fix_content_map()
|
||||
|> fix_addressing()
|
||||
|> fix_summary()
|
||||
|> fix_history(&fix_object/1)
|
||||
end
|
||||
|
||||
defp maybe_fix_object(%{"attributedTo" => _} = object), do: fix_object(object)
|
||||
defp maybe_fix_object(object), do: object
|
||||
|
||||
defp fix_history(%{"formerRepresentations" => %{"orderedItems" => list}} = obj, fix_fun)
|
||||
when is_list(list) do
|
||||
update_in(obj["formerRepresentations"]["orderedItems"], fn h -> Enum.map(h, fix_fun) end)
|
||||
end
|
||||
|
||||
defp fix_history(obj, _), do: obj
|
||||
|
||||
defp fix_recursive(obj, fun) do
|
||||
# unlike Erlang, Elixir does not support recursive inline functions
|
||||
# which would allow us to avoid reconstructing this on every recursion
|
||||
rec_fun = fn
|
||||
obj when is_map(obj) -> fix_recursive(obj, fun)
|
||||
# there may be simple AP IDs in history (or object field)
|
||||
obj -> obj
|
||||
end
|
||||
|
||||
obj
|
||||
|> fun.()
|
||||
|> fix_history(rec_fun)
|
||||
|> then(fn
|
||||
%{"object" => object} = doc when is_map(object) ->
|
||||
update_in(doc["object"], rec_fun)
|
||||
|
||||
apdoc ->
|
||||
apdoc
|
||||
end)
|
||||
end
|
||||
|
||||
def fix_summary(%{"summary" => nil} = object) do
|
||||
|
@ -414,17 +447,10 @@ defp get_reported(objects) do
|
|||
end
|
||||
|
||||
def handle_incoming(data, options \\ []) do
|
||||
data = normalise_addressing_public(data)
|
||||
|
||||
data =
|
||||
if data["object"] != nil do
|
||||
object = normalise_addressing_public(data["object"])
|
||||
Map.put(data, "object", object)
|
||||
else
|
||||
data
|
||||
end
|
||||
|
||||
handle_incoming_normalised(data, options)
|
||||
data
|
||||
|> fix_recursive(&normalise_addressing_public/1)
|
||||
|> fix_recursive(&strip_internal_fields/1)
|
||||
|> handle_incoming_normalised(options)
|
||||
end
|
||||
|
||||
defp handle_incoming_normalised(data, options)
|
||||
|
@ -490,7 +516,6 @@ defp handle_incoming_normalised(
|
|||
|
||||
object =
|
||||
data["object"]
|
||||
|> strip_internal_fields()
|
||||
|> fix_type(fetch_options)
|
||||
|> fix_in_reply_to(fetch_options)
|
||||
|> fix_quote_url(fetch_options)
|
||||
|
@ -545,6 +570,9 @@ defp handle_incoming_normalised(
|
|||
_options
|
||||
)
|
||||
when type in ~w{Update Block Follow Accept Reject} do
|
||||
fixed_obj = maybe_fix_object(data["object"])
|
||||
data = if fixed_obj != nil, do: %{data | "object" => fixed_obj}, else: data
|
||||
|
||||
with {:ok, %User{}} <- ObjectValidator.fetch_actor(data),
|
||||
{:ok, activity, _} <-
|
||||
Pipeline.common_pipeline(data, local: false) do
|
||||
|
@ -1043,6 +1071,8 @@ def prepare_attachments(object) do
|
|||
Map.put(object, "attachment", attachments)
|
||||
end
|
||||
|
||||
# for outgoing docs immediately stripping internal fields recursively breaks later emoji transformations
|
||||
# (XXX: it would be better to reorder operations so we can always use recursive stripping)
|
||||
def strip_internal_fields(object) do
|
||||
Map.drop(object, Pleroma.Constants.object_internal_fields())
|
||||
end
|
||||
|
|
|
@ -164,6 +164,183 @@ test "it accepts quote posts" do
|
|||
# It fetched the quoted post
|
||||
assert Object.normalize("https://misskey.io/notes/8vs6wxufd0")
|
||||
end
|
||||
|
||||
test "doesn't allow remote edits to fake local likes" do
|
||||
# as a spot check for no internal fields getting injected
|
||||
now = DateTime.utc_now()
|
||||
pub_date = DateTime.to_iso8601(Timex.subtract(now, Timex.Duration.from_minutes(3)))
|
||||
edit_date = DateTime.to_iso8601(now)
|
||||
|
||||
local_user = insert(:user)
|
||||
|
||||
create_data = %{
|
||||
"type" => "Create",
|
||||
"id" => "http://mastodon.example.org/users/admin/statuses/2619539638/activity",
|
||||
"actor" => "http://mastodon.example.org/users/admin",
|
||||
"to" => ["as:Public"],
|
||||
"cc" => [],
|
||||
"object" => %{
|
||||
"type" => "Note",
|
||||
"id" => "http://mastodon.example.org/users/admin/statuses/2619539638",
|
||||
"attributedTo" => "http://mastodon.example.org/users/admin",
|
||||
"to" => ["as:Public"],
|
||||
"cc" => [],
|
||||
"published" => pub_date,
|
||||
"content" => "miaow",
|
||||
"likes" => [local_user.ap_id]
|
||||
}
|
||||
}
|
||||
|
||||
update_data =
|
||||
create_data
|
||||
|> Map.put("type", "Update")
|
||||
|> Map.put("id", create_data["object"]["id"] <> "/update/1")
|
||||
|> put_in(["object", "content"], "miaow :3")
|
||||
|> put_in(["object", "updated"], edit_date)
|
||||
|> put_in(["object", "formerRepresentations"], %{
|
||||
"type" => "OrderedCollection",
|
||||
"totalItems" => 1,
|
||||
"orderedItems" => [create_data["object"]]
|
||||
})
|
||||
|
||||
{:ok, %Pleroma.Activity{} = activity} = Transmogrifier.handle_incoming(create_data)
|
||||
%Pleroma.Object{} = object = Object.get_by_ap_id(activity.data["object"])
|
||||
assert object.data["content"] == "miaow"
|
||||
assert object.data["likes"] == []
|
||||
assert object.data["like_count"] == 0
|
||||
|
||||
{:ok, %Pleroma.Activity{} = activity} = Transmogrifier.handle_incoming(update_data)
|
||||
%Pleroma.Object{} = object = Object.get_by_ap_id(activity.data["object"]["id"])
|
||||
assert object.data["content"] == "miaow :3"
|
||||
assert object.data["likes"] == []
|
||||
assert object.data["like_count"] == 0
|
||||
end
|
||||
|
||||
test "doesn't trip over remote likes in notes" do
|
||||
now = DateTime.utc_now()
|
||||
pub_date = DateTime.to_iso8601(Timex.subtract(now, Timex.Duration.from_minutes(3)))
|
||||
edit_date = DateTime.to_iso8601(now)
|
||||
|
||||
create_data = %{
|
||||
"type" => "Create",
|
||||
"id" => "http://mastodon.example.org/users/admin/statuses/3409297097/activity",
|
||||
"actor" => "http://mastodon.example.org/users/admin",
|
||||
"to" => ["as:Public"],
|
||||
"cc" => [],
|
||||
"object" => %{
|
||||
"type" => "Note",
|
||||
"id" => "http://mastodon.example.org/users/admin/statuses/3409297097",
|
||||
"attributedTo" => "http://mastodon.example.org/users/admin",
|
||||
"to" => ["as:Public"],
|
||||
"cc" => [],
|
||||
"published" => pub_date,
|
||||
"content" => "miaow",
|
||||
"likes" => %{
|
||||
"id" => "http://mastodon.example.org/users/admin/statuses/3409297097/likes",
|
||||
"totalItems" => 0,
|
||||
"type" => "Collection"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
update_data =
|
||||
create_data
|
||||
|> Map.put("type", "Update")
|
||||
|> Map.put("id", create_data["object"]["id"] <> "/update/1")
|
||||
|> put_in(["object", "content"], "miaow :3")
|
||||
|> put_in(["object", "updated"], edit_date)
|
||||
|> put_in(["object", "likes", "totalItems"], 666)
|
||||
|> put_in(["object", "formerRepresentations"], %{
|
||||
"type" => "OrderedCollection",
|
||||
"totalItems" => 1,
|
||||
"orderedItems" => [create_data["object"]]
|
||||
})
|
||||
|
||||
{:ok, %Pleroma.Activity{} = activity} = Transmogrifier.handle_incoming(create_data)
|
||||
%Pleroma.Object{} = object = Object.get_by_ap_id(activity.data["object"])
|
||||
assert object.data["content"] == "miaow"
|
||||
assert object.data["likes"] == []
|
||||
assert object.data["like_count"] == 0
|
||||
|
||||
{:ok, %Pleroma.Activity{} = activity} = Transmogrifier.handle_incoming(update_data)
|
||||
%Pleroma.Object{} = object = Object.get_by_ap_id(activity.data["object"]["id"])
|
||||
assert object.data["content"] == "miaow :3"
|
||||
assert object.data["likes"] == []
|
||||
# in the future this should retain remote likes, but for now:
|
||||
assert object.data["like_count"] == 0
|
||||
end
|
||||
|
||||
test "doesn't trip over remote likes in polls" do
|
||||
now = DateTime.utc_now()
|
||||
pub_date = DateTime.to_iso8601(Timex.subtract(now, Timex.Duration.from_minutes(3)))
|
||||
edit_date = DateTime.to_iso8601(now)
|
||||
|
||||
create_data = %{
|
||||
"type" => "Create",
|
||||
"id" => "http://mastodon.example.org/users/admin/statuses/2471790073/activity",
|
||||
"actor" => "http://mastodon.example.org/users/admin",
|
||||
"to" => ["as:Public"],
|
||||
"cc" => [],
|
||||
"object" => %{
|
||||
"type" => "Question",
|
||||
"id" => "http://mastodon.example.org/users/admin/statuses/2471790073",
|
||||
"attributedTo" => "http://mastodon.example.org/users/admin",
|
||||
"to" => ["as:Public"],
|
||||
"cc" => [],
|
||||
"published" => pub_date,
|
||||
"content" => "vote!",
|
||||
"anyOf" => [
|
||||
%{
|
||||
"type" => "Note",
|
||||
"name" => "a",
|
||||
"replies" => %{
|
||||
"type" => "Collection",
|
||||
"totalItems" => 3
|
||||
}
|
||||
},
|
||||
%{
|
||||
"type" => "Note",
|
||||
"name" => "b",
|
||||
"replies" => %{
|
||||
"type" => "Collection",
|
||||
"totalItems" => 1
|
||||
}
|
||||
}
|
||||
],
|
||||
"likes" => %{
|
||||
"id" => "http://mastodon.example.org/users/admin/statuses/2471790073/likes",
|
||||
"totalItems" => 0,
|
||||
"type" => "Collection"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
update_data =
|
||||
create_data
|
||||
|> Map.put("type", "Update")
|
||||
|> Map.put("id", create_data["object"]["id"] <> "/update/1")
|
||||
|> put_in(["object", "content"], "vote now!")
|
||||
|> put_in(["object", "updated"], edit_date)
|
||||
|> put_in(["object", "likes", "totalItems"], 666)
|
||||
|> put_in(["object", "formerRepresentations"], %{
|
||||
"type" => "OrderedCollection",
|
||||
"totalItems" => 1,
|
||||
"orderedItems" => [create_data["object"]]
|
||||
})
|
||||
|
||||
{:ok, %Pleroma.Activity{} = activity} = Transmogrifier.handle_incoming(create_data)
|
||||
%Pleroma.Object{} = object = Object.get_by_ap_id(activity.data["object"])
|
||||
assert object.data["content"] == "vote!"
|
||||
assert object.data["likes"] == []
|
||||
assert object.data["like_count"] == 0
|
||||
|
||||
{:ok, %Pleroma.Activity{} = activity} = Transmogrifier.handle_incoming(update_data)
|
||||
%Pleroma.Object{} = object = Object.get_by_ap_id(activity.data["object"]["id"])
|
||||
assert object.data["content"] == "vote now!"
|
||||
assert object.data["likes"] == []
|
||||
# in the future this should retain remote likes, but for now:
|
||||
assert object.data["like_count"] == 0
|
||||
end
|
||||
end
|
||||
|
||||
describe "prepare outgoing" do
|
||||
|
|
Loading…
Reference in a new issue