rack / rack-attack

Rack middleware for blocking & throttling
MIT License
5.57k stars 337 forks source link

Potential Bug in Rack-Attack Gem with Ruby on Rails #668

Closed dadiaz18 closed 2 months ago

dadiaz18 commented 2 months ago

Gem Versions:

Context

I encountered an issue with the rack-attack gem when applied to a Rails application. I noticed that the TTL (Time-to-Live) values for Redis keys are inconsistently applied, causing problems with request rate limiting.

Problem Description

When blocking requests based on actions taken on an endpoint, the filters apply a TTL in Redis that does not align with the expected duration. This inconsistency happens randomly. For instance, if a 3-minute window is supposed to validate more than 20 hits, the Redis key should have a TTL of approximately 180 seconds. However, in some cases, it only applies a TTL of 80 seconds, making it impossible to validate the conditions because the data persists incorrectly.

This issue is not noticeable with smaller numbers of hits in a shorter time frame (e.g., 10 hits in one minute) because the TTL mismatch is less obvious. For example, the expected TTL might be 60 seconds, but it sometimes applies 30 seconds, thus obscuring the error.

Example code for implementation

RA_REDIS_CONF = {
  url: ENV.fetch("REDIS_URL"),
  ssl_params: { verify_mode: OpenSSL::SSL::VERIFY_NONE },
}

rack_attack = Rack::Attack
rack_attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new(**RA_REDIS_CONF)

API_ENDPOINT = "/api/endpoint"

rack_attack.blocklist("filter 1") do |req|
  rack_attack::Allow2Ban.filter("filter_1:#{req.ip}", maxretry: 20, findtime: 180, bantime: 1.hour) do
    req.path == API_ENDPOINT && req.post?
  end
end

rack_attack.blocklist("filter 2") do |req|
  rack_attack::Allow2Ban.filter("filter_2:#{req.ip}", maxretry: 10, findtime: 60, bantime: 3.minutes) do
    req.path == API_ENDPOINT && req.post?
  end
end

rack_attack.blocklisted_responder = lambda do |request_env|
  [403, { "Content-Type" => "application/json" }, [{ error: "Too many requests. Please try again later." }.to_json]]
end

Logs of TTLs wrongly applied when I start the counters

TTL for rack::attack:95485:allow2ban:count:filter_1:14.0.4.4: --> 68  # (should be approx 180)
TTL for rack::attack:25841:allow2ban:count:filter_2:14.0.4.4: --> 33  # (should be approx 60)

Please, any possible solution related to this?

Thanks.

santib commented 2 months ago

Hi @dadiaz18, is it possible that this report is related to https://github.com/rack/rack-attack/pull/578 ?

That the throttling/blocking happens in time windows of the specified duration but that not necessarily blocks one requester for the exact specified time?

related code https://github.com/rack/rack-attack/blob/427fdfabbc4b6283af14b6916dec4d2d4074e9e4/lib/rack/attack/cache.rb#L69-L74

dadiaz18 commented 2 months ago

The truth is that I didn’t analyze the behavior of that function, but it could be closely related. All I did was run tests on a code implemented with the example above and measured the TTL times of Redis that were assigned to track behaviors starting from the first action. I concluded that the problem wasn’t Redis or the versions I had installed; it was a custom implementation of Rack::Attack that I didn’t examine closely.

santib commented 2 months ago

Ok, makes sense.

I was also confused the first time I used rack-attack TBH 😅

I'll close this issue, given that it's how it has been working since the creation of the gem. That said, I'm open to change its behavior, but it should be done under a non-default configuration since it'd change a core aspect of the gem.

dadiaz18 commented 2 months ago

I get that the current approach just does not cut it for me. Thank you!