Skip to content

Nginx: Added SecDisableBackendCompression support #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 22, 2013
Merged

Nginx: Added SecDisableBackendCompression support #44

merged 1 commit into from
Mar 22, 2013

Conversation

chaizhenhua
Copy link
Contributor

Nginx: Added SecDisableBackendCompression support
Nginx: Added internel redirected request processing

@brenosilva
Copy link
Contributor

Something is still wrong:
"This pull request cannot be automatically merged."

I think i can try to merge it manually. I will try tomorrow.
But please add ngx-pool-context as part of our code in the tree.

Thanks

Breno

@chaizhenhua
Copy link
Contributor Author

shall we add to pool context as a separate module(source code tree) or merge it and modsecurity into one module?

@brenosilva
Copy link
Contributor

I would suggest you add it as apr_bucket_nginx.[ch], then call the API into ngx_http_modsecurity.c. Should be a clean way to make it, what do you think ?

Thanks

Breno

@brenosilva
Copy link
Contributor

I mean add ngx-pool-context.[ch] not as module, but call the API inside ngx_http_modsecurity.c

Thanks

Breno

@chaizhenhua
Copy link
Contributor Author

ngx-pool-context not only has APIs, but also has a configure command which set the hash table size. so if not as module, we should rewrite this part.

can we move ngx-pool-context.[ch] to modsecurity folder and combine ngx-pool-context and modsecurity config file like this?(https://github.com/chaizhenhua/ModSecurity/blob/60423e0004b903f94ec51fd25edf28cb054827eb/nginx/modsecurity/config). I have added this patch here.

Nginx: Added internel redirected request processing
@chaizhenhua
Copy link
Contributor Author

hi, Breno

I have fixed the merge. now you can merge.

brenosilva added a commit that referenced this pull request Mar 22, 2013
Nginx: Added SecDisableBackendCompression support
@brenosilva brenosilva merged commit bc235a8 into owasp-modsecurity:remotes/trunk Mar 22, 2013
@brenosilva
Copy link
Contributor

Applied. Thanks

@chaizhenhua chaizhenhua deleted the SecDisableBackendCompression branch March 22, 2013 15:02
@brenosilva
Copy link
Contributor

Hello chaizhenhua

After this patch i'm getting:

root@ubuntu:/home/vmplanet/nginx-1.2.7# /usr/local/nginx/sbin/nginx
nginx: [emerg] unknown directive "ModSecurityEnabled" in /usr/local/nginx/conf/nginx.conf:46
root@ubuntu:/home/vmplanet/nginx-1.2.7#

This my config file:
gx_addon_name=ngx_http_modsecurity
CORE_MODULES="$CORE_MODULES ngx_pool_context_module"
HTTP_AUX_FILTER_MODULE="ngx_http_modsecurity $HTTP_AUX_FILTER_MODULE"
NGX_ADDON_SRCS="$NGX_ADDON_SRCS $ngx_addon_dir/ngx_http_modsecurity.c $ngx_addon_dir/apr_bucket_nginx.c $ngx_addon_dir/ngx_pool_context.c"
NGX_ADDON_DEPS="$NGX_ADDON_DEPS $ngx_addon_dir/apr_bucket_nginx.h $ngx_addon_dir/ngx_pool_context.h"
CORE_LIBS="$CORE_LIBS $ngx_addon_dir/../../standalone/.libs/standalone.a -lapr-1 -laprutil-1 -lxml2 -lm "
CORE_INCS="$CORE_INCS /usr/include/apache2 /usr/include/apr-1.0 /usr/include/httpd /usr/include/apr-1 $ngx_addon_dir $ngx_addon_dir/../../standalone $ngx_addon_dir/../../apache2 /usr/include/libxml2 "
#have=REQUEST_EARLY . auto/have

Can you take a look please ?

@chaizhenhua
Copy link
Contributor Author

what's in your nginx.conf file? the directive has never been changed.

@brenosilva
Copy link
Contributor

nginx.conf

    location / {
        root   html;
        index  index.html index.htm;
        ModSecurityEnabled on;
        ModSecurityConfig modsecurity.conf;
        proxy_pass http://192.168.0.104:80;
    }

Shouldn't been related to the new ngx_pool_context_module ?

@chaizhenhua
Copy link
Contributor Author

hi, brenosilva
i checkout from remote/chunk (2fcc089), the compile failed with this error:

config.c:1156:4: error: use of undeclared identifier '__try'

@brenosilva
Copy link
Contributor

I changed :

from
CORE_MODULES="$CORE_MODULES ngx_pool_context_module"

to
CORE_MODULES="$CORE_MODULES ngx_http_modsecurity"

And it loads the directive. But i think it is wrong. Because for each request nginx process is exiting and doesn't serving anything:

error.log:
2013/03/26 01:56:05 [notice] 664#0: signal 17 (SIGCHLD) received
2013/03/26 01:56:05 [alert] 664#0: worker process 669 exited on signal 8
2013/03/26 01:56:05 [notice] 664#0: start worker process 690
2013/03/26 01:56:05 [notice] 664#0: signal 29 (SIGIO) received

@brenosilva
Copy link
Contributor

Yes.

I already asked Greg to fix it. However to test the nginx module now. Just comment it:

                    //__try
                    //{
                            errmsg = invoke_cmd(cmd, parms, mconfig, args);^M
                    //}
                    //__except(EXCEPTION_EXECUTE_HANDLER)
                    //{
                    //      errmsg = "Command failed to execute (check file/folder permissions, syntax, etc.).";
                    //}^M

@chaizhenhua
Copy link
Contributor Author

my env has changed .I cant build nginx, i will check it tomorrow. this is the error message.

/home/chaizhenhua/Source/ModSecurity/standalone/../apache2/modsecurity.c:131: undefined reference to `ap_unixd_set_global_mutex_perms'
/home/chaizhenhua/Source/ModSecurity/standalone/../apache2/modsecurity.c:149: undefined reference to `ap_unixd_set_global_mutex_perms'

@brenosilva
Copy link
Contributor

Ok. I updated the standalone/Makefile.am. so Just don't forget to execute "make install: when compiling mod_security. It will auto-create the right config file

Thanks

@brenosilva
Copy link
Contributor

I added this config manually:

ngx_addon_name=ngx_pool_context_module
CORE_MODULES="$CORE_MODULES ngx_pool_context_module"
HTTP_AUX_FILTER_MODULES="ngx_http_modsecurity $HTTP_AUX_FILTER_MODULES"
NGX_ADDON_SRCS="$NGX_ADDON_SRCS $ngx_addon_dir/ngx_http_modsecurity.c $ngx_addon_dir/apr_bucket_nginx.c $ngx_addon_dir/ngx_pool_context.c"
NGX_ADDON_DEPS="$NGX_ADDON_DEPS $ngx_addon_dir/apr_bucket_nginx.h $ngx_addon_dir/ngx_pool_context.h"
CORE_LIBS="$CORE_LIBS $ngx_addon_dir/../../standalone/.libs/standalone.a -lapr-1 -laprutil-1 -lxml2 -lm "
CORE_INCS="$CORE_INCS /usr/include/apache2 /usr/include/apr-1.0 /usr/include/httpd /usr/include/apr-1 $ngx_addon_dir $ngx_addon_dir/../../standalone $ngx_addon_dir/../../apache2 /usr/include/libxml2 "
have=REQUEST_EARLY . auto/have

And looks like it fixed the issue. However i noticed unwanted data in audit.log like:

--5c5c930e-B--
GET /index.html?a=aabbb HTTP/1.1/index.html?a=aabbb/index.htmlhtmlubuntu
Host: 192.168.0.105

--5c5c930e-B--
GET /favicon.ico HTTP/1.1/favicon.ico/favicon.icoicoubuntui£<93>^H^HX^?^H^H

Looks like a memory leak.

Let me know if this config file is OK.
Any idea why this memory leak ?

Thanks

@brenosilva
Copy link
Contributor

hummm doing this change in the config file sometimes it works sometimes doesn't.... i'm seeing signal 11 in the error.log.

I think there is something related to new arch with ngx_pool_context

@brenosilva
Copy link
Contributor

Also...i saw you are using ngx_pstrdup to copy data. I think at least the data that is going to apr need to be null-terminated.

Maybe we can use a new version like:

u_char *ngx_pstrdup0(ngx_pool_t *pool, ngx_str_t *src) {
u_char *dst;
dst = ngx_pnalloc(pool, src->len + 1);
if (dst == NULL) {
return NULL;
}
ngx_memcpy(dst, src->data, src->len);
dst[src->len] = '\0';
return dst;
}

@brenosilva
Copy link
Contributor

chaizhenhua,

I can confirm the issue with ngx_pool_context.

If i comment the ngx_pool_context code inside ngx_http_modsecurity.c and add the ngx_pstrdup0 everything works fine No more memory leaks and Nginx serves the requests.

Also need to set back the config file to:

ngx_addon_name=ngx_http_modsecurity

HTTP_MODULES="$HTTP_MODULES ngx_http_modsecurity"

HTTP_HEADERS_FILTER_MODULE="ngx_http_modsecurity $HTTP_HEADERS_FILTER_MODULE"
NGX_ADDON_SRCS="$NGX_ADDON_SRCS $ngx_addon_dir/ngx_http_modsecurity.c $ngx_addon_dir/apr_bucket_nginx.c"
NGX_ADDON_DEPS="$NGX_ADDON_DEPS"
CORE_LIBS="$CORE_LIBS $ngx_addon_dir/../../standalone/.libs/standalone.a -lapr-1 -laprutil-1 -lxml2 -lm"
CORE_INCS="$CORE_INCS /usr/include/apache2 /usr/include/apr-1.0 /usr/include/httpd /usr/include/apr-1 $ngx_addon_dir $ngx_addon_dir/../../standalone $ngx_addon_dir/../../apache2 /usr/include/libxml2"
have=REQUEST_EARLY . auto/have

Thanks

Breno

@chaizhenhua
Copy link
Contributor Author

hello,Breno

#49 will fix this. it's typeset error.

zimmerle pushed a commit that referenced this pull request Apr 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants