HTTP Signature verification: fetch and use keyId from signature

#315

activitypub.py

@@ -13,6 +13,7 @@ from google.cloud import ndb
 from google.cloud.ndb import OR
 from granary import as1, as2
 from httpsig import HeaderVerifier
+from httpsig.utils import parse_signature_header
 from oauth_dropins.webutil import flask_util, util
 from oauth_dropins.webutil.flask_util import error
 from oauth_dropins.webutil.util import json_dumps, json_loads
@@ -142,26 +143,25 @@ def inbox(domain=None):
         if not user:
             return f'User {domain} not found', 404
 
-    # load actor
-    if actor and isinstance(actor, str):
-        actor = activity['actor'] = \
-            json_loads(common.get_object(actor, user=user).as2)
-
     # optionally verify signature
     # TODO: switch this from erroring to logging lots of detail. need to see
     # which headers, key shapes, etc we get in the wild.
-    if request.headers.get('Signature'):
+    sig = request.headers.get('Signature')
+    if sig:
+        logger.info(f'Headers: {json_dumps(dict(request.headers), indent=2)}')
+        # parse_signature_header lower-cases all keys
+        keyId = parse_signature_header(sig).get('keyid')
         digest = request.headers.get('Digest') or ''
         expected = b64encode(sha256(request.data).digest()).decode()
-        if not digest:
+        if not keyId:
+            logger.warning('HTTP Signature missing keyId')
+        elif not digest:
             logger.warning('Missing Digest header, required for HTTP Signature')
         elif digest.removeprefix('SHA-256=') != expected:
             logger.warning('Invalid Digest header, required for HTTP Signature')
         else:
-            # TODO: check keyId
-            key = actor.get('publicKey', {}).get('publicKeyPem')
-            logger.info(f'publicKey: {json_dumps(actor.get("publicKey"), indent=2)}')
-            logger.info(f'Headers: {json_dumps(dict(request.headers), indent=2)}')
+            key_actor = json_loads(common.get_object(keyId, user=user).as2)
+            key = key_actor.get("publicKey", {}).get('publicKeyPem')
             try:
                 if HeaderVerifier(request.headers, key, method='GET', path=request.path,
                                   required_headers=common.HTTP_SIG_HEADERS,
@@ -216,6 +216,11 @@ def inbox(domain=None):
         ndb.put_multi(followers)
         return 'OK'
 
+    # fetch actor if necessary so we have name, profile photo, etc
+    if actor and isinstance(actor, str):
+        actor = activity['actor'] = \
+            json_loads(common.get_object(actor, user=user).as2)
+
     # fetch object if necessary so we can render it in feeds
     if type in FETCH_OBJECT_TYPES and isinstance(activity.get('object'), str):
         obj_as2 = activity['object'] = \

tests/test_activitypub.py

@@ -4,6 +4,7 @@ from base64 import b64encode
 import copy
 from datetime import datetime, timedelta
 from hashlib import sha256
+import logging
 from unittest.mock import ANY, call, patch
 
 from google.cloud import ndb
@@ -700,15 +701,15 @@ class ActivityPubTest(testutil.TestCase):
 
         self.assertIsNone(Object.get_by_id(bad_url))
 
-    @patch('activitypub.logger.warning')
-    @patch('activitypub.logger.info')
+    @patch('activitypub.logger.warning', side_effect=logging.warning)
+    @patch('activitypub.logger.info', side_effect=logging.info)
     def test_inbox_verify_http_signature(self, mock_info, mock_warning, _, mock_get, ___):
         # actor with a public key
         mock_get.return_value = self.as2_resp({
             **ACTOR,
             'publicKey': {
-                'id': 'my-key-id',
-                'owner': 'http://sen/der',
+                'id': 'http://my/key/id#unused',
+                'owner': 'http://own/er',
                 'publicKeyPem': self.user.public_pem().decode(),
             },
         })
@@ -722,7 +723,7 @@ class ActivityPubTest(testutil.TestCase):
             'Content-Type': as2.CONTENT_TYPE,
             'Digest': f'SHA-256={digest}',
         }
-        hs = HeaderSigner('my-key-id', self.user.private_pem().decode(),
+        hs = HeaderSigner('http://my/key/id#unused', self.user.private_pem().decode(),
                           algorithm='rsa-sha256', sign_header='signature',
                           headers=('Date', 'Host', 'Digest', '(request-target)'))
         headers = hs.sign(headers, method='GET', path='/inbox')
@@ -730,8 +731,24 @@ class ActivityPubTest(testutil.TestCase):
         # valid signature
         resp = self.client.post('/inbox', data=body, headers=headers)
         self.assertEqual(200, resp.status_code, resp.get_data(as_text=True))
+        mock_get.assert_has_calls((
+            self.as2_req('http://my/key/id'),
+        ))
         mock_info.assert_any_call('HTTP Signature verified!')
 
+        # invalid signature, missing keyId
+        activitypub.seen_ids.clear()
+        obj_key = ndb.Key(Object, NOTE['id'])
+        obj_key.delete()
+
+        resp = self.client.post('/inbox', data=body, headers={
+            **headers,
+            'signature': headers['signature'].replace(
+                'keyId="http://my/key/id#unused",', ''),
+        })
+        self.assertEqual(200, resp.status_code, resp.get_data(as_text=True))
+        mock_warning.assert_any_call('HTTP Signature missing keyId')
+
         # invalid signature, content changed
         activitypub.seen_ids.clear()
         obj_key = ndb.Key(Object, NOTE['id'])