Move everything into access phase

tomsommer · web-flow · commit 1db43b1f81f5 · 2025-09-23T09:26:46.000+02:00
diff --git a/config b/config
@@ -152,7 +152,6 @@ else
 	    $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="\
diff --git a/src/ngx_http_modsecurity_access.c b/src/ngx_http_modsecurity_access.c
@@ -45,19 +45,248 @@ ngx_http_modsecurity_request_read(ngx_http_request_t *r)
 ngx_int_t
 ngx_http_modsecurity_access_handler(ngx_http_request_t *r)
 {
-#if 1
+
     ngx_pool_t                   *old_pool;
     ngx_http_modsecurity_ctx_t   *ctx;
     ngx_http_modsecurity_conf_t  *mcf;
 
-    dd("catching a new _access_ phase handler");
-
     mcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module);
-    if (mcf == NULL || mcf->enable != 1)
-    {
+    if (mcf == NULL || mcf->enable != 1) {
         dd("ModSecurity not enabled... returning");
         return NGX_DECLINED;
     }
+
+    /*
+    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;
+    }
+    */
+
+    dd("catching a new _rewrite_ phase handler");
+
+    ctx = ngx_http_modsecurity_get_module_ctx(r);
+
+    dd("recovering ctx: %p", ctx);
+
+    if (ctx == NULL)
+    {
+        int ret = 0;
+
+        ngx_connection_t *connection = r->connection;
+        /**
+         * FIXME: We may want to use struct sockaddr instead of addr_text.
+         *
+         */
+        ngx_str_t addr_text = connection->addr_text;
+
+        ctx = ngx_http_modsecurity_create_ctx(r);
+
+        dd("ctx was NULL, creating new context: %p", ctx);
+
+        if (ctx == NULL) {
+            dd("ctx still null; Nothing we can do, returning an error.");
+            return NGX_HTTP_INTERNAL_SERVER_ERROR;
+        }
+
+        /**
+         * FIXME: Check if it is possible to hook on nginx on a earlier phase.
+         *
+         * At this point we are doing an late connection process. Maybe
+         * we have to hook into NGX_HTTP_FIND_CONFIG_PHASE, it seems to be the
+         * erliest phase that nginx allow us to attach those kind of hooks.
+         *
+         */
+        int client_port = ngx_inet_get_port(connection->sockaddr);
+        int server_port = ngx_inet_get_port(connection->local_sockaddr);
+
+        const char *client_addr = ngx_str_to_char(addr_text, r->pool);
+        if (client_addr == (char*)-1) {
+            return NGX_HTTP_INTERNAL_SERVER_ERROR;
+        }
+
+#if defined(MODSECURITY_CHECK_VERSION)
+#if MODSECURITY_VERSION_NUM >= 30130100
+        ngx_str_t hostname;
+        hostname.len = 0;
+        // first check if Nginx received a Host header and it's usable
+        // (i.e. not empty)
+        // if yes, we can use that
+        if (r->headers_in.server.len > 0) {
+            hostname.len = r->headers_in.server.len;
+            hostname.data = r->headers_in.server.data;
+        }
+        else {
+            // otherwise we try to use the server config, namely the
+            // server_name $SERVER_NAME
+            // directive
+            // for eg. in default config, server_name is "_"
+            // possible all requests without a Host header will be
+            // handled by this server block
+            ngx_http_core_srv_conf_t  *cscf;
+            cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module);
+            if (cscf->server_name.len > 0) {
+                hostname.len = cscf->server_name.len;
+                hostname.data = cscf->server_name.data;
+            }
+        }
+        if (hostname.len > 0) {
+            const char *host_name = ngx_str_to_char(hostname, r->pool);
+            if (host_name == (char*)-1 || host_name == NULL) {
+                return NGX_HTTP_INTERNAL_SERVER_ERROR;
+            }
+            else {
+                // set the hostname in the transaction
+                // this function is only available in ModSecurity 3.0.13 and later
+                msc_set_request_hostname(ctx->modsec_transaction, (const unsigned char *)host_name);
+            }
+        }
+#endif
+#endif
+
+        ngx_str_t s;
+        u_char addr[NGX_SOCKADDR_STRLEN];
+        s.len = NGX_SOCKADDR_STRLEN;
+        s.data = addr;
+        if (ngx_connection_local_sockaddr(r->connection, &s, 0) != NGX_OK) {
+            return NGX_HTTP_INTERNAL_SERVER_ERROR;
+        }
+
+        const char *server_addr = ngx_str_to_char(s, r->pool);
+        if (server_addr == (char*)-1) {
+            return NGX_HTTP_INTERNAL_SERVER_ERROR;
+        }
+
+        old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
+        ret = msc_process_connection(ctx->modsec_transaction,
+            client_addr, client_port,
+            server_addr, server_port);
+        ngx_http_modsecurity_pcre_malloc_done(old_pool);
+        if (ret != 1){
+            dd("Was not able to extract connection information.");
+        }
+        /**
+         *
+         * FIXME: Check how we can finalize a request without crash nginx.
+         *
+         * I don't think nginx is expecting to finalize a request at that
+         * point as it seems that it clean the ngx_http_request_t information
+         * and try to use it later.
+         *
+         */
+        dd("Processing intervention with the connection information filled in");
+        ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 1);
+        if (ret > 0) {
+            ctx->intervention_triggered = 1;
+            return ret;
+        }
+
+        const char *http_version;
+        switch (r->http_version) {
+            case NGX_HTTP_VERSION_9 :
+                http_version = "0.9";
+                break;
+            case NGX_HTTP_VERSION_10 :
+                http_version = "1.0";
+                break;
+            case NGX_HTTP_VERSION_11 :
+                http_version = "1.1";
+                break;
+#if defined(nginx_version) && nginx_version >= 1009005
+            case NGX_HTTP_VERSION_20 :
+                http_version = "2.0";
+                break;
+#endif
+            default :
+                http_version = ngx_str_to_char(r->http_protocol, r->pool);
+                if (http_version == (char*)-1) {
+                    return NGX_HTTP_INTERNAL_SERVER_ERROR;
+                }
+                if ((http_version != NULL) && (strlen(http_version) > 5) && (!strncmp("HTTP/", http_version, 5))) {
+                    http_version += 5;
+                } else {
+                    http_version = "1.0";
+                }
+                break;
+        }
+
+        const char *n_uri = ngx_str_to_char(r->unparsed_uri, r->pool);
+        const char *n_method = ngx_str_to_char(r->method_name, r->pool);
+        if (n_uri == (char*)-1 || n_method == (char*)-1) {
+            return NGX_HTTP_INTERNAL_SERVER_ERROR;
+        }
+        if (n_uri == NULL) {
+            dd("uri is of length zero");
+            return NGX_HTTP_INTERNAL_SERVER_ERROR;
+        }
+        old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
+        msc_process_uri(ctx->modsec_transaction, n_uri, n_method, http_version);
+        ngx_http_modsecurity_pcre_malloc_done(old_pool);
+
+        dd("Processing intervention with the transaction information filled in (uri, method and version)");
+        ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 1);
+        if (ret > 0) {
+            ctx->intervention_triggered = 1;
+            return ret;
+        }
+
+        /**
+         * Since incoming request headers are already in place, lets send it to ModSecurity
+         *
+         */
+        ngx_list_part_t *part = &r->headers_in.headers.part;
+        ngx_table_elt_t *data = part->elts;
+        ngx_uint_t i = 0;
+        for (i = 0 ; /* void */ ; i++) {
+            if (i >= part->nelts) {
+                if (part->next == NULL) {
+                    break;
+                }
+
+                part = part->next;
+                data = part->elts;
+                i = 0;
+            }
+
+            /**
+             * By using u_char (utf8_t) I believe nginx is hoping to deal
+             * with utf8 strings.
+             * Casting those into to unsigned char * in order to pass
+             * it to ModSecurity, it will handle with those later.
+             *
+             */
+
+            dd("Adding request header: %.*s with value %.*s", (int)data[i].key.len, data[i].key.data, (int) data[i].value.len, data[i].value.data);
+            msc_add_n_request_header(ctx->modsec_transaction,
+                (const unsigned char *) data[i].key.data,
+                data[i].key.len,
+                (const unsigned char *) data[i].value.data,
+                data[i].value.len);
+        }
+
+        /**
+         * Since ModSecurity already knew about all headers, i guess it is safe
+         * to process this information.
+         */
+
+        old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
+        msc_process_request_headers(ctx->modsec_transaction);
+        ngx_http_modsecurity_pcre_malloc_done(old_pool);
+        dd("Processing intervention with the request headers information filled in");
+        ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 1);
+        if (r->error_page) {
+            return NGX_DECLINED;
+            }
+        if (ret > 0) {
+            ctx->intervention_triggered = 1;
+            return ret;
+        }
+    }
+
+#if 1
+
     /*
      * FIXME:
      * In order to perform some tests, let's accept everything.
diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c
@@ -551,7 +551,6 @@ 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_access;
     ngx_http_handler_pt *h_log;
     ngx_http_core_main_conf_t *cmcf;
@@ -569,21 +568,7 @@ ngx_http_modsecurity_init(ngx_conf_t *cf)
      * ngx_http_limit_*_module to run
      *
      */
-    h_rewrite = ngx_array_push(&cmcf->phases[NGX_HTTP_ACCESS_PHASE].handlers);
-    if (h_rewrite == NULL)
-    {
-        dd("Not able to create a new NGX_HTTP_ACCESS_PHASE handle");
-        return NGX_ERROR;
-    }
-    *h_rewrite = ngx_http_modsecurity_rewrite_handler;
 
-    /**
-     *
-     * Processing the request body on the access phase.
-     *
-     * TODO: check if hook into separated phases is the best thing to do.
-     *
-     */
     h_access = ngx_array_push(&cmcf->phases[NGX_HTTP_ACCESS_PHASE].handlers);
     if (h_access == NULL)
     {
@@ -592,6 +577,7 @@ ngx_http_modsecurity_init(ngx_conf_t *cf)
     }
     *h_access = ngx_http_modsecurity_access_handler;
 
+
     /**
      * Process the log phase.
      *
diff --git a/src/ngx_http_modsecurity_rewrite.c b/src/ngx_http_modsecurity_rewrite.c

Original file line number	Diff line number	Diff line change
`@@ -152,7 +152,6 @@ else`
`152`	`152`	`$ngx_addon_dir/src/ngx_http_modsecurity_header_filter.c \`
`153`	`153`	`$ngx_addon_dir/src/ngx_http_modsecurity_body_filter.c \`
`154`	`154`	`$ngx_addon_dir/src/ngx_http_modsecurity_log.c \`
`155`		`- $ngx_addon_dir/src/ngx_http_modsecurity_rewrite.c \`
`156`	`155`	`"`
`157`	`156`
`158`	`157`	`NGX_ADDON_DEPS="\`