diff --git a/api/config/settings/common.py b/api/config/settings/common.py index 12cb102dd812fec0a5263a96f5f1aa83281d2b3b..45480c9ea356f861c972a2867340099ffce60e91 100644 --- a/api/config/settings/common.py +++ b/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) diff --git a/api/funkwhale_api/federation/activity.py b/api/funkwhale_api/federation/activity.py index 86c79107a3f562af0bdd77881861fc9c63177acf..b9f8ffd69292fe1868417e058ad81d3e094327c4 100644 --- a/api/funkwhale_api/federation/activity.py +++ b/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 = [] diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index f1b94809d779dcbd5f7963b591f8808f2aab9806..c7a0c7c6b61c0bdbe83253286b5cb156b0370ba0 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/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) diff --git a/api/funkwhale_api/federation/authentication.py b/api/funkwhale_api/federation/authentication.py index adfb8a18161dc91f11b0957530054ed85c33b460..dd7a142dfe483aa9ee51b1d50de72c4c52a3ce4c 100644 --- a/api/funkwhale_api/federation/authentication.py +++ b/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 diff --git a/api/funkwhale_api/federation/tasks.py b/api/funkwhale_api/federation/tasks.py index d7c48957ab6a3b47e46d688ab8c9cd7495ffa724..f7d8913b7634f7b1529f65300aa8aea97a59bb81 100644 --- a/api/funkwhale_api/federation/tasks.py +++ b/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"]) diff --git a/api/tests/federation/test_activity.py b/api/tests/federation/test_activity.py index 67f60b4861a2af90be332ee41707cbf28580b1e0..fa83ed1f4d7048de95638a9bf8d9a87006d86adf 100644 --- a/api/tests/federation/test_activity.py +++ b/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 + ) diff --git a/api/tests/federation/test_authentication.py b/api/tests/federation/test_authentication.py index 2888588b88bc7ae4b60ea058d49827cc7c243b3a..7af7089f66c19bcadbb77a98e3c4eac253cf4382 100644 --- a/api/tests/federation/test_authentication.py +++ b/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 diff --git a/api/tests/federation/test_tasks.py b/api/tests/federation/test_tasks.py index 5e10dfa509189781b6e3e588579a46986c2b63bc..f3216eed77f235322ef61cb716f2c02b5a7e9bf1 100644 --- a/api/tests/federation/test_tasks.py +++ b/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" diff --git a/changes/changelog.d/658.enhancement b/changes/changelog.d/658.enhancement new file mode 100644 index 0000000000000000000000000000000000000000..4b31d84039b51cada273917fe2c8ac69f4aeae0f --- /dev/null +++ b/changes/changelog.d/658.enhancement @@ -0,0 +1 @@ +Support blind key rotation in HTTP Signatures (#658)