From 72bb3e03fdf4d8c886d41f3459000b336a3a362b Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 21 Aug 2017 22:57:34 +0200 Subject: [PATCH] Support more variations of ActivityPub keyId in signature (#4630) - Tries to avoid performing HTTP request if the keyId is an actor URI - Likewise if the URI is a fragment URI on top of actor URI - Resolves public key, returns owner if the owner links back to the key --- .../concerns/signature_verification.rb | 4 +- app/helpers/jsonld_helper.rb | 6 ++- app/lib/activitypub/activity.rb | 2 +- app/lib/activitypub/activity/accept.rb | 2 +- app/lib/activitypub/activity/reject.rb | 2 +- app/lib/activitypub/activity/undo.rb | 2 +- app/lib/activitypub/tag_manager.rb | 2 +- .../activitypub/fetch_remote_key_service.rb | 47 +++++++++++++++++++ 8 files changed, 60 insertions(+), 7 deletions(-) create mode 100644 app/services/activitypub/fetch_remote_key_service.rb diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb index aeb8da879..4211283ed 100644 --- a/app/controllers/concerns/signature_verification.rb +++ b/app/controllers/concerns/signature_verification.rb @@ -98,7 +98,9 @@ module SignatureVerification if key_id.start_with?('acct:') ResolveRemoteAccountService.new.call(key_id.gsub(/\Aacct:/, '')) elsif !ActivityPub::TagManager.instance.local_uri?(key_id) - ActivityPub::FetchRemoteAccountService.new.call(key_id) + account = ActivityPub::TagManager.instance.uri_to_resource(key_id, Account) + account ||= ActivityPub::FetchRemoteKeyService.new.call(key_id) + account end end end diff --git a/app/helpers/jsonld_helper.rb b/app/helpers/jsonld_helper.rb index c750a7038..d8b3ddf18 100644 --- a/app/helpers/jsonld_helper.rb +++ b/app/helpers/jsonld_helper.rb @@ -9,6 +9,10 @@ module JsonLdHelper value.is_a?(Array) ? value.first : value end + def value_or_id(value) + value.is_a?(String) ? value : value['id'] + end + def supported_context?(json) equals_or_includes?(json['@context'], ActivityPub::TagManager::CONTEXT) end @@ -20,7 +24,7 @@ module JsonLdHelper end def body_to_json(body) - body.nil? ? nil : Oj.load(body, mode: :strict) + body.is_a?(String) ? Oj.load(body, mode: :strict) : body rescue Oj::ParseError nil end diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index f8de8060c..14e3ca784 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -58,7 +58,7 @@ class ActivityPub::Activity end def object_uri - @object_uri ||= @object.is_a?(String) ? @object : @object['id'] + @object_uri ||= value_or_id(@object) end def redis diff --git a/app/lib/activitypub/activity/accept.rb b/app/lib/activitypub/activity/accept.rb index 44c432ae7..bd90c9019 100644 --- a/app/lib/activitypub/activity/accept.rb +++ b/app/lib/activitypub/activity/accept.rb @@ -20,6 +20,6 @@ class ActivityPub::Activity::Accept < ActivityPub::Activity end def target_uri - @target_uri ||= @object['actor'] + @target_uri ||= value_or_id(@object['actor']) end end diff --git a/app/lib/activitypub/activity/reject.rb b/app/lib/activitypub/activity/reject.rb index 6a234994e..d815feeb6 100644 --- a/app/lib/activitypub/activity/reject.rb +++ b/app/lib/activitypub/activity/reject.rb @@ -20,6 +20,6 @@ class ActivityPub::Activity::Reject < ActivityPub::Activity end def target_uri - @target_uri ||= @object['actor'] + @target_uri ||= value_or_id(@object['actor']) end end diff --git a/app/lib/activitypub/activity/undo.rb b/app/lib/activitypub/activity/undo.rb index 078e97ed4..097b1dba4 100644 --- a/app/lib/activitypub/activity/undo.rb +++ b/app/lib/activitypub/activity/undo.rb @@ -64,6 +64,6 @@ class ActivityPub::Activity::Undo < ActivityPub::Activity end def target_uri - @target_uri ||= @object['object'].is_a?(String) ? @object['object'] : @object['object']['id'] + @target_uri ||= value_or_id(@object['object']) end end diff --git a/app/lib/activitypub/tag_manager.rb b/app/lib/activitypub/tag_manager.rb index 855881612..3c16006cb 100644 --- a/app/lib/activitypub/tag_manager.rb +++ b/app/lib/activitypub/tag_manager.rb @@ -93,7 +93,7 @@ class ActivityPub::TagManager elsif ::TagManager.instance.local_id?(uri) klass.find_by(id: ::TagManager.instance.unique_tag_to_local_id(uri, klass.to_s)) else - klass.find_by(uri: uri) + klass.find_by(uri: uri.split('#').first) end end end diff --git a/app/services/activitypub/fetch_remote_key_service.rb b/app/services/activitypub/fetch_remote_key_service.rb new file mode 100644 index 000000000..ebd64071e --- /dev/null +++ b/app/services/activitypub/fetch_remote_key_service.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +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) + + return unless supported_context?(@json) && expected_type? + return find_account(uri, @json) if person? + + @owner = fetch_resource(owner_uri) + + return unless supported_context?(@owner) && confirmed_owner? + + find_account(owner_uri, @owner) + end + + private + + def find_account(uri, prefetched_json) + account = ActivityPub::TagManager.instance.uri_to_resource(uri, Account) + account ||= ActivityPub::FetchRemoteAccountService.new.call(uri, prefetched_json) + account + end + + def expected_type? + person? || public_key? + end + + def person? + @json['type'] == 'Person' + end + + def public_key? + @json['publicKeyPem'].present? && @json['owner'].present? + end + + def owner_uri + @owner_uri ||= value_or_id(@json['owner']) + end + + def confirmed_owner? + @owner['type'] == 'Person' && value_or_id(@owner['publicKey']) == @json['id'] + end +end