From 31ad09010e03f30643b9e921c4fd85c113071a4d Mon Sep 17 00:00:00 2001 From: floatingghost Date: Sun, 6 Nov 2022 23:50:32 +0000 Subject: [PATCH] Fix regex usage in MRF (#254) fixes #235 fixes #228 Co-authored-by: FloatingGhost Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/254 --- CHANGELOG.md | 4 ++ lib/pleroma/web/activity_pub/mrf.ex | 11 ++- test/pleroma/user_test.exs | 29 ++++---- .../activity_pub/mrf/simple_policy_test.exs | 68 +++++++++---------- test/pleroma/web/activity_pub/mrf_test.exs | 16 ++--- 5 files changed, 72 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bc87ada1..05d24a821 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Unreleased +## UPGRADE NOTES +- Change your instance blocks to remove any `*.` prefixes. `example.com` will block `*.example.com` by default now + ## Added - Officially supported docker release - Ability to remove followers unilaterally without a block @@ -14,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Changes - Follows no longer override domain blocks, a domain block is final - Deletes are now the lowest priority to publish and will be handled after creates +- Domain blocks are now subdomain-matches by default ## Fixed - Registrations via ldap are now compatible with the latest OTP24 diff --git a/lib/pleroma/web/activity_pub/mrf.ex b/lib/pleroma/web/activity_pub/mrf.ex index 4df226e80..7b7f44646 100644 --- a/lib/pleroma/web/activity_pub/mrf.ex +++ b/lib/pleroma/web/activity_pub/mrf.ex @@ -149,9 +149,18 @@ defp get_policies(policy) when is_atom(policy), do: [policy] defp get_policies(policies) when is_list(policies), do: policies defp get_policies(_), do: [] + # Matches the following: + # - https://baddomain.net + # - https://extra.baddomain.net/ + # Does NOT match the following: + # - https://maybebaddomain.net/ + def subdomain_regex(domain) do + ~r/^(.+\.)?#{Regex.escape(domain)}$/i + end + @spec subdomains_regex([String.t()]) :: [Regex.t()] def subdomains_regex(domains) when is_list(domains) do - for domain <- domains, do: ~r(^#{String.replace(domain, "*.", "(.*\\.)*")}$)i + Enum.map(domains, &subdomain_regex/1) end @spec subdomain_match?([Regex.t()], String.t()) :: boolean() diff --git a/test/pleroma/user_test.exs b/test/pleroma/user_test.exs index f11aec136..195df2a03 100644 --- a/test/pleroma/user_test.exs +++ b/test/pleroma/user_test.exs @@ -444,17 +444,20 @@ test "it sends a welcome message if it is set" do end setup do: - clear_config(:mrf_simple, - media_removal: [], - media_nsfw: [], - federated_timeline_removal: [], - report_removal: [], - reject: [], - followers_only: [], - accept: [], - avatar_removal: [], - banner_removal: [], - reject_deletes: [] + clear_config( + [:mrf_simple], + %{ + media_removal: [], + media_nsfw: [], + federated_timeline_removal: [], + report_removal: [], + reject: [], + followers_only: [], + accept: [], + avatar_removal: [], + banner_removal: [], + reject_deletes: [] + } ) setup do: @@ -1324,7 +1327,7 @@ test "does not block domain with same end if wildcard added" do collateral_user = insert(:user, %{ap_id: "https://another-awful-and-rude-instance.com/user/bully"}) - {:ok, user} = User.block_domain(user, "*.awful-and-rude-instance.com") + {:ok, user} = User.block_domain(user, "awful-and-rude-instance.com") refute User.blocks?(user, collateral_user) end @@ -1342,7 +1345,7 @@ test "blocks domain with wildcard for subdomain" do user_domain = insert(:user, %{ap_id: "https://awful-and-rude-instance.com/user/bully"}) - {:ok, user} = User.block_domain(user, "*.awful-and-rude-instance.com") + {:ok, user} = User.block_domain(user, "awful-and-rude-instance.com") assert User.blocks?(user, user_from_subdomain) assert User.blocks?(user, user_with_two_subdomains) diff --git a/test/pleroma/web/activity_pub/mrf/simple_policy_test.exs b/test/pleroma/web/activity_pub/mrf/simple_policy_test.exs index 0569bfed3..5f80c1629 100644 --- a/test/pleroma/web/activity_pub/mrf/simple_policy_test.exs +++ b/test/pleroma/web/activity_pub/mrf/simple_policy_test.exs @@ -46,8 +46,8 @@ test "has a matching host" do end test "match with wildcard domain" do - clear_config([:mrf_simple, :media_removal], [{"*.remote.instance", "Whatever reason"}]) - media_message = build_media_message() + clear_config([:mrf_simple, :media_removal], [{"remote.instance", "Whatever reason"}]) + media_message = build_media_message("sub.remote.instance") local_message = build_local_message() assert SimplePolicy.filter(media_message) == @@ -81,8 +81,8 @@ test "has a matching host" do end test "match with wildcard domain" do - clear_config([:mrf_simple, :media_nsfw], [{"*.remote.instance", "yeah yeah"}]) - media_message = build_media_message() + clear_config([:mrf_simple, :media_nsfw], [{"remote.instance", "yeah yeah"}]) + media_message = build_media_message("sub.remote.instance") local_message = build_local_message() assert SimplePolicy.filter(media_message) == @@ -92,9 +92,9 @@ test "match with wildcard domain" do end end - defp build_media_message do + defp build_media_message(domain \\ "remote.instance") do %{ - "actor" => "https://remote.instance/users/bob", + "actor" => "https://#{domain}/users/bob", "type" => "Create", "object" => %{ "attachment" => [%{}], @@ -124,8 +124,8 @@ test "has a matching host" do end test "match with wildcard domain" do - clear_config([:mrf_simple, :report_removal], [{"*.remote.instance", "suya"}]) - report_message = build_report_message() + clear_config([:mrf_simple, :report_removal], [{"remote.instance", "suya"}]) + report_message = build_report_message("sub.remote.instance") local_message = build_local_message() assert {:reject, _} = SimplePolicy.filter(report_message) @@ -133,9 +133,9 @@ test "match with wildcard domain" do end end - defp build_report_message do + defp build_report_message(domain \\ "remote.instance") do %{ - "actor" => "https://remote.instance/users/bob", + "actor" => "https://#{domain}/users/bob", "type" => "Flag" } end @@ -143,7 +143,7 @@ defp build_report_message do describe "when :federated_timeline_removal" do test "is empty" do clear_config([:mrf_simple, :federated_timeline_removal], []) - {_, ftl_message} = build_ftl_actor_and_message() + {_, ftl_message} = build_ftl_actor_and_message("https://remote.instance/users/bob") local_message = build_local_message() assert SimplePolicy.filter(ftl_message) == {:ok, ftl_message} @@ -151,7 +151,7 @@ test "is empty" do end test "has a matching host" do - {actor, ftl_message} = build_ftl_actor_and_message() + {actor, ftl_message} = build_ftl_actor_and_message("https://remote.instance/users/bob") ftl_message_actor_host = ftl_message @@ -172,7 +172,7 @@ test "has a matching host" do end test "match with wildcard domain" do - {actor, ftl_message} = build_ftl_actor_and_message() + {actor, ftl_message} = build_ftl_actor_and_message("https://sub.remote.instance/users/bob") ftl_message_actor_host = ftl_message @@ -181,7 +181,7 @@ test "match with wildcard domain" do |> Map.fetch!(:host) clear_config([:mrf_simple, :federated_timeline_removal], [ - {"*." <> ftl_message_actor_host, "owo"} + {ftl_message_actor_host, "owo"} ]) local_message = build_local_message() @@ -196,7 +196,7 @@ test "match with wildcard domain" do end test "has a matching host but only as:Public in to" do - {_actor, ftl_message} = build_ftl_actor_and_message() + {_actor, ftl_message} = build_ftl_actor_and_message("https://remote.instance/users/bob") ftl_message_actor_host = ftl_message @@ -253,8 +253,8 @@ test "obfuscates domains listed in :transparency_obfuscate_domains" do end end - defp build_ftl_actor_and_message do - actor = insert(:user) + defp build_ftl_actor_and_message(ap_id) do + actor = insert(:user, ap_id: ap_id) {actor, %{ @@ -282,9 +282,9 @@ test "activity has a matching host" do end test "activity matches with wildcard domain" do - clear_config([:mrf_simple, :reject], [{"*.remote.instance", ""}]) + clear_config([:mrf_simple, :reject], [{"remote.instance", ""}]) - remote_message = build_remote_message() + remote_message = build_remote_message("sub.remote.instance") assert {:reject, _} = SimplePolicy.filter(remote_message) end @@ -325,7 +325,7 @@ test "reject by URI object" do describe "when :followers_only" do test "is empty" do clear_config([:mrf_simple, :followers_only], []) - {_, ftl_message} = build_ftl_actor_and_message() + {_, ftl_message} = build_ftl_actor_and_message("https://remote.instance/users/alice") local_message = build_local_message() assert SimplePolicy.filter(ftl_message) == {:ok, ftl_message} @@ -412,10 +412,10 @@ test "activity has a matching host" do end test "activity matches with wildcard domain" do - clear_config([:mrf_simple, :accept], [{"*.remote.instance", ""}]) + clear_config([:mrf_simple, :accept], [{"remote.instance", ""}]) local_message = build_local_message() - remote_message = build_remote_message() + remote_message = build_remote_message("sub.remote.instance") assert SimplePolicy.filter(local_message) == {:ok, local_message} assert SimplePolicy.filter(remote_message) == {:ok, remote_message} @@ -457,9 +457,9 @@ test "has a matching host" do end test "match with wildcard domain" do - clear_config([:mrf_simple, :avatar_removal], [{"*.remote.instance", ""}]) + clear_config([:mrf_simple, :avatar_removal], [{"remote.instance", ""}]) - remote_user = build_remote_user() + remote_user = build_remote_user("sub.remote.instance") {:ok, filtered} = SimplePolicy.filter(remote_user) refute filtered["icon"] @@ -493,9 +493,9 @@ test "has a matching host" do end test "match with wildcard domain" do - clear_config([:mrf_simple, :banner_removal], [{"*.remote.instance", ""}]) + clear_config([:mrf_simple, :banner_removal], [{"remote.instance", ""}]) - remote_user = build_remote_user() + remote_user = build_remote_user("sub.remote.instance") {:ok, filtered} = SimplePolicy.filter(remote_user) refute filtered["image"] @@ -553,10 +553,10 @@ test "it rejects the deletion" do end describe "when :reject_deletes match with wildcard domain" do - setup do: clear_config([:mrf_simple, :reject_deletes], [{"*.remote.instance", ""}]) + setup do: clear_config([:mrf_simple, :reject_deletes], [{"remote.instance", ""}]) test "it rejects the deletion" do - deletion_message = build_remote_deletion_message() + deletion_message = build_remote_deletion_message("sub.remote.instance") assert {:reject, _} = SimplePolicy.filter(deletion_message) end @@ -570,13 +570,13 @@ defp build_local_message do } end - defp build_remote_message do - %{"actor" => "https://remote.instance/users/bob"} + defp build_remote_message(domain \\ "remote.instance") do + %{"actor" => "https://#{domain}/users/bob"} end - defp build_remote_user do + defp build_remote_user(domain \\ "remote.instance") do %{ - "id" => "https://remote.instance/users/bob", + "id" => "https://#{domain}/users/bob", "icon" => %{ "url" => "http://example.com/image.jpg", "type" => "Image" @@ -589,10 +589,10 @@ defp build_remote_user do } end - defp build_remote_deletion_message do + defp build_remote_deletion_message(domain \\ "remote.instance") do %{ "type" => "Delete", - "actor" => "https://remote.instance/users/bob" + "actor" => "https://#{domain}/users/bob" } end end diff --git a/test/pleroma/web/activity_pub/mrf_test.exs b/test/pleroma/web/activity_pub/mrf_test.exs index ed3233758..ec4dab30f 100644 --- a/test/pleroma/web/activity_pub/mrf_test.exs +++ b/test/pleroma/web/activity_pub/mrf_test.exs @@ -9,8 +9,8 @@ defmodule Pleroma.Web.ActivityPub.MRFTest do test "subdomains_regex/1" do assert MRF.subdomains_regex(["unsafe.tld", "*.unsafe.tld"]) == [ - ~r/^unsafe.tld$/i, - ~r/^(.*\.)*unsafe.tld$/i + ~r/^(.+\.)?unsafe\.tld$/i, + ~r/^(.+\.)?\*\.unsafe\.tld$/i ] end @@ -18,7 +18,7 @@ test "subdomains_regex/1" do test "common domains" do regexes = MRF.subdomains_regex(["unsafe.tld", "unsafe2.tld"]) - assert regexes == [~r/^unsafe.tld$/i, ~r/^unsafe2.tld$/i] + assert regexes == [~r/^(.+\.)?unsafe\.tld$/i, ~r/^(.+\.)?unsafe2\.tld$/i] assert MRF.subdomain_match?(regexes, "unsafe.tld") assert MRF.subdomain_match?(regexes, "unsafe2.tld") @@ -27,9 +27,9 @@ test "common domains" do end test "wildcard domains with one subdomain" do - regexes = MRF.subdomains_regex(["*.unsafe.tld"]) + regexes = MRF.subdomains_regex(["unsafe.tld"]) - assert regexes == [~r/^(.*\.)*unsafe.tld$/i] + assert regexes == [~r/^(.+\.)?unsafe\.tld$/i] assert MRF.subdomain_match?(regexes, "unsafe.tld") assert MRF.subdomain_match?(regexes, "sub.unsafe.tld") @@ -38,9 +38,9 @@ test "wildcard domains with one subdomain" do end test "wildcard domains with two subdomains" do - regexes = MRF.subdomains_regex(["*.unsafe.tld"]) + regexes = MRF.subdomains_regex(["unsafe.tld"]) - assert regexes == [~r/^(.*\.)*unsafe.tld$/i] + assert regexes == [~r/^(.+\.)?unsafe\.tld$/i] assert MRF.subdomain_match?(regexes, "unsafe.tld") assert MRF.subdomain_match?(regexes, "sub.sub.unsafe.tld") @@ -51,7 +51,7 @@ test "wildcard domains with two subdomains" do test "matches are case-insensitive" do regexes = MRF.subdomains_regex(["UnSafe.TLD", "UnSAFE2.Tld"]) - assert regexes == [~r/^UnSafe.TLD$/i, ~r/^UnSAFE2.Tld$/i] + assert regexes == [~r/^(.+\.)?UnSafe\.TLD$/i, ~r/^(.+\.)?UnSAFE2\.Tld$/i] assert MRF.subdomain_match?(regexes, "UNSAFE.TLD") assert MRF.subdomain_match?(regexes, "UNSAFE2.TLD")