From 791e55c7f5eee26792b90b94668d5f0dbbc767f9 Mon Sep 17 00:00:00 2001 From: Thomas Sileo Date: Sat, 2 Jun 2018 09:07:57 +0200 Subject: [PATCH] Add more security check/verification --- activitypub.py | 92 ++++++++++++++++++++++++++++++++++++++++++++------ app.py | 11 ++++-- 2 files changed, 90 insertions(+), 13 deletions(-) diff --git a/activitypub.py b/activitypub.py index 15a0454..316d620 100644 --- a/activitypub.py +++ b/activitypub.py @@ -10,7 +10,9 @@ from html2text import html2text from feedgen.feed import FeedGenerator from utils.actor_service import NotAnActorError -from utils.errors import BadActivityError, UnexpectedActivityTypeError +from utils.errors import BadActivityError +from utils.errors import UnexpectedActivityTypeError +from utils.errors import NotFromOutboxError from utils import activitypub_utils from config import USERNAME, BASE_URL, ID from config import CTX_AS, CTX_SECURITY, AS_PUBLIC @@ -94,18 +96,25 @@ class BaseActivity(object): ACTIVITY_TYPE: Optional[ActivityType] = None ALLOWED_OBJECT_TYPES: List[ActivityType] = [] + OBJECT_REQUIRED = False def __init__(self, **kwargs) -> None: # Ensure the class has an activity type defined if not self.ACTIVITY_TYPE: raise BadActivityError('Missing ACTIVITY_TYPE') + # XXX(tsileo): what to do about this check? # Ensure the activity has a type and a valid one - if kwargs.get('type') is not None and kwargs.pop('type') != self.ACTIVITY_TYPE.value: - raise UnexpectedActivityTypeError('Expect the type to be {}'.format(self.ACTIVITY_TYPE)) + # if kwargs.get('type') is None: + # raise BadActivityError('missing activity type') + + if kwargs.get('type') and kwargs.pop('type') != self.ACTIVITY_TYPE.value: + raise UnexpectedActivityTypeError(f'Expect the type to be {self.ACTIVITY_TYPE.value!r}') # Initialize the object - self._data: Dict[str, Any] = {'type': self.ACTIVITY_TYPE.value} + self._data: Dict[str, Any] = { + 'type': self.ACTIVITY_TYPE.value + } logger.debug(f'initializing a {self.ACTIVITY_TYPE.value} activity: {kwargs}') if 'id' in kwargs: @@ -118,6 +127,7 @@ class BaseActivity(object): actor = self._validate_person(actor) self._data['actor'] = actor else: + # FIXME(tsileo): uses a special method to set the actor as "the instance" if not self.NO_CONTEXT and self.ACTIVITY_TYPE != ActivityType.TOMBSTONE: actor = ID self._data['actor'] = actor @@ -166,7 +176,7 @@ class BaseActivity(object): # Allows an extra to (like for Accept and Follow) kwargs.pop('to', None) if len(set(kwargs.keys()) - set(allowed_keys)) > 0: - raise BadActivityError('extra data left: {}'.format(kwargs)) + raise BadActivityError(f'extra data left: {kwargs!r}') else: # Remove keys with `None` value valid_kwargs = {} @@ -183,6 +193,10 @@ class BaseActivity(object): raise NotImplementedError def verify(self) -> None: + """Verifies that the activity is valid.""" + if self.OBJECT_REQUIRED and 'object' not in self._data: + raise BadActivityError('activity must have an "object"') + try: self._verify() except NotImplementedError: @@ -275,12 +289,18 @@ class BaseActivity(object): actor_id = self._actor_id(actor) return Person(**ACTOR_SERVICE.get(actor_id)) + def _pre_post_to_outbox(self) -> None: + raise NotImplementedError + def _post_to_outbox(self, obj_id: str, activity: ObjectType, recipients: List[str]) -> None: raise NotImplementedError def _undo_outbox(self) -> None: raise NotImplementedError + def _pre_process_from_inbox(self) -> None: + raise NotImplementedError + def _process_from_inbox(self) -> None: raise NotImplementedError @@ -293,9 +313,6 @@ class BaseActivity(object): def _should_purge_cache(self) -> bool: raise NotImplementedError - # FIXME(tsileo): _pre_process_from_inbox, _pre_post_to_outbox, allow to prevent saving, - # check for undo, delete, update both inbox and outbox - def process_from_inbox(self) -> None: logger.debug(f'calling main process from inbox hook for {self}') self.verify() @@ -313,6 +330,12 @@ class BaseActivity(object): logger.info(f'received duplicate activity {self}, dropping it') return + try: + self._pre_process_from_inbox() + logger.debug('called pre process from inbox hook') + except NotImplementedError: + logger.debug('pre process from inbox hook not implemented') + activity = self.to_dict() DB.inbox.insert_one({ 'activity': activity, @@ -333,6 +356,13 @@ class BaseActivity(object): obj_id = random_object_id() self.set_id(f'{ID}/outbox/{obj_id}', obj_id) self.verify() + + try: + self._pre_post_to_outbox() + logger.debug(f'called pre post to outbox hook') + except NotImplementedError: + logger.debug('pre post to outbox hook not implemented') + activity = self.to_dict() DB.outbox.insert_one({ 'id': obj_id, @@ -434,6 +464,7 @@ class Person(BaseActivity): class Block(BaseActivity): ACTIVITY_TYPE = ActivityType.BLOCK + OBJECT_REQUIRED = True class Collection(BaseActivity): @@ -456,6 +487,7 @@ class Image(BaseActivity): class Follow(BaseActivity): ACTIVITY_TYPE = ActivityType.FOLLOW ALLOWED_OBJECT_TYPES = [ActivityType.PERSON] + OBJECT_REQUIRED = True def _build_reply(self, reply_type: ActivityType) -> BaseActivity: if reply_type == ActivityType.ACCEPT: @@ -515,6 +547,7 @@ class Accept(BaseActivity): class Undo(BaseActivity): ACTIVITY_TYPE = ActivityType.UNDO ALLOWED_OBJECT_TYPES = [ActivityType.FOLLOW, ActivityType.LIKE, ActivityType.ANNOUNCE] + OBJECT_REQUIRED = True def _recipients(self) -> List[str]: obj = self.get_object() @@ -525,6 +558,13 @@ class Undo(BaseActivity): # TODO(tsileo): handle like and announce raise Exception('TODO') + def _pre_process_from_inbox(self) -> None: + """Ensures an Undo activity comes from the same actor as the updated activity.""" + obj = self.get_object() + actor = self.get_actor() + if actor.id != obj.get_actor().id: + raise BadActivityError(f'{actor!r} cannot update {obj!r}') + def _process_from_inbox(self) -> None: obj = self.get_object() DB.inbox.update_one({'remote_id': obj.id}, {'$set': {'meta.undo': True}}) @@ -545,6 +585,12 @@ class Undo(BaseActivity): return False + def _pre_post_to_outbox(self) -> None: + """Ensures an Undo activity references an activity owned by the instance.""" + obj = self.get_object() + if not obj.id.startswith(ID): + raise NotFromOutboxError(f'object {obj["id"]} is not owned by this instance') + def _post_to_outbox(self, obj_id: str, activity: ObjectType, recipients: List[str]) -> None: logger.debug('processing undo to outbox') logger.debug('self={}'.format(self)) @@ -563,6 +609,7 @@ class Undo(BaseActivity): class Like(BaseActivity): ACTIVITY_TYPE = ActivityType.LIKE ALLOWED_OBJECT_TYPES = [ActivityType.NOTE] + OBJECT_REQUIRED = True def _recipients(self) -> List[str]: return [self.get_object().get_actor().id] @@ -680,6 +727,7 @@ class Announce(BaseActivity): class Delete(BaseActivity): ACTIVITY_TYPE = ActivityType.DELETE ALLOWED_OBJECT_TYPES = [ActivityType.NOTE, ActivityType.TOMBSTONE] + OBJECT_REQUIRED = True def _get_actual_object(self) -> BaseActivity: obj = self.get_object() @@ -691,6 +739,13 @@ class Delete(BaseActivity): obj = self._get_actual_object() return obj._recipients() + def _pre_process_from_inbox(self) -> None: + """Ensures a Delete activity comes from the same actor as the deleted activity.""" + obj = self._get_actual_object() + actor = self.get_actor() + if actor.id != obj.get_actor().id: + raise BadActivityError(f'{actor!r} cannot delete {obj!r}') + def _process_from_inbox(self) -> None: DB.inbox.update_one({'activity.object.id': self.get_object().id}, {'$set': {'meta.deleted': True}}) obj = self._get_actual_object() @@ -699,6 +754,12 @@ class Delete(BaseActivity): # TODO(tsileo): also purge the cache if it's a reply of a published activity + def _pre_post_to_outbox(self) -> None: + """Ensures the Delete activity references a activity from the outbox (i.e. owned by the instance).""" + obj = self._get_actual_object() + if not obj.id.startswith(ID): + raise NotFromOutboxError(f'object {obj["id"]} is not owned by this instance') + def _post_to_outbox(self, obj_id: str, activity: ObjectType, recipients: List[str]) -> None: DB.outbox.update_one({'activity.object.id': self.get_object().id}, {'$set': {'meta.deleted': True}}) @@ -706,9 +767,14 @@ class Delete(BaseActivity): class Update(BaseActivity): ACTIVITY_TYPE = ActivityType.UPDATE ALLOWED_OBJECT_TYPES = [ActivityType.NOTE, ActivityType.PERSON] + OBJECT_REQUIRED = True - # TODO(tsileo): ensure the actor updating is the same as the orinial activity - # (ensuring that the Update and its object are of same origin) + def _pre_process_from_inbox(self) -> None: + """Ensures an Update activity comes from the same actor as the updated activity.""" + obj = self.get_object() + actor = self.get_actor() + if actor.id != obj.get_actor().id: + raise BadActivityError(f'{actor!r} cannot update {obj!r}') def _process_from_inbox(self): obj = self.get_object() @@ -721,6 +787,11 @@ class Update(BaseActivity): # TODO(tsileo): implements _should_purge_cache if it's a reply of a published activity (i.e. in the outbox) + def _pre_post_to_outbox(self) -> None: + obj = self.get_object() + if not obj.id.startswith(ID): + raise NotFromOutboxError(f'object {obj["id"]} is not owned by this instance') + def _post_to_outbox(self, obj_id: str, activity: ObjectType, recipients: List[str]) -> None: obj = self._data['object'] @@ -748,6 +819,7 @@ class Update(BaseActivity): class Create(BaseActivity): ACTIVITY_TYPE = ActivityType.CREATE ALLOWED_OBJECT_TYPES = [ActivityType.NOTE] + OBJECT_REQUIRED = True def _set_id(self, uri: str, obj_id: str) -> None: self._data['object']['id'] = uri + '/activity' diff --git a/app.py b/app.py index 0391e40..0fd10db 100644 --- a/app.py +++ b/app.py @@ -901,14 +901,19 @@ def inbox(): logger.debug(f'req_headers={request.headers}') logger.debug(f'raw_data={data}') try: - print(verify_request(ACTOR_SERVICE)) - except Exception: + if not verify_request(ACTOR_SERVICE): + raise Exception('failed to verify request') + except Exception: logger.exception('failed to verify request, trying to verify the payload by fetching the remote') try: data = OBJECT_SERVICE.get(data['id']) except Exception: logger.exception(f'failed to fetch remote id at {data["id"]}') - abort(422) + return Response( + status=422, + headers={'Content-Type': 'application/json'}, + response=json.dumps({'error': 'failed to verify request (using HTTP signatures or fetching the IRI)'}), + ) activity = activitypub.parse_activity(data) logger.debug(f'inbox activity={activity}/{data}')