Allow cross-domain redirects on AP requests
Since we now remember the final location redirects lead to
and use it for all further checks since
3e134b07fa, these redirects
can no longer be exploited to serve counterfeit objects.
This fixes:
- display URLs from independent webapp clients
redirecting to the canonical domain
- Peertube display URLs for remote content
(acting like the above)
This commit is contained in:
parent
940792f8ba
commit
d5b0720596
2 changed files with 29 additions and 21 deletions
|
|
@ -317,7 +317,7 @@ def fetch_and_contain_remote_object_from_id(id, is_ap_id) when is_binary(id) do
|
|||
|
||||
{:containment, reason} ->
|
||||
log_fetch_error(id, reason)
|
||||
{:error, reason}
|
||||
{:error, {:containment, reason}}
|
||||
|
||||
{:error, e} ->
|
||||
{:error, e}
|
||||
|
|
@ -330,22 +330,10 @@ def fetch_and_contain_remote_object_from_id(id, is_ap_id) when is_binary(id) do
|
|||
def fetch_and_contain_remote_object_from_id(_id, _is_ap_id),
|
||||
do: {:error, :invalid_id}
|
||||
|
||||
defp check_crossdomain_redirect(final_host, original_url)
|
||||
|
||||
# HOPEFULLY TEMPORARY
|
||||
# Basically none of our Tesla mocks in tests set the (supposed to
|
||||
# exist for Tesla proper) url parameter for their responses
|
||||
# causing almost every fetch in test to fail otherwise
|
||||
if @mix_env == :test do
|
||||
defp check_crossdomain_redirect(nil, _) do
|
||||
{:cross_domain_redirect, false}
|
||||
end
|
||||
end
|
||||
|
||||
defp check_crossdomain_redirect(final_host, original_url) do
|
||||
{:cross_domain_redirect, final_host != URI.parse(original_url).host}
|
||||
end
|
||||
|
||||
if @mix_env == :test do
|
||||
defp get_final_id(nil, initial_url), do: initial_url
|
||||
defp get_final_id("", initial_url), do: initial_url
|
||||
|
|
@ -371,10 +359,6 @@ def get_object(id) do
|
|||
with {:ok, %{body: body, status: code, headers: headers, url: final_url}}
|
||||
when code in 200..299 <-
|
||||
HTTP.Backoff.get(id, headers),
|
||||
remote_host <-
|
||||
URI.parse(final_url).host,
|
||||
{:cross_domain_redirect, false} <-
|
||||
check_crossdomain_redirect(remote_host, id),
|
||||
{:has_content_type, {_, content_type}} <-
|
||||
{:has_content_type, List.keyfind(headers, "content-type", 0)},
|
||||
{:parse_content_type, {:ok, "application", subtype, type_params}} <-
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ defp spoofed_object_with_ids(
|
|||
|> Jason.decode!()
|
||||
|> Map.put("id", id)
|
||||
|> Map.put("actor", actor_id)
|
||||
|> Map.put("attributedTo", actor_id)
|
||||
|> Jason.encode!()
|
||||
end
|
||||
|
||||
|
|
@ -109,7 +110,7 @@ defp spoofed_object_with_ids(
|
|||
body: spoofed_object_with_ids("https://patch.cx/objects/spoof_media_redirect1")
|
||||
}
|
||||
|
||||
# Spoof: cross-domain redirect with final domain id
|
||||
# Spoof: cross-domain redirect with final domain id, but original id actor
|
||||
%{method: :get, url: "https://patch.cx/objects/spoof_media_redirect2"} ->
|
||||
%Tesla.Env{
|
||||
status: 200,
|
||||
|
|
@ -118,6 +119,19 @@ defp spoofed_object_with_ids(
|
|||
body: spoofed_object_with_ids("https://media.patch.cx/objects/spoof_media_redirect2")
|
||||
}
|
||||
|
||||
# No-Spoof: cross-domain redirect with id and actor from final domain
|
||||
%{method: :get, url: "https://patch.cx/objects/spoof_media_redirect3"} ->
|
||||
%Tesla.Env{
|
||||
status: 200,
|
||||
url: "https://media.patch.cx/objects/spoof_media_redirect3",
|
||||
headers: [{"content-type", "application/activity+json"}],
|
||||
body:
|
||||
spoofed_object_with_ids(
|
||||
"https://media.patch.cx/objects/spoof_media_redirect3",
|
||||
"https://media.patch.cx/users/rin"
|
||||
)
|
||||
}
|
||||
|
||||
# No-Spoof: same domain redirect
|
||||
%{method: :get, url: "https://patch.cx/objects/spoof_redirect"} ->
|
||||
%Tesla.Env{
|
||||
|
|
@ -264,19 +278,29 @@ test "it does not fetch a spoofed object with id different from URL" do
|
|||
end
|
||||
|
||||
test "it does not fetch an object via cross-domain redirects (initial id)" do
|
||||
assert {:error, {:cross_domain_redirect, true}} =
|
||||
assert {:error, {:containment, _}} =
|
||||
Fetcher.fetch_and_contain_remote_object_from_id(
|
||||
"https://patch.cx/objects/spoof_media_redirect1"
|
||||
)
|
||||
end
|
||||
|
||||
test "it does not fetch an object via cross-domain redirects (final id)" do
|
||||
assert {:error, {:cross_domain_redirect, true}} =
|
||||
test "it does not fetch an object via cross-domain redirect if the actor is from the original domain" do
|
||||
assert {:error, {:containment, :error}} =
|
||||
Fetcher.fetch_and_contain_remote_object_from_id(
|
||||
"https://patch.cx/objects/spoof_media_redirect2"
|
||||
)
|
||||
end
|
||||
|
||||
test "it allows cross-domain redirects when id and author are from final domain" do
|
||||
assert {:ok, %{"id" => id, "attributedTo" => author}} =
|
||||
Fetcher.fetch_and_contain_remote_object_from_id(
|
||||
"https://patch.cx/objects/spoof_media_redirect3"
|
||||
)
|
||||
|
||||
assert URI.parse(id).host == "media.patch.cx"
|
||||
assert URI.parse(author).host == "media.patch.cx"
|
||||
end
|
||||
|
||||
test "it accepts same-domain redirects" do
|
||||
assert {:ok, %{"id" => id} = _object} =
|
||||
Fetcher.fetch_and_contain_remote_object_from_id(
|
||||
|
|
|
|||
Loading…
Reference in a new issue