Remove IP matching from e-mail domain blocks (#18190)

Clear out e-mail domain blocks created from automatically resolved DNS records
shrike
Eugen Rochko 2022-04-29 23:27:03 +02:00 committed by GitHub
parent 7b0fe4aef9
commit f6d35ed57d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 43 additions and 114 deletions

View File

@ -3,16 +3,19 @@
#
# Table name: email_domain_blocks
#
# id :bigint(8) not null, primary key
# domain :string default(""), not null
# created_at :datetime not null
# updated_at :datetime not null
# parent_id :bigint(8)
# ips :inet is an Array
# last_refresh_at :datetime
# id :bigint(8) not null, primary key
# domain :string default(""), not null
# created_at :datetime not null
# updated_at :datetime not null
# parent_id :bigint(8)
#
class EmailDomainBlock < ApplicationRecord
self.ignored_columns = %w(
ips
last_refresh_at
)
include DomainNormalizable
belongs_to :parent, class_name: 'EmailDomainBlock', optional: true
@ -27,7 +30,7 @@ class EmailDomainBlock < ApplicationRecord
@history ||= Trends::History.new('email_domain_blocks', id)
end
def self.block?(domain_or_domains, ips: [], attempt_ip: nil)
def self.block?(domain_or_domains, attempt_ip: nil)
domains = Array(domain_or_domains).map do |str|
domain = begin
if str.include?('@')
@ -48,10 +51,7 @@ class EmailDomainBlock < ApplicationRecord
blocked = domains.any?(&:nil?)
scope = where(domain: domains)
scope = scope.or(where('ips && ARRAY[?]::inet[]', ips)) if ips.any?
scope.find_each do |block|
where(domain: domains).find_each do |block|
blocked = true
block.history.add(attempt_ip) if attempt_ip.present?
end

View File

@ -15,7 +15,7 @@ class EmailMxValidator < ActiveModel::Validator
if resolved_ips.empty?
user.errors.add(:email, :unreachable)
elsif on_blacklist?(resolved_domains, resolved_ips, user.sign_up_ip)
elsif on_blacklist?(resolved_domains, user.sign_up_ip)
user.errors.add(:email, :blocked)
end
end
@ -57,7 +57,7 @@ class EmailMxValidator < ActiveModel::Validator
[ips, records]
end
def on_blacklist?(domains, resolved_ips, attempt_ip)
EmailDomainBlock.block?(domains, ips: resolved_ips, attempt_ip: attempt_ip)
def on_blacklist?(domains, attempt_ip)
EmailDomainBlock.block?(domains, attempt_ip: attempt_ip)
end
end

View File

@ -1,31 +0,0 @@
# frozen_string_literal: true
class Scheduler::EmailDomainBlockRefreshScheduler
include Sidekiq::Worker
include Redisable
sidekiq_options retry: 0
def perform
Resolv::DNS.open do |dns|
dns.timeouts = 5
EmailDomainBlock.find_each do |email_domain_block|
ips = begin
if ip?(email_domain_block.domain)
[email_domain_block.domain]
else
resources = dns.getresources(email_domain_block.domain, Resolv::DNS::Resource::IN::A).to_a + dns.getresources(email_domain_block.domain, Resolv::DNS::Resource::IN::AAAA).to_a
resources.map { |resource| resource.address.to_s }
end
end
email_domain_block.update(ips: ips, last_refresh_at: Time.now.utc)
end
end
end
def ip?(str)
str =~ Regexp.union([Resolv::IPv4::Regex, Resolv::IPv6::Regex])
end
end

View File

@ -17,10 +17,6 @@
every: '5m'
class: Scheduler::Trends::RefreshScheduler
queue: scheduler
email_domain_block_refresh_scheduler:
every: '1h'
class: Scheduler::EmailDomainBlockRefreshScheduler
queue: scheduler
trends_review_notifications_scheduler:
every: '6h'
class: Scheduler::Trends::ReviewNotificationsScheduler

View File

@ -5,7 +5,7 @@ class AddAutofollowToInvites < ActiveRecord::Migration[5.2]
disable_ddl_transaction!
def change
def up
safety_assured do
add_column_with_default :invites, :autofollow, :bool, default: false, allow_null: false
end

View File

@ -0,0 +1,12 @@
# frozen_string_literal: true
class RemoveIpsFromEmailDomainBlocks < ActiveRecord::Migration[5.2]
disable_ddl_transaction!
def change
safety_assured do
remove_column :email_domain_blocks, :ips, :inet, array: true
remove_column :email_domain_blocks, :last_refresh_at, :datetime
end
end
end

View File

@ -0,0 +1,14 @@
# frozen_string_literal: true
class ClearEmailDomainBlocks < ActiveRecord::Migration[5.2]
disable_ddl_transaction!
class EmailDomainBlock < ApplicationRecord
end
def up
EmailDomainBlock.where.not(parent_id: nil).in_batches.delete_all
end
def down; end
end

View File

@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2022_04_28_114902) do
ActiveRecord::Schema.define(version: 2022_04_29_101850) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@ -389,8 +389,6 @@ ActiveRecord::Schema.define(version: 2022_04_28_114902) do
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.bigint "parent_id"
t.inet "ips", array: true
t.datetime "last_refresh_at"
t.index ["domain"], name: "index_email_domain_blocks_on_domain", unique: true
end

View File

@ -56,66 +56,6 @@ describe EmailMxValidator do
expect(user.errors).to have_received(:add)
end
it 'adds an error if the A record is blacklisted' do
EmailDomainBlock.create!(domain: 'alternate-example.com', ips: ['1.2.3.4'])
resolver = double
allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([])
allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([double(address: '1.2.3.4')])
allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::AAAA).and_return([])
allow(resolver).to receive(:timeouts=).and_return(nil)
allow(Resolv::DNS).to receive(:open).and_yield(resolver)
subject.validate(user)
expect(user.errors).to have_received(:add)
end
it 'adds an error if the AAAA record is blacklisted' do
EmailDomainBlock.create!(domain: 'alternate-example.com', ips: ['fd00::1'])
resolver = double
allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([])
allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([])
allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::AAAA).and_return([double(address: 'fd00::1')])
allow(resolver).to receive(:timeouts=).and_return(nil)
allow(Resolv::DNS).to receive(:open).and_yield(resolver)
subject.validate(user)
expect(user.errors).to have_received(:add)
end
it 'adds an error if the A record of the MX record is blacklisted' do
EmailDomainBlock.create!(domain: 'mail.other-domain.com', ips: ['2.3.4.5'])
resolver = double
allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([double(exchange: 'mail.example.com')])
allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([])
allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::AAAA).and_return([])
allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::A).and_return([double(address: '2.3.4.5')])
allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::AAAA).and_return([])
allow(resolver).to receive(:timeouts=).and_return(nil)
allow(Resolv::DNS).to receive(:open).and_yield(resolver)
subject.validate(user)
expect(user.errors).to have_received(:add)
end
it 'adds an error if the AAAA record of the MX record is blacklisted' do
EmailDomainBlock.create!(domain: 'mail.other-domain.com', ips: ['fd00::2'])
resolver = double
allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::MX).and_return([double(exchange: 'mail.example.com')])
allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::A).and_return([])
allow(resolver).to receive(:getresources).with('example.com', Resolv::DNS::Resource::IN::AAAA).and_return([])
allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::A).and_return([])
allow(resolver).to receive(:getresources).with('mail.example.com', Resolv::DNS::Resource::IN::AAAA).and_return([double(address: 'fd00::2')])
allow(resolver).to receive(:timeouts=).and_return(nil)
allow(Resolv::DNS).to receive(:open).and_yield(resolver)
subject.validate(user)
expect(user.errors).to have_received(:add)
end
it 'adds an error if the MX record is blacklisted' do
EmailDomainBlock.create!(domain: 'mail.example.com')
resolver = double