bad behavior - fix 500 error and do not pass objects with another lifetime to timers

2023-04-24 10:36:29 +02:00 · 2023-04-24 10:36:29 +02:00 · a60b6f3ada
parent d32102376f
commit a60b6f3ada
2 changed files with 54 additions and 102 deletions
--- a/src/common/confs/server-http/access-lua.conf
+++ b/src/common/confs/server-http/access-lua.conf
@ -31,10 +31,10 @@ end
 logger:log(ngx.INFO, "ngx.ctx filled (ret = " .. ret .. ")")

 -- Process bans as soon as possible
-local ok, reason = datastore:get("bans_ip_" .. ngx.ctx.bw.remote_addr)
-if not ok and reason ~= "not found" then
+local reason, err = datastore:get("bans_ip_" .. ngx.ctx.bw.remote_addr)
+if not reason and err ~= "not found" then
 	logger:log(ngx.ERR, "error while checking if client is banned : " .. reason)
-elseif ok and reason ~= "not found" then
+elseif reason and err ~= "not found" then
 	logger:log(ngx.WARN, "IP " .. ngx.ctx.bw.remote_addr .. " is banned with reason : " .. reason)
 	return ngx.exit(utils.get_deny_status())
 end
--- a/src/common/core/badbehavior/badbehavior.lua
+++ b/src/common/core/badbehavior/badbehavior.lua
@ -1,8 +1,6 @@
 local class			= require "middleclass"
 local plugin		= require "bunkerweb.plugin"
 local utils			= require "bunkerweb.utils"
-local datastore		= require "bunkerweb.datastore"
-local clusterstore	= require "bunkerweb.clusterstore"

 local badbehavior = class("badbehavior", plugin)

@ -36,7 +34,7 @@ function badbehavior:log()
 		return self:ret(true, "already banned")
 	end
 	-- Call increase function later and with cosocket enabled
-	local ok, err = ngx.timer.at(0, badbehavior.increase, self, ngx.ctx.bw.remote_addr)
+	local ok, err = ngx.timer.at(0, badbehavior.increase, ngx.ctx.bw.remote_addr, tonumber(self.variables["BAD_BEHAVIOR_COUNT_TIME"]), tonumber(self.variables["BAD_BEHAVIOR_BAN_TIME"]), tonumber(self.variables["BAD_BEHAVIOR_THRESHOLD"]), self.use_redis)
 	if not ok then
 		return self:ret(false, "can't create increase timer : " .. err)
 	end
@ -47,27 +45,26 @@ function badbehavior:log_default()
 	return self:log()
 end

-function badbehavior.increase(premature, obj, ip)
-	-- Our vars
-	local count_time = tonumber(obj.variables["BAD_BEHAVIOR_COUNT_TIME"])
-	local ban_time = tonumber(obj.variables["BAD_BEHAVIOR_BAN_TIME"])
-	local threshold = tonumber(obj.variables["BAD_BEHAVIOR_THRESHOLD"])
+function badbehavior.increase(premature, ip, count_time, ban_time, threshold, use_redis)
+	-- Instantiate objects
+	local logger = require "bunkerweb.logger":new("badbehavior")
+	local datastore = require "bunkerweb.datastore":new()
 	-- Declare counter
 	local counter = false
 	-- Redis case
-	if obj.use_redis then
-		local redis_counter, err = obj:redis_increase(ip)
+	if use_redis then
+		local redis_counter, err = bad_behavior.redis_increase(ip, count_time, ban_time)
 		if not redis_counter then
-			obj.logger:log(ngx.ERR, "(increase) redis_increase failed, falling back to local : " .. err)
+			logger:log(ngx.ERR, "(increase) redis_increase failed, falling back to local : " .. err)
 		else
 			counter = redis_counter
 		end
 	end
 	-- Local case
 	if not counter then
-		local local_counter, err = obj.datastore:get("plugin_badbehavior_count_" .. ip)
+		local local_counter, err = datastore:get("plugin_badbehavior_count_" .. ip)
 		if not local_counter and err ~= "not found" then
-			obj.logger:log(ngx.ERR, "(increase) can't get counts from the datastore : " .. err)
+			logger:log(ngx.ERR, "(increase) can't get counts from the datastore : " .. err)
 		end
 		if local_counter == nil then
 			local_counter = 0
@ -75,48 +72,48 @@ function badbehavior.increase(premature, obj, ip)
 		counter = local_counter + 1
 	end
 	-- Call decrease later
-	local ok, err = ngx.timer.at(count_time, badbehavior.decrease, obj, ip)
+	local ok, err = ngx.timer.at(count_time, badbehavior.decrease, ip, count_time, threshold, use_redis)
 	if not ok then
-		obj.logger:log(ngx.ERR, "(increase) can't create decrease timer : " .. err)
+		logger:log(ngx.ERR, "(increase) can't create decrease timer : " .. err)
 	end
 	-- Store local counter
-	local ok, err = obj.datastore:set("plugin_badbehavior_count_" .. ip, counter)
+	local ok, err = datastore:set("plugin_badbehavior_count_" .. ip, counter, count_time)
 	if not ok then
-		obj.logger:log(ngx.ERR, "(increase) can't save counts to the datastore : " .. err)
+		logger:log(ngx.ERR, "(increase) can't save counts to the datastore : " .. err)
 		return
 	end
 	-- Store local ban
 	if counter > threshold then
-		local ok, err = obj.datastore:set("bans_ip_" .. ip, "bad behavior", ban_time)
+		local ok, err = datastore:set("bans_ip_" .. ip, "bad behavior", ban_time)
 		if not ok then
-			obj.logger:log(ngx.ERR, "(increase) can't save ban to the datastore : " .. err)
+			logger:log(ngx.ERR, "(increase) can't save ban to the datastore : " .. err)
 			return
 		end
-		obj.logger:log(ngx.WARN, "IP " .. ip .. " is banned for " .. ban_time .. "s (" .. tostring(counter) .. "/" .. tostring(threshold) .. ")")
+		logger:log(ngx.WARN, "IP " .. ip .. " is banned for " .. ban_time .. "s (" .. tostring(counter) .. "/" .. tostring(threshold) .. ")")
 	end
+	logger:log(ngx.NOTICE, "increased counter for IP " .. ip .. " (" .. tostring(counter) .. "/" .. tostring(threshold) .. ")")
 end

-function badbehavior.decrease(premature, obj, ip)
-	-- Our vars
-	local count_time = tonumber(obj.variables["BAD_BEHAVIOR_COUNT_TIME"])
-	local ban_time = tonumber(obj.variables["BAD_BEHAVIOR_BAN_TIME"])
-	local threshold = tonumber(obj.variables["BAD_BEHAVIOR_THRESHOLD"])
+function badbehavior.decrease(premature, ip, count_time, threshold, use_redis)
+	-- Instantiate objects
+	local logger = require "bunkerweb.logger":new("badbehavior")
+	local datastore = require "bunkerweb.datastore":new()
 	-- Declare counter
 	local counter = false
 	-- Redis case
-	if obj.use_redis then
-		local redis_counter, err = obj:redis_decrease(ip)
+	if use_redis then
+		local redis_counter, err = badbehavior.redis_decrease(ip)
 		if not redis_counter then
-			obj.logger:log(ngx.ERR, "(increase) redis_increase failed, falling back to local : " .. err)
+			logger:log(ngx.ERR, "(increase) redis_increase failed, falling back to local : " .. err)
 		else
 			counter = redis_counter
 		end
 	end
 	-- Local case
 	if not counter then
-		local local_counter, err = obj.datastore:get("plugin_badbehavior_count_" .. ip)
+		local local_counter, err = datastore:get("plugin_badbehavior_count_" .. ip)
 		if not local_counter and err ~= "not found" then
-			obj.logger:log(ngx.ERR, "(increase) can't get counts from the datastore : " .. err)
+			logger:log(ngx.ERR, "(increase) can't get counts from the datastore : " .. err)
 		end
 		if local_counter == nil or local_counter <= 1 then
 			counter = 0
@ -126,20 +123,20 @@ function badbehavior.decrease(premature, obj, ip)
 	end
 	-- Store local counter
 	if counter <= 0 then
-		local ok, err = obj.datastore:delete("plugin_badbehavior_count_" .. ip)
+		local ok, err = datastore:delete("plugin_badbehavior_count_" .. ip)
 	else
-		local ok, err = obj.datastore:delete("plugin_badbehavior_count_" .. ip, counter)
+		local ok, err = datastore:set("plugin_badbehavior_count_" .. ip, counter, count_time)
 		if not ok then
-			obj.logger:log(ngx.ERR, "(increase) can't save counts to the datastore : " .. err)
+			logger:log(ngx.ERR, "(increase) can't save counts to the datastore : " .. err)
 			return
 		end
 	end
+	logger:log(ngx.NOTICE, "decreased counter for IP " .. ip .. " (" .. tostring(counter) .. "/" .. tostring(threshold) .. ")")
 end

-function badbehavior:redis_increase(ip)
-	-- Our vars
-	local count_time = tonumber(self.variables["BAD_BEHAVIOR_COUNT_TIME"])
-	local ban_time = tonumber(self.variables["BAD_BEHAVIOR_BAN_TIME"])
+function badbehavior.redis_increase(ip, count_time, ban_time)
+	-- Instantiate objects
+	local clusterstore = require "bunkerweb.clusterstore":new()
 	-- Our LUA script to execute on redis
 	local redis_script = [[
 		local ret_incr = redis.pcall("INCR", KEYS[1])
@ -162,54 +159,24 @@ function badbehavior:redis_increase(ip)
 		return ret_incr
 	]]
 	-- Connect to server
-	local cstore = clusterstore:new()
-	local ok, err = cstore:connect()
+	local ok, err = clusterstore:connect()
 	if not ok then
 		return false, err
 	end
 	-- Execute LUA script
-	local counter, err = cstore:call("eval", redis_script, 2, "bad_behavior_" .. ip, "ban_" .. ip, "bad behavior", count_time, ban_time)
-	self.logger(ngx.ERR, counter)
+	local counter, err = clusterstore:call("eval", redis_script, 2, "bad_behavior_" .. ip, "bans_ip" .. ip, count_time, ban_time)
 	if not counter then
-		cstore:close()
+		clusterstore:close()
 		return false, err
 	end
-	-- Exec transaction
-	-- local calls = {
-	-- 	{"incr", {"bad_behavior_" .. ip}},
-	-- 	{"expire", {"bad_behavior_" .. ip, count_time}}
-	-- }
-	-- local ok, err, exec = clusterstore:multi(calls)
-	-- if not ok then
-	-- 	clusterstore:close()
-	-- 	return false, err
-	-- end
-	-- -- Extract counter
-	-- local counter = exec[1]
-	-- if type(counter) == "table" then
-	-- 	clusterstore:close()
-	-- 	return false, counter[2]
-	-- end
-	-- -- Check expire result
-	-- local expire = exec[2]
-	-- if type(expire) == "table" then
-	-- 	clusterstore:close()
-	-- 	return false, expire[2]
-	-- end
-	-- -- Add IP to redis bans if needed
-	-- if counter > threshold then
-	-- 	local ok, err = clusterstore:call("set", "ban_" .. ip, "bad behavior", "EX", ban_time)
-	-- 	if err then
-	-- 		clusterstore:close()
-	-- 		return false, err
-	-- 	end
-	-- end
 	-- End connection
-	cstore:close()
+	clusterstore:close()
 	return counter
 end

-function badbehavior:redis_decrease(ip)
+function badbehavior.redis_decrease(ip, count_time)
+	-- Instantiate objects
+	local clusterstore = require "bunkerweb.clusterstore":new()
 	-- Our LUA script to execute on redis
 	local redis_script = [[
 		local ret_decr = redis.pcall("DECR", KEYS[1])
@ -217,46 +184,31 @@ function badbehavior:redis_decrease(ip)
 			redis.log(redis.LOG_WARNING, "Bad behavior decrease DECR error : " .. ret_decr["err"])
 			return ret_decr
 		end
+		local ret_expire = redis.pcall("EXPIRE", KEYS[1], ARGV[1])
+		if type(ret_expire) == "table" and ret_expire["err"] ~= nil then
+			redis.log(redis.LOG_WARNING, "Bad behavior decrease EXPIRE error : " .. ret_expire["err"])
+			return ret_expire
+		end
 		if ret_decr <= 0 then
 			local ret_del = redis.pcall("DEL", KEYS[1])
 			if type(ret_del) == "table" and ret_del["err"] ~= nil then
-				redis.log(redis.LOG_WARNING, "Bad behavior increase DEL error : " .. ret_del["err"])
+				redis.log(redis.LOG_WARNING, "Bad behavior decrease DEL error : " .. ret_del["err"])
 				return ret_del
 			end
 		end
 		return ret_decr
 	]]
 	-- Connect to server
-	local cstore = clusterstore:new()
-	local ok, err = cstore:connect()
+	local ok, err = clusterstore:connect()
 	if not ok then
 		return false, err
 	end
-	local counter, err = cstore:call("eval", redis_script, 1, "bad_behavior_" .. ip)
-	self.logger(ngx.ERR, counter)
+	local counter, err = clusterstore:call("eval", redis_script, 1, "bad_behavior_" .. ip, count_time)
 	if not counter then
-		cstore:close()
+		clusterstore:close()
 		return false, err
 	end
-	-- -- Decrement counter
-	-- local counter, err = clusterstore:call("decr", "bad_behavior_" .. ip)
-	-- if err then
-	-- 	clusterstore:close()
-	-- 	return false, err
-	-- end
-	-- -- Delete counter
-	-- if counter < 0 then
-	-- 	counter = 0
-	-- end
-	-- if counter == 0 then
-	-- 	local ok, err = clusterstore:call("del", "bad_behavior_" .. ip)
-	-- 	if err then
-	-- 		clusterstore:close()
-	-- 		return false, err
-	-- 	end
-	-- end
-	-- End connection
-	cstore:close()
+	clusterstore:close()
 	return counter
 end