From deb4b126592c97896165329bdb78553ad2fc354c Mon Sep 17 00:00:00 2001 From: Ryan Barrett Date: Thu, 9 Mar 2023 19:56:04 -0800 Subject: [PATCH] AS2: short circuit out on Delete actor that we don't have stored ...since when we try to fetch the actor to get their key to verify the signature, we get an HTTP 410 response, at least from Mastodon. --- activitypub.py | 23 ++++++++++++++++------- protocol.py | 1 - tests/test_activitypub.py | 23 +++++++++++------------ 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/activitypub.py b/activitypub.py index ba9b10c..0474aa7 100644 --- a/activitypub.py +++ b/activitypub.py @@ -5,13 +5,13 @@ from hashlib import sha256 import itertools import logging -from flask import request +from flask import abort, request from granary import as1, as2 from httpsig import HeaderVerifier from httpsig.requests_auth import HTTPSignatureAuth from httpsig.utils import parse_signature_header from oauth_dropins.webutil import flask_util, util -from oauth_dropins.webutil.util import json_dumps, json_loads +from oauth_dropins.webutil.util import fragmentless, json_dumps, json_loads import requests from werkzeug.exceptions import BadGateway @@ -138,13 +138,14 @@ class ActivityPub(Protocol): _error(resp) @classmethod - def verify_signature(cls, user): + def verify_signature(cls, activity, *, user=None): """Verifies the current request's HTTP Signature. Args: - user: :class:`User` + activity: dict, AS2 activity + user: optional :class:`User` - Logs details of the result. Raises :class:`werkzeug.HTTPSignature` if the + Logs details of the result. Raises :class:`werkzeug.HTTPError` if the signature is missing or invalid, otherwise does nothing and returns None. """ sig = request.headers.get('Signature') @@ -166,7 +167,15 @@ class ActivityPub(Protocol): if digest.removeprefix('SHA-256=') != expected: error('Invalid Digest header, required for HTTP Signature', status=401) - key_actor = cls.get_object(keyId, user=user).as2 + try: + key_actor = cls.get_object(keyId, user=user).as2 + except BadGateway: + if (activity.get('type') == 'Delete' and + fragmentless(keyId) == fragmentless(activity.get('object'))): + logging.info("Object/actor being deleted is also keyId; ignoring") + abort(202, 'OK') + raise + key = key_actor.get("publicKey", {}).get('publicKeyPem') logger.info(f'Verifying signature for {request.path} with key {key}') try: @@ -546,7 +555,7 @@ def inbox(domain=None): if not user: error(f'User {domain} not found', status=404) - ActivityPub.verify_signature(user) + ActivityPub.verify_signature(activity, user=user) # check that this activity is public. only do this for creates, not likes, # follows, or other activity types, since Mastodon doesn't currently mark diff --git a/protocol.py b/protocol.py index 6fb4286..0eef126 100644 --- a/protocol.py +++ b/protocol.py @@ -96,7 +96,6 @@ class Protocol: Raises: :class:`werkzeug.HTTPException` if the request is invalid - """ if not id: error('Activity has no id') diff --git a/tests/test_activitypub.py b/tests/test_activitypub.py index d963c4f..2b58d3d 100644 --- a/tests/test_activitypub.py +++ b/tests/test_activitypub.py @@ -331,8 +331,7 @@ class ActivityPubTest(testutil.TestCase): def _test_inbox_reply(self, reply, expected_props, mock_head, mock_get, mock_post): mock_head.return_value = requests_response(url='http://or.ig/post') - mock_get.return_value = requests_response( - '') + mock_get.return_value = WEBMENTION_DISCOVERY mock_post.return_value = requests_response() got = self.post('/foo.com/inbox', json=reply) @@ -413,11 +412,7 @@ class ActivityPubTest(testutil.TestCase): def test_repost_of_federated_post(self, mock_head, mock_get, mock_post): mock_head.return_value = requests_response(url='https://foo.com/orig') - mock_get.side_effect = [ - # webmention discovery - requests_response( - ''), - ] + mock_get.return_value = WEBMENTION_DISCOVERY mock_post.return_value = requests_response() # webmention orig_url = 'https://foo.com/orig' @@ -886,23 +881,27 @@ class ActivityPubTest(testutil.TestCase): self.assertEqual({'error': 'No HTTP Signature'}, resp.json) mock_common_log.assert_any_call('Returning 401: No HTTP Signature') - def test_delete_actor(self, _, mock_get, ___): + def test_delete_actor(self, *mocks): follower = Follower.get_or_create('foo.com', DELETE['actor']) followee = Follower.get_or_create(DELETE['actor'], 'snarfed.org') # other unrelated follower other = Follower.get_or_create('foo.com', 'https://mas.to/users/other') self.assertEqual(3, Follower.query().count()) - mock_get.side_effect = [ - self.as2_resp(ACTOR), - ] - got = self.post('/inbox', json=DELETE) self.assertEqual(200, got.status_code) self.assertEqual('inactive', follower.key.get().status) self.assertEqual('inactive', followee.key.get().status) self.assertEqual('active', other.key.get().status) + def test_delete_actor_not_stored(self, _, mock_get, ___): + self.key_id_obj.delete() + Protocol.get_object.cache.clear() + + mock_get.return_value = requests_response(status=410) + got = self.post('/inbox', json={**DELETE, 'object': 'http://my/key/id'}) + self.assertEqual(202, got.status_code) + def test_delete_note(self, _, mock_get, ___): obj = Object(id='http://an/obj', as2={}) obj.put()