owasp-modsecurity / ModSecurity-nginx

ModSecurity v3 Nginx Connector
Apache License 2.0
1.59k stars 282 forks source link

two server fields in json #230

Open nickvth opened 3 years ago

nickvth commented 3 years ago

When server-tokens off and more_clear_headers server are configured we get two server fields in json. Elasticsearch will not parse this, because field need te be unique.

Please fix this, so we get 1 server field

example

"http_code":403,"headers":{"Server":"","Server":"","Date":"Wed, 02 Dec 2020 20:57:05 GMT","Content-Length"

zimmerle commented 3 years ago

Hi @nickvth

I am assuming that more_clear_headers is the project available at: https://github.com/openresty/headers-more-nginx-module#more_clear_headers is that right? If so, do you happens to have any directive configured? or just have the project enabled without any configuration?

nickvth commented 3 years ago

@zimmerle ingress-nginx configured this settings by default

server_tokens off;
more_clear_headers Server;

The project module is right.

More information why this is configured. https://medium.com/@getpagespeed/how-to-remove-the-server-header-in-nginx-e74c7b431b

ingress-nginx project: https://github.com/kubernetes/ingress-nginx

zimmerle commented 3 years ago

For the reference, here is where ModSecurity collects the headers variables -

https://github.com/SpiderLabs/ModSecurity-nginx/blob/22e53aba4e3ae8c7d59a3672d6727e49246afe96/src/ngx_http_modsecurity_header_filter.c#L453-L502

more_clear_headers removes the header here - https://github.com/openresty/headers-more-nginx-module/blob/d6d7ebab3c0c5b32ab421ba186783d3e5d2c6a17/src/ngx_http_headers_more_util.c#L266-L380

@nickvth What happens if you use more_set_headers instead? -

more_set_headers    "Server: my_server";
Ricardolaponder commented 3 years ago

Hi,

I'm having the issue on the same environment.

I've tested your suggestion with more_set_headers and it results in another duplicate Server header:

"headers": {
    "Server": "my_server",
    "Server": "my_server",
zimmerle commented 3 years ago

Thank you fro the feedback @Ricardolaponder. The list structure apparently has some left overs or we are reading it incorrectly on ModSecurity. It needs further investigation. Do you happens to have other add-ons on the same server that also uses those headers?

Ricardolaponder commented 3 years ago
bash-5.0$ nginx -V
nginx version: nginx/1.19.4
built by gcc 9.3.0 (Alpine 9.3.0)
built with OpenSSL 1.1.1g  21 Apr 2020
TLS SNI support enabled
configure arguments: 
--prefix=/usr/local/nginx 
--conf-path=/etc/nginx/nginx.conf 
--modules-path=/etc/nginx/modules 
--http-log-path=/var/log/nginx/access.log 
--error-log-path=/var/log/nginx/error.log 
--lock-path=/var/lock/nginx.lock 
--pid-path=/run/nginx.pid 
--http-client-body-temp-path=/var/lib/nginx/body 
--http-fastcgi-temp-path=/var/lib/nginx/fastcgi 
--http-proxy-temp-path=/var/lib/nginx/proxy 
--http-scgi-temp-path=/var/lib/nginx/scgi 
--http-uwsgi-temp-path=/var/lib/nginx/uwsgi 
--with-debug 
--with-compat 
--with-pcre-jit 
--with-http_ssl_module 
--with-http_stub_status_module
--with-http_realip_module
--with-http_auth_request_module
--with-http_addition_module
--with-http_geoip_module
--with-http_gzip_static_module
--with-http_sub_module
--with-http_v2_module
--with-stream
--with-stream_ssl_module
--with-stream_realip_module
--with-stream_ssl_preread_module
--with-threads
--with-http_secure_link_module
--with-http_gunzip_module
--with-file-aio
--without-mail_pop3_module
--without-mail_smtp_module
--without-mail_imap_module
--without-http_uwsgi_module
--without-http_scgi_module
--with-cc-opt='-g -Og -fPIE -fstack-protector-strong -Wformat -Werror=format-security -Wno-deprecated-declarations -fno-strict-aliasing -D_FORTIFY_SOURCE=2 --param=ssp-buffer-size=4 -DTCP_FASTOPEN=23 -fPIC -I/root/.hunter/_Base/2c5c6fc/b895437/92161a9/Install/include -Wno-cast-function-type -m64 -mtune=native'
--with-ld-opt='-fPIE -fPIC -pie -Wl,-z,relro -Wl,-z,now -L/root/.hunter/_Base/2c5c6fc/b895437/92161a9/Install/lib'
--user=www-data
--group=www-data
--add-module=/tmp/build/ngx_devel_kit-0.3.1
--add-module=/tmp/build/set-misc-nginx-module-0.32
--add-module=/tmp/build/headers-more-nginx-module-0.33
--add-module=/tmp/build/ngx_http_substitutions_filter_module-bc58cb11844bc42735bbaef7085ea86ace46d05b
--add-module=/tmp/build/lua-nginx-module-0.10.18rc4
--add-module=/tmp/build/stream-lua-nginx-module-0.0.9rc3
--add-module=/tmp/build/lua-upstream-nginx-module-0.07
--add-module=/tmp/build/nginx_ajp_module-bf6cd93f2098b59260de8d494f0f4b1f11a84627
--add-dynamic-module=/tmp/build/nginx-http-auth-digest-cd8641886c873cf543255aeda20d23e4cd603d05
--add-dynamic-module=/tmp/build/nginx-influxdb-module-5b09391cb7b9a889687c0aa67964c06a2d933e8b
--add-dynamic-module=/tmp/build/nginx-opentracing-0.10.0/opentracing
--add-dynamic-module=/tmp/build/ModSecurity-nginx-22e53aba4e3ae8c7d59a3672d6727e49246afe96
--add-dynamic-module=/tmp/build/ngx_http_geoip2_module-3.3
--add-dynamic-module=/tmp/build/ngx_brotli

These are all the modules installed, it's al default from the nginx-ingress controller image.

martinhsv commented 3 years ago

Looking at this from a slightly different angle ...

If there are multiple response headers with the same name, and you are using 'SecAuditLogFormat JSON' within your ModSecurity configuration, then you can indeed get audit log output with duplicate json keys.

Duplicate json keys do not actually violate the json specification, however RFC-7159 does state that 'The names within an object SHOULD be unique.' [Note: 'SHOULD' not 'MUST']

In most normal situations, multiple response headers with the same name should not be present. There is at least one notable exception, however: it is not exactly rare for there to be multiple 'Set-Cookie' headers in a response.

We could consider changing the json audit-log-writing to use array format for this use case, e.g.

    "headers": {
        "Set-Cookie": ["mycookie1=abc", "mycookie2=def"]
    }
Ricardolaponder commented 3 years ago

@martinhsv You're right, it doesn't violate the json specification, but on why it's a problem for us is that we want to index the json events from Modsecurity into Elasticsearch. Elasticsearch by design doesn't allow duplicate field names for their documents. So as a workaround we've disabled the more_clear_headers Server; setting, but that's not preferred from a Security perspective.

I still find it weird that Modsecurity returns 2 times an empty Server header, when nginx removes the Server header entirely in it's response. It doesn't feel like a "expected" response from Modsecurity.

NeckBeardPrince commented 3 years ago

Running into the same issue.

lchdev commented 3 years ago

I've just hit the same issue so I will share my observations:

It seems indeed that there is an issue with modsecurity logs when the more headers directive are used.

martinhsv commented 3 years ago

Just to clarify this item a little ...

It seems there are really two sub-issues here: A) That if ModSecurity believes it has been provided two headers with the same name, it will report them in a non-recommended JSON format. See, for example, my comment from Dec. 11. B) Surprise and some potential confusion that, with this specific configuration, ModSecurity is reporting two Server headers.

I'm just wondering how to weigh these two sub-issues.

If (A) above were to be addressed via array format in the json audit log writing, would that alleviate the great majority of the concern and/or nuisance?

Or is resolving, or at least explaining, (B) really the core issue?

OmarHawk commented 1 year ago

Puh, such an old issue - we also do have the very same issue :(