bw - refactor session handling and fix antibot

2026-05-24 09:28:37 +00:00 · 2024-01-22 10:51:53 +01:00 · 2024-01-22 10:51:53 +01:00 · 0efc31c4d1
commit 0efc31c4d1
parent 9cce27228d
4 changed files with 107 additions and 120 deletions
--- a/src/bw/lua/bunkerweb/utils.lua
+++ b/src/bw/lua/bunkerweb/utils.lua
@ -569,73 +569,71 @@ utils.get_deny_status = function()
 	return 444
 end

-utils.check_session = function(ctx)
-	local _session, _, exists, _ = session_start({ audience = "metadata" })
+utils.get_session = function(ctx)
+	-- Return session from ctx if already there
+	if ctx.bw.sessions_session then
+		return ctx.bw.sessions_session
+	end
+	-- Open/create and do an optional refresh
+	local session, err, exists, refreshed = session_start()
+	if not session then
+		return nil, err
+	end
+	if err then
+		logger:log(WARN, "can't open session : " .. err)
+	end
+	local checks = {
+		["IP"] = ctx.bw.remote_addr,
+		["USER_AGENT"] = ctx.bw.http_user_agent or "",
+	}
 	if exists then
-		for _, check in ipairs(ctx.bw.sessions_checks) do
-			local key = check[1]
-			local value = check[2]
-			if _session:get(key) ~= value then
-				_session:clear_request_cookie()
-				local ok, err = _session:destroy()
-				if not ok then
-					return false, "session:destroy() error : " .. err
+		logger:log(INFO, "opening an existing session")
+		if refreshed then
+			logger:log(INFO, "existing session refreshed")
+		end
+		-- Get metadata
+		local metadata = session:get("metadata")
+		if metadata then
+			-- Check if session passes the checks
+			for check, value in pairs(checks) do
+				local check_value
+				check_value, err = utils.get_variable("SESSIONS_CHECK_" .. check, false, nil)
+				if not check_value then
+					logger:log(ERR, "error while getting variable SESSIONS_CHECK_" .. check .. " : " .. err)
+				elseif check_value == "yes" and value ~= metadata[check] then
+					logger:log(WARN, "session check failed : " .. check .. "!=" .. metadata[check])
+					local ok
+					ok, err = session:destroy()
+					if not ok then
+						return nil, err
+					end
+					return utils.get_session(ctx)
 				end
-				logger:log(WARN, "session check " .. key .. " failed, destroying session")
-				return utils.check_session(ctx)
 			end
 		end
 	else
-		for _, check in ipairs(ctx.bw.sessions_checks) do
-			_session:set(check[1], check[2])
-		end
-		local ok, err = _session:save()
-		if not ok then
-			_session:close()
-			return false, "session:save() error : " .. err
-		end
+		logger:log(INFO, "creating a new session")
+		session:set("metadata", checks)
+		ctx.bw.sessions_updated = true
 	end
-	ctx.bw.sessions_is_checked = true
-	return true, exists
+	ctx.bw.sessions_session = session
+	return session
 end

-utils.get_session = function(audience, ctx)
-	-- Check session
-	if not ctx.bw.sessions_is_checked then
-		local ok, err = utils.check_session(ctx)
-		if not ok then
-			return false, "error while checking session, " .. err
+utils.save_session = function(ctx)
+	if ctx.bw.sessions_session then
+		if ctx.bw.sessions_updated then
+			local ok, err = ctx.bw.sessions_session:save()
+			if not err then
+				err = "session saved"
+			end
+			return ok, err
+		else
+			return true, "session not updated"
 		end
+	else
+		return true, "no session"
 	end
-	-- Open session with specific audience
-	local _session, err, _ = session_open({ audience = audience })
-	if err then
-		logger:log(INFO, "session:open() error : " .. err)
-	end
-	return _session
-end
-
-- luacheck: ignore 214
-utils.get_session_data = function(_session, site, ctx)
-	local site_only = site == nil or site
-	local data = _session:get_data()
-	if site_only then
-		return data[ctx.bw.server_name] or {}
-	end
-	return data
-end
-
-- luacheck: ignore 214
-utils.set_session_data = function(_session, data, site, ctx)
-	local site_only = site == nil or site
-	if site_only then
-		local all_data = _session:get_data()
-		all_data[ctx.bw.server_name] = data
-		_session:set_data(all_data)
-		return _session:save()
-	end
-	_session:set_data(data)
-	return _session:save()
 end

 utils.is_banned = function(ip)
--- a/src/common/confs/server-http/access-lua.conf
+++ b/src/common/confs/server-http/access-lua.conf
@ -22,6 +22,7 @@ access_by_lua_block {
 	local is_banned = utils.is_banned
 	local set_reason = utils.set_reason
 	local get_deny_status = utils.get_deny_status
+	local save_session = utils.save_session
 	local tostring = tostring

 	-- Don't process internal requests
@ -120,6 +121,14 @@ access_by_lua_block {
 	end
 	logger:log(INFO, "called access() methods of plugins")

+	-- Save session
+	ok, err = save_session(ctx)
+	if ok then
+		logger:log(INFO, err)
+	else
+		logger:log(ERR, err)
+	end
+
 	-- Save ctx
 	save_ctx(ctx)

--- a/src/common/core/antibot/antibot.lua
+++ b/src/common/core/antibot/antibot.lua
@ -12,6 +12,7 @@ local ngx = ngx
 local subsystem = ngx.config.subsystem
 local HTTP_INTERNAL_SERVER_ERROR = ngx.HTTP_INTERNAL_SERVER_ERROR
 local OK = ngx.OK
+local INFO = ngx.INFO
 local tonumber = tonumber
 local tostring = tostring
 local get_session = utils.get_session
@ -46,37 +47,26 @@ function antibot:header()
 		return self:ret(true, "antibot not activated")
 	end
 	-- Check if antibot uri
-	if self.ctx.bw.uri ~= self.variables["ANTIBOT_URI"] then
-		return self:ret(true, "Not antibot uri")
-	end
-
-	-- Get session data
-	local session, err = get_session("antibot", self.ctx)
-	if not session then
-		return self:ret(false, "can't get session : " .. err, HTTP_INTERNAL_SERVER_ERROR)
-	end
-	self.session = session
-	self.session_data = get_session_data(self.session, true, self.ctx)
-	-- Check if session is valid
-	self:check_session()
-
-	-- Don't go further if client resolved the challenge
-	if self.session_data.resolved then
-		if self.ctx.bw.uri == self.variables["ANTIBOT_URI"] then
-			return self:ret(true, "client already resolved the challenge", nil, self.session_data.original_uri)
-		end
-		return self:ret(true, "client already resolved the challenge")
-	end
-
 	if self.ctx.bw.uri ~= self.variables["ANTIBOT_URI"] then
 		return self:ret(true, "not antibot uri")
 	end

+	-- Get session data
+	self.session_data = self.ctx.bw.antibot_session_data
+	if not self.session_data then
+		return self:ret(false, "can't get session data", HTTP_INTERNAL_SERVER_ERROR)
+	end
+
+	-- Don't go further if client resolved the challenge
+	if self.session_data.resolved then
+		return self:ret(true, "client already resolved the challenge", nil, self.session_data.original_uri)
+	end
+
+	-- Override headers
 	local header = "Content-Security-Policy"
 	if self.variables["CONTENT_SECURITY_POLICY_REPORT_ONLY"] == "yes" then
 		header = header .. "-Report-Only"
 	end
-
 	if self.session_data.type == "recaptcha" then
 		ngx.header[header] = "default-src 'none'; form-action 'self'; script-src 'strict-dynamic' 'nonce-"
 			.. self.session_data.nonce_script
@ -108,7 +98,7 @@ function antibot:header()
 			.. self.session_data.nonce_style
 			.. "'; font-src 'self' data:; base-uri 'self';"
 	end
-	return self:ret(true, "Successfully overridden CSP header")
+	return self:ret(true, "successfully overridden CSP header")
 end

 function antibot:access()
@ -118,14 +108,17 @@ function antibot:access()
 	end

 	-- Get session data
-	local session, err = get_session("antibot", self.ctx)
+	local session, err = get_session(self.ctx)
 	if not session then
-		return self:ret(false, "can't get session : " .. err, HTTP_INTERNAL_SERVER_ERROR)
+		return self:ret(false, "can't get session : " .. err)
 	end
 	self.session = session
-	self.session_data = get_session_data(self.session, true, self.ctx)
+	self.session_data = session:get("antibot") or {}
+	self.ctx.bw.antibot_session_data = self.session_data
+
 	-- Check if session is valid
-	self:check_session()
+	local msg = self:check_session()
+	self.logger:log(INFO, "check_session returned : " .. msg)

 	-- Don't go further if client resolved the challenge
 	if self.session_data.resolved then
@ -137,10 +130,6 @@ function antibot:access()

 	-- Prepare challenge if needed
 	self:prepare_challenge()
-	local ok, err = self:set_session_data()
-	if not ok then
-		return self:ret(false, "can't save session : " .. err, HTTP_INTERNAL_SERVER_ERROR)
-	end

 	-- Redirect to challenge page
 	if self.ctx.bw.uri ~= self.variables["ANTIBOT_URI"] then
@ -162,10 +151,6 @@ function antibot:access()
 	if self.ctx.bw.request_method == "POST" then
 		-- luacheck: ignore 421
 		local ok, err, redirect = self:check_challenge()
-		local set_ok, set_err = self:set_session_data()
-		if not set_ok then
-			return self:ret(false, "can't save session : " .. set_err, HTTP_INTERNAL_SERVER_ERROR)
-		end
 		if ok == nil then
 			return self:ret(false, "check challenge error : " .. err, HTTP_INTERNAL_SERVER_ERROR)
 		elseif not ok then
@ -175,10 +160,6 @@ function antibot:access()
 			return self:ret(true, "check challenge redirect : " .. redirect, nil, redirect)
 		end
 		self:prepare_challenge()
-		ok, err = self:set_session_data()
-		if not ok then
-			return self:ret(false, "can't save session : " .. err, HTTP_INTERNAL_SERVER_ERROR)
-		end
 		self.ctx.bw.antibot_display_content = true
 		return self:ret(true, "displaying challenge to client", OK)
 	end
@ -202,12 +183,10 @@ function antibot:content()
 	end

 	-- Get session data
-	local session, err = get_session("antibot", self.ctx)
-	if not session then
-		return self:ret(false, "can't get session : " .. err, HTTP_INTERNAL_SERVER_ERROR)
+	self.session_data = self.ctx.bw.antibot_session_data
+	if not self.session_data then
+		return self:ret(false, "missing session data", HTTP_INTERNAL_SERVER_ERROR)
 	end
-	self.session = session
-	self.session_data = get_session_data(self.session, true, self.ctx)

 	-- Direct access without session
 	if not self.session_data.prepared then
@ -229,42 +208,36 @@ function antibot:check_session()
 	-- Not resolved and not prepared
 	if not time_resolve and not time_valid then
 		self.session_data = {}
-		self.session_updated = true
-		return
+		self:set_session_data()
+		return "not prepared"
 	end
 	-- Check if still valid
-	local time = ngx.now()
+	local time = now()
 	local resolved = self.session_data.resolved
 	if resolved and (time_valid > time or time - time_valid > tonumber(self.variables["ANTIBOT_TIME_VALID"])) then
 		self.session_data = {}
-		self.session_updated = true
-		return
+		self:set_session_data()
+		return "need new resolve"
 	end
 	-- Check if new prepare is needed
 	if
 		not resolved and (time_resolve > time or time - time_resolve > tonumber(self.variables["ANTIBOT_TIME_RESOLVE"]))
 	then
 		self.session_data = {}
-		self.session_updated = true
-		return
+		self:set_session_data()
+		return "need new prepare"
 	end
+	return "valid"
 end

 function antibot:set_session_data()
-	if self.session_updated then
-		local ok, err = set_session_data(self.session, self.session_data, true, self.ctx)
-		if not ok then
-			return false, err
-		end
-		self.session_updated = false
-		return true, "updated"
-	end
-	return true, "no update"
+	self.session:set("antibot", self.session_data)
+	self.ctx.bw.antibot_session_data = self.session_data
+	self.ctx.bw.sessions_updated = true
 end

 function antibot:prepare_challenge()
 	if not self.session_data.prepared then
-		self.session_updated = true
 		self.session_data.prepared = true
 		self.session_data.time_resolve = ngx.now()
 		self.session_data.type = self.variables["USE_ANTIBOT"]
@ -283,6 +256,7 @@ function antibot:prepare_challenge()
 		elseif self.session_data.type == "captcha" then
 			self.session_data.captcha = rand(6, true)
 		end
+		self:set_session_data()
 	end
 end

@ -363,6 +337,7 @@ function antibot:check_challenge()
 		end
 		self.session_data.resolved = true
 		self.session_data.time_valid = now()
+		self:set_session_data()
 		return true, "resolved", self.session_data.original_uri
 	end

@ -374,10 +349,11 @@ function antibot:check_challenge()
 			return nil, "missing challenge arg", nil
 		end
 		if self.session_data.captcha ~= args["captcha"] then
-			return false, "wrong value", nil
+			return false, "wrong value, expected " .. self.session_data.captcha, nil
 		end
 		self.session_data.resolved = true
 		self.session_data.time_valid = now()
+		self:set_session_data()
 		return true, "resolved", self.session_data.original_uri
 	end

@ -417,6 +393,7 @@ function antibot:check_challenge()
 		end
 		self.session_data.resolved = true
 		self.session_data.time_valid = now()
+		self:set_session_data()
 		return true, "resolved", self.session_data.original_uri
 	end

@ -456,6 +433,7 @@ function antibot:check_challenge()
 		end
 		self.session_data.resolved = true
 		self.session_data.time_valid = now()
+		self:set_session_data()
 		return true, "resolved", self.session_data.original_uri
 	end

@ -495,6 +473,7 @@ function antibot:check_challenge()
 		end
 		self.session_data.resolved = true
 		self.session_data.time_valid = now()
+		self:set_session_data()
 		return true, "resolved", self.session_data.original_uri
 	end

--- a/src/common/core/antibot/files/captcha.html
+++ b/src/common/core/antibot/files/captcha.html
@ -250,6 +250,7 @@
            class="mt-3 px-2 text-gray-800 h-8 w-full max-w-[300px] rounded-lg outline-secondary"
            type="text"
            name="captcha"
+            required
          />
          <button
            class="hover:brightness-90 mt-2 rounded-lg bg-secondary px-6 py-2 text-white text-sm xs:text-base font-bold"