fix wrong env parsing in init phase, bypass modsec/crs when method is not allowed, refactor ALLOWED_METHODS and improve error page management

.github/workflows/staging.yml

@@ -144,6 +144,7 @@ jobs:
   tests-core:
     needs: prepare-tests-core
     strategy:
+      fail-fast: false
       matrix:
         test: ${{ fromJson(needs.prepare-tests-core.outputs.tests) }}
     uses: ./.github/workflows/test-core.yml

.github/workflows/test-core.yml

@@ -15,7 +15,7 @@ on:
       PRIVATE_REGISTRY_TOKEN:
         required: true
 jobs:
-  build:
+  test:
     runs-on: ubuntu-latest
     steps:
       # Prepare

src/common/confs/http.conf

@@ -39,7 +39,7 @@ resolver {{ DNS_RESOLVERS }} {% if USE_IPV6 == "no" %}ipv6=off{% endif %};
 # remove ports when sending redirects
 port_in_redirect off;
 
-# lua path and dicts
+# lua configs
 lua_package_path "/usr/share/bunkerweb/lua/?.lua;/usr/share/bunkerweb/core/?.lua;/etc/bunkerweb/plugins/?.lua;/usr/share/bunkerweb/deps/lib/lua/?.lua;;";
 lua_package_cpath "/usr/share/bunkerweb/deps/lib/?.so;/usr/share/bunkerweb/deps/lib/lua/?.so;;";
 lua_ssl_trusted_certificate "/usr/share/bunkerweb/misc/root-ca.pem";

src/common/confs/init-lua.conf

@@ -40,7 +40,7 @@ if not file then
 end
 file:close()
 for line in io.lines("/etc/nginx/variables.env") do
-	local variable, value = line:match("(.+)=(.*)")
+	local variable, value = line:match("^([^=]+)=(.*)$")
 	local ok, err = datastore:set("variable_" .. variable, value)
 	if not ok then
 		logger:log(ngx.ERR, "can't save variable " .. variable .. " into datastore : " .. err)

src/common/core/errors/confs/default-server-http/errors.conf

@@ -0,0 +1,22 @@
+{% for intercepted_error_code in "400 401 403 404 405 413 429 500 501 502 503 504".split(" ") %}
+
+{% if intercepted_error_code == "400" %}
+error_page 400 /bwerror400;
+{% else %}
+error_page {{ intercepted_error_code }} @bwerror{{ intercepted_error_code }};
+{% endif %}
+
+location {% if intercepted_error_code == "400" %}= /{% else %} @{% endif %}bwerror{{ intercepted_error_code }} {
+    auth_basic off;
+    internal;
+    modsecurity off;
+    default_type 'text/html';
+    root /usr/share/bunkerweb/core/errors/files;
+    content_by_lua_block {
+        local logger = require "bunkerweb.logger"
+        local cerrors = require "errors.errors"
+        local errors = cerrors:new()
+        errors:render_template(tostring(ngx.status))
+    }
+}
+{% endfor %}

src/common/core/errors/confs/server-http/errors.conf

@@ -8,6 +8,7 @@ location = {{ page }} {
     root {% if ROOT_FOLDER == "" %}/var/www/html/{% if MULTISITE == "yes" %}{{ SERVER_NAME.split(" ")[0] }}{% endif %}{% else %}{{ ROOT_FOLDER }}{% endif %};
     modsecurity off;
     internal;
+	auth_basic off;
 }
 	{% endfor %}
 {% endif %}
@@ -15,12 +16,13 @@ location = {{ page }} {
 {% if INTERCEPTED_ERROR_CODES != "" %}
 	{% for intercepted_error_code in INTERCEPTED_ERROR_CODES.split(" ") %}
 		{% if not intercepted_error_code + "=" in ERRORS +%}
-			{% if intercepted_error_code == "405" +%}
-	error_page 405 =200 @405;
-			{% else +%}
-	error_page {{ intercepted_error_code }} @{{ intercepted_error_code }};
-			{% endif +%}
-	location @{{ intercepted_error_code }} {
+			{% if intercepted_error_code == "400" %}
+	error_page 400 /bwerror400;
+			{% else %}
+	error_page {{ intercepted_error_code }} @bwerror{{ intercepted_error_code }};
+			{% endif %}
+
+	location {% if intercepted_error_code == "400" %}= /{% else %} @{% endif %}bwerror{{ intercepted_error_code }} {
 		auth_basic off;
 		internal;
 		modsecurity off;
@@ -30,11 +32,7 @@ location = {{ page }} {
 			local logger = require "bunkerweb.logger"
 			local cerrors = require "errors.errors"
 			local errors = cerrors:new()
-			if ngx.status == 200 then
-				errors:render_template(tostring(405))
-			else
-				errors:render_template(tostring(ngx.status))
-			end
+			errors:render_template(tostring(ngx.status))
 		}
 	}
 

src/common/core/misc/confs/default-server-http/errors.conf

@@ -1,43 +0,0 @@
-{% if ERRORS != "" %}
-	{% for element in ERRORS.split(" ") %}
-		{% set code = element.split("=")[0] %}
-		{% set page = element.split("=")[1] %}
-error_page {{ code }} {{ page }};
-
-location = {{ page }} {
-    root {% if ROOT_FOLDER == "" %}/var/www/html/{% if MULTISITE == "yes" %}{{ SERVER_NAME.split(" ")[0] }}{% endif %}{% else %}{{ ROOT_FOLDER }}{% endif %};
-    modsecurity off;
-    internal;
-}
-	{% endfor %}
-{% endif %}
-
-{% if INTERCEPTED_ERROR_CODES != "" %}
-	{% for intercepted_error_code in INTERCEPTED_ERROR_CODES.split(" ") %}
-		{% if not intercepted_error_code + "=" in ERRORS +%}
-			{% if intercepted_error_code == "405" +%}
-	error_page 405 =200 @405;
-			{% else +%}
-	error_page {{ intercepted_error_code }} @{{ intercepted_error_code }};
-			{% endif +%}
-	location @{{ intercepted_error_code }} {
-		auth_basic off;
-		internal;
-		modsecurity off;
-		default_type 'text/html';
-		root /usr/share/bunkerweb/core/errors/files;
-		content_by_lua_block {
-			local logger = require "bunkerweb.logger"
-			local cerrors = require "errors.errors"
-			local errors = cerrors:new()
-			if ngx.status == 200 then
-				errors:render_template(tostring(405))
-			else
-				errors:render_template(tostring(ngx.status))
-			end
-		}
-	}
-
-		{% endif %}
-	{% endfor %}
-{% endif %}

src/common/core/misc/confs/server-http/allowed-methods.conf

@@ -1,5 +0,0 @@
-{% if ALLOWED_METHODS != "" +%}
-if ($request_method !~ ^({{ ALLOWED_METHODS }})$) {
-	return 405;
-}
-{% endif %}

src/common/core/misc/misc.lua

@@ -0,0 +1,32 @@
+local class  = require "middleclass"
+local plugin = require "bunkerweb.plugin"
+local utils  = require "bunkerweb.utils"
+
+local misc = class("misc", plugin)
+
+function misc:initialize()
+	-- Call parent initialize
+	plugin.initialize(self, "misc")
+end
+
+function misc:set()
+    -- Check if method is allowed
+    local method = ngx.ctx.bw.request_method
+    for allowed_method in self.variables["ALLOWED_METHODS"]:gmatch("[^|]+") do
+        if method == allowed_method then
+            return self:ret(true, "method " .. method .. " is allowed")
+        end
+    end
+    ngx.ctx.bw.plugin_misc_method_not_allowed = true
+    return self:ret(true, "method " .. method .. " not is allowed")
+end
+
+function misc:access()
+    -- Check if method is allowed
+    if ngx.ctx.bw.plugin_misc_method_not_allowed then
+        return self:ret(true, "method " .. ngx.ctx.bw.request_method .. " is not allowed", ngx.HTTP_NOT_ALLOWED)
+    end
+    return self:ret(true, "method " .. ngx.ctx.bw.request_method .. " is allowed")
+end
+
+return misc

src/common/core/misc/plugin.json

@@ -1,6 +1,6 @@
 {
   "id": "misc",
-  "order": 999,
+  "order": 0,
   "name": "Miscellaneous",
   "description": "Miscellaneous settings.",
   "version": "1.0",

src/common/core/modsecurity/confs/server-http/modsecurity-rules.conf.modsec

@@ -86,7 +86,6 @@ include /etc/nginx/{{ SERVER_NAME.split(" ")[0] }}/modsec-crs/*.conf
 SecAction "nolog,phase:1,setenv:REASON=none"
 
 # Auto update allowed methods
-{% if ALLOWED_METHODS != "" +%}
 SecAction \
  "id:900200,\
   phase:1,\
@@ -94,11 +93,10 @@ SecAction \
   pass,\
   t:none,\
   setvar:'tx.allowed_methods={{ ALLOWED_METHODS.replace("|", " ") }}'"
-{% endif +%}
 
 # Check if client is whitelisted
 {% if USE_WHITELIST == "yes" +%}
-SecRule ENV:is_whitelisted "yes" "id:1000,phase:1,pass,nolog,ctl:ruleEngine=Off"
+SecRule ENV:is_whitelisted "yes" "id:1000,phase:1,allow,nolog,ctl:ruleEngine=Off"
 {% endif +%}
 
 # include OWASP CRS rules
@@ -119,8 +117,14 @@ include /etc/nginx/modsec/*.conf
 include /etc/nginx/{{ SERVER_NAME.split(" ")[0] }}/modsec/*.conf
 {% endif %}
 
-# set REASON env var
+
 {% if USE_MODSECURITY_CRS == "yes" %}
+
+# set REASON env var
 SecRuleUpdateActionById 949110 "t:none,deny,status:{{ DENY_HTTP_STATUS }},setenv:REASON=modsecurity"
 SecRuleUpdateActionById 959100 "t:none,deny,status:{{ DENY_HTTP_STATUS }},setenv:REASON=modsecurity"
-{% endif %}
+
+# let BW manage when method is not allowed (and save up some computing)
+SecRuleUpdateActionById 911100 "t:none,allow,nolog"
+
+{% endif %}