Skip to content

Conversation

@NOX73
Copy link

@NOX73 NOX73 commented Jan 6, 2024

I want to use rack-attack to block pentesters based on 4xx requests. It's not possible with the current gem API.

So here is a POC how it might look like:

    Rack::Attack.postrequest("fail2ban for 404") do |request, response|
      Rack::Attack::Fail2Ban.filter(request.ip, maxretry: 2, findtime: 30, bantime: 60) do
        if response.nil?
          false
        else
          response[0] == 404
        end
      end
    end

It might look clumsy, but you have to check if a response exists with such API.

So, I'm ready to add more specs and finish this to get it ready to merge if you think this is a good idea.

Naming is also debatable.

@seliverstov-maxim
Copy link

1 similar comment
@senzpo
Copy link

senzpo commented Jan 10, 2024

assert_equal 200, last_response.status
end


Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spec/acceptance/postrequest_spec.rb:29:1: C: [Correctable] Layout/EmptyLines: Extra blank line detected.
spec/acceptance/postrequest_spec.rb:51:1: C: [Correctable] Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body end.

68 files inspected, 2 offenses detected, 2 offenses auto-correctable
RuboCop failed!

@type = :postrequest
end

def matched_by?(request, response)
Copy link

@seliverstov-maxim seliverstov-maxim Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that this method calling twice:

  • the first time before app.call
  • the second time after app.call

I guess that we can accidentally mark a request as matched even when it doesn't blocked.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mb change postrequest.matched_by? to postrequest.block.call?

#lib/rack/attack/configuration.rb

def process_postrequests(request, response)
    @postrequests.each { |_name, postrequest| postrequest.matched_by?(request, response) }
end

@seliverstov-maxim
Copy link

@santib The solution is feet to the gem philosophy? Is there any chance the PR to be accepted? I would like to have the opportunity to use this feature in my projects.

@santib
Copy link
Collaborator

santib commented Jan 16, 2024

Hi @NOX73 and @seliverstov-maxim 👋 First of all, thanks for your contribution.

I've been thinking about this PR for some days already, but didn't comment because I haven't formed an opinion just yet.

You said that the reason to add this feature is because

I want to use rack-attack to block pentesters based on 4xx requests

Would you be able to expand it a bit more? Like why do you want to do it only based on 4xx? Why wouldn't throttling or blocklist+fail2ban be enough for your use case?

@NOX73
Copy link
Author

NOX73 commented Jan 17, 2024

Would you be able to expand it a bit more? Like why do you want to do it only based on 4xx? Why wouldn't throttling or blocklist+fail2ban be enough for your use case?

Sure, no problem. Some scanning programs scan some URLs, but there are some clients who might put same load, like the same requests per time. The only different we see - it's the response. In the scanning case it a lot of 404 errors. So would be very helpful to ban them based on what status code the request ended with.

@santib
Copy link
Collaborator

santib commented Jan 22, 2024

@NOX73 Honestly, I'm not sold on adding this feature to rack-attack. I'm wondering, would it be possible to add this feature as a separate gem? I mean something like a rack-attack extension (e.g. rack-attack-postrequest). If not, what would be needed to make it work? Also, would it make sense to hook into rack.response_finished for this postprocess stuff?

@NOX73
Copy link
Author

NOX73 commented Jan 23, 2024

@santib Do you think it does not fit the gem idea, or is it our corner case?

Sure I think it might be a separate gem, why not. I just was surprised there is no way to implement such common (in my opinion) functionality.

@seliverstov-maxim
Copy link

seliverstov-maxim commented Jan 23, 2024

I mean something like a rack-attack extension (e.g. rack-attack-postrequest)

Sounds good to me

@johnnyshields
Copy link

This seems totally reasonable to include in the RackAttack gem itself. I don't like the idea of having to make this a separate gem.

@JulienItard
Copy link

JulienItard commented Mar 20, 2024

@seliverstov-maxim @NOX73 Hello, i have more or less same use case. We have a public API and some of our customers use our API in wrong way, so we returns errors. I would like to add throlling when there is too many errors in a period. Like "error limits exceeded".

@seliverstov-maxim
Copy link

I investigated the point "I mean something like a rack-attack extension (e.g. rack-attack-postrequest)"
Imho it's impossible to do this, because @NOX73 changes interface of core classes. Support core classes in this gem and in extension looks like overhead.
One way to do this is big refactoring.

@alexsmartens
Copy link
Contributor


damn, this would be sick! Any progress?

Any suggestions for a temporary workaround? I was thinking about caching response codes and basing the throttle/blocking logic based on those cached response codes

@gregnavis
Copy link

I think this fits with the overall goal of rack-attack. An extension would be a good place for preventing specific kinds of attacks. This is different, though: we're adding a generic mechanism for taking responses into account. Such mechanism is something a third-party extension could be built upon.

Additionally, if we're talking about supporting extensions then some of rack-attack classes would become published interfaces, and we'd need to take backwards compatibility into account.

I have a use case for this. A side project of mine has been subjected to some account spam. Bots used by spammer send requests that cause ActionController::ParameterMissing to be raised. The parameters I can see in error captures indicate these requests were not generated by the app (it's impossible to send such malformed request + decoy input is filled indicating bot activity).

Currently, in order to handle that case, I'd need to replicate a lot of controller logic in my rack-attack initializer. This can be problematic for a single controller action, not to mention when many disparate controller actions need to be protected.

@alexsmartens
Copy link
Contributor

My use case is to prevent brute forcing things like login. If there are N failures I want to block anyone from logging in to this account for some time, say a day. Because this logic is based on the response code there is no good way to make it work with Rack::Attack without a workaround.

@santib
Copy link
Collaborator

santib commented May 8, 2025

Hi @NOX73 and @seliverstov-maxim, I come back with some good news I think.

I did a POC, based on the postrequest spec to see if it was possible to have the same assertions pass without having to change Rack::Attack, and seems like I could make it work.

In this branch you can find the same test but with a different setup. In short, I'm achieving the same behavior by:

  1. Creating a new middleware (that runs after the app) which makes use of Fail2Ban filter to increase the counter for the discriminator (request.ip) when status == 404.
  2. Configuring Rack::Attack with a normal blocklist which makes use of the same filter that is being used by the middleware. This is to block the requests, but NOT to increase the counter.

I do find the API a bit strange, but I think this is very cool for two reasons:

  1. You can have this feature in your app today without even upgrading Rack::Attack.
  2. I feel like there is an opportunity to create a gem that encapsulates this behavior and provides it with a simpler API.

Thank you for your patience, would love to hear your thoughts.

@santib
Copy link
Collaborator

santib commented May 14, 2025

Closing the PR since the requested feature can be solved without any additional changes.

I'm open to continue the conversation in an issue/discussion, or accept a PR adding documentation/example on how to use the response in Rack::Attack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants