diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 3350a4f6bc..479fa72037 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -529,7 +529,7 @@ async def async_broadcast(recipients: List[str], sender, data: str): async def sign_and_send( - session: aiohttp.ClientSession, sender, data: str, destination: str + session: aiohttp.ClientSession, sender, data: str, destination: str, **kwargs ): """Sign the messages and send them in an asynchronous bundle""" now = http_date() @@ -539,11 +539,19 @@ async def sign_and_send( raise ValueError("No private key found for sender") digest = make_digest(data) + signature = make_signature( + "post", + sender, + destination, + now, + digest=digest, + use_legacy_key=kwargs.get("use_legacy_key"), + ) headers = { "Date": now, "Digest": digest, - "Signature": make_signature("post", sender, destination, now, digest), + "Signature": signature, "Content-Type": "application/activity+json; charset=utf-8", "User-Agent": USER_AGENT, } @@ -554,6 +562,14 @@ async def sign_and_send( logger.exception( "Failed to send broadcast to %s: %s", destination, response.reason ) + if kwargs.get("use_legacy_key") is not True: + logger.info("Trying again with legacy keyId header value") + asyncio.ensure_future( + sign_and_send( + session, sender, data, destination, use_legacy_key=True + ) + ) + return response except asyncio.TimeoutError: logger.info("Connection timed out for url: %s", destination) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index df4bb2e4a4..3fe035f580 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -371,7 +371,7 @@ def field_to_activity(self, value): tags.append( activitypub.Link( href=item.remote_id, - name=getattr(item, item.name_field), + name=f"@{getattr(item, item.name_field)}", type=activity_type, ) ) @@ -379,7 +379,12 @@ def field_to_activity(self, value): def field_from_activity(self, value, allow_external_connections=True): if not isinstance(value, list): - return None + # GoToSocial DMs and single-user mentions are + # sent as objects, not as an array of objects + if isinstance(value, dict): + value = [value] + else: + return None items = [] for link_json in value: link = activitypub.Link(**link_json) diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 047d0aba6a..e51f2ba073 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -142,10 +142,17 @@ def ignore_activity( # keep notes if they mention local users if activity.tag == MISSING or activity.tag is None: return True - tags = [l["href"] for l in activity.tag if l["type"] == "Mention"] + # GoToSocial sends single tags as objects + # not wrapped in a list + tags = activity.tag if isinstance(activity.tag, list) else [activity.tag] user_model = apps.get_model("bookwyrm.User", require_ready=True) for tag in tags: - if user_model.objects.filter(remote_id=tag, local=True).exists(): + if ( + tag["type"] == "Mention" + and user_model.objects.filter( + remote_id=tag["href"], local=True + ).exists() + ): # we found a mention of a known use boost return False return True diff --git a/bookwyrm/signatures.py b/bookwyrm/signatures.py index 3102f8da21..08780b7317 100644 --- a/bookwyrm/signatures.py +++ b/bookwyrm/signatures.py @@ -22,7 +22,7 @@ def create_key_pair(): return private_key, public_key -def make_signature(method, sender, destination, date, digest=None): +def make_signature(method, sender, destination, date, **kwargs): """uses a private key to sign an outgoing message""" inbox_parts = urlparse(destination) signature_headers = [ @@ -31,6 +31,7 @@ def make_signature(method, sender, destination, date, digest=None): f"date: {date}", ] headers = "(request-target) host date" + digest = kwargs.get("digest") if digest is not None: signature_headers.append(f"digest: {digest}") headers = "(request-target) host date digest" @@ -38,8 +39,14 @@ def make_signature(method, sender, destination, date, digest=None): message_to_sign = "\n".join(signature_headers) signer = pkcs1_15.new(RSA.import_key(sender.key_pair.private_key)) signed_message = signer.sign(SHA256.new(message_to_sign.encode("utf8"))) + # For legacy reasons we need to use an incorrect keyId for older Bookwyrm versions + key_id = ( + f"{sender.remote_id}#main-key" + if kwargs.get("use_legacy_key") + else f"{sender.remote_id}/#main-key" + ) signature = { - "keyId": f"{sender.remote_id}#main-key", + "keyId": key_id, "algorithm": "rsa-sha256", "headers": headers, "signature": b64encode(signed_message).decode("utf8"), diff --git a/bookwyrm/tests/models/test_fields.py b/bookwyrm/tests/models/test_fields.py index c6e3957533..553a533d57 100644 --- a/bookwyrm/tests/models/test_fields.py +++ b/bookwyrm/tests/models/test_fields.py @@ -404,7 +404,7 @@ def test_tag_field(self, *_): self.assertIsInstance(result, list) self.assertEqual(len(result), 1) self.assertEqual(result[0].href, "https://e.b/c") - self.assertEqual(result[0].name, "Name") + self.assertEqual(result[0].name, "@Name") self.assertEqual(result[0].type, "Serializable") def test_tag_field_from_activity(self, *_): diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index d15d6eecf3..d61c32df5a 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -87,7 +87,7 @@ def send_test_request( # pylint: disable=too-many-arguments data = json.dumps(get_follow_activity(sender, self.rat)) digest = digest or make_digest(data) signature = make_signature( - "post", signer or sender, self.rat.inbox, now, digest + "post", signer or sender, self.rat.inbox, now, digest=digest ) with patch("bookwyrm.views.inbox.activity_task.apply_async"): with patch("bookwyrm.models.user.set_remote_server.delay"): @@ -111,6 +111,7 @@ def test_remote_signer(self): datafile = pathlib.Path(__file__).parent.joinpath("data/ap_user.json") data = json.loads(datafile.read_bytes()) data["id"] = self.fake_remote.remote_id + data["publicKey"]["id"] = f"{self.fake_remote.remote_id}/#main-key" data["publicKey"]["publicKeyPem"] = self.fake_remote.key_pair.public_key del data["icon"] # Avoid having to return an avatar. responses.add(responses.GET, self.fake_remote.remote_id, json=data, status=200) @@ -138,6 +139,7 @@ def test_key_needs_refresh(self): datafile = pathlib.Path(__file__).parent.joinpath("data/ap_user.json") data = json.loads(datafile.read_bytes()) data["id"] = self.fake_remote.remote_id + data["publicKey"]["id"] = f"{self.fake_remote.remote_id}/#main-key" data["publicKey"]["publicKeyPem"] = self.fake_remote.key_pair.public_key del data["icon"] # Avoid having to return an avatar. responses.add(responses.GET, self.fake_remote.remote_id, json=data, status=200) @@ -157,7 +159,7 @@ def test_key_needs_refresh(self): "bookwyrm.models.relationship.UserFollowRequest.accept" ) as accept_mock: response = self.send_test_request(sender=self.fake_remote) - self.assertEqual(response.status_code, 200) + self.assertEqual(response.status_code, 200) # BUG this is 401 self.assertTrue(accept_mock.called) # Old key is cached, so still works: diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index 2fb36507f8..5264eb395e 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -3,7 +3,6 @@ import re import logging -from urllib.parse import urldefrag import requests from django.http import HttpResponse, Http404 @@ -130,15 +129,18 @@ def has_valid_signature(request, activity): """verify incoming signature""" try: signature = Signature.parse(request) - - key_actor = urldefrag(signature.key_id).url - if key_actor != activity.get("actor"): - raise ValueError("Wrong actor created signature.") - - remote_user = activitypub.resolve_remote_id(key_actor, model=models.User) + remote_user = activitypub.resolve_remote_id( + activity.get("actor"), model=models.User + ) if not remote_user: return False + if signature.key_id != remote_user.key_pair.remote_id: + if ( + signature.key_id != f"{remote_user.remote_id}#main-key" + ): # legacy Bookwyrm + raise ValueError("Wrong actor created signature.") + try: signature.verify(remote_user.key_pair.public_key, request) except ValueError: