From 63f097979990bf5ba9db848b8a253056bad781af Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Wed, 4 Oct 2017 08:13:48 +0900 Subject: Validate id of ActivityPub representations (#5114) Additionally, ActivityPub::FetchRemoteStatusService no longer parses activities. OStatus::Activity::Creation no longer delegates to ActivityPub because the provided ActivityPub representations are not signed while OStatus representations are. --- .../activitypub/fetch_remote_account_service.rb | 10 ++++-- .../activitypub/fetch_remote_key_service.rb | 25 +++++++++++---- .../activitypub/fetch_remote_status_service.rb | 37 ++++++++++------------ .../activitypub/process_account_service.rb | 6 ++-- 4 files changed, 46 insertions(+), 32 deletions(-) (limited to 'app/services/activitypub') diff --git a/app/services/activitypub/fetch_remote_account_service.rb b/app/services/activitypub/fetch_remote_account_service.rb index cb6e40748..e6c6338be 100644 --- a/app/services/activitypub/fetch_remote_account_service.rb +++ b/app/services/activitypub/fetch_remote_account_service.rb @@ -5,14 +5,18 @@ class ActivityPub::FetchRemoteAccountService < BaseService # Should be called when uri has already been checked for locality # Does a WebFinger roundtrip on each call - def call(uri, prefetched_json = nil) - @json = body_to_json(prefetched_json) || fetch_resource(uri) + def call(uri, id: true, prefetched_body: nil) + @json = if prefetched_body.nil? + fetch_resource(uri, id) + else + body_to_json(prefetched_body) + end return unless supported_context? && expected_type? @uri = @json['id'] @username = @json['preferredUsername'] - @domain = Addressable::URI.parse(uri).normalized_host + @domain = Addressable::URI.parse(@uri).normalized_host return unless verified_webfinger? diff --git a/app/services/activitypub/fetch_remote_key_service.rb b/app/services/activitypub/fetch_remote_key_service.rb index ebd64071e..ce1048fee 100644 --- a/app/services/activitypub/fetch_remote_key_service.rb +++ b/app/services/activitypub/fetch_remote_key_service.rb @@ -4,13 +4,26 @@ class ActivityPub::FetchRemoteKeyService < BaseService include JsonLdHelper # Returns account that owns the key - def call(uri, prefetched_json = nil) - @json = body_to_json(prefetched_json) || fetch_resource(uri) + def call(uri, id: true, prefetched_body: nil) + if prefetched_body.nil? + if id + @json = fetch_resource_without_id_validation(uri) + if person? + @json = fetch_resource(@json['id'], true) + elsif uri != @json['id'] + return + end + else + @json = fetch_resource(uri, id) + end + else + @json = body_to_json(prefetched_body) + end return unless supported_context?(@json) && expected_type? - return find_account(uri, @json) if person? + return find_account(@json['id'], @json) if person? - @owner = fetch_resource(owner_uri) + @owner = fetch_resource(owner_uri, true) return unless supported_context?(@owner) && confirmed_owner? @@ -19,9 +32,9 @@ class ActivityPub::FetchRemoteKeyService < BaseService private - def find_account(uri, prefetched_json) + def find_account(uri, prefetched_body) account = ActivityPub::TagManager.instance.uri_to_resource(uri, Account) - account ||= ActivityPub::FetchRemoteAccountService.new.call(uri, prefetched_json) + account ||= ActivityPub::FetchRemoteAccountService.new.call(uri, prefetched_body: prefetched_body) account end diff --git a/app/services/activitypub/fetch_remote_status_service.rb b/app/services/activitypub/fetch_remote_status_service.rb index a95931afe..c7414f161 100644 --- a/app/services/activitypub/fetch_remote_status_service.rb +++ b/app/services/activitypub/fetch_remote_status_service.rb @@ -4,36 +4,33 @@ class ActivityPub::FetchRemoteStatusService < BaseService include JsonLdHelper # Should be called when uri has already been checked for locality - def call(uri, prefetched_json = nil) - @json = body_to_json(prefetched_json) || fetch_resource(uri) + def call(uri, id: true, prefetched_body: nil) + @json = if prefetched_body.nil? + fetch_resource(uri, id) + else + body_to_json(prefetched_body) + end - return unless supported_context? + return unless expected_type? && supported_context? - activity = activity_json - actor_id = value_or_id(activity['actor']) - - return unless expected_type?(activity) && trustworthy_attribution?(uri, actor_id) + return if actor_id.nil? || !trustworthy_attribution?(@json['id'], actor_id) actor = ActivityPub::TagManager.instance.uri_to_resource(actor_id, Account) - actor = ActivityPub::FetchRemoteAccountService.new.call(actor_id) if actor.nil? + actor = ActivityPub::FetchRemoteAccountService.new.call(actor_id, id: true) if actor.nil? return if actor.suspended? - ActivityPub::Activity.factory(activity, actor).perform + ActivityPub::Activity.factory(activity_json, actor).perform end private def activity_json - if %w(Note Article).include? @json['type'] - { - 'type' => 'Create', - 'actor' => first_of_value(@json['attributedTo']), - 'object' => @json, - } - else - @json - end + { 'type' => 'Create', 'actor' => actor_id, 'object' => @json } + end + + def actor_id + first_of_value(@json['attributedTo']) end def trustworthy_attribution?(uri, attributed_to) @@ -44,7 +41,7 @@ class ActivityPub::FetchRemoteStatusService < BaseService super(@json) end - def expected_type?(json) - %w(Create Announce).include? json['type'] + def expected_type? + %w(Note Article).include? @json['type'] end end diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 811209537..f93baf4b5 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -90,7 +90,7 @@ class ActivityPub::ProcessAccountService < BaseService return if value.nil? return value['url'] if value.is_a?(Hash) - image = fetch_resource(value) + image = fetch_resource_without_id_validation(value) image['url'] if image end @@ -100,7 +100,7 @@ class ActivityPub::ProcessAccountService < BaseService return if value.nil? return value['publicKeyPem'] if value.is_a?(Hash) - key = fetch_resource(value) + key = fetch_resource_without_id_validation(value) key['publicKeyPem'] if key end @@ -130,7 +130,7 @@ class ActivityPub::ProcessAccountService < BaseService return if @json[type].blank? return @collections[type] if @collections.key?(type) - collection = fetch_resource(@json[type]) + collection = fetch_resource_without_id_validation(@json[type]) @collections[type] = collection.is_a?(Hash) && collection['totalItems'].present? && collection['totalItems'].is_a?(Numeric) ? collection['totalItems'] : nil rescue HTTP::Error, OpenSSL::SSL::SSLError -- cgit