From e7d954b788103cecdaa3462e2676afd69092d2b8 Mon Sep 17 00:00:00 2001 From: Alain St-Denis Date: Sun, 26 Mar 2023 14:47:40 -0400 Subject: [PATCH] Fixes to address the reviewer's comments. Where appropriate, align with existing code structure. --- federation/entities/activitypub/__init__.py | 14 ++++--- federation/entities/activitypub/ldcontext.py | 40 +++++++++++--------- federation/entities/activitypub/ldsigning.py | 23 +++++------ federation/entities/activitypub/models.py | 40 ++++++++++++++------ federation/entities/mixins.py | 3 +- federation/hostmeta/django/generators.py | 2 +- federation/protocols/activitypub/protocol.py | 2 +- federation/protocols/activitypub/signing.py | 6 --- federation/utils/activitypub.py | 9 +++-- 9 files changed, 81 insertions(+), 58 deletions(-) diff --git a/federation/entities/activitypub/__init__.py b/federation/entities/activitypub/__init__.py index a777610..cc57a67 100644 --- a/federation/entities/activitypub/__init__.py +++ b/federation/entities/activitypub/__init__.py @@ -2,10 +2,12 @@ import json from datetime import timedelta from pyld import jsonld -from federation.utils.django import get_redis - -cache = get_redis() or {} -EXPIRATION = int(timedelta(weeks=4).total_seconds()) +try: + from federation.utils.django import get_redis + cache = get_redis() or {} + EXPIRATION = int(timedelta(weeks=4).total_seconds()) +except: + cache = {} # This is required to workaround a bug in pyld that has the Accept header @@ -24,9 +26,9 @@ def get_loader(*args, **kwargs): options['headers']['Accept'] = 'application/ld+json' doc = requests_loader(url, options) if isinstance(cache, dict): - cache[url] = json.dumps(doc) + cache[key] = json.dumps(doc) else: - cache.set(f'ld_cache:{url}', json.dumps(doc), ex=EXPIRATION) + cache.set(key, json.dumps(doc), ex=EXPIRATION) return doc return loader diff --git a/federation/entities/activitypub/ldcontext.py b/federation/entities/activitypub/ldcontext.py index 7976ee8..e46bfb9 100644 --- a/federation/entities/activitypub/ldcontext.py +++ b/federation/entities/activitypub/ldcontext.py @@ -6,6 +6,7 @@ from pyld import jsonld from federation.entities.activitypub.constants import CONTEXT_ACTIVITYSTREAMS, CONTEXT_SECURITY, NAMESPACE_PUBLIC + # Extract context information from the metadata parameter defined for fields # that are not part of the official AP spec. Use the same extended context for # inbound payload. For outbound payload, build a context with only the required @@ -22,16 +23,16 @@ class LdContextManager: self._extensions[klass] = {} ctx = getattr(klass, 'ctx', []) if ctx: - self._extensions[klass].update({klass.__name__:ctx}) + self._extensions[klass].update({klass.__name__: ctx}) for name, value in klass.schema().declared_fields.items(): ctx = value.metadata.get('ctx') or [] if ctx: - self._extensions[klass].update({name:ctx}) + self._extensions[klass].update({name: ctx}) merged = {} for field in self._extensions.values(): for ctx in field.values(): self._add_extensions(ctx, self._named, merged) - self._merged = copy.copy(self._named) + self._merged = copy.copy(self._named) self._merged.append(merged) def _add_extensions(self, field, named, extensions): @@ -41,7 +42,6 @@ class LdContextManager: elif isinstance(item, dict): extensions.update(item) - def _get_fields(self, obj): for klass in self._extensions.keys(): if issubclass(type(obj), klass): @@ -58,14 +58,16 @@ class LdContextManager: def patch_payload(payload, patched): for field in ('attachment', 'cc', 'tag', 'to'): value = payload.get(field) - if value and not isinstance(value, list): + if not value: + continue + if not isinstance(value, list): value = [value] - patched[field] = value + patched[field] = value if field in ('cc', 'to'): try: idx = value.index('as:Public') patched[field][idx] = value[idx].replace('as:Public', NAMESPACE_PUBLIC) - except: + except ValueError: pass if isinstance(payload.get('object'), dict): patch_payload(payload['object'], patched['object']) @@ -88,22 +90,25 @@ class LdContextManager: if field in to_add.keys(): if field_value is not missing or obj.signable and field == 'signature': self._add_extensions(to_add[field], final, extensions) - if not isinstance(field_value, list): field_value = [field_value] + if not isinstance(field_value, list): + field_value = [field_value] for value in field_value: if issubclass(type(value), (Object, Link)): walk_object(value) walk_object(obj) - if extensions: final.append(extensions) + if extensions: + final.append(extensions) # compact the array if len == 1 to minimize test changes return final if len(final) > 1 else final[0] def merge_context(self, ctx): # One platform sends a single string context - if isinstance(ctx, str): ctx = [ctx] + if isinstance(ctx, str): + ctx = [ctx] # add a # at the end of the python-federation string - # for socialhome payloads + # for legacy socialhome payloads s = json.dumps(ctx) if 'python-federation"' in s: ctx = json.loads(s.replace('python-federation', 'python-federation#', 1)) @@ -112,7 +117,7 @@ class LdContextManager: # is not a json-ld document. try: ctx.pop(ctx.index('http://joinmastodon.org/ns')) - except: + except ValueError: pass # remove @language in context since this directive is not @@ -124,13 +129,12 @@ class LdContextManager: for i, v in enumerate(ctx): if isinstance(v, dict): v.pop('@language', None) - if len(v) == 0: idx.insert(0, i) - for i in idx: ctx.pop(i) + if len(v) == 0: + idx.insert(0, i) + for i in idx: + ctx.pop(i) - # AP activities may be signed, but most platforms don't - # define RsaSignature2017. add it to the context - # hubzilla doesn't define the discoverable property in its context - # include all Mastodon extensions for platforms that only define http://joinmastodon.org/ns in their context + # Merge all defined AP extensions to the inbound context uris = [] defs = {} # Merge original context dicts in one dict diff --git a/federation/entities/activitypub/ldsigning.py b/federation/entities/activitypub/ldsigning.py index 3d3c1df..c199324 100644 --- a/federation/entities/activitypub/ldsigning.py +++ b/federation/entities/activitypub/ldsigning.py @@ -3,7 +3,7 @@ import logging import math import re from base64 import b64encode, b64decode -from copy import copy +from copy import copy from funcy import omit from pyld import jsonld @@ -22,13 +22,13 @@ def create_ld_signature(obj, author): sig = { 'created': datetime.datetime.now(tz=datetime.timezone.utc).isoformat(timespec='seconds'), 'creator': f'{author.id}#main-key', - '@context':'https://w3id.org/security/v1' + '@context': 'https://w3id.org/security/v1' } try: private_key = import_key(author.private_key) except (ValueError, TypeError) as exc: - logger.warning(f'ld_signature - {exc}') + logger.warning('ld_signature - %s', exc) return None signer = pkcs1_15.new(private_key) @@ -40,7 +40,7 @@ def create_ld_signature(obj, author): sig.update({'type': 'RsaSignature2017', 'signatureValue': b64encode(signature).decode()}) sig.pop('@context') - obj.update({'signature':sig}) + obj.update({'signature': sig}) def verify_ld_signature(payload): @@ -49,24 +49,25 @@ def verify_ld_signature(payload): """ signature = copy(payload.get('signature', None)) if not signature: - logger.warning(f'ld_signature - No signature in {payload.get("id", "the payload")}') + logger.warning('ld_signature - No signature in %s', payload.get("id", "the payload")) return None # retrieve the author's public key profile = retrieve_and_parse_document(signature.get('creator')) if not profile: - logger.warning(f'ld_signature - Failed to retrieve profile for {signature.get("creator")}') + + logger.warning('ld_signature - Failed to retrieve profile for %s', signature.get("creator")) return None try: pkey = import_key(profile.public_key) except ValueError as exc: - logger.warning(f'ld_signature - {exc}') + logger.warning('ld_signature - %s', exc) return None verifier = pkcs1_15.new(pkey) # Compute digests and verify signature sig = omit(signature, ('type', 'signatureValue')) - sig.update({'@context':'https://w3id.org/security/v1'}) + sig.update({'@context': 'https://w3id.org/security/v1'}) sig_digest = hash(sig) obj = omit(payload, 'signature') obj_digest = hash(obj) @@ -75,15 +76,15 @@ def verify_ld_signature(payload): sig_value = b64decode(signature.get('signatureValue')) try: verifier.verify(SHA256.new(digest), sig_value) - logger.debug(f'ld_signature - {payload.get("id")} has a valid signature') + logger.debug('ld_signature - %s has a valid signature', payload.get("id")) return profile.id except ValueError: - logger.warning(f'ld_signature - Invalid signature for {payload.get("id")}') + logger.warning('ld_signature - Invalid signature for %s', payload.get("id")) return None def hash(obj): - nquads = NormalizedDoubles().normalize(obj, options={'format':'application/nquads','algorithm':'URDNA2015'}) + nquads = NormalizedDoubles().normalize(obj, options={'format': 'application/nquads', 'algorithm': 'URDNA2015'}) return SHA256.new(nquads.encode('utf-8')).hexdigest() diff --git a/federation/entities/activitypub/models.py b/federation/entities/activitypub/models.py index 08d9b74..c3ab74b 100644 --- a/federation/entities/activitypub/models.py +++ b/federation/entities/activitypub/models.py @@ -10,6 +10,7 @@ import bleach from calamus import fields from calamus.schema import JsonLDAnnotation, JsonLDSchema, JsonLDSchemaOpts from calamus.utils import normalize_value +from cryptography.exceptions import InvalidSignature from marshmallow import exceptions, pre_load, post_load, post_dump from marshmallow.fields import Integer from marshmallow.utils import EXCLUDE, missing @@ -25,7 +26,6 @@ from federation.outbound import handle_send from federation.types import UserType, ReceiverVariant from federation.utils.activitypub import retrieve_and_parse_document, retrieve_and_parse_profile, \ get_profile_id_from_webfinger -from federation.utils.django import get_configuration from federation.utils.text import with_slash, validate_handle logger = logging.getLogger("federation") @@ -297,12 +297,23 @@ class Object(BaseEntity, metaclass=JsonLDAnnotation): # TODO: rework validation def validate(self, direction='inbound'): if direction == 'inbound': + # ensure marshmallow.missing is not sent to the client app for attr in type(self).schema().load_fields.keys(): if getattr(self, attr) is missing: setattr(self, attr, None) super().validate(direction) + def _validate_signatures(self): + # Always verify the inbound LD signature, for monitoring purposes + actor = verify_ld_signature(self._source_object) + if not self._sender: + return + if self.signable and self._sender not in (self.id, getattr(self, 'actor_id', None)): + # Relayed payload + if not actor: + raise InvalidSignature('no or invalid signature for %s, a relayed payload', self.id) + def to_string(self): # noinspection PyUnresolvedReferences return str(self.to_as2()) @@ -457,6 +468,7 @@ class Link(metaclass=JsonLDAnnotation): class Hashtag(Link): ctx = [{'Hashtag': 'as:Hashtag'}] + id = fields.Id() # Hubzilla uses id instead of href class Meta: rdf_type = as2.Hashtag @@ -702,7 +714,7 @@ class Note(Object, RawContentMixin): self._allowed_children += (base.Audio, base.Video) def to_as2(self): - #self.sensitive = 'nsfw' in self.tags + self.sensitive = 'nsfw' in self.tags self.url = self.id edited = False @@ -768,7 +780,13 @@ class Note(Object, RawContentMixin): # Skip when markdown return - hrefs = [tag.href.lower() for tag in self.tag_objects if isinstance(tag, Hashtag)] + hrefs = [] + for tag in self.tag_objects: + if isinstance(tag, Hashtag): + if tag.href is not missing: + hrefs.append(tag.href.lower()) + elif tag.id is not missing: + hrefs.append(tag.id.lower()) # noinspection PyUnusedLocal def remove_tag_links(attrs, new=False): # Hashtag object hrefs @@ -807,6 +825,7 @@ class Note(Object, RawContentMixin): Populate tags to the object.tag list. """ try: + from federation.utils.django import get_configuration config = get_configuration() except ImportError: tags_path = None @@ -1321,6 +1340,7 @@ def element_to_objects(element: Union[Dict, Object], sender: str = "") -> List: entity = model_to_objects(element) if not isinstance(element, Object) else element if entity and hasattr(entity, 'to_base'): entity = entity.to_base() + entity._sender = sender if isinstance(entity, ( base.Post, base.Comment, base.Profile, base.Share, base.Follow, base.Retraction, base.Accept,) @@ -1330,13 +1350,11 @@ def element_to_objects(element: Union[Dict, Object], sender: str = "") -> List: except ValueError as ex: logger.error("Failed to validate entity %s: %s", entity, ex) return [] - # Always verify the LD signature, for monitoring purposes - actor = verify_ld_signature(entity._source_object) - if entity.signable and sender not in (entity.id, getattr(entity, 'actor_id', None), ''): - # Relayed payload - if not actor: - logger.warning(f'no or invalid signature for a relayed payload, fetching {entity.id}') - entity = retrieve_and_parse_document(entity.id) + except InvalidSignature as exc: + logger.info('%s, fetching from remote', exc) + entity = retrieve_and_parse_document(entity.id) + if not entity: + return [] logger.info('Entity type "%s" was handled through the json-ld processor', entity.__class__.__name__) return [entity] elif entity: @@ -1355,7 +1373,7 @@ def model_to_objects(payload): try: entity = model.schema().load(payload) except (KeyError, jsonld.JsonLdError, exceptions.ValidationError) as exc : # Just give up for now. This must be made robust - logger.error(f"Error parsing jsonld payload ({exc})") + logger.error("Error parsing jsonld payload (%s)", exc) return None if isinstance(getattr(entity, 'object_', None), Object): diff --git a/federation/entities/mixins.py b/federation/entities/mixins.py index 2fea627..bbc4075 100644 --- a/federation/entities/mixins.py +++ b/federation/entities/mixins.py @@ -20,6 +20,7 @@ class BaseEntity: _source_protocol: str = "" # Contains the original object from payload as a string _source_object: Union[str, Dict] = None + _sender: str = "" _sender_key: str = "" # ActivityType activity: ActivityType = None @@ -231,8 +232,8 @@ class RawContentMixin(BaseEntity): @property def rendered_content(self) -> str: """Returns the rendered version of raw_content, or just raw_content.""" - from federation.utils.django import get_configuration try: + from federation.utils.django import get_configuration config = get_configuration() if config["tags_path"]: def linkifier(tag: str) -> str: diff --git a/federation/hostmeta/django/generators.py b/federation/hostmeta/django/generators.py index b81c2da..01d657c 100644 --- a/federation/hostmeta/django/generators.py +++ b/federation/hostmeta/django/generators.py @@ -60,7 +60,7 @@ def rfc7033_webfinger_view(request, *args, **kwargs): if not resource.startswith("acct:"): return HttpResponseBadRequest("Invalid resource") handle = resource.replace("acct:", "").lower() - logger.debug(f"{handle} requested with {request}") + logger.debug("%s requested with %s", handle, request) profile_func = get_function_from_config("get_profile_function") try: diff --git a/federation/protocols/activitypub/protocol.py b/federation/protocols/activitypub/protocol.py index 1526a27..fc2e95f 100644 --- a/federation/protocols/activitypub/protocol.py +++ b/federation/protocols/activitypub/protocol.py @@ -90,6 +90,6 @@ class Protocol: # Verify the HTTP signature self.sender = verify_request_signature(self.request) except (ValueError, KeyError, InvalidSignature) as exc: - logger.warning(f'HTTP signature verification failed: {exc}') + logger.warning('HTTP signature verification failed: %s', exc) return self.actor, {} return self.sender, self.payload diff --git a/federation/protocols/activitypub/signing.py b/federation/protocols/activitypub/signing.py index 4b420df..29ac4fd 100644 --- a/federation/protocols/activitypub/signing.py +++ b/federation/protocols/activitypub/signing.py @@ -79,9 +79,3 @@ def verify_request_signature(request: RequestType, required: bool=True): raise ValueError("Invalid signature") return signer.id - - - - - - diff --git a/federation/utils/activitypub.py b/federation/utils/activitypub.py index 8ea1e3c..b9cfb31 100644 --- a/federation/utils/activitypub.py +++ b/federation/utils/activitypub.py @@ -3,14 +3,17 @@ import logging from typing import Optional, Any from federation.protocols.activitypub.signing import get_http_authentication -from federation.utils.django import get_federation_user from federation.utils.network import fetch_document, try_retrieve_webfinger_document from federation.utils.text import decode_if_bytes, validate_handle logger = logging.getLogger('federation') -federation_user = get_federation_user() -if not federation_user: logger.warning("django is required for get requests signing") +try: + from federation.utils.django import get_federation_user + federation_user = get_federation_user() +except ImportError: + federation_user = None + logger.warning("django is required for get requests signing") def get_profile_id_from_webfinger(handle: str) -> Optional[str]: