naxsi icon indicating copy to clipboard operation
naxsi copied to clipboard

DeniedError

Open RekGRpth opened this issue 3 years ago • 4 comments

How about DeniedError instead DeniedUrl? like this

diff --git a/naxsi_src/naxsi.h b/naxsi_src/naxsi.h
index 0d81ba6..d9ac93b 100644
--- a/naxsi_src/naxsi.h
+++ b/naxsi_src/naxsi.h
@@ -376,6 +376,7 @@ typedef struct
   ngx_flag_t   libinjection_sql_enabled : 1;
   ngx_flag_t   libinjection_xss_enabled : 1;
   ngx_str_t*   denied_url;
+  ngx_int_t    denied_error;
   /* precomputed hash for dynamic variable lookup,
      variable themselves are boolean */
   ngx_uint_t flag_enable_h;
@@ -461,6 +462,7 @@ typedef struct ngx_http_nx_json_s
 } ngx_json_t;
 
 #define TOP_DENIED_URL_T       "DeniedUrl"
+#define TOP_DENIED_ERROR_T     "DeniedError"
 #define TOP_IGNORE_IP_T        "IgnoreIP"
 #define TOP_IGNORE_CIDR_T      "IgnoreCIDR"
 #define TOP_LEARNING_FLAG_T    "LearningMode"
@@ -475,6 +477,7 @@ typedef struct ngx_http_nx_json_s
 
 /* nginx-style names */
 #define TOP_DENIED_URL_N       "denied_url"
+#define TOP_DENIED_ERROR_N     "denied_error"
 #define TOP_IGNORE_IP_N        "ignore_ip"
 #define TOP_IGNORE_CIDR_N      "ignore_cidr"
 #define TOP_LEARNING_FLAG_N    "learning_mode"
diff --git a/naxsi_src/naxsi_runtime.c b/naxsi_src/naxsi_runtime.c
index 6a5dea2..99112d2 100644
--- a/naxsi_src/naxsi_runtime.c
+++ b/naxsi_src/naxsi_runtime.c
@@ -1444,7 +1444,7 @@ ngx_http_output_forbidden_page(ngx_http_request_ctx_t* ctx, ngx_http_request_t*
   }
 
   if (ctx->learning && !ctx->drop) {
-    if (ctx->post_action) {
+    if (ctx->post_action && cf->denied_url != NULL) {
       ngx_http_core_loc_conf_t* clcf;
       clcf                   = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
       clcf->post_action.data = cf->denied_url->data;
@@ -1452,6 +1452,8 @@ ngx_http_output_forbidden_page(ngx_http_request_ctx_t* ctx, ngx_http_request_t*
     }
     return (NGX_DECLINED);
   } else {
+    if (cf->denied_url == NULL)
+      return cf->denied_error;
     ngx_http_internal_redirect(r, cf->denied_url, &empty);
     return (NGX_HTTP_OK);
   }
diff --git a/naxsi_src/naxsi_skeleton.c b/naxsi_src/naxsi_skeleton.c
index 90d4fbc..62b86fe 100644
--- a/naxsi_src/naxsi_skeleton.c
+++ b/naxsi_src/naxsi_skeleton.c
@@ -115,6 +115,22 @@ static ngx_command_t ngx_http_naxsi_commands[] = {
     0,
     NULL },
 
+  /* DeniedError */
+  { ngx_string(TOP_DENIED_ERROR_T),
+    NGX_HTTP_MAIN_CONF | NGX_HTTP_SRV_CONF | NGX_HTTP_LOC_CONF | NGX_HTTP_LMT_CONF | NGX_CONF_1MORE,
+    ngx_http_naxsi_ud_loc_conf,
+    NGX_HTTP_LOC_CONF_OFFSET,
+    0,
+    NULL },
+
+  /* DeniedError - nginx style */
+  { ngx_string(TOP_DENIED_ERROR_N),
+    NGX_HTTP_MAIN_CONF | NGX_HTTP_SRV_CONF | NGX_HTTP_LOC_CONF | NGX_HTTP_LMT_CONF | NGX_CONF_1MORE,
+    ngx_http_naxsi_ud_loc_conf,
+    NGX_HTTP_LOC_CONF_OFFSET,
+    0,
+    NULL },
+
   /* WhitelistIP */
   { ngx_string(TOP_IGNORE_IP_T),
     NGX_HTTP_MAIN_CONF | NGX_HTTP_SRV_CONF | NGX_HTTP_LOC_CONF | NGX_HTTP_LMT_CONF | NGX_CONF_1MORE,
@@ -370,6 +386,10 @@ ngx_http_naxsi_merge_loc_conf(ngx_conf_t* cf, void* parent, void* child)
     conf->libinjection_xss_enabled = prev->libinjection_xss_enabled;
   if (conf->denied_url == NULL)
     conf->denied_url = prev->denied_url;
+  if (conf->denied_error == 0)
+    conf->denied_error = prev->denied_error;
+  if (conf->denied_error == 0)
+    conf->denied_error = NGX_HTTP_FORBIDDEN;
   if (conf->flag_enable_h == 0)
     conf->flag_enable_h = prev->flag_enable_h;
   if (conf->flag_learning_h == 0)
@@ -418,13 +438,6 @@ ngx_http_naxsi_init(ngx_conf_t* cf)
   loc_cf = main_cf->locations->elts;
 
   for (i = 0; i < main_cf->locations->nelts; i++) {
-    if (loc_cf[i]->enabled && (!loc_cf[i]->denied_url || loc_cf[i]->denied_url->len <= 0)) {
-      /* LCOV_EXCL_START */
-      ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "Missing DeniedURL, abort.");
-      return (NGX_ERROR);
-      /* LCOV_EXCL_STOP */
-    }
-
     loc_cf[i]->flag_enable_h   = ngx_hash_key_lc((u_char*)RT_ENABLE, strlen(RT_ENABLE));
     loc_cf[i]->flag_learning_h = ngx_hash_key_lc((u_char*)RT_LEARNING, strlen(RT_LEARNING));
     loc_cf[i]->flag_post_action_h =
@@ -889,6 +902,24 @@ ngx_http_naxsi_ud_loc_conf(ngx_conf_t* cf, ngx_command_t* cmd, void* conf)
     alcf->denied_url->len = value[1].len;
     return (NGX_CONF_OK);
   }
+  /* store denied ERROR for location */
+  else if ((!ngx_strcmp(value[0].data, TOP_DENIED_ERROR_N) ||
+       !ngx_strcmp(value[0].data, TOP_DENIED_ERROR_T)) &&
+      value[1].len) {
+
+    alcf->denied_error = ngx_atoi(value[1].data, value[1].len);
+
+    if (alcf->denied_error == NGX_ERROR || alcf->denied_error == 499) {
+      ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "invalid value \"%V\"", &value[1]);
+      return (NGX_CONF_ERROR); /* LCOV_EXCL_LINE */
+    }
+
+    if (alcf->denied_error < 300 || alcf->denied_error > 599) {
+      ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "value \"%V\" must be between 300 and 599", &value[1]);
+      return (NGX_CONF_ERROR); /* LCOV_EXCL_LINE */
+    }
+    return (NGX_CONF_OK);
+  }
 
   return NGX_CONF_ERROR;
 }

RekGRpth avatar Jul 07 '21 04:07 RekGRpth

Yes, this could be added as an alternative, but what happens if you define both?

wargio avatar Jul 07 '21 08:07 wargio

if I define both then DeniedUrl is prefer

RekGRpth avatar Jul 07 '21 08:07 RekGRpth

more, DeniedUrl requires location to be exists, but DeniedError does not reqire, but You may use location by error_page

RekGRpth avatar Jul 07 '21 08:07 RekGRpth

yes, sure, this is why i asked what would happen if both are defined. This is a good PR, if you could push it, i could approve it. please, also add some tests.

wargio avatar Jul 07 '21 11:07 wargio