Nginx Module Code error_page directive problems

Question

I am leveraging the ModSecurity WAF to help block tx's deemed dangerous on NGINX: https://github.com/SpiderLabs/ModSecurity https://github.com/SpiderLabs/ModSecurity-nginx

My issue can be found here: https://github.com/SpiderLabs/ModSecurity-nginx/issues/182

The TLDR of my problem is the nginx error_page directive resets the HTTP Request Method Header sent by the originating client during its redirect to GET. This causes false positive logs with the WAF to report client sent an HTTP Body with GET etc. when they really sent a POST and NGINX hits the error_page redirect due to some case with the upstream timing out on a reverse proxy call.

To fix this I need to hack in or PR something to this file section seemingly: https://github.com/SpiderLabs/ModSecurity-nginx/blob/master/src/ngx_http_modsecurity_rewrite.c#L145

With the goal of something like this in sudo code:

//SAFE to trust this value as its the originating client HTTP REQUEST Method HEADER when not error_page
if(!r->error_page){
   const char *n_method = ngx_str_to_char(r->method_name, r->pool);
  //HOW to add a transaction persistent context value here to store STORED_CTX_HTTP_REQUEST_METHOD_HEADER_VALUE???
}
else {  //IF ERROR_PAGE, then we need to grab the original stored CTX request http method header value.
  const char *n_method = ngx_str_to_char(STORED_CTX_HTTP_REQUEST_METHOD_HEADER_VALUE); 
}

And it is here that I have been unable to solve the above sudo code. How does one create a tx context safe variable here that can persist across the phases and internal error_page redirect to store the original client http method header?

I landed on these NGINX page guides but so far no dice really seeing anything cutting into what I want: https://www.evanmiller.org/nginx-modules-guide.html

Looking for any NGINX / C guru's that understand the issue and might be able to turn my sudo code into something I can test in my sandbox environment.

Note I don't have the luxury of doing away with my NGINX error_page directive calls. So I do need a solution to the above sudo code I believe.

Thanks!

score 1 · Accepted Answer · answered May 22 '20 at 07:48

The fix I ended up going with can be seen here: https://github.com/SpiderLabs/ModSecurity-nginx/pull/204

There is no reason to re-process the clients requests in the earliest phases. This was the simplest approach. I just needed to take a high level step back and consider what was occurring here for the most elegant solution.

score 0 · Answer 2 · answered May 14 '20 at 19:56

0

Thanks for sharing! We solved some similar problems while developing Wallarm Nginx module. The short answer is yes, the wrong Nginx phase was used here. The reason is simple - this is a ModSecurity port from Apache, where Nginx phases are irrelevant.

I'll ask our development team if we can help with the patch.

answered May 14 '20 at 19:56

Ivan Wallarm

1

Would be super cool to see Wallarm contribute back to an opensource WAF based on your own struggles as well. NGINX can be a tricky beast in a number of situations, this one included. The patch should be trivial I think, I am just struggling to find good docs on it from NGINX. Thanks! – Jeremy Justus May 14 '20 at 20:10
Sooo, I have some ideas on how to fix that. There are at least three ways you may try to use to pass some request data through Nginx call flow: – Ivan Wallarm May 14 '20 at 23:34
3. Store your data as a request's cleanup handler data. Just create a cleanup handler with ```ngx_pool_cleanup_add()```. You may use the cleanup function callback as a unique key and the data as your data associated with the key. – Ivan Wallarm May 14 '20 at 23:35
2. If Nginx resets modules records data (which it does in some cases like redirects), you may use a trick with Nginx variables. A module may register some variable with ```ngx_http_add_variable()```, then set & get the value with ```ngx_http_get_variable()``` – Ivan Wallarm May 14 '20 at 23:35
1. Store the data in a mod_security data record attached to the request: ```ngx_http_modsecurity_ctx_t``` – Ivan Wallarm May 14 '20 at 23:35
Appreciate the feedback, let me research those methods and see which is most appropriate and what it takes to whip up full fledged code with those methods. Thanks! – Jeremy Justus May 15 '20 at 02:38
Hmm this code throws me for a loop as a novice module programmer. every one of these ***_phase.c files does something like https://github.com/SpiderLabs/ModSecurity-nginx/blob/master/src/ngx_http_modsecurity_rewrite.c#L62 where its constantly setting the ctx to r (which is the request context as I understand it). I don't really see how this thing persists through the transaction and during redirects when it's constantly being init'ed to the current request context if I read this right. – Jeremy Justus May 16 '20 at 07:07
Might look at the other 2 approaches. Would be nice if a dead simple way was declare something at top of file and just a simple set and get call in the phase handling depending if ```r->error_page``` or not like my sudo code. – Jeremy Justus May 16 '20 at 07:07
Then I tried following guides and other modules, ex: https://github.com/giom/nginx_accept_language_module/blob/master/src/ngx_http_accept_language_module.c http://antoine.bonavita.free.fr/nginx_mod_dev_en.html But it is not clicking like I thought it would. where are structs declared, why does that one module do a variable add but never then get it anywhere else in the module(what would be the point..). where is cf, &name and how they relate. The flags field kinda makes sense, and are these variables tx specific so safe to use? Wish there was some full commented code for dummies on this. – Jeremy Justus May 16 '20 at 07:55

Nginx Module Code error_page directive problems

2 Answers2