Skip to content
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

two server fields in json #230

Open
nickvth opened this issue Dec 2, 2020 · 12 comments
Open

two server fields in json #230

nickvth opened this issue Dec 2, 2020 · 12 comments
Assignees

Comments

@nickvth
Copy link

nickvth commented Dec 2, 2020

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
Copy link
Contributor

zimmerle commented Dec 2, 2020

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?

@zimmerle zimmerle self-assigned this Dec 2, 2020
@nickvth
Copy link
Author

nickvth commented Dec 3, 2020

@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
Copy link
Contributor

zimmerle commented Dec 3, 2020

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
Copy link

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
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

martinhsv commented Dec 11, 2020

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
Copy link

@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
Copy link

Running into the same issue.

@lchdev
Copy link

lchdev commented Sep 8, 2021

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

  • Using more_clear_headers or more_set_headers causes the Server header to appear twice in the modsecurity JSON audit logs, while the response received by the client only contains it once.
  • Removing any more_*_headers directive, the Server: nginx header is correctly appearing only once in the modsecurity logs.

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

@martinhsv
Copy link
Contributor

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
Copy link

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

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

No branches or pull requests

7 participants