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.

4 files changed, 46 insertions, 32 deletions

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