Merge branch '658-blind-key-rotation' into 'develop'

Fix #658: Support blind key rotation in HTTP Signatures

Closes #658

See merge request funkwhale/funkwhale!530

api/config/settings/common.py

@@ -595,3 +595,5 @@ RSA_KEY_SIZE = 2048
 # for performance gain in tests, since we don't need to actually create the
 # thumbnails
 CREATE_IMAGE_THUMBNAILS = env.bool("CREATE_IMAGE_THUMBNAILS", default=True)
+# we rotate actor keys at most every two days by default
+ACTOR_KEY_ROTATION_DELAY = env.int("ACTOR_KEY_ROTATION_DELAY", default=3600 * 48)

api/funkwhale_api/federation/activity.py

@@ -1,6 +1,8 @@
 import uuid
 import logging
 
+from django.core.cache import cache
+from django.conf import settings
 from django.db import transaction, IntegrityError
 from django.db.models import Q
 
@@ -236,6 +238,21 @@ class InboxRouter(Router):
                 return
 
 
+ACTOR_KEY_ROTATION_LOCK_CACHE_KEY = "federation:actor-key-rotation-lock:{}"
+
+
+def should_rotate_actor_key(actor_id):
+    lock = cache.get(ACTOR_KEY_ROTATION_LOCK_CACHE_KEY.format(actor_id))
+    return lock is None
+
+
+def schedule_key_rotation(actor_id, delay):
+    from . import tasks
+
+    cache.set(ACTOR_KEY_ROTATION_LOCK_CACHE_KEY.format(actor_id), True, timeout=delay)
+    tasks.rotate_actor_key.apply_async(kwargs={"actor_id": actor_id}, countdown=delay)
+
+
 class OutboxRouter(Router):
     @transaction.atomic
     def dispatch(self, routing, context):
@@ -256,6 +273,15 @@ class OutboxRouter(Router):
                 # a route can yield zero, one or more activity payloads
                 if e:
                     activities_data.append(e)
+            deletions = [
+                a["actor"].id
+                for a in activities_data
+                if a["payload"]["type"] == "Delete"
+            ]
+            for actor_id in deletions:
+                # we way need to triggers a blind key rotation
+                if should_rotate_actor_key(actor_id):
+                    schedule_key_rotation(actor_id, settings.ACTOR_KEY_ROTATION_DELAY)
             inbox_items_by_activity_uuid = {}
             deliveries_by_activity_uuid = {}
             prepared_activities = []

api/funkwhale_api/federation/actors.py

@@ -25,17 +25,18 @@ def get_actor_data(actor_url):
         raise ValueError("Invalid actor payload: {}".format(response.text))
 
 
-def get_actor(fid):
-    try:
-        actor = models.Actor.objects.get(fid=fid)
-    except models.Actor.DoesNotExist:
-        actor = None
-    fetch_delta = datetime.timedelta(
-        minutes=preferences.get("federation__actor_fetch_delay")
-    )
-    if actor and actor.last_fetch_date > timezone.now() - fetch_delta:
-        # cache is hot, we can return as is
-        return actor
+def get_actor(fid, skip_cache=False):
+    if not skip_cache:
+        try:
+            actor = models.Actor.objects.get(fid=fid)
+        except models.Actor.DoesNotExist:
+            actor = None
+        fetch_delta = datetime.timedelta(
+            minutes=preferences.get("federation__actor_fetch_delay")
+        )
+        if actor and actor.last_fetch_date > timezone.now() - fetch_delta:
+            # cache is hot, we can return as is
+            return actor
     data = get_actor_data(fid)
     serializer = serializers.ActorSerializer(data=data)
     serializer.is_valid(raise_exception=True)

api/funkwhale_api/federation/authentication.py

@@ -49,7 +49,13 @@ class SignatureAuthentication(authentication.BaseAuthentication):
         try:
             signing.verify_django(request, actor.public_key.encode("utf-8"))
         except cryptography.exceptions.InvalidSignature:
-            raise rest_exceptions.AuthenticationFailed("Invalid signature")
+            # in case of invalid signature, we refetch the actor object
+            # to load a potentially new public key. This process is called
+            # Blind key rotation, and is described at
+            # https://blog.dereferenced.org/the-case-for-blind-key-rotation
+            # if signature verification fails after that, then we return a 403 error
+            actor = actors.get_actor(actor_url, skip_cache=True)
+            signing.verify_django(request, actor.public_key.encode("utf-8"))
 
         return actor
 

api/funkwhale_api/federation/tasks.py

@@ -14,6 +14,7 @@ from funkwhale_api.common import session
 from funkwhale_api.music import models as music_models
 from funkwhale_api.taskapp import celery
 
+from . import keys
 from . import models, signing
 from . import serializers
 from . import routes
@@ -229,3 +230,12 @@ def purge_actors(ids=[], domains=[], only=[]):
     found_ids = list(actors.values_list("id", flat=True))
     logger.info("Starting purging %s accounts", len(found_ids))
     handle_purge_actors(ids=found_ids, only=only)
+
+
+@celery.app.task(name="federation.rotate_actor_key")
+@celery.require_instance(models.Actor.objects.local(), "actor")
+def rotate_actor_key(actor):
+    pair = keys.get_key_pair()
+    actor.private_key = pair[0].decode()
+    actor.public_key = pair[1].decode()
+    actor.save(update_fields=["private_key", "public_key"])

api/tests/federation/test_activity.py

@@ -387,3 +387,47 @@ def test_prepare_deliveries_and_inbox_items(factories):
     ):
         assert inbox_item.actor == expected_inbox_item.actor
         assert inbox_item.type == "to"
+
+
+def test_should_rotate_actor_key(settings, cache, now):
+    actor_id = 42
+    settings.ACTOR_KEY_ROTATION_DELAY = 10
+
+    cache.set(activity.ACTOR_KEY_ROTATION_LOCK_CACHE_KEY.format(actor_id), True)
+
+    assert activity.should_rotate_actor_key(actor_id) is False
+
+    cache.delete(activity.ACTOR_KEY_ROTATION_LOCK_CACHE_KEY.format(actor_id))
+
+    assert activity.should_rotate_actor_key(actor_id) is True
+
+
+def test_schedule_key_rotation(cache, mocker):
+    actor_id = 42
+    rotate_actor_key = mocker.patch.object(tasks.rotate_actor_key, "apply_async")
+
+    activity.schedule_key_rotation(actor_id, 3600)
+    rotate_actor_key.assert_called_once_with(
+        kwargs={"actor_id": actor_id}, countdown=3600
+    )
+    assert cache.get(activity.ACTOR_KEY_ROTATION_LOCK_CACHE_KEY.format(actor_id), True)
+
+
+def test_outbox_dispatch_rotate_key_on_delete(mocker, factories, cache, settings):
+    router = activity.OutboxRouter()
+    actor = factories["federation.Actor"]()
+    r1 = factories["federation.Actor"]()
+    schedule_key_rotation = mocker.spy(activity, "schedule_key_rotation")
+
+    def handler(context):
+        yield {
+            "payload": {"type": "Delete", "actor": actor.fid, "to": [r1]},
+            "actor": actor,
+        }
+
+    router.connect({"type": "Delete"}, handler)
+    router.dispatch({"type": "Delete"}, {})
+
+    schedule_key_rotation.assert_called_once_with(
+        actor.id, settings.ACTOR_KEY_ROTATION_DELAY
+    )

api/tests/federation/test_authentication.py

@@ -126,3 +126,46 @@ def test_authenticate_ignore_inactive_policy(factories, api_request, mocker):
 
     assert actor.public_key == public.decode("utf-8")
     assert actor.fid == actor_url
+
+
+def test_autenthicate_supports_blind_key_rotation(factories, mocker, api_request):
+    actor = factories["federation.Actor"]()
+    actor_url = actor.fid
+    # request is signed with a pair of new keys
+    new_private, new_public = keys.get_key_pair()
+    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": new_public.decode("utf-8"),
+                "owner": actor_url,
+                "id": actor_url + "#main-key",
+            },
+        },
+    )
+    signed_request = factories["federation.SignedRequest"](
+        auth__key=new_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()
+    user, _ = authenticator.authenticate(django_request)
+    actor = django_request.actor
+
+    assert user.is_anonymous is True
+    assert actor.public_key == new_public.decode("utf-8")
+    assert actor.fid == actor_url

api/tests/federation/test_tasks.py

@@ -266,3 +266,20 @@ def test_purge_actors(factories, mocker):
     handle_purge_actors.assert_called_once_with(
         ids=[to_delete.pk, to_delete_domain.pk], only=["hello"]
     )
+
+
+def test_rotate_actor_key(factories, settings, mocker):
+    actor = factories["federation.Actor"](local=True)
+    get_key_pair = mocker.patch(
+        "funkwhale_api.federation.keys.get_key_pair",
+        return_value=(b"private", b"public"),
+    )
+
+    tasks.rotate_actor_key(actor_id=actor.pk)
+
+    actor.refresh_from_db()
+
+    get_key_pair.assert_called_once_with()
+
+    assert actor.public_key == "public"
+    assert actor.private_key == "private"

changes/changelog.d/658.enhancement

@@ -0,0 +1 @@
+Support blind key rotation in HTTP Signatures (#658)