From 1c55f2c9a6426ec69ae5dfe1f70526d1f44dc40e Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Wed, 9 Jan 2019 17:52:14 +0100
Subject: [PATCH] Use our instance policies to discard fetched and inbox
objects
---
api/funkwhale_api/federation/activity.py | 43 ++++++++-
api/funkwhale_api/federation/api_views.py | 8 +-
.../federation/authentication.py | 26 +++++-
api/funkwhale_api/federation/exceptions.py | 8 ++
api/funkwhale_api/federation/fields.py | 4 +-
api/funkwhale_api/federation/serializers.py | 2 +-
api/funkwhale_api/federation/utils.py | 22 ++++-
api/funkwhale_api/manage/serializers.py | 9 +-
api/funkwhale_api/moderation/models.py | 18 +++-
api/tests/federation/test_activity.py | 63 +++++++++++++
api/tests/federation/test_api_views.py | 2 +-
api/tests/federation/test_authentication.py | 90 ++++++++++++++++++-
api/tests/federation/test_serializers.py | 4 +-
api/tests/federation/test_utils.py | 37 ++++++--
api/tests/manage/test_serializers.py | 12 +++
15 files changed, 317 insertions(+), 31 deletions(-)
diff --git a/api/funkwhale_api/federation/activity.py b/api/funkwhale_api/federation/activity.py
index 211b8230..86c79107 100644
--- a/api/funkwhale_api/federation/activity.py
+++ b/api/funkwhale_api/federation/activity.py
@@ -80,6 +80,30 @@ OBJECT_TYPES = (
BROADCAST_TO_USER_ACTIVITIES = ["Follow", "Accept"]
+def should_reject(id, actor_id=None, payload={}):
+ from funkwhale_api.moderation import models as moderation_models
+
+ policies = moderation_models.InstancePolicy.objects.active()
+
+ media_types = ["Audio", "Artist", "Album", "Track", "Library", "Image"]
+ relevant_values = [
+ recursive_gettattr(payload, "type", permissive=True),
+ recursive_gettattr(payload, "object.type", permissive=True),
+ recursive_gettattr(payload, "target.type", permissive=True),
+ ]
+ # if one of the payload types match our internal media types, then
+ # we apply policies that reject media
+ if set(media_types) & set(relevant_values):
+ policy_type = Q(block_all=True) | Q(reject_media=True)
+ else:
+ policy_type = Q(block_all=True)
+
+ query = policies.matching_url_query(id) & policy_type
+ if actor_id:
+ query |= policies.matching_url_query(actor_id) & policy_type
+ return policies.filter(query).exists()
+
+
@transaction.atomic
def receive(activity, on_behalf_of):
from . import models
@@ -92,6 +116,16 @@ def receive(activity, on_behalf_of):
data=activity, context={"actor": on_behalf_of, "local_recipients": True}
)
serializer.is_valid(raise_exception=True)
+ if should_reject(
+ id=serializer.validated_data["id"],
+ actor_id=serializer.validated_data["actor"].fid,
+ payload=activity,
+ ):
+ logger.info(
+ "[federation] Discarding activity due to instance policies %s",
+ serializer.validated_data.get("id"),
+ )
+ return
try:
copy = serializer.save()
except IntegrityError:
@@ -283,7 +317,7 @@ class OutboxRouter(Router):
return activities
-def recursive_gettattr(obj, key):
+def recursive_gettattr(obj, key, permissive=False):
"""
Given a dictionary such as {'user': {'name': 'Bob'}} and
a dotted string such as user.name, returns 'Bob'.
@@ -292,7 +326,12 @@ def recursive_gettattr(obj, key):
"""
v = obj
for k in key.split("."):
- v = v.get(k)
+ try:
+ v = v.get(k)
+ except (TypeError, AttributeError):
+ if not permissive:
+ raise
+ return
if v is None:
return
diff --git a/api/funkwhale_api/federation/api_views.py b/api/funkwhale_api/federation/api_views.py
index 75ffad0b..4c5aaf92 100644
--- a/api/funkwhale_api/federation/api_views.py
+++ b/api/funkwhale_api/federation/api_views.py
@@ -13,6 +13,7 @@ from funkwhale_api.music import models as music_models
from . import activity
from . import api_serializers
+from . import exceptions
from . import filters
from . import models
from . import routes
@@ -128,11 +129,16 @@ class LibraryViewSet(mixins.RetrieveModelMixin, viewsets.GenericViewSet):
except KeyError:
return response.Response({"fid": ["This field is required"]})
try:
- library = utils.retrieve(
+ library = utils.retrieve_ap_object(
fid,
queryset=self.queryset,
serializer_class=serializers.LibrarySerializer,
)
+ except exceptions.BlockedActorOrDomain:
+ return response.Response(
+ {"detail": "This domain/account is blocked on your instance."},
+ status=400,
+ )
except requests.exceptions.RequestException as e:
return response.Response(
{"detail": "Error while fetching the library: {}".format(str(e))},
diff --git a/api/funkwhale_api/federation/authentication.py b/api/funkwhale_api/federation/authentication.py
index f32c78ff..6f0fd329 100644
--- a/api/funkwhale_api/federation/authentication.py
+++ b/api/funkwhale_api/federation/authentication.py
@@ -1,8 +1,14 @@
import cryptography
+import logging
+
from django.contrib.auth.models import AnonymousUser
from rest_framework import authentication, exceptions
-from . import actors, keys, signing, utils
+from funkwhale_api.moderation import models as moderation_models
+from . import actors, exceptions, keys, signing, utils
+
+
+logger = logging.getLogger(__name__)
class SignatureAuthentication(authentication.BaseAuthentication):
@@ -17,8 +23,24 @@ class SignatureAuthentication(authentication.BaseAuthentication):
raise exceptions.AuthenticationFailed(str(e))
try:
- actor = actors.get_actor(key_id.split("#")[0])
+ actor_url = key_id.split("#")[0]
+ except (TypeError, IndexError, AttributeError):
+ raise exceptions.AuthenticationFailed("Invalid key id")
+
+ policies = (
+ moderation_models.InstancePolicy.objects.active()
+ .filter(block_all=True)
+ .matching_url(actor_url)
+ )
+ if policies.exists():
+ raise exceptions.BlockedActorOrDomain()
+
+ try:
+ actor = actors.get_actor(actor_url)
except Exception as e:
+ logger.info(
+ "Discarding HTTP request from blocked actor/domain %s", actor_url
+ )
raise exceptions.AuthenticationFailed(str(e))
if not actor.public_key:
diff --git a/api/funkwhale_api/federation/exceptions.py b/api/funkwhale_api/federation/exceptions.py
index b3fb73ab..303a0c4b 100644
--- a/api/funkwhale_api/federation/exceptions.py
+++ b/api/funkwhale_api/federation/exceptions.py
@@ -1,6 +1,14 @@
+from rest_framework import authentication, exceptions
+from rest_framework import exceptions
+
+
class MalformedPayload(ValueError):
pass
class MissingSignature(KeyError):
pass
+
+
+class BlockedActorOrDomain(exceptions.AuthenticationFailed):
+ pass
diff --git a/api/funkwhale_api/federation/fields.py b/api/funkwhale_api/federation/fields.py
index f23d0907..3523396d 100644
--- a/api/funkwhale_api/federation/fields.py
+++ b/api/funkwhale_api/federation/fields.py
@@ -7,8 +7,8 @@ class ActorRelatedField(serializers.EmailField):
def to_representation(self, value):
return value.full_username
- def to_interal_value(self, value):
- value = super().to_interal_value(value)
+ def to_internal_value(self, value):
+ value = super().to_internal_value(value)
username, domain = value.split("@")
try:
return models.Actor.objects.get(
diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py
index 1cece3b9..d0e07cd8 100644
--- a/api/funkwhale_api/federation/serializers.py
+++ b/api/funkwhale_api/federation/serializers.py
@@ -567,7 +567,7 @@ class LibrarySerializer(PaginatedCollectionSerializer):
return r
def create(self, validated_data):
- actor = utils.retrieve(
+ actor = utils.retrieve_ap_object(
validated_data["actor"],
queryset=models.Actor,
serializer_class=ActorSerializer,
diff --git a/api/funkwhale_api/federation/utils.py b/api/funkwhale_api/federation/utils.py
index d02c8bf6..f29608a2 100644
--- a/api/funkwhale_api/federation/utils.py
+++ b/api/funkwhale_api/federation/utils.py
@@ -1,9 +1,12 @@
import unicodedata
import re
from django.conf import settings
+from django.db.models import Q
from funkwhale_api.common import session
+from funkwhale_api.moderation import models as moderation_models
+from . import exceptions
from . import signing
@@ -58,7 +61,14 @@ def slugify_username(username):
return re.sub(r"[-\s]+", "_", value)
-def retrieve(fid, actor=None, serializer_class=None, queryset=None):
+def retrieve_ap_object(
+ fid, actor=None, serializer_class=None, queryset=None, apply_instance_policies=True
+):
+ from . import activity, serializers
+
+ policies = moderation_models.InstancePolicy.objects.active().filter(block_all=True)
+ if apply_instance_policies and policies.matching_url(fid):
+ raise exceptions.BlockedActorOrDomain()
if queryset:
try:
# queryset can also be a Model class
@@ -83,6 +93,16 @@ def retrieve(fid, actor=None, serializer_class=None, queryset=None):
)
response.raise_for_status()
data = response.json()
+
+ # we match against moderation policies here again, because the FID of the returned
+ # object may not be the same as the URL used to access it
+ try:
+ id = data["id"]
+ except KeyError:
+ pass
+ else:
+ if apply_instance_policies and activity.should_reject(id=id, payload=data):
+ raise exceptions.BlockedActorOrDomain()
if not serializer_class:
return data
serializer = serializer_class(data=data)
diff --git a/api/funkwhale_api/manage/serializers.py b/api/funkwhale_api/manage/serializers.py
index 6795b30d..e8d1a294 100644
--- a/api/funkwhale_api/manage/serializers.py
+++ b/api/funkwhale_api/manage/serializers.py
@@ -316,15 +316,8 @@ class ManageInstancePolicySerializer(serializers.ModelSerializer):
@transaction.atomic
def save(self, *args, **kwargs):
- block_all = self.validated_data.get("block_all", False)
- need_purge = (
- # we purge when we create with block all
- (not self.instance and block_all)
- or
- # or when block all value switch from False to True
- (self.instance and block_all and not self.instance.block_all)
- )
instance = super().save(*args, **kwargs)
+ need_purge = self.instance.is_active and self.instance.block_all
if need_purge:
target = instance.target
diff --git a/api/funkwhale_api/moderation/models.py b/api/funkwhale_api/moderation/models.py
index 59b077ba..c184bbda 100644
--- a/api/funkwhale_api/moderation/models.py
+++ b/api/funkwhale_api/moderation/models.py
@@ -9,10 +9,22 @@ class InstancePolicyQuerySet(models.QuerySet):
def active(self):
return self.filter(is_active=True)
- def matching_url(self, url):
+ def matching_url(self, *urls):
+ if not urls:
+ return self.none()
+ query = None
+ for url in urls:
+ new_query = self.matching_url_query(url)
+ if query:
+ query = query | new_query
+ else:
+ query = new_query
+ return self.filter(query)
+
+ def matching_url_query(self, url):
parsed = urllib.parse.urlparse(url)
- return self.filter(
- models.Q(target_domain_id=parsed.hostname) | models.Q(target_actor__fid=url)
+ return models.Q(target_domain_id=parsed.hostname) | models.Q(
+ target_actor__fid=url
)
diff --git a/api/tests/federation/test_activity.py b/api/tests/federation/test_activity.py
index a65b7b0c..67f60b48 100644
--- a/api/tests/federation/test_activity.py
+++ b/api/tests/federation/test_activity.py
@@ -46,6 +46,69 @@ def test_receive_validates_basic_attributes_and_stores_activity(factories, now,
assert ii.is_read is False
+def test_receive_calls_should_reject(factories, now, mocker):
+ should_reject = mocker.patch.object(activity, "should_reject", return_value=True)
+ local_to_actor = factories["users.User"]().create_actor()
+ remote_actor = factories["federation.Actor"]()
+ a = {
+ "@context": [],
+ "actor": remote_actor.fid,
+ "type": "Noop",
+ "id": "https://test.activity",
+ "to": [local_to_actor.fid, remote_actor.fid],
+ }
+
+ copy = activity.receive(activity=a, on_behalf_of=remote_actor)
+ should_reject.assert_called_once_with(
+ id=a["id"], actor_id=remote_actor.fid, payload=a
+ )
+ assert copy is None
+
+
+@pytest.mark.parametrize(
+ "params, policy_kwargs, expected",
+ [
+ ({"id": "https://ok.test"}, {"target_domain__name": "notok.test"}, False),
+ (
+ {"id": "https://ok.test"},
+ {"target_domain__name": "ok.test", "is_active": False},
+ False,
+ ),
+ (
+ {"id": "https://ok.test"},
+ {"target_domain__name": "ok.test", "block_all": False},
+ False,
+ ),
+ # id match blocked domain
+ ({"id": "http://notok.test"}, {"target_domain__name": "notok.test"}, True),
+ # actor id match blocked domain
+ (
+ {"id": "http://ok.test", "actor_id": "https://notok.test"},
+ {"target_domain__name": "notok.test"},
+ True,
+ ),
+ # reject media
+ (
+ {
+ "payload": {"type": "Library"},
+ "id": "http://ok.test",
+ "actor_id": "http://notok.test",
+ },
+ {
+ "target_domain__name": "notok.test",
+ "block_all": False,
+ "reject_media": True,
+ },
+ True,
+ ),
+ ],
+)
+def test_should_reject(factories, params, policy_kwargs, expected):
+ factories["moderation.InstancePolicy"](for_domain=True, **policy_kwargs)
+
+ assert activity.should_reject(**params) is expected
+
+
def test_get_actors_from_audience_urls(settings, db):
settings.FEDERATION_HOSTNAME = "federation.hostname"
library_uuid1 = uuid.uuid4()
diff --git a/api/tests/federation/test_api_views.py b/api/tests/federation/test_api_views.py
index c2d69518..feb2ea24 100644
--- a/api/tests/federation/test_api_views.py
+++ b/api/tests/federation/test_api_views.py
@@ -23,7 +23,7 @@ def test_user_can_list_their_library_follows(factories, logged_in_api_client):
def test_user_can_fetch_library_using_url(mocker, factories, logged_in_api_client):
library = factories["music.Library"]()
mocked_retrieve = mocker.patch(
- "funkwhale_api.federation.utils.retrieve", return_value=library
+ "funkwhale_api.federation.utils.retrieve_ap_object", return_value=library
)
url = reverse("api:v1:federation:libraries-fetch")
response = logged_in_api_client.post(url, {"fid": library.fid})
diff --git a/api/tests/federation/test_authentication.py b/api/tests/federation/test_authentication.py
index 100971a3..2888588b 100644
--- a/api/tests/federation/test_authentication.py
+++ b/api/tests/federation/test_authentication.py
@@ -1,4 +1,6 @@
-from funkwhale_api.federation import authentication, keys
+import pytest
+
+from funkwhale_api.federation import authentication, exceptions, keys
def test_authenticate(factories, mocker, api_request):
@@ -38,3 +40,89 @@ def test_authenticate(factories, mocker, api_request):
assert user.is_anonymous is True
assert actor.public_key == public.decode("utf-8")
assert actor.fid == actor_url
+
+
+def test_authenticate_skips_blocked_domain(factories, api_request):
+ policy = factories["moderation.InstancePolicy"](block_all=True, for_domain=True)
+ private, public = keys.get_key_pair()
+ actor_url = "https://{}/actor".format(policy.target_domain.name)
+
+ signed_request = factories["federation.SignedRequest"](
+ auth__key=private, auth__key_id=actor_url + "#main-key", auth__headers=["date"]
+ )
+ prepared = signed_request.prepare()
+ django_request = api_request.get(
+ "/",
+ **{
+ "HTTP_DATE": prepared.headers["date"],
+ "HTTP_SIGNATURE": prepared.headers["signature"],
+ }
+ )
+ authenticator = authentication.SignatureAuthentication()
+
+ with pytest.raises(exceptions.BlockedActorOrDomain):
+ authenticator.authenticate(django_request)
+
+
+def test_authenticate_skips_blocked_actor(factories, api_request):
+ policy = factories["moderation.InstancePolicy"](block_all=True, for_actor=True)
+ private, public = keys.get_key_pair()
+ actor_url = policy.target_actor.fid
+
+ signed_request = factories["federation.SignedRequest"](
+ auth__key=private, auth__key_id=actor_url + "#main-key", auth__headers=["date"]
+ )
+ prepared = signed_request.prepare()
+ django_request = api_request.get(
+ "/",
+ **{
+ "HTTP_DATE": prepared.headers["date"],
+ "HTTP_SIGNATURE": prepared.headers["signature"],
+ }
+ )
+ authenticator = authentication.SignatureAuthentication()
+
+ with pytest.raises(exceptions.BlockedActorOrDomain):
+ authenticator.authenticate(django_request)
+
+
+def test_authenticate_ignore_inactive_policy(factories, api_request, mocker):
+ policy = factories["moderation.InstancePolicy"](
+ block_all=True, for_domain=True, is_active=False
+ )
+ private, public = keys.get_key_pair()
+ actor_url = "https://{}/actor".format(policy.target_domain.name)
+
+ signed_request = factories["federation.SignedRequest"](
+ auth__key=private, auth__key_id=actor_url + "#main-key", auth__headers=["date"]
+ )
+ mocker.patch(
+ "funkwhale_api.federation.actors.get_actor_data",
+ return_value={
+ "id": actor_url,
+ "type": "Person",
+ "outbox": "https://test.com",
+ "inbox": "https://test.com",
+ "followers": "https://test.com",
+ "preferredUsername": "test",
+ "publicKey": {
+ "publicKeyPem": public.decode("utf-8"),
+ "owner": actor_url,
+ "id": actor_url + "#main-key",
+ },
+ },
+ )
+ prepared = signed_request.prepare()
+ django_request = api_request.get(
+ "/",
+ **{
+ "HTTP_DATE": prepared.headers["date"],
+ "HTTP_SIGNATURE": prepared.headers["signature"],
+ }
+ )
+ authenticator = authentication.SignatureAuthentication()
+ authenticator.authenticate(django_request)
+ actor = django_request.actor
+
+ assert actor.public_key == public.decode("utf-8")
+ assert actor.fid == actor_url
diff --git a/api/tests/federation/test_serializers.py b/api/tests/federation/test_serializers.py
index fb151b2d..207a8fbe 100644
--- a/api/tests/federation/test_serializers.py
+++ b/api/tests/federation/test_serializers.py
@@ -507,7 +507,7 @@ def test_music_library_serializer_to_ap(factories):
def test_music_library_serializer_from_public(factories, mocker):
actor = factories["federation.Actor"]()
retrieve = mocker.patch(
- "funkwhale_api.federation.utils.retrieve", return_value=actor
+ "funkwhale_api.federation.utils.retrieve_ap_object", return_value=actor
)
data = {
"@context": [
@@ -550,7 +550,7 @@ def test_music_library_serializer_from_public(factories, mocker):
def test_music_library_serializer_from_private(factories, mocker):
actor = factories["federation.Actor"]()
retrieve = mocker.patch(
- "funkwhale_api.federation.utils.retrieve", return_value=actor
+ "funkwhale_api.federation.utils.retrieve_ap_object", return_value=actor
)
data = {
"@context": [
diff --git a/api/tests/federation/test_utils.py b/api/tests/federation/test_utils.py
index e89c5254..761f44d3 100644
--- a/api/tests/federation/test_utils.py
+++ b/api/tests/federation/test_utils.py
@@ -1,7 +1,7 @@
from rest_framework import serializers
import pytest
-from funkwhale_api.federation import utils
+from funkwhale_api.federation import exceptions, utils
@pytest.mark.parametrize(
@@ -53,20 +53,43 @@ def test_extract_headers_from_meta():
assert cleaned_headers == expected
-def test_retrieve(r_mock):
+def test_retrieve_ap_object(db, r_mock):
fid = "https://some.url"
m = r_mock.get(fid, json={"hello": "world"})
- result = utils.retrieve(fid)
+ result = utils.retrieve_ap_object(fid)
assert result == {"hello": "world"}
assert m.request_history[-1].headers["Accept"] == "application/activity+json"
+def test_retrieve_ap_object_honor_instance_policy_domain(factories):
+ domain = factories["moderation.InstancePolicy"](
+ block_all=True, for_domain=True
+ ).target_domain
+ fid = "https://{}/test".format(domain.name)
+
+ with pytest.raises(exceptions.BlockedActorOrDomain):
+ utils.retrieve_ap_object(fid)
+
+
+def test_retrieve_ap_object_honor_instance_policy_different_url_and_id(
+ r_mock, factories
+):
+ domain = factories["moderation.InstancePolicy"](
+ block_all=True, for_domain=True
+ ).target_domain
+ fid = "https://ok/test"
+ m = r_mock.get(fid, json={"id": "http://{}/test".format(domain.name)})
+
+ with pytest.raises(exceptions.BlockedActorOrDomain):
+ utils.retrieve_ap_object(fid)
+
+
def test_retrieve_with_actor(r_mock, factories):
actor = factories["federation.Actor"]()
fid = "https://some.url"
m = r_mock.get(fid, json={"hello": "world"})
- result = utils.retrieve(fid, actor=actor)
+ result = utils.retrieve_ap_object(fid, actor=actor)
assert result == {"hello": "world"}
assert m.request_history[-1].headers["Accept"] == "application/activity+json"
@@ -76,16 +99,16 @@ def test_retrieve_with_actor(r_mock, factories):
def test_retrieve_with_queryset(factories):
actor = factories["federation.Actor"]()
- assert utils.retrieve(actor.fid, queryset=actor.__class__)
+ assert utils.retrieve_ap_object(actor.fid, queryset=actor.__class__)
-def test_retrieve_with_serializer(r_mock):
+def test_retrieve_with_serializer(db, r_mock):
class S(serializers.Serializer):
def create(self, validated_data):
return {"persisted": "object"}
fid = "https://some.url"
r_mock.get(fid, json={"hello": "world"})
- result = utils.retrieve(fid, serializer_class=S)
+ result = utils.retrieve_ap_object(fid, serializer_class=S)
assert result == {"persisted": "object"}
diff --git a/api/tests/manage/test_serializers.py b/api/tests/manage/test_serializers.py
index 36dbe509..1cb3ff90 100644
--- a/api/tests/manage/test_serializers.py
+++ b/api/tests/manage/test_serializers.py
@@ -141,6 +141,18 @@ def test_instance_policy_serializer_save_domain(factories):
assert policy.target_domain == domain
+def test_instance_policy_serializer_save_actor(factories):
+ actor = factories["federation.Actor"]()
+
+ data = {"target": {"id": actor.full_username, "type": "actor"}, "block_all": True}
+
+ serializer = serializers.ManageInstancePolicySerializer(data=data)
+ serializer.is_valid(raise_exception=True)
+ policy = serializer.save()
+
+ assert policy.target_actor == actor
+
+
def test_manage_actor_action_purge(factories, mocker):
actors = factories["federation.Actor"].create_batch(size=3)
s = serializers.ManageActorActionSerializer(queryset=None)
--
GitLab