From 9088e58a64d5fed1084eed09e04ae7b097c343dc Mon Sep 17 00:00:00 2001 From: Diogo Cordeiro Date: Fri, 11 Oct 2019 19:09:08 +0100 Subject: [PATCH] [ActivityPub] The protocol allows content to be null, GNU social doesn't, we'll reject silentiously Reported by kaniini --- plugins/ActivityPub/ActivityPubPlugin.php | 16 ++-- plugins/ActivityPub/lib/explorer.php | 12 ++- plugins/ActivityPub/lib/inbox_handler.php | 20 +++-- .../lib/models/Activitypub_create.php | 31 +++---- .../lib/models/Activitypub_mention_tag.php | 4 +- .../lib/models/Activitypub_notice.php | 81 ++++++++++--------- 6 files changed, 90 insertions(+), 74 deletions(-) diff --git a/plugins/ActivityPub/ActivityPubPlugin.php b/plugins/ActivityPub/ActivityPubPlugin.php index 50ecbb6e68..07253c2eb4 100644 --- a/plugins/ActivityPub/ActivityPubPlugin.php +++ b/plugins/ActivityPub/ActivityPubPlugin.php @@ -41,6 +41,10 @@ const ACTIVITYPUB_PUBLIC_TO = ['https://www.w3.org/ns/activitystreams#Public', 'Public', 'as:Public' ]; +const ACTIVITYPUB_HTTP_CLIENT_HEADERS = [ + 'Accept: application/ld+json; profile="https://www.w3.org/ns/activitystreams"', + 'User-Agent: GNUsocialBot ' . GNUSOCIAL_VERSION . ' - https://gnusocial.network' +]; /** * @category Plugin @@ -117,13 +121,13 @@ class ActivityPubPlugin extends Plugin if ($grab_online) { /* Online Grabbing */ $client = new HTTPClient(); - $headers = []; - $headers[] = 'Accept: application/ld+json; profile="https://www.w3.org/ns/activitystreams"'; - $headers[] = 'User-Agent: GNUSocialBot ' . GNUSOCIAL_VERSION . ' - https://gnu.io/social'; - $response = $client->get($url, $headers); + $response = $client->get($url, ACTIVITYPUB_HTTP_CLIENT_HEADERS); $object = json_decode($response->getBody(), true); - Activitypub_notice::validate_note($object); - return Activitypub_notice::create_notice($object); + if (Activitypub_notice::validate_note($object)) { + return Activitypub_notice::create_notice($object); + } else { + throw new Exception("Valid ActivityPub Notice object but unsupported by GNU social."); + } } common_debug('ActivityPubPlugin Notice Grabber: failed to find: '.$url); diff --git a/plugins/ActivityPub/lib/explorer.php b/plugins/ActivityPub/lib/explorer.php index 62ff446163..da8718951c 100644 --- a/plugins/ActivityPub/lib/explorer.php +++ b/plugins/ActivityPub/lib/explorer.php @@ -40,8 +40,6 @@ class Activitypub_explorer { private $discovered_actor_profiles = []; private $temp_res; // global variable to hold a temporary http response - private static $headers = ['Accept: application/ld+json; profile="https://www.w3.org/ns/activitystreams"', - 'User-Agent: GNUsocialBot ' . GNUSOCIAL_VERSION . ' - https://gnusocial.network']; /** * Shortcut function to get a single profile from its URL. @@ -130,7 +128,7 @@ class Activitypub_explorer private function ensure_proper_remote_uri($url) { $client = new HTTPClient(); - $response = $client->get($url, self::$headers); + $response = $client->get($url, ACTIVITYPUB_HTTP_CLIENT_HEADERS); $res = json_decode($response->getBody(), true); if (self::validate_remote_response($res)) { $this->temp_res = $res; @@ -224,7 +222,7 @@ class Activitypub_explorer common_debug('ActivityPub Explorer: Trying to grab a remote actor for ' . $url); if (!isset($this->temp_res)) { $client = new HTTPClient(); - $response = $client->get($url, self::$headers); + $response = $client->get($url, ACTIVITYPUB_HTTP_CLIENT_HEADERS); $res = json_decode($response->getBody(), true); } else { $res = $this->temp_res; @@ -409,7 +407,7 @@ class Activitypub_explorer public static function get_actor_inboxes_uri($url) { $client = new HTTPClient(); - $response = $client->get($url, self::$headers); + $response = $client->get($url, ACTIVITYPUB_HTTP_CLIENT_HEADERS); if (!$response->isOk()) { throw new Exception('Invalid Actor URL.'); } @@ -437,7 +435,7 @@ class Activitypub_explorer private function travel_collection($url) { $client = new HTTPClient(); - $response = $client->get($url, self::$headers); + $response = $client->get($url, ACTIVITYPUB_HTTP_CLIENT_HEADERS); $res = json_decode($response->getBody(), true); if (!isset($res['orderedItems'])) { @@ -470,7 +468,7 @@ class Activitypub_explorer public static function get_remote_user_activity($url) { $client = new HTTPClient(); - $response = $client->get($url, self::$headers); + $response = $client->get($url, ACTIVITYPUB_HTTP_CLIENT_HEADERS); $res = json_decode($response->getBody(), true); if (Activitypub_explorer::validate_remote_response($res)) { common_debug('ActivityPub Explorer: Found a valid remote actor for ' . $url); diff --git a/plugins/ActivityPub/lib/inbox_handler.php b/plugins/ActivityPub/lib/inbox_handler.php index 1fea9577b7..da2b0d11e7 100644 --- a/plugins/ActivityPub/lib/inbox_handler.php +++ b/plugins/ActivityPub/lib/inbox_handler.php @@ -54,7 +54,9 @@ class Activitypub_inbox_handler $this->object = $activity['object']; // Validate Activity - $this->validate_activity(); + if (!$this->validate_activity()) { + return; // Just ignore + } // Get Actor's Profile if (!is_null($actor_profile)) { @@ -70,10 +72,11 @@ class Activitypub_inbox_handler /** * Validates if a given Activity is valid. Throws exception if not. * - * @throws Exception + * @throws Exception if invalid + * @return bool true if valid and acceptable, false if unsupported * @author Diogo Cordeiro */ - private function validate_activity() + private function validate_activity(): bool { // Activity validation // Validate data @@ -88,15 +91,16 @@ class Activitypub_inbox_handler } // Object validation + $valid = true; switch ($this->activity['type']) { case 'Accept': - Activitypub_accept::validate_object($this->object); + $valid &= Activitypub_accept::validate_object($this->object); break; case 'Create': - Activitypub_create::validate_object($this->object); + $valid &= Activitypub_create::validate_object($this->object); break; case 'Delete': - Activitypub_delete::validate_object($this->object); + $valid &= Activitypub_delete::validate_object($this->object); break; case 'Follow': case 'Like': @@ -106,11 +110,13 @@ class Activitypub_inbox_handler } break; case 'Undo': - Activitypub_undo::validate_object($this->object); + $valid &= Activitypub_undo::validate_object($this->object); break; default: throw new Exception('Unknown Activity Type.'); } + + return $valid; } /** diff --git a/plugins/ActivityPub/lib/models/Activitypub_create.php b/plugins/ActivityPub/lib/models/Activitypub_create.php index 6ea760329b..15207a61e7 100644 --- a/plugins/ActivityPub/lib/models/Activitypub_create.php +++ b/plugins/ActivityPub/lib/models/Activitypub_create.php @@ -39,23 +39,23 @@ class Activitypub_create /** * Generates an ActivityPub representation of a Create * - * @author Diogo Cordeiro * @param string $actor * @param array $object - * @param bool $directMesssage whether it is a private Create activity or not + * @param bool $directMessage whether it is a private Create activity or not * @return array pretty array to be used in a response + * @author Diogo Cordeiro */ public static function create_to_array(string $actor, array $object, bool $directMessage = false): array { $res = [ - '@context' => 'https://www.w3.org/ns/activitystreams', - 'id' => $object['id'].'/create', - 'type' => 'Create', + '@context' => 'https://www.w3.org/ns/activitystreams', + 'id' => $object['id'] . '/create', + 'type' => 'Create', 'directMessage' => $directMessage, - 'to' => $object['to'], - 'cc' => $object['cc'], - 'actor' => $actor, - 'object' => $object + 'to' => $object['to'], + 'cc' => $object['cc'], + 'actor' => $actor, + 'object' => $object ]; return $res; } @@ -63,11 +63,12 @@ class Activitypub_create /** * Verifies if a given object is acceptable for a Create Activity. * - * @author Diogo Cordeiro * @param array $object - * @throws Exception + * @return bool True if acceptable, false if valid but unsupported + * @throws Exception if invalid + * @author Diogo Cordeiro */ - public static function validate_object($object) + public static function validate_object($object): bool { if (!is_array($object)) { common_debug('ActivityPub Create Validator: Rejected because of invalid Object format.'); @@ -84,11 +85,12 @@ class Activitypub_create switch ($object['type']) { case 'Note': // Validate data - Activitypub_notice::validate_note($object); + return Activitypub_notice::validate_note($object); break; default: throw new Exception('This is not a supported Object Type for Create Activity.'); } + return true; } /** @@ -100,7 +102,8 @@ class Activitypub_create * @return bool true if note is private, false otherwise * @author Bruno casteleiro */ - public static function isPrivateNote(array $activity): bool { + public static function isPrivateNote(array $activity): bool + { if (isset($activity['directMessage'])) { return $activity['directMessage']; } diff --git a/plugins/ActivityPub/lib/models/Activitypub_mention_tag.php b/plugins/ActivityPub/lib/models/Activitypub_mention_tag.php index 33c7851259..330f8d455c 100644 --- a/plugins/ActivityPub/lib/models/Activitypub_mention_tag.php +++ b/plugins/ActivityPub/lib/models/Activitypub_mention_tag.php @@ -41,10 +41,10 @@ class Activitypub_mention_tag * * @author Diogo Cordeiro * @param string $href Actor Uri - * @param array $name Mention name + * @param string $name Mention name * @return array pretty array to be used in a response */ - public static function mention_tag_to_array_from_values($href, $name) + public static function mention_tag_to_array_from_values(string $href, string $name): array { $res = [ '@context' => 'https://www.w3.org/ns/activitystreams', diff --git a/plugins/ActivityPub/lib/models/Activitypub_notice.php b/plugins/ActivityPub/lib/models/Activitypub_notice.php index d5bc706639..da61c2a72c 100644 --- a/plugins/ActivityPub/lib/models/Activitypub_notice.php +++ b/plugins/ActivityPub/lib/models/Activitypub_notice.php @@ -44,6 +44,7 @@ class Activitypub_notice * @throws EmptyPkeyValueException * @throws InvalidUrlException * @throws ServerException + * @throws Exception * @author Diogo Cordeiro */ public static function notice_to_array($notice) @@ -74,24 +75,24 @@ class Activitypub_notice } foreach ($notice->getAttentionProfiles() as $to_profile) { - $to[] = $href = $to_profile->getUri(); - $tags[] = Activitypub_mention_tag::mention_tag_to_array_from_values($href, $to_profile->getNickname().'@'.parse_url($href, PHP_URL_HOST)); + $to[] = $href = $to_profile->getUri(); + $tags[] = Activitypub_mention_tag::mention_tag_to_array_from_values($href, $to_profile->getNickname() . '@' . parse_url($href, PHP_URL_HOST)); } $item = [ - '@context' => 'https://www.w3.org/ns/activitystreams', - 'id' => self::getUrl($notice), - 'type' => 'Note', - 'published' => str_replace(' ', 'T', $notice->getCreated()).'Z', - 'url' => self::getUrl($notice), - 'attributedTo' => ActivityPubPlugin::actor_uri($profile), - 'to' => $to, - 'cc' => $cc, - 'conversation' => $notice->getConversationUrl(), - 'content' => $notice->getRendered(), - 'isLocal' => $notice->isLocal(), - 'attachment' => $attachments, - 'tag' => $tags + '@context' => 'https://www.w3.org/ns/activitystreams', + 'id' => self::getUrl($notice), + 'type' => 'Note', + 'published' => str_replace(' ', 'T', $notice->getCreated()) . 'Z', + 'url' => self::getUrl($notice), + 'attributedTo' => ActivityPubPlugin::actor_uri($profile), + 'to' => $to, + 'cc' => $cc, + 'conversation' => $notice->getConversationUrl(), + 'content' => $notice->getRendered(), + 'isLocal' => $notice->isLocal(), + 'attachment' => $attachments, + 'tag' => $tags ]; // Is this a reply? @@ -102,7 +103,7 @@ class Activitypub_notice // Do we have a location for this notice? try { $location = Notice_location::locFromStored($notice); - $item['latitude'] = $location->lat; + $item['latitude'] = $location->lat; $item['longitude'] = $location->lon; } catch (Exception $e) { // Apparently no. @@ -115,17 +116,17 @@ class Activitypub_notice * Create a Notice via ActivityPub Note Object. * Returns created Notice. * - * @author Diogo Cordeiro * @param array $object * @param Profile $actor_profile * @param bool $directMessage * @return Notice * @throws Exception + * @author Diogo Cordeiro */ public static function create_notice(array $object, Profile $actor_profile = null, bool $directMessage = false): Notice { - $id = $object['id']; // int - $url = isset($object['url']) ? $object['url'] : $id; // string + $id = $object['id']; // int + $url = isset($object['url']) ? $object['url'] : $id; // string $content = $object['content']; // string // possible keys: ['inReplyTo', 'latitude', 'longitude'] @@ -134,7 +135,7 @@ class Activitypub_notice $settings['inReplyTo'] = $object['inReplyTo']; } if (isset($object['latitude'])) { - $settings['latitude'] = $object['latitude']; + $settings['latitude'] = $object['latitude']; } if (isset($object['longitude'])) { $settings['longitude'] = $object['longitude']; @@ -156,10 +157,10 @@ class Activitypub_notice $act->time = time(); $act->actor = $actor_profile->asActivityObject(); $act->context = new ActivityContext(); - $options = ['source' => 'ActivityPub', - 'uri' => $id, - 'url' => $url, - 'is_local' => self::getNotePolicyType($object, $actor_profile)]; + $options = ['source' => 'ActivityPub', + 'uri' => $id, + 'url' => $url, + 'is_local' => self::getNotePolicyType($object, $actor_profile)]; if ($directMessage) { $options['scope'] = Notice::MESSAGE_SCOPE; @@ -169,7 +170,7 @@ class Activitypub_notice if (isset($settings['inReplyTo'])) { try { $inReplyTo = ActivityPubPlugin::grab_notice_from_url($settings['inReplyTo']); - $act->context->replyToID = $inReplyTo->getUri(); + $act->context->replyToID = $inReplyTo->getUri(); $act->context->replyToUrl = $inReplyTo->getUrl(); } catch (Exception $e) { // It failed to grab, maybe we got this note from another source @@ -234,8 +235,8 @@ class Activitypub_notice * Validates a note. * * @param array $object - * @return bool - * @throws Exception + * @return bool false if unacceptable for GS but valid ActivityPub object + * @throws Exception if invalid ActivityPub object * @author Diogo Cordeiro */ public static function validate_note($object) @@ -251,10 +252,6 @@ class Activitypub_notice common_debug('ActivityPub Notice Validator: Rejected because of Type.'); throw new Exception('Invalid Object type.'); } - if (!isset($object['content'])) { - common_debug('ActivityPub Notice Validator: Rejected because Content was not specified (GNU social requires content in notes).'); - throw new Exception('Object content was not specified.'); - } if (isset($object['url']) && !filter_var($object['url'], FILTER_VALIDATE_URL)) { common_debug('ActivityPub Notice Validator: Rejected because Object URL is invalid.'); throw new Exception('Invalid Object URL.'); @@ -263,6 +260,10 @@ class Activitypub_notice common_debug('ActivityPub Notice Validator: Rejected because either Object CC or TO wasn\'t specified.'); throw new Exception('Either Object CC or TO wasn\'t specified.'); } + if (!isset($object['content'])) { + common_debug('ActivityPub Notice Validator: Rejected because Content was not specified (GNU social requires content in notes).'); + return false; + } return true; } @@ -271,14 +272,17 @@ class Activitypub_notice * * @param Notice $notice notice from which to retrieve the URL * @return string URL + * @throws InvalidUrlException + * @throws Exception * @author Bruno Casteleiro */ - public static function getUrl(Notice $notice): string { - if ($notice->isLocal()) { - return common_local_url('apNotice', ['id' => $notice->getID()]); - } else { - return $notice->getUrl(); - } + public static function getUrl(Notice $notice): string + { + if ($notice->isLocal()) { + return common_local_url('apNotice', ['id' => $notice->getID()]); + } else { + return $notice->getUrl(); + } } /** @@ -289,7 +293,8 @@ class Activitypub_notice * @return int Notice policy type * @author Bruno Casteleiro */ - public static function getNotePolicyType(array $note, Profile $actor_profile): int { + public static function getNotePolicyType(array $note, Profile $actor_profile): int + { if (in_array('https://www.w3.org/ns/activitystreams#Public', $note['to'])) { return $actor_profile->isLocal() ? Notice::LOCAL_PUBLIC : Notice::REMOTE; } else {