diff --git a/README.md b/README.md index c7c4c46d..be7958d6 100644 --- a/README.md +++ b/README.md @@ -237,6 +237,26 @@ Rack::Attack.blocklist('allow2ban login scrapers') do |req| end ``` +#### Remaining ban time + +`Fail2Ban.retry_after(discriminator)` and `Allow2Ban.retry_after(discriminator)` return the number of seconds until the +ban expires, rounded up to a whole second, or `nil` if the discriminator is not banned. Pass the same discriminator +that was given to `filter`. It can be used to send a `Retry-After` header: + +```ruby +Rack::Attack.blocklisted_responder = lambda do |request| + headers = {} + if (retry_after = Rack::Attack::Allow2Ban.retry_after(request.ip)) + headers['retry-after'] = retry_after.to_s + end + + [ 403, headers, ["Forbidden\n"]] +end +``` + +`retry_after` also returns `nil` for bans created by previous versions of Rack::Attack, which did not store the ban +expiration time. + ### Throttling Throttle state is stored in a [configurable cache](#cache-store-configuration) (which defaults to `Rails.cache` if present). diff --git a/lib/rack/attack/fail2ban.rb b/lib/rack/attack/fail2ban.rb index b43c7cba..eb798851 100644 --- a/lib/rack/attack/fail2ban.rb +++ b/lib/rack/attack/fail2ban.rb @@ -28,6 +28,19 @@ def banned?(discriminator) cache.read("#{key_prefix}:ban:#{discriminator}") ? true : false end + # Returns the number of seconds until the ban on discriminator expires, + # rounded up to a whole second, or nil if discriminator is not banned. + # + # Also returns nil for bans created by versions of Rack::Attack that did not + # store the ban expiration time, even though banned? is true for them. + def retry_after(discriminator) + banned_until = cache.read("#{key_prefix}:ban:#{discriminator}").to_f + # Legacy bans store 1 instead of the expiration time + return if banned_until <= 1 + + [(banned_until - Time.now.to_f).ceil, 1].max + end + protected def key_prefix @@ -46,7 +59,8 @@ def fail!(discriminator, bantime, findtime, maxretry) private def ban!(discriminator, bantime) - cache.write("#{key_prefix}:ban:#{discriminator}", 1, bantime) + # Store the ban expiration time, since not every cache store can report the TTL of a key + cache.write("#{key_prefix}:ban:#{discriminator}", Time.now.to_f + bantime.to_f, bantime) end def cache diff --git a/spec/allow2ban_spec.rb b/spec/allow2ban_spec.rb index 105e0ad0..1b89788a 100644 --- a/spec/allow2ban_spec.rb +++ b/spec/allow2ban_spec.rb @@ -63,7 +63,7 @@ it 'is banned' do key = "rack::attack:allow2ban:ban:1.2.3.4" - _(@cache.store.read(key)).must_equal 1 + _(@cache.store.read(key)).must_be :>, Time.now.to_f end end end @@ -100,7 +100,7 @@ it 'is still banned' do key = "rack::attack:allow2ban:ban:1.2.3.4" - _(@cache.store.read(key)).must_equal 1 + _(@cache.store.read(key)).must_be :>, Time.now.to_f end end @@ -120,7 +120,7 @@ it 'is still banned' do key = "rack::attack:allow2ban:ban:1.2.3.4" - _(@cache.store.read(key)).must_equal 1 + _(@cache.store.read(key)).must_be :>, Time.now.to_f end end end diff --git a/spec/fail2ban_retry_after_spec.rb b/spec/fail2ban_retry_after_spec.rb new file mode 100644 index 00000000..52abb738 --- /dev/null +++ b/spec/fail2ban_retry_after_spec.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +require_relative 'spec_helper' +require 'timecop' + +[Rack::Attack::Fail2Ban, Rack::Attack::Allow2Ban].each do |klass| + describe "#{klass}.retry_after" do + let(:key_prefix) { klass.name.split('::').last.downcase } + let(:ban_key) { "#{key_prefix}:ban:1.2.3.4" } + let(:start_time) { Time.at(1_700_000_000.4) } + + before do + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + options = { bantime: 60, findtime: 60, maxretry: 2 } + + Rack::Attack.blocklist('pentest') do |req| + klass.filter(req.ip, options) { req.query_string =~ /OMGHAX/ } + end + end + + after { Timecop.return } + + def ban + Timecop.freeze(start_time) + get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' + get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' + end + + it 'is nil when not banned' do + _(klass.retry_after('1.2.3.4')).must_be_nil + end + + it 'stores the ban expiration time' do + ban + + _(klass.banned?('1.2.3.4')).must_equal true + _(Rack::Attack.cache.read(ban_key)).must_equal start_time.to_f + 60 + end + + it 'returns the remaining ban time' do + ban + _(klass.retry_after('1.2.3.4')).must_equal 60 + + Timecop.freeze(start_time + 15) + _(klass.retry_after('1.2.3.4')).must_equal 45 + end + + it 'rounds partial seconds up' do + ban + + Timecop.freeze(start_time + 0.25) + _(klass.retry_after('1.2.3.4')).must_equal 60 + + Timecop.freeze(start_time + 59.9) + _(klass.retry_after('1.2.3.4')).must_equal 1 + end + + it 'does not extend the ban on further requests' do + ban + + Timecop.freeze(start_time + 30) + get '/?foo=OMGHAX', {}, 'REMOTE_ADDR' => '1.2.3.4' + get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' + + _(last_response.status).must_equal 403 + _(klass.retry_after('1.2.3.4')).must_equal 30 + end + + it 'is nil once the ban expires' do + ban + + Timecop.freeze(start_time + 61) + _(klass.banned?('1.2.3.4')).must_equal false + _(klass.retry_after('1.2.3.4')).must_be_nil + end + + it 'is nil after reset' do + ban + klass.reset('1.2.3.4', findtime: 60) + + _(klass.retry_after('1.2.3.4')).must_be_nil + end + + it 'reads expiration times stored as strings by raw cache stores' do + Rack::Attack.cache.write(ban_key, (Time.now.to_f + 10.5).to_s, 60) + + _(klass.banned?('1.2.3.4')).must_equal true + _(klass.retry_after('1.2.3.4')).must_equal 11 + end + + it 'is at least 1 while the ban key has not expired yet' do + Rack::Attack.cache.write(ban_key, Time.now.to_f - 0.5, 60) + + _(klass.banned?('1.2.3.4')).must_equal true + _(klass.retry_after('1.2.3.4')).must_equal 1 + end + + [1, '1'].each do |legacy_value| + it "is nil for legacy bans stored as #{legacy_value.inspect}" do + Rack::Attack.cache.write(ban_key, legacy_value, 60) + + _(klass.banned?('1.2.3.4')).must_equal true + _(klass.retry_after('1.2.3.4')).must_be_nil + end + end + end +end diff --git a/spec/fail2ban_spec.rb b/spec/fail2ban_spec.rb index 6a9d9bcf..ff8e135b 100644 --- a/spec/fail2ban_spec.rb +++ b/spec/fail2ban_spec.rb @@ -61,7 +61,7 @@ it 'is banned' do key = "rack::attack:fail2ban:ban:1.2.3.4" - _(@cache.store.read(key)).must_equal 1 + _(@cache.store.read(key)).must_be :>, Time.now.to_f end end @@ -119,7 +119,7 @@ it 'is still banned' do key = "rack::attack:fail2ban:ban:1.2.3.4" - _(@cache.store.read(key)).must_equal 1 + _(@cache.store.read(key)).must_be :>, Time.now.to_f end end @@ -139,7 +139,7 @@ it 'is still banned' do key = "rack::attack:fail2ban:ban:1.2.3.4" - _(@cache.store.read(key)).must_equal 1 + _(@cache.store.read(key)).must_be :>, Time.now.to_f end end end diff --git a/spec/support/cache_store_helper.rb b/spec/support/cache_store_helper.rb index 8295ac00..473cd432 100644 --- a/spec/support/cache_store_helper.rb +++ b/spec/support/cache_store_helper.rb @@ -39,6 +39,7 @@ def self.it_works_for_cache_backed_features(options) get "/" assert_equal 403, last_response.status + assert_equal 60, Rack::Attack::Fail2Ban.retry_after("127.0.0.1") end end @@ -64,9 +65,28 @@ def self.it_works_for_cache_backed_features(options) get "/" assert_equal 403, last_response.status + assert_equal 60, Rack::Attack::Allow2Ban.retry_after("127.0.0.1") end end + it "expires bans" do + Rack::Attack.blocklist("fail2ban pentesters") do |request| + Rack::Attack::Fail2Ban.filter(request.ip, maxretry: 1, findtime: 30, bantime: 1) do + request.path.include?("private-place") + end + end + + get "/private-place" + assert_equal 403, last_response.status + assert_equal 1, Rack::Attack::Fail2Ban.retry_after("127.0.0.1") + + sleep 2.1 + + assert_nil Rack::Attack::Fail2Ban.retry_after("127.0.0.1") + get "/" + assert_equal 200, last_response.status + end + it "doesn't leak keys" do Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| request.ip