From 5bf67ca91350e40e6f329271d3ca2bdcba87ab64 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 11 Jul 2019 20:11:09 +0200 Subject: [PATCH] Add ActivityPub secure mode (#11269) * Add HTTP signature requirement for served ActivityPub resources * Change `SECURE_MODE` to `AUTHORIZED_FETCH` * Add 'Signature' to 'Vary' header and improve code style * Improve code style by adding `public_fetch_mode?` method --- app/controllers/accounts_controller.rb | 13 +++++++-- .../activitypub/collections_controller.rb | 3 ++- .../activitypub/inboxes_controller.rb | 27 +++++++++++-------- .../activitypub/outboxes_controller.rb | 4 +-- .../activitypub/replies_controller.rb | 2 ++ app/controllers/application_controller.rb | 10 ++++++- .../concerns/account_controller_concern.rb | 2 +- .../concerns/signature_verification.rb | 19 ++++++++++--- .../follower_accounts_controller.rb | 12 ++++++--- .../following_accounts_controller.rb | 12 ++++++--- app/controllers/statuses_controller.rb | 9 ++++--- app/controllers/tags_controller.rb | 5 +++- app/lib/activitypub/adapter.rb | 1 + .../activitypub/inboxes_controller_spec.rb | 4 +-- 14 files changed, 89 insertions(+), 34 deletions(-) diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 3184a73cb1..fc913c2ecd 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -4,6 +4,7 @@ class AccountsController < ApplicationController PAGE_SIZE = 20 include AccountControllerConcern + include SignatureAuthentication before_action :set_cache_headers before_action :set_body_classes @@ -39,8 +40,8 @@ class AccountsController < ApplicationController end format.json do - expires_in 3.minutes, public: true - render json: @account, content_type: 'application/activity+json', serializer: ActivityPub::ActorSerializer, adapter: ActivityPub::Adapter + expires_in 3.minutes, public: !(authorized_fetch_mode? && signed_request_account.present?) + render json: @account, content_type: 'application/activity+json', serializer: ActivityPub::ActorSerializer, adapter: ActivityPub::Adapter, fields: restrict_fields_to end end end @@ -132,4 +133,12 @@ class AccountsController < ApplicationController filtered_statuses.paginate_by_max_id(PAGE_SIZE, params[:max_id], params[:since_id]).to_a end end + + def restrict_fields_to + if signed_request_account.present? || public_fetch_mode? + # Return all fields + else + %i(id type preferred_username inbox public_key endpoints) + end + end end diff --git a/app/controllers/activitypub/collections_controller.rb b/app/controllers/activitypub/collections_controller.rb index dd2f111b03..035467f417 100644 --- a/app/controllers/activitypub/collections_controller.rb +++ b/app/controllers/activitypub/collections_controller.rb @@ -4,12 +4,13 @@ class ActivityPub::CollectionsController < Api::BaseController include SignatureVerification include AccountOwnedConcern + before_action :require_signature!, if: :authorized_fetch_mode? before_action :set_size before_action :set_statuses before_action :set_cache_headers def show - expires_in 3.minutes, public: true + expires_in 3.minutes, public: public_fetch_mode? render json: collection_presenter, content_type: 'application/activity+json', serializer: ActivityPub::CollectionSerializer, adapter: ActivityPub::Adapter, skip_activities: true end diff --git a/app/controllers/activitypub/inboxes_controller.rb b/app/controllers/activitypub/inboxes_controller.rb index 9be0676e14..7cfd9a25e1 100644 --- a/app/controllers/activitypub/inboxes_controller.rb +++ b/app/controllers/activitypub/inboxes_controller.rb @@ -5,23 +5,24 @@ class ActivityPub::InboxesController < Api::BaseController include JsonLdHelper include AccountOwnedConcern + before_action :skip_unknown_actor_delete + before_action :require_signature! + def create - if unknown_deleted_account? - head 202 - elsif signed_request_account - upgrade_account - process_payload - head 202 - else - render plain: signature_verification_failure_reason, status: 401 - end + upgrade_account + process_payload + head 202 end private + def skip_unknown_actor_delete + head 202 if unknown_deleted_account? + end + def unknown_deleted_account? json = Oj.load(body, mode: :strict) - json['type'] == 'Delete' && json['actor'].present? && json['actor'] == value_or_id(json['object']) && !Account.where(uri: json['actor']).exists? + json.is_a?(Hash) && json['type'] == 'Delete' && json['actor'].present? && json['actor'] == value_or_id(json['object']) && !Account.where(uri: json['actor']).exists? rescue Oj::ParseError false end @@ -32,8 +33,12 @@ class ActivityPub::InboxesController < Api::BaseController def body return @body if defined?(@body) - @body = request.body.read.force_encoding('UTF-8') + + @body = request.body.read + @body.force_encoding('UTF-8') if @body.present? + request.body.rewind if request.body.respond_to?(:rewind) + @body end diff --git a/app/controllers/activitypub/outboxes_controller.rb b/app/controllers/activitypub/outboxes_controller.rb index 4c0b769f0f..cdfd28ba84 100644 --- a/app/controllers/activitypub/outboxes_controller.rb +++ b/app/controllers/activitypub/outboxes_controller.rb @@ -6,12 +6,12 @@ class ActivityPub::OutboxesController < Api::BaseController include SignatureVerification include AccountOwnedConcern + before_action :require_signature!, if: :authorized_fetch_mode? before_action :set_statuses before_action :set_cache_headers def show - expires_in 1.minute, public: true unless page_requested? - + expires_in(page_requested? ? 0 : 3.minutes, public: public_fetch_mode?) render json: outbox_presenter, serializer: ActivityPub::OutboxSerializer, adapter: ActivityPub::Adapter, content_type: 'application/activity+json' end diff --git a/app/controllers/activitypub/replies_controller.rb b/app/controllers/activitypub/replies_controller.rb index 99b7b310f4..020c077ab0 100644 --- a/app/controllers/activitypub/replies_controller.rb +++ b/app/controllers/activitypub/replies_controller.rb @@ -7,11 +7,13 @@ class ActivityPub::RepliesController < Api::BaseController DESCENDANTS_LIMIT = 60 + before_action :require_signature!, if: :authorized_fetch_mode? before_action :set_status before_action :set_cache_headers before_action :set_replies def index + expires_in 0, public: public_fetch_mode? render json: replies_collection_presenter, serializer: ActivityPub::CollectionSerializer, adapter: ActivityPub::Adapter, content_type: 'application/activity+json', skip_activities: true end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index cc8b8e4da6..16e7d70a37 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -36,6 +36,14 @@ class ApplicationController < ActionController::Base Rails.env.production? end + def authorized_fetch_mode? + ENV['AUTHORIZED_FETCH'] == 'true' + end + + def public_fetch_mode? + !authorized_fetch_mode? + end + def store_current_location store_location_for(:user, request.url) unless request.format == :json end @@ -152,6 +160,6 @@ class ApplicationController < ActionController::Base end def set_cache_headers - response.headers['Vary'] = 'Accept' + response.headers['Vary'] = 'Accept, Signature' end end diff --git a/app/controllers/concerns/account_controller_concern.rb b/app/controllers/concerns/account_controller_concern.rb index 287a930da4..11eac0eb6b 100644 --- a/app/controllers/concerns/account_controller_concern.rb +++ b/app/controllers/concerns/account_controller_concern.rb @@ -11,7 +11,7 @@ module AccountControllerConcern layout 'public' before_action :set_instance_presenter - before_action :set_link_headers + before_action :set_link_headers, if: -> { request.format.nil? || request.format == :html } end private diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb index 0ccdf5ec98..7b251cf804 100644 --- a/app/controllers/concerns/signature_verification.rb +++ b/app/controllers/concerns/signature_verification.rb @@ -7,12 +7,20 @@ module SignatureVerification include DomainControlHelper + def require_signature! + render plain: signature_verification_failure_reason, status: signature_verification_failure_code unless signed_request_account + end + def signed_request? request.headers['Signature'].present? end def signature_verification_failure_reason - return @signature_verification_failure_reason if defined?(@signature_verification_failure_reason) + @signature_verification_failure_reason + end + + def signature_verification_failure_code + @signature_verification_failure_code || 401 end def signed_request_account @@ -125,11 +133,16 @@ module SignatureVerification end def account_from_key_id(key_id) + domain = key_id.start_with?('acct:') ? key_id.split('@').last : key_id + + if domain_not_allowed?(domain) + @signature_verification_failure_code = 403 + return + end + if key_id.start_with?('acct:') stoplight_wrap_request { ResolveAccountService.new.call(key_id.gsub(/\Aacct:/, '')) } elsif !ActivityPub::TagManager.instance.local_uri?(key_id) - return if domain_not_allowed?(key_id) - account = ActivityPub::TagManager.instance.uri_to_resource(key_id, Account) account ||= stoplight_wrap_request { ActivityPub::FetchRemoteKeyService.new.call(key_id, id: false) } account diff --git a/app/controllers/follower_accounts_controller.rb b/app/controllers/follower_accounts_controller.rb index 8baa64490f..6e873de5b6 100644 --- a/app/controllers/follower_accounts_controller.rb +++ b/app/controllers/follower_accounts_controller.rb @@ -2,7 +2,9 @@ class FollowerAccountsController < ApplicationController include AccountControllerConcern + include SignatureVerification + before_action :require_signature!, if: -> { request.format == :json && authorized_fetch_mode? } before_action :set_cache_headers def index @@ -17,9 +19,9 @@ class FollowerAccountsController < ApplicationController end format.json do - raise Mastodon::NotPermittedError if params[:page].present? && @account.user_hides_network? + raise Mastodon::NotPermittedError if page_requested? && @account.user_hides_network? - expires_in 3.minutes, public: true if params[:page].blank? + expires_in(page_requested? ? 0 : 3.minutes, public: public_fetch_mode?) render json: collection_presenter, serializer: ActivityPub::CollectionSerializer, @@ -35,12 +37,16 @@ class FollowerAccountsController < ApplicationController @follows ||= Follow.where(target_account: @account).recent.page(params[:page]).per(FOLLOW_PER_PAGE).preload(:account) end + def page_requested? + params[:page].present? + end + def page_url(page) account_followers_url(@account, page: page) unless page.nil? end def collection_presenter - if params[:page].present? + if page_requested? ActivityPub::CollectionPresenter.new( id: account_followers_url(@account, page: params.fetch(:page, 1)), type: :ordered, diff --git a/app/controllers/following_accounts_controller.rb b/app/controllers/following_accounts_controller.rb index 4d1ea4594e..07d62f7dd1 100644 --- a/app/controllers/following_accounts_controller.rb +++ b/app/controllers/following_accounts_controller.rb @@ -2,7 +2,9 @@ class FollowingAccountsController < ApplicationController include AccountControllerConcern + include SignatureVerification + before_action :require_signature!, if: -> { request.format == :json && authorized_fetch_mode? } before_action :set_cache_headers def index @@ -17,9 +19,9 @@ class FollowingAccountsController < ApplicationController end format.json do - raise Mastodon::NotPermittedError if params[:page].present? && @account.user_hides_network? + raise Mastodon::NotPermittedError if page_requested? && @account.user_hides_network? - expires_in 3.minutes, public: true if params[:page].blank? + expires_in(page_requested? ? 0 : 3.minutes, public: public_fetch_mode?) render json: collection_presenter, serializer: ActivityPub::CollectionSerializer, @@ -35,12 +37,16 @@ class FollowingAccountsController < ApplicationController @follows ||= Follow.where(account: @account).recent.page(params[:page]).per(FOLLOW_PER_PAGE).preload(:target_account) end + def page_requested? + params[:page].present? + end + def page_url(page) account_following_index_url(@account, page: page) unless page.nil? end def collection_presenter - if params[:page].present? + if page_requested? ActivityPub::CollectionPresenter.new( id: account_following_index_url(@account, page: params.fetch(:page, 1)), type: :ordered, diff --git a/app/controllers/statuses_controller.rb b/app/controllers/statuses_controller.rb index 13ce5c691a..22e7519f9a 100644 --- a/app/controllers/statuses_controller.rb +++ b/app/controllers/statuses_controller.rb @@ -8,11 +8,12 @@ class StatusesController < ApplicationController layout 'public' + before_action :require_signature!, only: :show, if: -> { request.format == :json && authorized_fetch_mode? } before_action :set_status before_action :set_instance_presenter before_action :set_link_headers - before_action :redirect_to_original, only: [:show] - before_action :set_referrer_policy_header, only: [:show] + before_action :redirect_to_original, only: :show + before_action :set_referrer_policy_header, only: :show before_action :set_cache_headers before_action :set_body_classes before_action :set_autoplay, only: :embed @@ -30,14 +31,14 @@ class StatusesController < ApplicationController end format.json do - expires_in 3.minutes, public: @status.distributable? + expires_in 3.minutes, public: @status.distributable? && public_fetch_mode? render json: @status, content_type: 'application/activity+json', serializer: ActivityPub::NoteSerializer, adapter: ActivityPub::Adapter end end end def activity - expires_in 3.minutes, public: @status.distributable? + expires_in 3.minutes, public: @status.distributable? && public_fetch_mode? render json: @status, content_type: 'application/activity+json', serializer: ActivityPub::ActivitySerializer, adapter: ActivityPub::Adapter end diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index 2ecce0ca22..d08e5a61a5 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -1,10 +1,13 @@ # frozen_string_literal: true class TagsController < ApplicationController + include SignatureVerification + PAGE_SIZE = 20 layout 'public' + before_action :require_signature!, if: -> { request.format == :json && authorized_fetch_mode? } before_action :set_tag before_action :set_body_classes before_action :set_instance_presenter @@ -30,7 +33,7 @@ class TagsController < ApplicationController end format.json do - expires_in 3.minutes, public: true + expires_in 3.minutes, public: public_fetch_mode? @statuses = HashtagQueryService.new.call(@tag, params.slice(:any, :all, :none), current_account, params[:local]).paginate_by_max_id(PAGE_SIZE, params[:max_id]) @statuses = cache_collection(@statuses, Status) diff --git a/app/lib/activitypub/adapter.rb b/app/lib/activitypub/adapter.rb index c259c96f41..a1d84de2fb 100644 --- a/app/lib/activitypub/adapter.rb +++ b/app/lib/activitypub/adapter.rb @@ -33,6 +33,7 @@ class ActivityPub::Adapter < ActiveModelSerializers::Adapter::Base def serializable_hash(options = nil) options = serialization_options(options) serialized_hash = serializer.serializable_hash(options) + serialized_hash = serialized_hash.select { |k, _| options[:fields].include?(k) } if options[:fields] serialized_hash = self.class.transform_key_casing!(serialized_hash, instance_options) { '@context' => serialized_context }.merge(serialized_hash) diff --git a/spec/controllers/activitypub/inboxes_controller_spec.rb b/spec/controllers/activitypub/inboxes_controller_spec.rb index eab4b8c3e6..a9ee754900 100644 --- a/spec/controllers/activitypub/inboxes_controller_spec.rb +++ b/spec/controllers/activitypub/inboxes_controller_spec.rb @@ -4,7 +4,7 @@ require 'rails_helper' RSpec.describe ActivityPub::InboxesController, type: :controller do describe 'POST #create' do - context 'if signed_request_account' do + context 'with signed_request_account' do it 'returns 202' do allow(controller).to receive(:signed_request_account) do Fabricate(:account) @@ -15,7 +15,7 @@ RSpec.describe ActivityPub::InboxesController, type: :controller do end end - context 'not signed_request_account' do + context 'without signed_request_account' do it 'returns 401' do allow(controller).to receive(:signed_request_account) do false