Time based deprecate for ngx.shared.DICT

[rezaalavi]

Hi,

ngx.shared.DICT can usually be used for ip block lists. A cool feature that is available on modsecurity is deprecatevar which works as follow:

You define a variable which acts as a counter (for instance number of failed login attempts by a particular ip). Then in their rules they define deprecatevar 1/180 on that counter which reduces the counter by 1 in 180 seconds. In other words the counter only has the count of failed attempts in the last 180 seconds.

This would be an extremely useful function to help us write webserver firewall rules and even translate modsecuirty rules to native lua and just use lua for nginx.

My suggestion is to have ngx.shared.DICT:deprecate(key, amount,time) which will deprecate the value of "key" by the value of "amount" parameter only once in the next "time" seconds. This function then usually should get called right after a ngx.shared.DICT:incr(key, value) to effectively expire result of the last increment after a set amount of time.

[agentzh]

@rezaalavi By deprecating a key, you mean invalidating it? I'm not sure if I completely understand you. Some concrete examples can be helpful I guess?

[rezaalavi]

Hi,

Thanks for the quick reply. It does not invalidate the key, it simply decreases the value of the key when the time is up.

So lets say we want to write a code to block an ip that has got 10 return code of 403 in the past 3 minutes.

What we need to do is to first have a shared dict where we increment the value for each ip for each 403 they get. Something like this: ngx.shared.DICT:incr(request_ip_address, 1) nxg.shared.DICT.deprecatevar(request_ip,1,180)

So for the above after 180 seconds it is like we are going to in the background run: ngx.shared.DICT:incr(request_ip_address, -1)

Here is a rule from modsecurity to block bruteforce attacks to wp-login

SecRule RESPONSE_STATUS "^200" "phase:5,chain,t:none,nolog,pass,setvar:ip.bf_counter=+1,deprecatevar:ip.bf_counter=1/180,id:5000137" SecRule ip:bf_counter "@gt 10" "t:none,setvar:user.bf_block=1,expirevar:user.bf_block=300,setvar:ip.bf_counter=0"

A simple translation of the above rules is : if the condition for failed login is met (in this case response of 200) increment the value of counter for that ip by 1 but in 3 minutes decrease the ip counter by 1. Then it checks the value of counter and if it is more than 10 it rejects the request! As you can see because of the deprecate function the counter is only the count of incidents in the last 3 minutes.

Please let me know if this make sense.

[agentzh]

@rezaalavi Okay, I see. A counter for the the most recent N milliseconds is useful in general. I'm just not sure how to implement such things efficiently. Are you willing to propose a patch? Thanks! BTW, IMHO, deprecate() is a bad method name for this; we need some better name to be clearer on our intentions :)

[rezaalavi]

@agentzh my understanding of nginx-lua is not that good, I am afraid this needs to be implemented in c level to manage resource allocation and performance for a case where every single request has to go through this function.

[agentzh]

@rezaalavi I've rechecked this. It seems that you can easily implement this with the ngx.timer.at API. I'm not sure how modsecurity implements it, but it seems expensive from the description in its official reference manual:

https://github.com/SpiderLabs/ModSecurity/wiki/Reference-Manual#deprecatevar

Unlike expirevar, the deprecate action must be executed on every request.

But I'm not very sure what exactly this sentence means without checking out its implementation :)

[p0pr0ck5]

deprecatevar doesn't quite work like a time-based decrement via recurring ngx.timer.at would implement. when called, it decrements the value of the given variable by the amount determined from the ratio associated with the deprecatevar. this only requires that the action in question is matched with essentially every single request, because it doesn't fire via a timer. that's not terribly expensive (SecAction matches are relatively cheap). Having said all that, I don't think adding this functionality directly to ngx.shared.DICT is worthwhile, particularly when it can be managed more intelligently by a ngx.timer.

BTW, using shared dictionaries to represent ModSecurity persistent storage is not ideal, because you're essentially representing a 2-d array as a single k/v store. Either some serialization or a nasty call to get_keys() is necessary. It's ugly ;)