Hook into Nginx NGX_HTTP_ACCESS_PHASE instead of NGX_HTTP_PREACCESS_PHASE

Author: tomsommer

config

@@ -117,11 +117,10 @@ if test -n "$ngx_module_link"; then
 	ngx_module_type=HTTP_FILTER
 	ngx_module_name="$ngx_addon_name"
 	ngx_module_srcs="$ngx_addon_dir/src/ngx_http_modsecurity_module.c \
-            $ngx_addon_dir/src/ngx_http_modsecurity_pre_access.c \
+            $ngx_addon_dir/src/ngx_http_modsecurity_access.c \
             $ngx_addon_dir/src/ngx_http_modsecurity_header_filter.c \
             $ngx_addon_dir/src/ngx_http_modsecurity_body_filter.c \
             $ngx_addon_dir/src/ngx_http_modsecurity_log.c \
-            $ngx_addon_dir/src/ngx_http_modsecurity_rewrite.c \
             "
 	ngx_module_deps="$ngx_addon_dir/src/ddebug.h \
             $ngx_addon_dir/src/ngx_http_modsecurity_common.h \
@@ -148,11 +147,10 @@ else
 	NGX_ADDON_SRCS="\
 	    $NGX_ADDON_SRCS \
 	    $ngx_addon_dir/src/ngx_http_modsecurity_module.c \
-	    $ngx_addon_dir/src/ngx_http_modsecurity_pre_access.c \
+	    $ngx_addon_dir/src/ngx_http_modsecurity_access.c \
 	    $ngx_addon_dir/src/ngx_http_modsecurity_header_filter.c \
 	    $ngx_addon_dir/src/ngx_http_modsecurity_body_filter.c \
 	    $ngx_addon_dir/src/ngx_http_modsecurity_log.c \
-	    $ngx_addon_dir/src/ngx_http_modsecurity_rewrite.c \
 	    "
 
 	NGX_ADDON_DEPS="\

src/ngx_http_modsecurity_rewrite.c renamed to src/ngx_http_modsecurity_access.c

@@ -22,9 +22,30 @@
 
 #include "ngx_http_modsecurity_common.h"
 
+void
+ngx_http_modsecurity_request_read(ngx_http_request_t *r)
+{
+    ngx_http_modsecurity_ctx_t *ctx;
+
+    ctx = ngx_http_modsecurity_get_module_ctx(r);
+
+#if defined(nginx_version) && nginx_version >= 8011
+    r->main->count--;
+#endif
+
+    if (ctx->waiting_more_body)
+    {
+        ctx->waiting_more_body = 0;
+        r->write_event_handler = ngx_http_core_run_phases;
+        ngx_http_core_run_phases(r);
+    }
+}
+
+
 ngx_int_t
-ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r)
+ngx_http_modsecurity_access_handler(ngx_http_request_t *r)
 {
+
     ngx_pool_t                   *old_pool;
     ngx_http_modsecurity_ctx_t   *ctx;
     ngx_http_modsecurity_conf_t  *mcf;
@@ -44,7 +65,7 @@ ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r)
     }
     */
 
-    dd("catching a new _rewrite_ phase handler");
+    dd("catching a new _access_ phase handler");
 
     ctx = ngx_http_modsecurity_get_module_ctx(r);
 
@@ -264,6 +285,181 @@ ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r)
         }
     }
 
+#if 1
+
+    /*
+     * FIXME:
+     * In order to perform some tests, let's accept everything.
+     *
+    if (r->method != NGX_HTTP_GET &&
+        r->method != NGX_HTTP_POST && r->method != NGX_HTTP_HEAD) {
+        dd("ModSecurity is not ready to deal with anything different from " \
+            "POST, GET or HEAD");
+        return NGX_DECLINED;
+    }
+    */
+
+    ctx = ngx_http_modsecurity_get_module_ctx(r);
+
+    dd("recovering ctx: %p", ctx);
 
+    if (ctx == NULL)
+    {
+        dd("ctx is null; Nothing we can do, returning an error.");
+        return NGX_HTTP_INTERNAL_SERVER_ERROR;
+    }
+
+    if (ctx->request_body_processed) {
+        // should we use r->internal or r->filter_finalize?
+        return NGX_DECLINED;
+    }
+
+    if (ctx->intervention_triggered) {
+        return NGX_DECLINED;
+    }
+
+    if (ctx->waiting_more_body == 1)
+    {
+        dd("waiting for more data before proceed. / count: %d",
+            r->main->count);
+
+        return NGX_DONE;
+    }
+
+    if (ctx->body_requested == 0)
+    {
+        ngx_int_t rc = NGX_OK;
+
+        ctx->body_requested = 1;
+
+        dd("asking for the request body, if any. Count: %d",
+            r->main->count);
+        /**
+         * TODO: Check if there is any benefit to use request_body_in_single_buf set to 1.
+         *
+         *       saw some module using this request_body_in_single_buf
+         *       but not sure what exactly it does, same for the others options below.
+         *
+         * r->request_body_in_single_buf = 1;
+         */
+        r->request_body_in_single_buf = 1;
+        r->request_body_in_persistent_file = 1;
+        if (!r->request_body_in_file_only) {
+            // If the above condition fails, then the flag below will have been
+            // set correctly elsewhere. We need to set the flag here for other
+            // conditions (client_body_in_file_only not used but
+            // client_body_buffer_size is)
+            r->request_body_in_clean_file = 1;
+        }
+
+        rc = ngx_http_read_client_request_body(r,
+            ngx_http_modsecurity_request_read);
+        if (rc == NGX_ERROR || rc >= NGX_HTTP_SPECIAL_RESPONSE) {
+#if (nginx_version < 1002006) ||                                             \
+    (nginx_version >= 1003000 && nginx_version < 1003009)
+            r->main->count--;
+#endif
+
+            return rc;
+        }
+        if (rc == NGX_AGAIN)
+        {
+            dd("nginx is asking us to wait for more data.");
+
+            ctx->waiting_more_body = 1;
+            return NGX_DONE;
+        }
+    }
+
+    if (ctx->waiting_more_body == 0)
+    {
+        int ret = 0;
+        int already_inspected = 0;
+
+        dd("request body is ready to be processed");
+
+        r->write_event_handler = ngx_http_core_run_phases;
+
+        ngx_chain_t *chain = r->request_body->bufs;
+
+        /**
+         * TODO: Speed up the analysis by sending chunk while they arrive.
+         *
+         * Notice that we are waiting for the full request body to
+         * start to process it, it may not be necessary. We may send
+         * the chunks to ModSecurity while nginx keep calling this
+         * function.
+         */
+
+        if (r->request_body->temp_file != NULL) {
+            ngx_str_t file_path = r->request_body->temp_file->file.name;
+            const char *file_name = ngx_str_to_char(file_path, r->pool);
+            if (file_name == (char*)-1) {
+                return NGX_HTTP_INTERNAL_SERVER_ERROR;
+            }
+            /*
+             * Request body was saved to a file, probably we don't have a
+             * copy of it in memory.
+             */
+            dd("request body inspection: file -- %s", file_name);
+
+            msc_request_body_from_file(ctx->modsec_transaction, file_name);
+
+            already_inspected = 1;
+        } else {
+            dd("inspection request body in memory.");
+        }
+
+        while (chain && !already_inspected)
+        {
+            u_char *data = chain->buf->pos;
+
+            msc_append_request_body(ctx->modsec_transaction, data,
+                chain->buf->last - data);
+
+            if (chain->buf->last_buf) {
+                break;
+            }
+            chain = chain->next;
+
+/* XXX: chains are processed one-by-one, maybe worth to pass all chains and then call intervention() ? */
+
+            /**
+             * ModSecurity may perform stream inspection on this buffer,
+             * it may ask for a intervention in consequence of that.
+             *
+             */
+            ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
+            if (ret > 0) {
+                return ret;
+            }
+        }
+
+        /**
+         * At this point, all the request body was sent to ModSecurity
+         * and we want to make sure that all the request body inspection
+         * happened; consequently we have to check if ModSecurity have
+         * returned any kind of intervention.
+         */
+
+/* XXX: once more -- is body can be modified ?  content-length need to be adjusted ? */
+
+        old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
+        msc_process_request_body(ctx->modsec_transaction);
+        ctx->request_body_processed = 1;
+        ngx_http_modsecurity_pcre_malloc_done(old_pool);
+
+        ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
+        if (r->error_page) {
+            return NGX_DECLINED;
+            }
+        if (ret > 0) {
+            return ret;
+        }
+    }
+
+    dd("Nothing to add on the body inspection, reclaiming a NGX_DECLINED");
+#endif
     return NGX_DECLINED;
 }
+

src/ngx_http_modsecurity_common.h

@@ -165,11 +165,7 @@ int ngx_http_modsecurity_store_ctx_header(ngx_http_request_t *r, ngx_str_t *name
 void ngx_http_modsecurity_log(void *log, const void* data);
 ngx_int_t ngx_http_modsecurity_log_handler(ngx_http_request_t *r);
 
-/* ngx_http_modsecurity_pre_access.c */
-ngx_int_t ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r);
-
-/* ngx_http_modsecurity_rewrite.c */
-ngx_int_t ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r);
-
+/* ngx_http_modsecurity_access.c */
+ngx_int_t ngx_http_modsecurity_access_handler(ngx_http_request_t *r);
 
 #endif /* _NGX_HTTP_MODSECURITY_COMMON_H_INCLUDED_ */

src/ngx_http_modsecurity_module.c

@@ -551,8 +551,7 @@ ngx_module_t ngx_http_modsecurity_module = {
 static ngx_int_t
 ngx_http_modsecurity_init(ngx_conf_t *cf)
 {
-    ngx_http_handler_pt *h_rewrite;
-    ngx_http_handler_pt *h_preaccess;
+    ngx_http_handler_pt *h_access;
     ngx_http_handler_pt *h_log;
     ngx_http_core_main_conf_t *cmcf;
     int rc = 0;
@@ -565,35 +564,19 @@ ngx_http_modsecurity_init(ngx_conf_t *cf)
     }
     /**
      *
-     * Seems like we cannot do this very same thing with
-     * NGX_HTTP_FIND_CONFIG_PHASE. it does not seems to
-     * be an array. Our next option is the REWRITE.
-     *
-     * TODO: check if we can hook prior to NGX_HTTP_REWRITE_PHASE phase.
+     * We want to process everything in the NGX_HTTP_ACCESS_PHASE because we need to allow 
+     * ngx_http_limit_*_module to run
      *
      */
-    h_rewrite = ngx_array_push(&cmcf->phases[NGX_HTTP_REWRITE_PHASE].handlers);
-    if (h_rewrite == NULL)
-    {
-        dd("Not able to create a new NGX_HTTP_REWRITE_PHASE handle");
-        return NGX_ERROR;
-    }
-    *h_rewrite = ngx_http_modsecurity_rewrite_handler;
 
-    /**
-     *
-     * Processing the request body on the preaccess phase.
-     *
-     * TODO: check if hook into separated phases is the best thing to do.
-     *
-     */
-    h_preaccess = ngx_array_push(&cmcf->phases[NGX_HTTP_PREACCESS_PHASE].handlers);
-    if (h_preaccess == NULL)
+    h_access = ngx_array_push(&cmcf->phases[NGX_HTTP_ACCESS_PHASE].handlers);
+    if (h_access == NULL)
     {
-        dd("Not able to create a new NGX_HTTP_PREACCESS_PHASE handle");
+        dd("Not able to create a new NGX_HTTP_ACCESS_PHASE handle");
         return NGX_ERROR;
     }
-    *h_preaccess = ngx_http_modsecurity_pre_access_handler;
+    *h_access = ngx_http_modsecurity_access_handler;
+
 
     /**
      * Process the log phase.