Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce DoSFilter Listener for Alert messages #5185

Closed
gonphsn opened this issue Aug 21, 2020 · 7 comments · Fixed by #5195
Closed

Introduce DoSFilter Listener for Alert messages #5185

gonphsn opened this issue Aug 21, 2020 · 7 comments · Fixed by #5195
Assignees
Labels
Enhancement Question Sponsored This issue affects a user with a commercial support agreement

Comments

@gonphsn
Copy link

gonphsn commented Aug 21, 2020

Presently the "DOS Alert" family of messages in the DosFilter class is transparently passed to the assigned Logger to the warn level as shown:

// Reject this request.
LOG.warn("DOS ALERT: Request rejected ip={}, session={}, user={}", request.getRemoteAddr(), request.getRequestedSessionId(), request.getUserPrincipal());
if (insertHeaders)
    response.addHeader("DoSFilter", "unavailable");
response.sendError(getTooManyCode());
return;

each time a DOS event is triggered. While this certainly succeeds in communicating the DOS condition to the user, we have had feedback that the repeated messages from a same source, as is likely to actually be the case in DOS conditions, has the undesired affect of overrunning log spools. Consider a sustained DOS from a single source:

WARNING [09:39:42 21-Aug-20 EDT][jetty] DOS ALERT: Request rejected ip=192.168.1.8, session=null, user=null
WARNING [09:39:42 21-Aug-20 EDT][jetty] DOS ALERT: Request rejected ip=192.168.1.8, session=null, user=null
WARNING [09:39:42 21-Aug-20 EDT][jetty] DOS ALERT: Request rejected ip=192.168.1.8, session=null, user=null
WARNING [09:39:42 21-Aug-20 EDT][jetty] DOS ALERT: Request rejected ip=192.168.1.8, session=null, user=null
WARNING [09:39:42 21-Aug-20 EDT][jetty] DOS ALERT: Request rejected ip=192.168.1.8, session=null, user=null
WARNING [09:39:42 21-Aug-20 EDT][jetty] DOS ALERT: Request rejected ip=192.168.1.8, session=null, user=null

If this condition was sustained, and the webserver not restarted or the client not added to a block list, the client could succeed in causing more interesting log messages that preceded this activity to leave the log messages in memory or displayed to the screen. If possible, such as described in the (admittedly closed issue) CocoaLumberjack/CocoaLumberjack#88, it would be a fair compromise to see something like:

WARNING [09:39:42 21-Aug-20 EDT][jetty] DOS ALERT: Request rejected ip=192.168.1.8, session=null, user=null (repeated XXXX times)

We certainly want to know that the DOS Alert was triggered; however, we do not want this condition to dominant the logs and push out other preceding events.

@joakime
Copy link
Contributor

joakime commented Aug 21, 2020

This is usually a feature of your logging framework.

logback, log4j2 both have duplicate message suppression features.

We've had feedback this kind of message is useful to determine the severity of the DOS event and automatically trigger actions based on various thresholds.

Such as .. if the same IP occurs X times in N seconds then ban IP at firewall for Z minutes.

There's various tooling on linux for this.
Example: fail2ban will monitor specific log files, for specific messages, and trigger firewall ban events based on your fail2ban configuration.

@gonphsn
Copy link
Author

gonphsn commented Aug 21, 2020

Thanks for the feedback - We are just using a JUL integration without any overlaying log framework. I agree that the present solution would allow you to see the "severity" of the DOS condition, but I suppose I'm still interested in some manner of Jetty application layer correction independent of the logger framework present. I absolutely want this logger on since it is valuable, I just want to avoid Jetty causing the display rollover. Is there a callback available in 9.4.x DosFilter that might possibly allow me to switch the Logger to severe under sustained conditions and then off again after condition passes?

@joakime
Copy link
Contributor

joakime commented Aug 21, 2020

Having those events be sent to a Listener, where the default implementation is to log is sane enough.

But switching the event level to severe will just suppress other valid events.
That's not a great solution if you had the listener.
You would need to suppress based on the combination of remote.address and remote.port at a minimum.
Allowing other addresses and port combinations to still trigger events.

If you had used literally any other logging framework, you could have easily tossed the remote address / remote port pair into a NDC or MDC and had the logger do the suppression, even doing that suppression at the specific named logger level.

@gonphsn
Copy link
Author

gonphsn commented Aug 21, 2020

Agree on the short comings of vanilla JUL, but framework switch is a completely different beast at this point. Even with context I agree it might be difficult to properly tune the suppression if the attacker was clever enough to manipulate the principal user or requested session, so IP address and port, assuming they can't be manipulated, would have to be the basis of filtering. Seems I'm stuck in a position where I have to accept the spam in order to get the value of the notification, but I appreciate the discussion. I might be able to filter out some of the messages by prefix "DOS Alert" before they get to the underlying system logger since we use JUL Handler to push to native logger, but I suppose I was hoping to articulate some DosFilter configurations that might aid in avoiding these spammy conditions.

@gonphsn
Copy link
Author

gonphsn commented Aug 21, 2020

Thinking your comment over:

"Having those events be sent to a Listener, where the default implementation is to log is sane enough."

I think this is the ideal scenario. Rather than sending the rejected, throttled and delayed events to the log, send them to a listener that the embedded instance can override and then I can decide whether or not to send them to the log. A default listener that maintains the current behavior sounds great, but I would like to supplant that listener and only log the messages for conditions I consider appropriate (i.e. not the same address/port as previous time). The "coalesce" behavior here then just becomes something to do at the listeners discretion, among other things they might want to do, and not the principle focus of the change. Perhaps this should be renamed to "Allow custom handling of DoSFilter rejected, throttled and delayed events"? That would be great since it solves my issue and permits more substantial handling for other things you might want to do (such as dynamically update the block list and such).

@joakime joakime added the Sponsored This issue affects a user with a commercial support agreement label Aug 24, 2020
@joakime joakime self-assigned this Aug 24, 2020
joakime added a commit that referenced this issue Aug 24, 2020
+ Currently there's no way to respond to rejected/throttled/delayed
  requests that the DoSFilter impacts.
  A Listener has been added to allow for any behaviors needed
  by a user of the DoSFilter on requests that have been
  impacted by the DoSFilter.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime
Copy link
Contributor

joakime commented Aug 24, 2020

Opened PR #5195

joakime added a commit that referenced this issue Aug 24, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Aug 25, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Aug 25, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Aug 26, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Sep 2, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Sep 2, 2020
+ Currently there's no way to respond to rejected/throttled/delayed
  requests that the DoSFilter impacts.
  A Listener has been added to allow for any behaviors needed
  by a user of the DoSFilter on requests that have been
  impacted by the DoSFilter.
+ Introducing OverLimit and RateType to DoSFilter internals

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Sep 3, 2020
…ener

Issue #5185 - Add DoSFilter Listener to allow extensible behavior
@joakime joakime changed the title Coalesce DOS Alert log messages Introduce DoSFilter Listener for Alert messages Sep 3, 2020
@joakime
Copy link
Contributor

joakime commented Sep 3, 2020

PR #5195 has been merged.

@joakime joakime closed this as completed Sep 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Question Sponsored This issue affects a user with a commercial support agreement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants