From d872acc94bb84e68b8b392b4f638d831a56c2049 Mon Sep 17 00:00:00 2001 From: Woody Peterson Date: Thu, 7 Mar 2024 12:12:23 -0800 Subject: [PATCH] Instrument fail2ban Previously, if a request triggered a ban, there was no instrumentation for knowing the ban occurred. This instruments bans under the `ban.rack_attack` notification. --- README.md | 29 +++++++++++++++++++++++++++ lib/rack/attack/allow2ban.rb | 8 +++----- lib/rack/attack/check.rb | 7 +++++-- lib/rack/attack/fail2ban.rb | 25 ++++++++++++++++++++++-- spec/fail2ban_spec.rb | 38 +++++++++++++++++++++++++++++++++++- 5 files changed, 97 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 199021f6..efe18437 100644 --- a/README.md +++ b/README.md @@ -217,6 +217,35 @@ end Note that `Fail2Ban` filters are not automatically scoped to the blocklist, so when using multiple filters in an application the scoping must be added to the discriminator e.g. `"pentest:#{req.ip}"`. +Instrumentation of fail2ban requires passing the request to the filter, and +subscribing to `ban.rack_attack`: + +```ruby +Rack::Attack.blocklist('fail2ban') do |req| + # same as above, plus passing in the request + Rack::Attack::Fail2Ban.filter(..., request: req) { ... } +end + +# Then you can subscribe to the ban event +ActiveSupport::Notifications.subscribe("ban.rack_attack") do |name, start, finish, request_id, payload| + # Request object available in payload[:request]. + # + # For ease of use, the payload echoes the fail2ban settings that triggered + # the ban: + # + # payload[:request].env['rack.attack.match_data'] #=> { + # name: "fail2ban", + # discriminator: "1.2.3.4", + # count: 2, + # maxretry: 5, + # findtime: 60, + # bantime: 3600 + # } + + # Your code here +end +``` + #### Allow2Ban `Allow2Ban.filter` works the same way as the `Fail2Ban.filter` except that it *allows* requests from misbehaving diff --git a/lib/rack/attack/allow2ban.rb b/lib/rack/attack/allow2ban.rb index faa3518d..a08e96a4 100644 --- a/lib/rack/attack/allow2ban.rb +++ b/lib/rack/attack/allow2ban.rb @@ -12,11 +12,9 @@ def key_prefix # everything is the same here except we only return true # (blocking the request) if they have tripped the limit. - def fail!(discriminator, bantime, findtime, maxretry) - count = cache.count("#{key_prefix}:count:#{discriminator}", findtime) - if count >= maxretry - ban!(discriminator, bantime) - end + def fail!(discriminator, bantime, findtime, maxretry, request) + super + # we may not block them this time, but they're banned for next time false end diff --git a/lib/rack/attack/check.rb b/lib/rack/attack/check.rb index c9f3ff7d..c7a8a56c 100644 --- a/lib/rack/attack/check.rb +++ b/lib/rack/attack/check.rb @@ -14,8 +14,11 @@ def initialize(name, options = {}, &block) def matched_by?(request) block.call(request).tap do |match| if match - request.env["rack.attack.matched"] = name - request.env["rack.attack.match_type"] = type + # Can already be set if the match just resulted in a ban, + # in which case we want to notify about the ban, not the + # blocklist check + request.env["rack.attack.matched"] ||= name + request.env["rack.attack.match_type"] ||= type Rack::Attack.instrument(request) end end diff --git a/lib/rack/attack/fail2ban.rb b/lib/rack/attack/fail2ban.rb index b43c7cba..2c851884 100644 --- a/lib/rack/attack/fail2ban.rb +++ b/lib/rack/attack/fail2ban.rb @@ -8,12 +8,13 @@ def filter(discriminator, options) bantime = options[:bantime] or raise ArgumentError, "Must pass bantime option" findtime = options[:findtime] or raise ArgumentError, "Must pass findtime option" maxretry = options[:maxretry] or raise ArgumentError, "Must pass maxretry option" + request = options[:request] if banned?(discriminator) # Return true for blocklist true elsif yield - fail!(discriminator, bantime, findtime, maxretry) + fail!(discriminator, bantime, findtime, maxretry, request) end end @@ -34,10 +35,23 @@ def key_prefix 'fail2ban' end - def fail!(discriminator, bantime, findtime, maxretry) + def fail!(discriminator, bantime, findtime, maxretry, request) count = cache.count("#{key_prefix}:count:#{discriminator}", findtime) if count >= maxretry ban!(discriminator, bantime) + + if request # must be passed in just for instrumentation + annotate_request_with_matched_data( + request, + name: key_prefix, + discriminator: discriminator, + count: count, + maxretry: maxretry, + findtime: findtime, + bantime: bantime + ) + Rack::Attack.instrument(request) + end end true @@ -52,6 +66,13 @@ def ban!(discriminator, bantime) def cache Rack::Attack.cache end + + def annotate_request_with_matched_data(request, data) + request.env['rack.attack.matched'] = data[:name] + request.env['rack.attack.match_discriminator'] = data[:discriminator] + request.env['rack.attack.match_type'] = :ban + request.env['rack.attack.match_data'] = data + end end end end diff --git a/spec/fail2ban_spec.rb b/spec/fail2ban_spec.rb index 6a9d9bcf..a97cfb6b 100644 --- a/spec/fail2ban_spec.rb +++ b/spec/fail2ban_spec.rb @@ -12,7 +12,13 @@ @f2b_options = { bantime: @bantime, findtime: @findtime, maxretry: 2 } Rack::Attack.blocklist('pentest') do |req| - Rack::Attack::Fail2Ban.filter(req.ip, @f2b_options) { req.query_string =~ /OMGHAX/ } + Rack::Attack::Fail2Ban.filter(req.ip, @f2b_options.merge(request: req)) do + req.query_string =~ /OMGHAX/ + end + end + + ActiveSupport::Notifications.subscribe("ban.rack_attack") do |name, start, finish, id, payload| + @notification = { name: name, start: start, finish: finish, id: id, payload: payload } end end @@ -41,6 +47,10 @@ key = "rack::attack:fail2ban:1.2.3.4" _(@cache.store.read(key)).must_be_nil end + + it 'does not notify' do + _(@notification).must_be_nil + end end describe 'when at maxretry' do @@ -63,6 +73,22 @@ key = "rack::attack:fail2ban:ban:1.2.3.4" _(@cache.store.read(key)).must_equal 1 end + + it "notifies" do + _(@notification).wont_be_nil + + _(@notification[:payload][:request].env['rack.attack.match_type'])\ + .must_equal(:ban) + _(@notification[:payload][:request].env['rack.attack.match_data'])\ + .must_equal( + name: "fail2ban", + discriminator: "1.2.3.4", + count: 2, + maxretry: 2, + findtime: 60, + bantime: 60 + ) + end end describe 'reset after success' do @@ -105,6 +131,7 @@ describe 'making ok request' do before do + @notification = nil get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' end @@ -121,10 +148,15 @@ key = "rack::attack:fail2ban:ban:1.2.3.4" _(@cache.store.read(key)).must_equal 1 end + + it 'does not notify' do + _(@notification).must_be_nil + end end describe 'making failing request' do before do + @notification = nil get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' end @@ -141,6 +173,10 @@ key = "rack::attack:fail2ban:ban:1.2.3.4" _(@cache.store.read(key)).must_equal 1 end + + it 'does not notify' do + _(@notification).must_be_nil + end end end end