Wall time usage in DoSFilter RateTracker results in false positive alert

[gonphsn]

Jetty version(s)
9.4.46

Java version/vendor (use: java -version)
openjdk version "1.8.0_312"
OpenJDK Runtime Environment (build 1.8.0_312-8u312-b07-0ubuntu1~20.04-b07)
OpenJDK 64-Bit Server VM (build 25.312-b07, mixed mode)

OS type/version
Ubuntu 20.04 LTS

Description
We have observed that when the system time is set backwards in our environment, either through manual intervention or
by NTP/chrony/timesync, any active clients captured in the RateTracker in the DoSFilter will instantly demonstrate an ALERT on the next request.

I see that the following issue:

#121

highlights that Wall time is used in the filter RateTracker to measure time elapsed:

jetty-servlets/src/main/java/org/eclipse/jetty/servlets/DoSFilter.java:318:            final boolean overRateLimit = tracker.isRateExceeded(System.currentTimeMillis());
jetty-servlets/src/main/java/org/eclipse/jetty/servlets/DoSFilter.java:1280:            boolean hasRecentRequest = last != 0 && (System.currentTimeMillis() - last) < 1000L;

Current DoSFilter class definition demonstrates this is still in place at:

jetty-servlets\src\main\java\org\eclipse\jetty\servlets\DoSFilter.java(329): final OverLimit overLimit = tracker.isRateExceeded(System.currentTimeMillis());
jetty-servlets\src\main\java\org\eclipse\jetty\servlets\DoSFilter.java(1330): boolean hasRecentRequest = last != 0 && (System.currentTimeMillis() - last) < 1000L;

If this time is set backwards (date used is Sat 26 Oct 1985 01:21:00 AM) , the alert will appear as:

WARNING [01:21:05 26-Oct-85 UTC] DOS ALERT: Request rejected ip=192.168.56.11, overlimit=OverLimit@36fec835[type=IP, id=192.168.56.11, duration=PT-320728H-7M-49.388S, count=25], session=null, user=null

and the alert condition will not clear until time has moved forward once more past the original millis.

The condition may also manifest if time is moved forward, but in a less problematic way as it could mean that a potential DOS condition is not recognized; however, the time moving backwards scenario is the one more likely to cause frustration.

I've worked around this by using the removeFromRateTracker(String id) function to fix specific client's being actively tracked, but I think it would be very useful if the DoSFilter class exposed the _rateTracker().clear() functionality so that I could more easily inform the filter that time has meaningfully changed in the host and all clients should be reset. Our workaround is inadequate for some of the boundary conditions and a solution in the RateTracker itself is preferred.

The ideal solution may be to use nanoTime() or some other monotonic clock to track duration in the RateTracker. If switching to the monotonic clock is too disruptive, perhaps simply allow a negative duration value (in above duration=PT-320728H-7M-49.388S) to be thrown out as a recognized condition that time has gone backwards and my alert is false?

How to reproduce?
Enable the DoSFilter in a Jetty server instance.
Make a request to the server that will be captured by RateTracker, IP type used
Set system time back a significant amount
Make another request to the server from same IP address
Observe the DOS Alert is triggered

[joakime]

This is why production systems try everything in their power to not adjust the clock during operation.

Google even goes so far as to smear a 1 second leap second across the entire day in tiny increments (as even a 1 second adjustment can cause issues).
https://developers.google.com/time/smear

[gregw]

However, in this particular case.... I think we should switch the RateTracker to use nano time rather than system time. Some usages do need wall clock time, but RateTracker is not one of them.

However, HOWEVER, we are exceedingly unlikely to make this change in jetty-9.4, as it approaches end of community support (1st June.... ie tomorrow). Although we do have a 9.4 release coming out soon..... hmmm let me see if I can quickly squeeze this one in....

[gonphsn]

I agree there is wisdom in avoiding time changes completely; however, part of our API is specifically allowing for remote time change so that is not a direction we can move to quickly. I'm certainly open to workarounds like considering the negative duration, or allowing us access to clear the RateTracker universally; however, this particular usage of system time seemed as though the DoSFilter became the very thing it wanted to protect against. If the timing was correct, these false positives themselves become a form of denial to that particular client.

Regarding the EOL, I would be happy to sponsor the correction through our support channel in a future release if that is the preferred option. I see there is some progress on this issue for 9.4.47 so please let me know if you have any further guidance on how to move forward. Thanks!

[jmcc0nn3ll]

@gonphsn (or anyone really) Webtide is the company behind the open source Jetty and CometD projects. If you are interested in learning more about how our support service works for things like this feel free to reach out to me at [email protected] and I would be happy to chat. I am a long time committer on the project and I chat with lots of folks in the community, companies and individuals.