Nginx Rate limiting for HTTPS requests

[Maxim Dounin]

Hello!

On Wed, May 23, 2018 at 12:58:53AM -0400, rickGsp wrote:

> >>Please show "uname -a", "nginx -V", and "ps -alxww | grep nginx" 
> >>output.
> 
> #uname -a
> Linux localhost.localdomain 3.10.0-693.11.6.el7.x86_64 #1 SMP Thu Jan 4
> 01:06:37 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
> 
> #nginx -V
> nginx version: nginx/1.14.0
> built by gcc 4.8.5 20150623 (Red Hat 4.8.5-16) (GCC) 
> built with OpenSSL 1.0.2k-fips  26 Jan 2017

[...]

Thank you, I was able to reproduce this.

This indeed seems to be a problem with request rate limiting which 
manifests itself when high request rates are used, there are 
multiple worker processes, and request processing results in long 
delays during event loop iterations (so that's why HTTPS requests 
were special).

The problem is that time, as updated by each worker process at the 
event loop iteration start, can be different in different workers 
due to delays.  Moreover, a worker may be in the future relative 
to another worker.  And this interfered nicely with nginx request 
rate limiting logic which trying to be tolerant to time changes.

Quick and dirty patch below.  It is expected to work fine on 
systems with monotonic clocks available, but may need more work to 
handle time changes on systems without monotonic clocks.

diff --git a/src/http/modules/ngx_http_limit_req_module.c b/src/http/modules/ngx_http_limit_req_module.c
--- a/src/http/modules/ngx_http_limit_req_module.c
+++ b/src/http/modules/ngx_http_limit_req_module.c
@@ -398,8 +398,9 @@ ngx_http_limit_req_lookup(ngx_http_limit
             ngx_queue_insert_head(&ctx->sh->queue, &lr->queue);
 
             ms = (ngx_msec_int_t) (now - lr->last);
+            ms = ngx_max(ms, 0);
 
-            excess = lr->excess - ctx->rate * ngx_abs(ms) / 1000 + 1000;
+            excess = lr->excess - ctx->rate * ms / 1000 + 1000;
 
             if (excess < 0) {
                 excess = 0;
@@ -413,7 +414,7 @@ ngx_http_limit_req_lookup(ngx_http_limit
 
             if (account) {
                 lr->excess = excess;
-                lr->last = now;
+                lr->last += ms;
                 return NGX_OK;
             }
 
@@ -508,14 +509,15 @@ ngx_http_limit_req_account(ngx_http_limi
 
         now = ngx_current_msec;
         ms = (ngx_msec_int_t) (now - lr->last);
+        ms = ngx_max(ms, 0);
 
-        excess = lr->excess - ctx->rate * ngx_abs(ms) / 1000 + 1000;
+        excess = lr->excess - ctx->rate * ms / 1000 + 1000;
 
         if (excess < 0) {
             excess = 0;
         }
 
-        lr->last = now;
+        lr->last += ms;
         lr->excess = excess;
         lr->count--;
 


-- 
Maxim Dounin
http://mdounin.ru/