From 7d985f2aac639dc5fae528db2dbc4422ca10f276 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 8 Oct 2020 00:34:57 +0200 Subject: Remove dependency on goldfinger gem (#14919) There are edge cases where requests to certain hosts timeout when using the vanilla HTTP.rb gem, which the goldfinger gem uses. Now that we no longer need to support OStatus servers, webfinger logic is so simple that there is no point encapsulating it in a gem, so we can just use our own Request class. With that, we benefit from more robust timeout code and IPv4/IPv6 resolution. Fix #14091 --- spec/controllers/remote_follow_controller_spec.rb | 10 ++++------ spec/controllers/well_known/host_meta_controller_spec.rb | 2 +- spec/lib/feed_manager_spec.rb | 1 + 3 files changed, 6 insertions(+), 7 deletions(-) (limited to 'spec') diff --git a/spec/controllers/remote_follow_controller_spec.rb b/spec/controllers/remote_follow_controller_spec.rb index 3ef8f14d9..7312dde58 100644 --- a/spec/controllers/remote_follow_controller_spec.rb +++ b/spec/controllers/remote_follow_controller_spec.rb @@ -43,8 +43,7 @@ describe RemoteFollowController do end it 'renders new when template is nil' do - link_with_nil_template = double(template: nil) - resource_with_link = double(link: link_with_nil_template) + resource_with_link = double(link: nil) allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_return(resource_with_link) post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } } @@ -55,8 +54,7 @@ describe RemoteFollowController do context 'when webfinger values are good' do before do - link_with_template = double(template: 'http://example.com/follow_me?acct={uri}') - resource_with_link = double(link: link_with_template) + resource_with_link = double(link: 'http://example.com/follow_me?acct={uri}') allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_return(resource_with_link) post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } } end @@ -78,8 +76,8 @@ describe RemoteFollowController do expect(response).to render_template(:new) end - it 'renders new with error when goldfinger fails' do - allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_raise(Goldfinger::Error) + it 'renders new with error when webfinger fails' do + allow_any_instance_of(WebfingerHelper).to receive(:webfinger!).with('acct:user@example.com').and_raise(Webfinger::Error) post :create, params: { account_username: @account.to_param, remote_follow: { acct: 'user@example.com' } } expect(response).to render_template(:new) diff --git a/spec/controllers/well_known/host_meta_controller_spec.rb b/spec/controllers/well_known/host_meta_controller_spec.rb index b43ae19d8..643ba9cd3 100644 --- a/spec/controllers/well_known/host_meta_controller_spec.rb +++ b/spec/controllers/well_known/host_meta_controller_spec.rb @@ -12,7 +12,7 @@ describe WellKnown::HostMetaController, type: :controller do expect(response.body).to eq < - + XML end diff --git a/spec/lib/feed_manager_spec.rb b/spec/lib/feed_manager_spec.rb index d9c17470f..7d775a86d 100644 --- a/spec/lib/feed_manager_spec.rb +++ b/spec/lib/feed_manager_spec.rb @@ -108,6 +108,7 @@ RSpec.describe FeedManager do it 'returns false for status by followee mentioning another account' do bob.follow!(alice) + jeff.follow!(alice) status = PostStatusService.new.call(alice, text: 'Hey @jeff') expect(FeedManager.instance.filter?(:home, status, bob)).to be false end -- cgit From 5e1364c448222c964faa469b6b5bfe9adf701c1a Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 12 Oct 2020 16:33:49 +0200 Subject: Add IP-based rules (#14963) --- app/controllers/admin/ip_blocks_controller.rb | 56 +++++++++ app/controllers/api/v1/accounts_controller.rb | 2 +- app/controllers/auth/registrations_controller.rb | 6 +- app/helpers/admin/action_logs_helper.rb | 4 + app/lib/fast_ip_map.rb | 32 +++++ app/models/concerns/expireable.rb | 10 +- app/models/form/ip_block_batch.rb | 31 +++++ app/models/ip_block.rb | 41 +++++++ app/models/user.rb | 16 ++- app/policies/ip_block_policy.rb | 15 +++ app/services/app_sign_up_service.rb | 4 +- app/views/admin/ip_blocks/_ip_block.html.haml | 11 ++ app/views/admin/ip_blocks/index.html.haml | 28 +++++ app/views/admin/ip_blocks/new.html.haml | 20 ++++ .../admin/pending_accounts/_account.html.haml | 2 +- app/workers/scheduler/ip_cleanup_scheduler.rb | 18 ++- config/initializers/rack_attack.rb | 4 + config/locales/en.yml | 19 +++ config/locales/simple_form.en.yml | 15 +++ config/navigation.rb | 1 + config/routes.rb | 6 + db/migrate/20201008202037_create_ip_blocks.rb | 12 ++ .../20201008220312_add_sign_up_ip_to_users.rb | 5 + db/schema.rb | 12 +- lib/cli.rb | 4 + lib/mastodon/ip_blocks_cli.rb | 132 +++++++++++++++++++++ spec/fabricators/ip_block_fabricator.rb | 6 + spec/lib/fast_ip_map_spec.rb | 21 ++++ spec/models/ip_block_spec.rb | 5 + spec/services/app_sign_up_service_spec.rb | 13 +- 30 files changed, 530 insertions(+), 21 deletions(-) create mode 100644 app/controllers/admin/ip_blocks_controller.rb create mode 100644 app/lib/fast_ip_map.rb create mode 100644 app/models/form/ip_block_batch.rb create mode 100644 app/models/ip_block.rb create mode 100644 app/policies/ip_block_policy.rb create mode 100644 app/views/admin/ip_blocks/_ip_block.html.haml create mode 100644 app/views/admin/ip_blocks/index.html.haml create mode 100644 app/views/admin/ip_blocks/new.html.haml create mode 100644 db/migrate/20201008202037_create_ip_blocks.rb create mode 100644 db/migrate/20201008220312_add_sign_up_ip_to_users.rb create mode 100644 lib/mastodon/ip_blocks_cli.rb create mode 100644 spec/fabricators/ip_block_fabricator.rb create mode 100644 spec/lib/fast_ip_map_spec.rb create mode 100644 spec/models/ip_block_spec.rb (limited to 'spec') diff --git a/app/controllers/admin/ip_blocks_controller.rb b/app/controllers/admin/ip_blocks_controller.rb new file mode 100644 index 000000000..92b8b0d2b --- /dev/null +++ b/app/controllers/admin/ip_blocks_controller.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module Admin + class IpBlocksController < BaseController + def index + authorize :ip_block, :index? + + @ip_blocks = IpBlock.page(params[:page]) + @form = Form::IpBlockBatch.new + end + + def new + authorize :ip_block, :create? + + @ip_block = IpBlock.new(ip: '', severity: :no_access, expires_in: 1.year) + end + + def create + authorize :ip_block, :create? + + @ip_block = IpBlock.new(resource_params) + + if @ip_block.save + log_action :create, @ip_block + redirect_to admin_ip_blocks_path, notice: I18n.t('admin.ip_blocks.created_msg') + else + render :new + end + end + + def batch + @form = Form::IpBlockBatch.new(form_ip_block_batch_params.merge(current_account: current_account, action: action_from_button)) + @form.save + rescue ActionController::ParameterMissing + flash[:alert] = I18n.t('admin.ip_blocks.no_ip_block_selected') + rescue Mastodon::NotPermittedError + flash[:alert] = I18n.t('admin.custom_emojis.not_permitted') + ensure + redirect_to admin_ip_blocks_path + end + + private + + def resource_params + params.require(:ip_block).permit(:ip, :severity, :comment, :expires_in) + end + + def action_from_button + 'delete' if params[:delete] + end + + def form_ip_block_batch_params + params.require(:form_ip_block_batch).permit(ip_block_ids: []) + end + end +end diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index aef51a647..4a97f0251 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -20,7 +20,7 @@ class Api::V1::AccountsController < Api::BaseController end def create - token = AppSignUpService.new.call(doorkeeper_token.application, account_params) + token = AppSignUpService.new.call(doorkeeper_token.application, request.remote_ip, account_params) response = Doorkeeper::OAuth::TokenResponse.new(token) headers.merge!(response.headers) diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb index d31966248..eb0924190 100644 --- a/app/controllers/auth/registrations_controller.rb +++ b/app/controllers/auth/registrations_controller.rb @@ -45,9 +45,9 @@ class Auth::RegistrationsController < Devise::RegistrationsController def build_resource(hash = nil) super(hash) - resource.locale = I18n.locale - resource.invite_code = params[:invite_code] if resource.invite_code.blank? - resource.current_sign_in_ip = request.remote_ip + resource.locale = I18n.locale + resource.invite_code = params[:invite_code] if resource.invite_code.blank? + resource.sign_up_ip = request.remote_ip resource.build_account if resource.account.nil? end diff --git a/app/helpers/admin/action_logs_helper.rb b/app/helpers/admin/action_logs_helper.rb index 8e398c3b2..0f3ca36e2 100644 --- a/app/helpers/admin/action_logs_helper.rb +++ b/app/helpers/admin/action_logs_helper.rb @@ -29,6 +29,8 @@ module Admin::ActionLogsHelper link_to record.target_account.acct, admin_account_path(record.target_account_id) when 'Announcement' link_to truncate(record.text), edit_admin_announcement_path(record.id) + when 'IpBlock' + "#{record.ip}/#{record.ip.prefix} (#{I18n.t("simple_form.labels.ip_block.severities.#{record.severity}")})" end end @@ -48,6 +50,8 @@ module Admin::ActionLogsHelper end when 'Announcement' truncate(attributes['text'].is_a?(Array) ? attributes['text'].last : attributes['text']) + when 'IpBlock' + "#{attributes['ip']}/#{attributes['ip'].prefix} (#{I18n.t("simple_form.labels.ip_block.severities.#{attributes['severity']}")})" end end end diff --git a/app/lib/fast_ip_map.rb b/app/lib/fast_ip_map.rb new file mode 100644 index 000000000..ba30b45f3 --- /dev/null +++ b/app/lib/fast_ip_map.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +class FastIpMap + MAX_IPV4_PREFIX = 32 + MAX_IPV6_PREFIX = 128 + + # @param [Enumerable] addresses + def initialize(addresses) + @fast_lookup = {} + @ranges = [] + + # Hash look-up is faster but only works for exact matches, so we split + # exact addresses from non-exact ones + addresses.each do |address| + if (address.ipv4? && address.prefix == MAX_IPV4_PREFIX) || (address.ipv6? && address.prefix == MAX_IPV6_PREFIX) + @fast_lookup[address.to_s] = true + else + @ranges << address + end + end + + # We're more likely to hit wider-reaching ranges when checking for + # inclusion, so make sure they're sorted first + @ranges.sort_by!(&:prefix) + end + + # @param [IPAddr] address + # @return [Boolean] + def include?(address) + @fast_lookup[address.to_s] || @ranges.any? { |cidr| cidr.include?(address) } + end +end diff --git a/app/models/concerns/expireable.rb b/app/models/concerns/expireable.rb index f7d2bab49..a66a4661b 100644 --- a/app/models/concerns/expireable.rb +++ b/app/models/concerns/expireable.rb @@ -6,7 +6,15 @@ module Expireable included do scope :expired, -> { where.not(expires_at: nil).where('expires_at < ?', Time.now.utc) } - attr_reader :expires_in + def expires_in + return @expires_in if defined?(@expires_in) + + if expires_at.nil? + nil + else + (expires_at - created_at).to_i + end + end def expires_in=(interval) self.expires_at = interval.to_i.seconds.from_now if interval.present? diff --git a/app/models/form/ip_block_batch.rb b/app/models/form/ip_block_batch.rb new file mode 100644 index 000000000..f6fe9b593 --- /dev/null +++ b/app/models/form/ip_block_batch.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +class Form::IpBlockBatch + include ActiveModel::Model + include Authorization + include AccountableConcern + + attr_accessor :ip_block_ids, :action, :current_account + + def save + case action + when 'delete' + delete! + end + end + + private + + def ip_blocks + @ip_blocks ||= IpBlock.where(id: ip_block_ids) + end + + def delete! + ip_blocks.each { |ip_block| authorize(ip_block, :destroy?) } + + ip_blocks.each do |ip_block| + ip_block.destroy + log_action :destroy, ip_block + end + end +end diff --git a/app/models/ip_block.rb b/app/models/ip_block.rb new file mode 100644 index 000000000..aedd3ca0d --- /dev/null +++ b/app/models/ip_block.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: ip_blocks +# +# id :bigint(8) not null, primary key +# created_at :datetime not null +# updated_at :datetime not null +# expires_at :datetime +# ip :inet default(#), not null +# severity :integer default(NULL), not null +# comment :text default(""), not null +# + +class IpBlock < ApplicationRecord + CACHE_KEY = 'blocked_ips' + + include Expireable + + enum severity: { + sign_up_requires_approval: 5000, + no_access: 9999, + } + + validates :ip, :severity, presence: true + + after_commit :reset_cache + + class << self + def blocked?(remote_ip) + blocked_ips_map = Rails.cache.fetch(CACHE_KEY) { FastIpMap.new(IpBlock.where(severity: :no_access).pluck(:ip)) } + blocked_ips_map.include?(remote_ip) + end + end + + private + + def reset_cache + Rails.cache.delete(CACHE_KEY) + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 2c460e1fd..7c8124fed 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -41,6 +41,7 @@ # sign_in_token :string # sign_in_token_sent_at :datetime # webauthn_id :string +# sign_up_ip :inet # class User < ApplicationRecord @@ -97,7 +98,7 @@ class User < ApplicationRecord scope :inactive, -> { where(arel_table[:current_sign_in_at].lt(ACTIVE_DURATION.ago)) } scope :active, -> { confirmed.where(arel_table[:current_sign_in_at].gteq(ACTIVE_DURATION.ago)).joins(:account).where(accounts: { suspended_at: nil }) } scope :matches_email, ->(value) { where(arel_table[:email].matches("#{value}%")) } - scope :matches_ip, ->(value) { left_joins(:session_activations).where('users.current_sign_in_ip <<= ?', value).or(left_joins(:session_activations).where('users.last_sign_in_ip <<= ?', value)).or(left_joins(:session_activations).where('session_activations.ip <<= ?', value)) } + scope :matches_ip, ->(value) { left_joins(:session_activations).where('users.current_sign_in_ip <<= ?', value).or(left_joins(:session_activations).where('users.sign_up_ip <<= ?', value)).or(left_joins(:session_activations).where('users.last_sign_in_ip <<= ?', value)).or(left_joins(:session_activations).where('session_activations.ip <<= ?', value)) } scope :emailable, -> { confirmed.enabled.joins(:account).merge(Account.searchable) } before_validation :sanitize_languages @@ -331,6 +332,7 @@ class User < ApplicationRecord arr << [current_sign_in_at, current_sign_in_ip] if current_sign_in_ip.present? arr << [last_sign_in_at, last_sign_in_ip] if last_sign_in_ip.present? + arr << [created_at, sign_up_ip] if sign_up_ip.present? arr.sort_by { |pair| pair.first || Time.now.utc }.uniq(&:last).reverse! end @@ -385,7 +387,17 @@ class User < ApplicationRecord end def set_approved - self.approved = open_registrations? || valid_invitation? || external? + self.approved = begin + if sign_up_from_ip_requires_approval? + false + else + open_registrations? || valid_invitation? || external? + end + end + end + + def sign_up_from_ip_requires_approval? + !sign_up_ip.nil? && IpBlock.where(severity: :sign_up_requires_approval).where('ip >>= ?', sign_up_ip.to_s).exists? end def open_registrations? diff --git a/app/policies/ip_block_policy.rb b/app/policies/ip_block_policy.rb new file mode 100644 index 000000000..34dbd746a --- /dev/null +++ b/app/policies/ip_block_policy.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class IpBlockPolicy < ApplicationPolicy + def index? + admin? + end + + def create? + admin? + end + + def destroy? + admin? + end +end diff --git a/app/services/app_sign_up_service.rb b/app/services/app_sign_up_service.rb index c9739c77d..e00694157 100644 --- a/app/services/app_sign_up_service.rb +++ b/app/services/app_sign_up_service.rb @@ -1,13 +1,13 @@ # frozen_string_literal: true class AppSignUpService < BaseService - def call(app, params) + def call(app, remote_ip, params) return unless allowed_registrations? user_params = params.slice(:email, :password, :agreement, :locale) account_params = params.slice(:username) invite_request_params = { text: params[:reason] } - user = User.create!(user_params.merge(created_by_application: app, password_confirmation: user_params[:password], account_attributes: account_params, invite_request_attributes: invite_request_params)) + user = User.create!(user_params.merge(created_by_application: app, sign_up_ip: remote_ip, password_confirmation: user_params[:password], account_attributes: account_params, invite_request_attributes: invite_request_params)) Doorkeeper::AccessToken.create!(application: app, resource_owner_id: user.id, diff --git a/app/views/admin/ip_blocks/_ip_block.html.haml b/app/views/admin/ip_blocks/_ip_block.html.haml new file mode 100644 index 000000000..e07e2b444 --- /dev/null +++ b/app/views/admin/ip_blocks/_ip_block.html.haml @@ -0,0 +1,11 @@ +.batch-table__row + %label.batch-table__row__select.batch-table__row__select--aligned.batch-checkbox + = f.check_box :ip_block_ids, { multiple: true, include_hidden: false }, ip_block.id + .batch-table__row__content + .batch-table__row__content__text + %samp= "#{ip_block.ip}/#{ip_block.ip.prefix}" + - if ip_block.comment.present? + • + = ip_block.comment + %br/ + = t("simple_form.labels.ip_block.severities.#{ip_block.severity}") diff --git a/app/views/admin/ip_blocks/index.html.haml b/app/views/admin/ip_blocks/index.html.haml new file mode 100644 index 000000000..a282a4cfe --- /dev/null +++ b/app/views/admin/ip_blocks/index.html.haml @@ -0,0 +1,28 @@ +- content_for :page_title do + = t('admin.ip_blocks.title') + +- content_for :header_tags do + = javascript_pack_tag 'admin', integrity: true, async: true, crossorigin: 'anonymous' + +- if can?(:create, :ip_block) + - content_for :heading_actions do + = link_to t('admin.ip_blocks.add_new'), new_admin_ip_block_path, class: 'button' + += form_for(@form, url: batch_admin_ip_blocks_path) do |f| + = hidden_field_tag :page, params[:page] || 1 + + .batch-table + .batch-table__toolbar + %label.batch-table__toolbar__select.batch-checkbox-all + = check_box_tag :batch_checkbox_all, nil, false + .batch-table__toolbar__actions + - if can?(:destroy, :ip_block) + = f.button safe_join([fa_icon('times'), t('admin.ip_blocks.delete')]), name: :delete, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') } + .batch-table__body + - if @ip_blocks.empty? + = nothing_here 'nothing-here--under-tabs' + - else + = render partial: 'ip_block', collection: @ip_blocks, locals: { f: f } + += paginate @ip_blocks + diff --git a/app/views/admin/ip_blocks/new.html.haml b/app/views/admin/ip_blocks/new.html.haml new file mode 100644 index 000000000..69f6b98b9 --- /dev/null +++ b/app/views/admin/ip_blocks/new.html.haml @@ -0,0 +1,20 @@ +- content_for :page_title do + = t('.title') + += simple_form_for @ip_block, url: admin_ip_blocks_path do |f| + = render 'shared/error_messages', object: @ip_block + + .fields-group + = f.input :ip, as: :string, wrapper: :with_block_label, input_html: { placeholder: '192.0.2.0/24' } + + .fields-group + = f.input :expires_in, wrapper: :with_block_label, collection: [1.day, 2.weeks, 1.month, 6.months, 1.year, 3.years].map(&:to_i), label_method: lambda { |i| I18n.t("admin.ip_blocks.expires_in.#{i}") }, prompt: I18n.t('invites.expires_in_prompt') + + .fields-group + = f.input :severity, as: :radio_buttons, collection: IpBlock.severities.keys, include_blank: false, wrapper: :with_block_label, label_method: lambda { |severity| safe_join([I18n.t("simple_form.labels.ip_block.severities.#{severity}"), content_tag(:span, I18n.t("simple_form.hints.ip_block.severities.#{severity}"), class: 'hint')]) } + + .fields-group + = f.input :comment, as: :string, wrapper: :with_block_label + + .actions + = f.button :button, t('admin.ip_blocks.add_new'), type: :submit diff --git a/app/views/admin/pending_accounts/_account.html.haml b/app/views/admin/pending_accounts/_account.html.haml index 7a9796a67..5b475b59a 100644 --- a/app/views/admin/pending_accounts/_account.html.haml +++ b/app/views/admin/pending_accounts/_account.html.haml @@ -7,7 +7,7 @@ %strong= account.user_email = "(@#{account.username})" %br/ - = account.user_current_sign_in_ip + %samp= account.user_current_sign_in_ip • = t 'admin.accounts.time_in_queue', time: time_ago_in_words(account.user&.created_at) diff --git a/app/workers/scheduler/ip_cleanup_scheduler.rb b/app/workers/scheduler/ip_cleanup_scheduler.rb index 6d38b52a2..853f20e25 100644 --- a/app/workers/scheduler/ip_cleanup_scheduler.rb +++ b/app/workers/scheduler/ip_cleanup_scheduler.rb @@ -3,13 +3,23 @@ class Scheduler::IpCleanupScheduler include Sidekiq::Worker - RETENTION_PERIOD = 1.year + IP_RETENTION_PERIOD = 1.year.freeze sidekiq_options lock: :until_executed, retry: 0 def perform - time_ago = RETENTION_PERIOD.ago - SessionActivation.where('updated_at < ?', time_ago).in_batches.destroy_all - User.where('last_sign_in_at < ?', time_ago).where.not(last_sign_in_ip: nil).in_batches.update_all(last_sign_in_ip: nil) + clean_ip_columns! + clean_expired_ip_blocks! + end + + private + + def clean_ip_columns! + SessionActivation.where('updated_at < ?', IP_RETENTION_PERIOD.ago).in_batches.destroy_all + User.where('current_sign_in_at < ?', IP_RETENTION_PERIOD.ago).in_batches.update_all(last_sign_in_ip: nil, current_sign_in_ip: nil, sign_up_ip: nil) + end + + def clean_expired_ip_blocks! + IpBlock.expired.in_batches.destroy_all end end diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index cd29afac5..6662ef40b 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -42,6 +42,10 @@ class Rack::Attack req.remote_ip == '127.0.0.1' || req.remote_ip == '::1' end + Rack::Attack.blocklist('deny from blocklist') do |req| + IpBlock.blocked?(req.remote_ip) + end + throttle('throttle_authenticated_api', limit: 300, period: 5.minutes) do |req| req.authenticated_user_id if req.api_request? end diff --git a/config/locales/en.yml b/config/locales/en.yml index 427b2c3fc..084006a2a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -223,12 +223,14 @@ en: create_domain_allow: Create Domain Allow create_domain_block: Create Domain Block create_email_domain_block: Create E-mail Domain Block + create_ip_block: Create IP rule demote_user: Demote User destroy_announcement: Delete Announcement destroy_custom_emoji: Delete Custom Emoji destroy_domain_allow: Delete Domain Allow destroy_domain_block: Delete Domain Block destroy_email_domain_block: Delete e-mail domain block + destroy_ip_block: Delete IP rule destroy_status: Delete Status disable_2fa_user: Disable 2FA disable_custom_emoji: Disable Custom Emoji @@ -259,12 +261,14 @@ en: create_domain_allow: "%{name} allowed federation with domain %{target}" create_domain_block: "%{name} blocked domain %{target}" create_email_domain_block: "%{name} blocked e-mail domain %{target}" + create_ip_block: "%{name} created rule for IP %{target}" demote_user: "%{name} demoted user %{target}" destroy_announcement: "%{name} deleted announcement %{target}" destroy_custom_emoji: "%{name} destroyed emoji %{target}" destroy_domain_allow: "%{name} disallowed federation with domain %{target}" destroy_domain_block: "%{name} unblocked domain %{target}" destroy_email_domain_block: "%{name} unblocked e-mail domain %{target}" + destroy_ip_block: "%{name} deleted rule for IP %{target}" destroy_status: "%{name} removed status by %{target}" disable_2fa_user: "%{name} disabled two factor requirement for user %{target}" disable_custom_emoji: "%{name} disabled emoji %{target}" @@ -449,6 +453,21 @@ en: expired: Expired title: Filter title: Invites + ip_blocks: + add_new: Create rule + created_msg: Successfully added new IP rule + delete: Delete + expires_in: + '1209600': 2 weeks + '15778476': 6 months + '2629746': 1 month + '31556952': 1 year + '86400': 1 day + '94670856': 3 years + new: + title: Create new IP rule + no_ip_block_selected: No IP rules were changed as none were selected + title: IP rules pending_accounts: title: Pending accounts (%{count}) relationships: diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml index 9b0af6d24..b69487953 100644 --- a/config/locales/simple_form.en.yml +++ b/config/locales/simple_form.en.yml @@ -65,6 +65,14 @@ en: data: CSV file exported from another Mastodon server invite_request: text: This will help us review your application + ip_block: + comment: Optional. Remember why you added this rule. + expires_in: IP addresses are a finite resource, they are sometimes shared and often change hands. For this reason, indefinite IP blocks are not recommended. + ip: Enter an IPv4 or IPv6 address. You can block entire ranges using the CIDR syntax. Be careful not to lock yourself out! + severities: + no_access: Block access to all resources + sign_up_requires_approval: New sign-ups will require your approval + severity: Choose what will happen with requests from this IP sessions: otp: 'Enter the two-factor code generated by your phone app or use one of your recovery codes:' webauthn: If it's an USB key be sure to insert it and, if necessary, tap it. @@ -170,6 +178,13 @@ en: comment: Comment invite_request: text: Why do you want to join? + ip_block: + comment: Comment + ip: IP + severities: + no_access: Block access + sign_up_requires_approval: Limit sign-ups + severity: Rule notification_emails: digest: Send digest e-mails favourite: Someone favourited your status diff --git a/config/navigation.rb b/config/navigation.rb index c113a3c3e..4a56abe18 100644 --- a/config/navigation.rb +++ b/config/navigation.rb @@ -41,6 +41,7 @@ SimpleNavigation::Configuration.run do |navigation| s.item :tags, safe_join([fa_icon('hashtag fw'), t('admin.tags.title')]), admin_tags_path, highlights_on: %r{/admin/tags} s.item :instances, safe_join([fa_icon('cloud fw'), t('admin.instances.title')]), admin_instances_url(limited: whitelist_mode? ? nil : '1'), highlights_on: %r{/admin/instances|/admin/domain_blocks|/admin/domain_allows}, if: -> { current_user.admin? } s.item :email_domain_blocks, safe_join([fa_icon('envelope fw'), t('admin.email_domain_blocks.title')]), admin_email_domain_blocks_url, highlights_on: %r{/admin/email_domain_blocks}, if: -> { current_user.admin? } + s.item :ip_blocks, safe_join([fa_icon('ban fw'), t('admin.ip_blocks.title')]), admin_ip_blocks_url, highlights_on: %r{/admin/ip_blocks}, if: -> { current_user.admin? } end n.item :admin, safe_join([fa_icon('cogs fw'), t('admin.title')]), admin_dashboard_url, if: proc { current_user.staff? } do |s| diff --git a/config/routes.rb b/config/routes.rb index 8d9bc317b..a21dbd45e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -283,6 +283,12 @@ Rails.application.routes.draw do end end + resources :ip_blocks, only: [:index, :new, :create] do + collection do + post :batch + end + end + resources :account_moderation_notes, only: [:create, :destroy] resources :tags, only: [:index, :show, :update] do diff --git a/db/migrate/20201008202037_create_ip_blocks.rb b/db/migrate/20201008202037_create_ip_blocks.rb new file mode 100644 index 000000000..32acd6ede --- /dev/null +++ b/db/migrate/20201008202037_create_ip_blocks.rb @@ -0,0 +1,12 @@ +class CreateIpBlocks < ActiveRecord::Migration[5.2] + def change + create_table :ip_blocks do |t| + t.inet :ip, null: false, default: '0.0.0.0' + t.integer :severity, null: false, default: 0 + t.datetime :expires_at + t.text :comment, null: false, default: '' + + t.timestamps + end + end +end diff --git a/db/migrate/20201008220312_add_sign_up_ip_to_users.rb b/db/migrate/20201008220312_add_sign_up_ip_to_users.rb new file mode 100644 index 000000000..66cd624bb --- /dev/null +++ b/db/migrate/20201008220312_add_sign_up_ip_to_users.rb @@ -0,0 +1,5 @@ +class AddSignUpIpToUsers < ActiveRecord::Migration[5.2] + def change + add_column :users, :sign_up_ip, :inet + end +end diff --git a/db/schema.rb b/db/schema.rb index 0029d620a..5805f3105 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_09_17_222734) do +ActiveRecord::Schema.define(version: 2020_10_08_220312) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -463,6 +463,15 @@ ActiveRecord::Schema.define(version: 2020_09_17_222734) do t.index ["user_id"], name: "index_invites_on_user_id" end + create_table "ip_blocks", force: :cascade do |t| + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.datetime "expires_at" + t.inet "ip", default: "0.0.0.0", null: false + t.integer "severity", default: 0, null: false + t.text "comment", default: "", null: false + end + create_table "list_accounts", force: :cascade do |t| t.bigint "list_id", null: false t.bigint "account_id", null: false @@ -891,6 +900,7 @@ ActiveRecord::Schema.define(version: 2020_09_17_222734) do t.string "sign_in_token" t.datetime "sign_in_token_sent_at" t.string "webauthn_id" + t.inet "sign_up_ip" t.index ["account_id"], name: "index_users_on_account_id" t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true t.index ["created_by_application_id"], name: "index_users_on_created_by_application_id" diff --git a/lib/cli.rb b/lib/cli.rb index 9162144cc..2a4dd11b2 100644 --- a/lib/cli.rb +++ b/lib/cli.rb @@ -13,6 +13,7 @@ require_relative 'mastodon/preview_cards_cli' require_relative 'mastodon/cache_cli' require_relative 'mastodon/upgrade_cli' require_relative 'mastodon/email_domain_blocks_cli' +require_relative 'mastodon/ip_blocks_cli' require_relative 'mastodon/version' module Mastodon @@ -57,6 +58,9 @@ module Mastodon desc 'email_domain_blocks SUBCOMMAND ...ARGS', 'Manage e-mail domain blocks' subcommand 'email_domain_blocks', Mastodon::EmailDomainBlocksCLI + desc 'ip_blocks SUBCOMMAND ...ARGS', 'Manage IP blocks' + subcommand 'ip_blocks', Mastodon::IpBlocksCLI + option :dry_run, type: :boolean desc 'self-destruct', 'Erase the server from the federation' long_desc <<~LONG_DESC diff --git a/lib/mastodon/ip_blocks_cli.rb b/lib/mastodon/ip_blocks_cli.rb new file mode 100644 index 000000000..6aff36d90 --- /dev/null +++ b/lib/mastodon/ip_blocks_cli.rb @@ -0,0 +1,132 @@ +# frozen_string_literal: true + +require 'rubygems/package' +require_relative '../../config/boot' +require_relative '../../config/environment' +require_relative 'cli_helper' + +module Mastodon + class IpBlocksCLI < Thor + def self.exit_on_failure? + true + end + + option :severity, required: true, enum: %w(no_access sign_up_requires_approval), desc: 'Severity of the block' + option :comment, aliases: [:c], desc: 'Optional comment' + option :duration, aliases: [:d], type: :numeric, desc: 'Duration of the block in seconds' + option :force, type: :boolean, aliases: [:f], desc: 'Overwrite existing blocks' + desc 'add IP...', 'Add one or more IP blocks' + long_desc <<-LONG_DESC + Add one or more IP blocks. You can use CIDR syntax to + block IP ranges. You must specify --severity of the block. All + options will be copied for each IP block you create in one command. + + You can add a --comment. If an IP block already exists for one of + the provided IPs, it will be skipped unless you use the --force + option to overwrite it. + LONG_DESC + def add(*addresses) + if addresses.empty? + say('No IP(s) given', :red) + exit(1) + end + + skipped = 0 + processed = 0 + failed = 0 + + addresses.each do |address| + ip_block = IpBlock.find_by(ip: address) + + if ip_block.present? && !options[:force] + say("#{address} is already blocked", :yellow) + skipped += 1 + next + end + + ip_block ||= IpBlock.new(ip: address) + + ip_block.severity = options[:severity] + ip_block.comment = options[:comment] + ip_block.expires_in = options[:duration] + + if ip_block.save + processed += 1 + else + say("#{address} could not be saved", :red) + failed += 1 + end + end + + say("Added #{processed}, skipped #{skipped}, failed #{failed}", color(processed, failed)) + end + + option :force, type: :boolean, aliases: [:f], desc: 'Remove blocks for ranges that cover given IP(s)' + desc 'remove IP...', 'Remove one or more IP blocks' + long_desc <<-LONG_DESC + Remove one or more IP blocks. Normally, only exact matches are removed. If + you want to ensure that all of the given IP addresses are unblocked, you + can use --force which will also remove any blocks for IP ranges that would + cover the given IP(s). + LONG_DESC + def remove(*addresses) + if addresses.empty? + say('No IP(s) given', :red) + exit(1) + end + + processed = 0 + skipped = 0 + + addresses.each do |address| + ip_blocks = begin + if options[:force] + IpBlock.where('ip >>= ?', address) + else + IpBlock.where('ip <<= ?', address) + end + end + + if ip_blocks.empty? + say("#{address} is not yet blocked", :yellow) + skipped += 1 + next + end + + ip_blocks.in_batches.destroy_all + processed += 1 + end + + say("Removed #{processed}, skipped #{skipped}", color(processed, 0)) + end + + option :format, aliases: [:f], enum: %w(plain nginx), desc: 'Format of the output' + desc 'export', 'Export blocked IPs' + long_desc <<-LONG_DESC + Export blocked IPs. Different formats are supported for usage with other + tools. Only blocks with no_access severity are returned. + LONG_DESC + def export + IpBlock.where(severity: :no_access).find_each do |ip_block| + case options[:format] + when 'nginx' + puts "deny #{ip_block.ip}/#{ip_block.ip.prefix};" + else + puts "#{ip_block.ip}/#{ip_block.ip.prefix}" + end + end + end + + private + + def color(processed, failed) + if !processed.zero? && failed.zero? + :green + elsif failed.zero? + :yellow + else + :red + end + end + end +end diff --git a/spec/fabricators/ip_block_fabricator.rb b/spec/fabricators/ip_block_fabricator.rb new file mode 100644 index 000000000..31dc336e6 --- /dev/null +++ b/spec/fabricators/ip_block_fabricator.rb @@ -0,0 +1,6 @@ +Fabricator(:ip_block) do + ip "" + severity "" + expires_at "2020-10-08 22:20:37" + comment "MyText" +end \ No newline at end of file diff --git a/spec/lib/fast_ip_map_spec.rb b/spec/lib/fast_ip_map_spec.rb new file mode 100644 index 000000000..c66f64828 --- /dev/null +++ b/spec/lib/fast_ip_map_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe FastIpMap do + describe '#include?' do + subject { described_class.new([IPAddr.new('20.4.0.0/16'), IPAddr.new('145.22.30.0/24'), IPAddr.new('189.45.86.3')])} + + it 'returns true for an exact match' do + expect(subject.include?(IPAddr.new('189.45.86.3'))).to be true + end + + it 'returns true for a range match' do + expect(subject.include?(IPAddr.new('20.4.45.7'))).to be true + end + + it 'returns false for no match' do + expect(subject.include?(IPAddr.new('145.22.40.64'))).to be false + end + end +end diff --git a/spec/models/ip_block_spec.rb b/spec/models/ip_block_spec.rb new file mode 100644 index 000000000..6603c6417 --- /dev/null +++ b/spec/models/ip_block_spec.rb @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe IpBlock, type: :model do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/services/app_sign_up_service_spec.rb b/spec/services/app_sign_up_service_spec.rb index e7c7f3ba1..e0c83b704 100644 --- a/spec/services/app_sign_up_service_spec.rb +++ b/spec/services/app_sign_up_service_spec.rb @@ -3,6 +3,7 @@ require 'rails_helper' RSpec.describe AppSignUpService, type: :service do let(:app) { Fabricate(:application, scopes: 'read write') } let(:good_params) { { username: 'alice', password: '12345678', email: 'good@email.com', agreement: true } } + let(:remote_ip) { IPAddr.new('198.0.2.1') } subject { described_class.new } @@ -10,16 +11,16 @@ RSpec.describe AppSignUpService, type: :service do it 'returns nil when registrations are closed' do tmp = Setting.registrations_mode Setting.registrations_mode = 'none' - expect(subject.call(app, good_params)).to be_nil + expect(subject.call(app, remote_ip, good_params)).to be_nil Setting.registrations_mode = tmp end it 'raises an error when params are missing' do - expect { subject.call(app, {}) }.to raise_error ActiveRecord::RecordInvalid + expect { subject.call(app, remote_ip, {}) }.to raise_error ActiveRecord::RecordInvalid end it 'creates an unconfirmed user with access token' do - access_token = subject.call(app, good_params) + access_token = subject.call(app, remote_ip, good_params) expect(access_token).to_not be_nil user = User.find_by(id: access_token.resource_owner_id) expect(user).to_not be_nil @@ -27,13 +28,13 @@ RSpec.describe AppSignUpService, type: :service do end it 'creates access token with the app\'s scopes' do - access_token = subject.call(app, good_params) + access_token = subject.call(app, remote_ip, good_params) expect(access_token).to_not be_nil expect(access_token.scopes.to_s).to eq 'read write' end it 'creates an account' do - access_token = subject.call(app, good_params) + access_token = subject.call(app, remote_ip, good_params) expect(access_token).to_not be_nil user = User.find_by(id: access_token.resource_owner_id) expect(user).to_not be_nil @@ -42,7 +43,7 @@ RSpec.describe AppSignUpService, type: :service do end it 'creates an account with invite request text' do - access_token = subject.call(app, good_params.merge(reason: 'Foo bar')) + access_token = subject.call(app, remote_ip, good_params.merge(reason: 'Foo bar')) expect(access_token).to_not be_nil user = User.find_by(id: access_token.resource_owner_id) expect(user).to_not be_nil -- cgit From ca56527140034952002f8f7334da9f94c4f486a8 Mon Sep 17 00:00:00 2001 From: ThibG Date: Wed, 21 Oct 2020 18:04:09 +0200 Subject: Add follower synchronization mechanism (#14510) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add support for followers synchronization on the receiving end Check the `collectionSynchronization` attribute on `Create` and `Announce` activities and synchronize followers from provided collection if possible. * Add tests for followers synchronization on the receiving end * Add support for follower synchronization on the sender's end * Add tests for the sending end * Switch from AS attributes to HTTP header Replace the custom `collectionSynchronization` ActivityStreams attribute by an HTTP header (`X-AS-Collection-Synchronization`) with the same syntax as the `Signature` header and the following fields: - `collectionId` to specify which collection to synchronize - `digest` for the SHA256 hex-digest of the list of followers known on the receiving instance (where “receiving instance” is determined by accounts sharing the same host name for their ActivityPub actor `id`) - `url` of a collection that should be fetched by the instance actor Internally, move away from the webfinger-based `domain` attribute and use account `uri` prefix to group accounts. * Add environment variable to disable followers synchronization Since the whole mechanism relies on some new preconditions that, in some extremely rare cases, might not be met, add an environment variable (DISABLE_FOLLOWERS_SYNCHRONIZATION) to disable the mechanism altogether and avoid followers being incorrectly removed. The current conditions are: 1. all managed accounts' actor `id` and inbox URL have the same URI scheme and netloc. 2. all accounts whose actor `id` or inbox URL share the same URI scheme and netloc as a managed account must be managed by the same Mastodon instance as well. As far as Mastodon is concerned, breaking those preconditions require extensive configuration changes in the reverse proxy and might also cause other issues. Therefore, this environment variable provides a way out for people with highly unusual configurations, and can be safely ignored for the overwhelming majority of Mastodon administrators. * Only set follower synchronization header on non-public statuses This is to avoid unnecessary computations and allow Follow-related activities to be handled by the usual codepath instead of going through the synchronization mechanism (otherwise, any Follow/Undo/Accept activity would trigger the synchronization mechanism even if processing the activity itself would be enough to re-introduce synchronization) * Change how ActivityPub::SynchronizeFollowersService handles follow requests If the remote lists a local follower which we only know has sent a follow request, consider the follow request as accepted instead of sending an Undo. * Integrate review feeback - rename X-AS-Collection-Synchronization to Collection-Synchronization - various minor refactoring and code style changes * Only select required fields when computing followers_hash * Use actor URI rather than webfinger domain in synchronization endpoint * Change hash computation to be a XOR of individual hashes Makes it much easier to be memory-efficient, and avoid sorting discrepancy issues. * Marginally improve followers_hash computation speed * Further improve hash computation performances by using pluck_each --- Gemfile | 3 + Gemfile.lock | 6 ++ .../followers_synchronizations_controller.rb | 36 +++++++ app/controllers/activitypub/inboxes_controller.rb | 14 +++ app/lib/activitypub/tag_manager.rb | 4 + app/models/account.rb | 6 ++ app/models/concerns/account_interactions.rb | 20 ++++ app/models/follow.rb | 8 ++ .../prepare_followers_synchronization_service.rb | 13 +++ .../activitypub/synchronize_followers_service.rb | 74 +++++++++++++++ app/workers/activitypub/delivery_worker.rb | 10 ++ app/workers/activitypub/distribution_worker.rb | 2 +- .../followers_synchronization_worker.rb | 14 +++ config/routes.rb | 1 + .../followers_synchronizations_controller_spec.rb | 58 ++++++++++++ .../activitypub/inboxes_controller_spec.rb | 50 ++++++++++ spec/models/concerns/account_interactions_spec.rb | 43 +++++++++ .../synchronize_followers_service_spec.rb | 105 +++++++++++++++++++++ spec/workers/activitypub/delivery_worker_spec.rb | 10 +- 19 files changed, 474 insertions(+), 3 deletions(-) create mode 100644 app/controllers/activitypub/followers_synchronizations_controller.rb create mode 100644 app/services/activitypub/prepare_followers_synchronization_service.rb create mode 100644 app/services/activitypub/synchronize_followers_service.rb create mode 100644 app/workers/activitypub/followers_synchronization_worker.rb create mode 100644 spec/controllers/activitypub/followers_synchronizations_controller_spec.rb create mode 100644 spec/services/activitypub/synchronize_followers_service_spec.rb (limited to 'spec') diff --git a/Gemfile b/Gemfile index 138566780..1ad600224 100644 --- a/Gemfile +++ b/Gemfile @@ -159,3 +159,6 @@ end gem 'concurrent-ruby', require: false gem 'connection_pool', require: false + +gem 'xorcist', '~> 1.1' +gem 'pluck_each', '~> 0.1.3' diff --git a/Gemfile.lock b/Gemfile.lock index aea592da1..915b45512 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -403,6 +403,9 @@ GEM pghero (2.7.2) activerecord (>= 5) pkg-config (1.4.4) + pluck_each (0.1.3) + activerecord (> 3.2.0) + activesupport (> 3.0.0) posix-spawn (0.3.15) premailer (1.14.2) addressable @@ -662,6 +665,7 @@ GEM websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) wisper (2.0.1) + xorcist (1.1.2) xpath (3.2.0) nokogiri (~> 1.8) @@ -748,6 +752,7 @@ DEPENDENCIES pg (~> 1.2) pghero (~> 2.7) pkg-config (~> 1.4) + pluck_each (~> 0.1.3) posix-spawn premailer-rails private_address_check (~> 0.5) @@ -796,3 +801,4 @@ DEPENDENCIES webmock (~> 3.9) webpacker (~> 5.2) webpush + xorcist (~> 1.1) diff --git a/app/controllers/activitypub/followers_synchronizations_controller.rb b/app/controllers/activitypub/followers_synchronizations_controller.rb new file mode 100644 index 000000000..525031105 --- /dev/null +++ b/app/controllers/activitypub/followers_synchronizations_controller.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +class ActivityPub::FollowersSynchronizationsController < ActivityPub::BaseController + include SignatureVerification + include AccountOwnedConcern + + before_action :require_signature! + before_action :set_items + before_action :set_cache_headers + + def show + expires_in 0, public: false + render json: collection_presenter, + serializer: ActivityPub::CollectionSerializer, + adapter: ActivityPub::Adapter, + content_type: 'application/activity+json' + end + + private + + def uri_prefix + signed_request_account.uri[/http(s?):\/\/[^\/]+\//] + end + + def set_items + @items = @account.followers.where(Account.arel_table[:uri].matches(uri_prefix + '%', false, true)).pluck(:uri) + end + + def collection_presenter + ActivityPub::CollectionPresenter.new( + id: account_followers_synchronization_url(@account), + type: :ordered, + items: @items + ) + end +end diff --git a/app/controllers/activitypub/inboxes_controller.rb b/app/controllers/activitypub/inboxes_controller.rb index 0a561e7f0..fdb60d590 100644 --- a/app/controllers/activitypub/inboxes_controller.rb +++ b/app/controllers/activitypub/inboxes_controller.rb @@ -11,6 +11,7 @@ class ActivityPub::InboxesController < ActivityPub::BaseController def create upgrade_account + process_collection_synchronization process_payload head 202 end @@ -52,6 +53,19 @@ class ActivityPub::InboxesController < ActivityPub::BaseController DeliveryFailureTracker.reset!(signed_request_account.inbox_url) end + def process_collection_synchronization + raw_params = request.headers['Collection-Synchronization'] + return if raw_params.blank? || ENV['DISABLE_FOLLOWERS_SYNCHRONIZATION'] == 'true' + + # Re-using the syntax for signature parameters + tree = SignatureParamsParser.new.parse(raw_params) + params = SignatureParamsTransformer.new.apply(tree) + + ActivityPub::PrepareFollowersSynchronizationService.new.call(signed_request_account, params) + rescue Parslet::ParseFailed + Rails.logger.warn 'Error parsing Collection-Synchronization header' + end + def process_payload ActivityPub::ProcessingWorker.perform_async(signed_request_account.id, body, @account&.id) end diff --git a/app/lib/activitypub/tag_manager.rb b/app/lib/activitypub/tag_manager.rb index 3f98dad2e..3f2ae1106 100644 --- a/app/lib/activitypub/tag_manager.rb +++ b/app/lib/activitypub/tag_manager.rb @@ -40,6 +40,10 @@ class ActivityPub::TagManager end end + def uri_for_username(username) + account_url(username: username) + end + def generate_uri_for(_target) URI.join(root_url, 'payloads', SecureRandom.uuid) end diff --git a/app/models/account.rb b/app/models/account.rb index 5acc8d621..59d338f5a 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -352,6 +352,12 @@ class Account < ApplicationRecord shared_inbox_url.presence || inbox_url end + def synchronization_uri_prefix + return 'local' if local? + + @synchronization_uri_prefix ||= uri[/http(s?):\/\/[^\/]+\//] + end + class Field < ActiveModelSerializers::Model attributes :name, :value, :verified_at, :account, :errors diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index 6a0ad5aa9..e2c4b8acf 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -243,6 +243,26 @@ module AccountInteractions .where('users.current_sign_in_at > ?', User::ACTIVE_DURATION.ago) end + def remote_followers_hash(url_prefix) + Rails.cache.fetch("followers_hash:#{id}:#{url_prefix}") do + digest = "\x00" * 32 + followers.where(Account.arel_table[:uri].matches(url_prefix + '%', false, true)).pluck_each(:uri) do |uri| + Xorcist.xor!(digest, Digest::SHA256.digest(uri)) + end + digest.unpack('H*')[0] + end + end + + def local_followers_hash + Rails.cache.fetch("followers_hash:#{id}:local") do + digest = "\x00" * 32 + followers.where(domain: nil).pluck_each(:username) do |username| + Xorcist.xor!(digest, Digest::SHA256.digest(ActivityPub::TagManager.instance.uri_for_username(username))) + end + digest.unpack('H*')[0] + end + end + private def remove_potential_friendship(other_account, mutual = false) diff --git a/app/models/follow.rb b/app/models/follow.rb index 0b4ddbf3f..55a9da792 100644 --- a/app/models/follow.rb +++ b/app/models/follow.rb @@ -41,8 +41,10 @@ class Follow < ApplicationRecord before_validation :set_uri, only: :create after_create :increment_cache_counters + after_create :invalidate_hash_cache after_destroy :remove_endorsements after_destroy :decrement_cache_counters + after_destroy :invalidate_hash_cache private @@ -63,4 +65,10 @@ class Follow < ApplicationRecord account&.decrement_count!(:following_count) target_account&.decrement_count!(:followers_count) end + + def invalidate_hash_cache + return if account.local? && target_account.local? + + Rails.cache.delete("followers_hash:#{target_account_id}:#{account.synchronization_uri_prefix}") + end end diff --git a/app/services/activitypub/prepare_followers_synchronization_service.rb b/app/services/activitypub/prepare_followers_synchronization_service.rb new file mode 100644 index 000000000..2d22ed701 --- /dev/null +++ b/app/services/activitypub/prepare_followers_synchronization_service.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class ActivityPub::PrepareFollowersSynchronizationService < BaseService + include JsonLdHelper + + def call(account, params) + @account = account + + return if params['collectionId'] != @account.followers_url || invalid_origin?(params['url']) || @account.local_followers_hash == params['digest'] + + ActivityPub::FollowersSynchronizationWorker.perform_async(@account.id, params['url']) + end +end diff --git a/app/services/activitypub/synchronize_followers_service.rb b/app/services/activitypub/synchronize_followers_service.rb new file mode 100644 index 000000000..d83fcf55e --- /dev/null +++ b/app/services/activitypub/synchronize_followers_service.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +class ActivityPub::SynchronizeFollowersService < BaseService + include JsonLdHelper + include Payloadable + + def call(account, partial_collection_url) + @account = account + + items = collection_items(partial_collection_url) + return if items.nil? + + # There could be unresolved accounts (hence the call to .compact) but this + # should never happen in practice, since in almost all cases we keep an + # Account record, and should we not do that, we should have sent a Delete. + # In any case there is not much we can do if that occurs. + @expected_followers = items.map { |uri| ActivityPub::TagManager.instance.uri_to_resource(uri, Account) }.compact + + remove_unexpected_local_followers! + handle_unexpected_outgoing_follows! + end + + private + + def remove_unexpected_local_followers! + @account.followers.local.where.not(id: @expected_followers.map(&:id)).each do |unexpected_follower| + UnfollowService.new.call(unexpected_follower, @account) + end + end + + def handle_unexpected_outgoing_follows! + @expected_followers.each do |expected_follower| + next if expected_follower.following?(@account) + + if expected_follower.requested?(@account) + # For some reason the follow request went through but we missed it + expected_follower.follow_requests.find_by(target_account: @account)&.authorize! + else + # Since we were not aware of the follow from our side, we do not have an + # ID for it that we can include in the Undo activity. For this reason, + # the Undo may not work with software that relies exclusively on + # matching activity IDs and not the actor and target + follow = Follow.new(account: expected_follower, target_account: @account) + ActivityPub::DeliveryWorker.perform_async(build_undo_follow_json(follow), follow.account_id, follow.target_account.inbox_url) + end + end + end + + def build_undo_follow_json(follow) + Oj.dump(serialize_payload(follow, ActivityPub::UndoFollowSerializer)) + end + + def collection_items(collection_or_uri) + collection = fetch_collection(collection_or_uri) + return unless collection.is_a?(Hash) + + collection = fetch_collection(collection['first']) if collection['first'].present? + return unless collection.is_a?(Hash) + + case collection['type'] + when 'Collection', 'CollectionPage' + collection['items'] + when 'OrderedCollection', 'OrderedCollectionPage' + collection['orderedItems'] + end + end + + def fetch_collection(collection_or_uri) + return collection_or_uri if collection_or_uri.is_a?(Hash) + return if invalid_origin?(collection_or_uri) + + fetch_resource_without_id_validation(collection_or_uri, nil, true) + end +end diff --git a/app/workers/activitypub/delivery_worker.rb b/app/workers/activitypub/delivery_worker.rb index 60775787a..6c5a576a7 100644 --- a/app/workers/activitypub/delivery_worker.rb +++ b/app/workers/activitypub/delivery_worker.rb @@ -2,6 +2,7 @@ class ActivityPub::DeliveryWorker include Sidekiq::Worker + include RoutingHelper include JsonLdHelper STOPLIGHT_FAILURE_THRESHOLD = 10 @@ -38,9 +39,18 @@ class ActivityPub::DeliveryWorker Request.new(:post, @inbox_url, body: @json, http_client: http_client).tap do |request| request.on_behalf_of(@source_account, :uri, sign_with: @options[:sign_with]) request.add_headers(HEADERS) + request.add_headers({ 'Collection-Synchronization' => synchronization_header }) if ENV['DISABLE_FOLLOWERS_SYNCHRONIZATION'] != 'true' && @options[:synchronize_followers] end end + def synchronization_header + "collectionId=\"#{account_followers_url(@source_account)}\", digest=\"#{@source_account.remote_followers_hash(inbox_url_prefix)}\", url=\"#{account_followers_synchronization_url(@source_account)}\"" + end + + def inbox_url_prefix + @inbox_url[/http(s?):\/\/[^\/]+\//] + end + def perform_request light = Stoplight(@inbox_url) do request_pool.with(@host) do |http_client| diff --git a/app/workers/activitypub/distribution_worker.rb b/app/workers/activitypub/distribution_worker.rb index e4997ba0e..9b4814644 100644 --- a/app/workers/activitypub/distribution_worker.rb +++ b/app/workers/activitypub/distribution_worker.rb @@ -13,7 +13,7 @@ class ActivityPub::DistributionWorker return if skip_distribution? ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url| - [payload, @account.id, inbox_url] + [payload, @account.id, inbox_url, { synchronize_followers: !@status.distributable? }] end relay! if relayable? diff --git a/app/workers/activitypub/followers_synchronization_worker.rb b/app/workers/activitypub/followers_synchronization_worker.rb new file mode 100644 index 000000000..35a3ef0b9 --- /dev/null +++ b/app/workers/activitypub/followers_synchronization_worker.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class ActivityPub::FollowersSynchronizationWorker + include Sidekiq::Worker + + sidekiq_options queue: 'push', lock: :until_executed + + def perform(account_id, url) + @account = Account.find_by(id: account_id) + return true if @account.nil? + + ActivityPub::SynchronizeFollowersService.new.call(@account, url) + end +end diff --git a/config/routes.rb b/config/routes.rb index a21dbd45e..e0ef23723 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -83,6 +83,7 @@ Rails.application.routes.draw do resource :inbox, only: [:create], module: :activitypub resource :claim, only: [:create], module: :activitypub resources :collections, only: [:show], module: :activitypub + resource :followers_synchronization, only: [:show], module: :activitypub end resource :inbox, only: [:create], module: :activitypub diff --git a/spec/controllers/activitypub/followers_synchronizations_controller_spec.rb b/spec/controllers/activitypub/followers_synchronizations_controller_spec.rb new file mode 100644 index 000000000..a24d3f8e0 --- /dev/null +++ b/spec/controllers/activitypub/followers_synchronizations_controller_spec.rb @@ -0,0 +1,58 @@ +require 'rails_helper' + +RSpec.describe ActivityPub::FollowersSynchronizationsController, type: :controller do + let!(:account) { Fabricate(:account) } + let!(:follower_1) { Fabricate(:account, domain: 'example.com', uri: 'https://example.com/users/a') } + let!(:follower_2) { Fabricate(:account, domain: 'example.com', uri: 'https://example.com/users/b') } + let!(:follower_3) { Fabricate(:account, domain: 'foo.com', uri: 'https://foo.com/users/a') } + + before do + follower_1.follow!(account) + follower_2.follow!(account) + follower_3.follow!(account) + end + + before do + allow(controller).to receive(:signed_request_account).and_return(remote_account) + end + + describe 'GET #show' do + context 'without signature' do + let(:remote_account) { nil } + + before do + get :show, params: { account_username: account.username } + end + + it 'returns http not authorized' do + expect(response).to have_http_status(401) + end + end + + context 'with signature from example.com' do + let(:remote_account) { Fabricate(:account, domain: 'example.com', uri: 'https://example.com/instance') } + + before do + get :show, params: { account_username: account.username } + end + + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'returns application/activity+json' do + expect(response.content_type).to eq 'application/activity+json' + end + + it 'returns orderedItems with followers from example.com' do + json = body_as_json + expect(json[:orderedItems]).to be_an Array + expect(json[:orderedItems].sort).to eq [follower_1.uri, follower_2.uri] + end + + it 'returns private Cache-Control header' do + expect(response.headers['Cache-Control']).to eq 'max-age=0, private' + end + end + end +end diff --git a/spec/controllers/activitypub/inboxes_controller_spec.rb b/spec/controllers/activitypub/inboxes_controller_spec.rb index f3bc23953..e5c004611 100644 --- a/spec/controllers/activitypub/inboxes_controller_spec.rb +++ b/spec/controllers/activitypub/inboxes_controller_spec.rb @@ -22,6 +22,56 @@ RSpec.describe ActivityPub::InboxesController, type: :controller do end end + context 'with Collection-Synchronization header' do + let(:remote_account) { Fabricate(:account, followers_url: 'https://example.com/followers', domain: 'example.com', uri: 'https://example.com/actor', protocol: :activitypub) } + let(:synchronization_collection) { remote_account.followers_url } + let(:synchronization_url) { 'https://example.com/followers-for-domain' } + let(:synchronization_hash) { 'somehash' } + let(:synchronization_header) { "collectionId=\"#{synchronization_collection}\", digest=\"#{synchronization_hash}\", url=\"#{synchronization_url}\"" } + + before do + allow(ActivityPub::FollowersSynchronizationWorker).to receive(:perform_async).and_return(nil) + allow_any_instance_of(Account).to receive(:local_followers_hash).and_return('somehash') + + request.headers['Collection-Synchronization'] = synchronization_header + post :create, body: '{}' + end + + context 'with mismatching target collection' do + let(:synchronization_collection) { 'https://example.com/followers2' } + + it 'does not start a synchronization job' do + expect(ActivityPub::FollowersSynchronizationWorker).not_to have_received(:perform_async) + end + end + + context 'with mismatching domain in partial collection attribute' do + let(:synchronization_url) { 'https://example.org/followers' } + + it 'does not start a synchronization job' do + expect(ActivityPub::FollowersSynchronizationWorker).not_to have_received(:perform_async) + end + end + + context 'with matching digest' do + it 'does not start a synchronization job' do + expect(ActivityPub::FollowersSynchronizationWorker).not_to have_received(:perform_async) + end + end + + context 'with mismatching digest' do + let(:synchronization_hash) { 'wronghash' } + + it 'starts a synchronization job' do + expect(ActivityPub::FollowersSynchronizationWorker).to have_received(:perform_async) + end + end + + it 'returns http accepted' do + expect(response).to have_http_status(202) + end + end + context 'without signature' do before do post :create, body: '{}' diff --git a/spec/models/concerns/account_interactions_spec.rb b/spec/models/concerns/account_interactions_spec.rb index f0380179c..85fbf7e79 100644 --- a/spec/models/concerns/account_interactions_spec.rb +++ b/spec/models/concerns/account_interactions_spec.rb @@ -539,6 +539,49 @@ describe AccountInteractions do end end + describe '#followers_hash' do + let(:me) { Fabricate(:account, username: 'Me') } + let(:remote_1) { Fabricate(:account, username: 'alice', domain: 'example.org', uri: 'https://example.org/users/alice') } + let(:remote_2) { Fabricate(:account, username: 'bob', domain: 'example.org', uri: 'https://example.org/users/bob') } + let(:remote_3) { Fabricate(:account, username: 'eve', domain: 'foo.org', uri: 'https://foo.org/users/eve') } + + before do + remote_1.follow!(me) + remote_2.follow!(me) + remote_3.follow!(me) + me.follow!(remote_1) + end + + context 'on a local user' do + it 'returns correct hash for remote domains' do + expect(me.remote_followers_hash('https://example.org/')).to eq '707962e297b7bd94468a21bc8e506a1bcea607a9142cd64e27c9b106b2a5f6ec' + expect(me.remote_followers_hash('https://foo.org/')).to eq 'ccb9c18a67134cfff9d62c7f7e7eb88e6b803446c244b84265565f4eba29df0e' + end + + it 'invalidates cache as needed when removing or adding followers' do + expect(me.remote_followers_hash('https://example.org/')).to eq '707962e297b7bd94468a21bc8e506a1bcea607a9142cd64e27c9b106b2a5f6ec' + remote_1.unfollow!(me) + expect(me.remote_followers_hash('https://example.org/')).to eq '241b00794ce9b46aa864f3220afadef128318da2659782985bac5ed5bd436bff' + remote_1.follow!(me) + expect(me.remote_followers_hash('https://example.org/')).to eq '707962e297b7bd94468a21bc8e506a1bcea607a9142cd64e27c9b106b2a5f6ec' + end + end + + context 'on a remote user' do + it 'returns correct hash for remote domains' do + expect(remote_1.local_followers_hash).to eq Digest::SHA256.hexdigest(ActivityPub::TagManager.instance.uri_for(me)) + end + + it 'invalidates cache as needed when removing or adding followers' do + expect(remote_1.local_followers_hash).to eq Digest::SHA256.hexdigest(ActivityPub::TagManager.instance.uri_for(me)) + me.unfollow!(remote_1) + expect(remote_1.local_followers_hash).to eq '0000000000000000000000000000000000000000000000000000000000000000' + me.follow!(remote_1) + expect(remote_1.local_followers_hash).to eq Digest::SHA256.hexdigest(ActivityPub::TagManager.instance.uri_for(me)) + end + end + end + describe 'muting an account' do let(:me) { Fabricate(:account, username: 'Me') } let(:you) { Fabricate(:account, username: 'You') } diff --git a/spec/services/activitypub/synchronize_followers_service_spec.rb b/spec/services/activitypub/synchronize_followers_service_spec.rb new file mode 100644 index 000000000..75dcf204b --- /dev/null +++ b/spec/services/activitypub/synchronize_followers_service_spec.rb @@ -0,0 +1,105 @@ +require 'rails_helper' + +RSpec.describe ActivityPub::SynchronizeFollowersService, type: :service do + let(:actor) { Fabricate(:account, domain: 'example.com', uri: 'http://example.com/account', inbox_url: 'http://example.com/inbox') } + let(:alice) { Fabricate(:account, username: 'alice') } + let(:bob) { Fabricate(:account, username: 'bob') } + let(:eve) { Fabricate(:account, username: 'eve') } + let(:mallory) { Fabricate(:account, username: 'mallory') } + let(:collection_uri) { 'http://example.com/partial-followers' } + + let(:items) do + [ + ActivityPub::TagManager.instance.uri_for(alice), + ActivityPub::TagManager.instance.uri_for(eve), + ActivityPub::TagManager.instance.uri_for(mallory), + ] + end + + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Collection', + id: collection_uri, + items: items, + }.with_indifferent_access + end + + subject { described_class.new } + + shared_examples 'synchronizes followers' do + before do + alice.follow!(actor) + bob.follow!(actor) + mallory.request_follow!(actor) + + allow(ActivityPub::DeliveryWorker).to receive(:perform_async) + + subject.call(actor, collection_uri) + end + + it 'keeps expected followers' do + expect(alice.following?(actor)).to be true + end + + it 'removes local followers not in the remote list' do + expect(bob.following?(actor)).to be false + end + + it 'converts follow requests to follow relationships when they have been accepted' do + expect(mallory.following?(actor)).to be true + end + + it 'sends an Undo Follow to the actor' do + expect(ActivityPub::DeliveryWorker).to have_received(:perform_async).with(anything, eve.id, actor.inbox_url) + end + end + + describe '#call' do + context 'when the endpoint is a Collection of actor URIs' do + before do + stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload)) + end + + it_behaves_like 'synchronizes followers' + end + + context 'when the endpoint is an OrderedCollection of actor URIs' do + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'OrderedCollection', + id: collection_uri, + orderedItems: items, + }.with_indifferent_access + end + + before do + stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload)) + end + + it_behaves_like 'synchronizes followers' + end + + context 'when the endpoint is a paginated Collection of actor URIs' do + let(:payload) do + { + '@context': 'https://www.w3.org/ns/activitystreams', + type: 'Collection', + id: collection_uri, + first: { + type: 'CollectionPage', + partOf: collection_uri, + items: items, + } + }.with_indifferent_access + end + + before do + stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload)) + end + + it_behaves_like 'synchronizes followers' + end + end +end diff --git a/spec/workers/activitypub/delivery_worker_spec.rb b/spec/workers/activitypub/delivery_worker_spec.rb index 351be185c..f4633731e 100644 --- a/spec/workers/activitypub/delivery_worker_spec.rb +++ b/spec/workers/activitypub/delivery_worker_spec.rb @@ -3,16 +3,22 @@ require 'rails_helper' describe ActivityPub::DeliveryWorker do + include RoutingHelper + subject { described_class.new } let(:sender) { Fabricate(:account) } let(:payload) { 'test' } + before do + allow_any_instance_of(Account).to receive(:remote_followers_hash).with('https://example.com/').and_return('somehash') + end + describe 'perform' do it 'performs a request' do stub_request(:post, 'https://example.com/api').to_return(status: 200) - subject.perform(payload, sender.id, 'https://example.com/api') - expect(a_request(:post, 'https://example.com/api')).to have_been_made.once + subject.perform(payload, sender.id, 'https://example.com/api', { synchronize_followers: true }) + expect(a_request(:post, 'https://example.com/api').with(headers: { 'Collection-Synchronization' => "collectionId=\"#{account_followers_url(sender)}\", digest=\"somehash\", url=\"#{account_followers_synchronization_url(sender)}\"" })).to have_been_made.once end it 'raises when request fails' do -- cgit