Fix federation with GoToSocial and inconsistent KeyId in headers

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)

bookwyrm/models/fields.py

@@ -371,15 +371,20 @@ 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,
                 )
             )
         return tags
 
     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)

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

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,15 +31,22 @@ 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"
 
     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"),

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, *_):

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:

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: