unsafe nginx config

It is easy.
Your error:

root@server:~/server-scripts# gixy
[nginx_parser]	WARNING	File not found: /etc/nginx/conf.d/*.conf

==================== Results ===================

Problem: [http_splitting] Possible HTTP-Splitting vulnerability.
Description: Using variables that can contain "\n" may lead to http injection.
Additional info: https://github.com/yandex/gixy/blob/master/docs/en/plugins/httpsplitting.md
Reason: At least variable "$uri" can contain "\n"
Pseudo config:
include /etc/nginx/sites-enabled/sync.patrikx3.tk;

	server {
		server_name sync.patrikx3.tk;

		location / {
			rewrite ^ /index.php$uri;
		}
	}


==================== Summary ===================
Total issues:
    Unspecified: 0
    Low: 0
    Medium: 0
    High: 1

Just do not use $uri for NGINX, use $request_uri;

For use like this:

  location / {
        rewrite ^ /index.php$request_uri;
    }

@josh4trunks

@josh4trunks Any input? :)

Hmm, never heard of this before but I can see the issue.
Here is an explanation that helped me understand http://blog.volema.com/nginx-insecurities.html

I agree we should change this. There are 3 lines where this should be changed for both...
"https://github.com/nextcloud/documentation/blob/master/admin_manual/installation/nginx.rst#nextcloud-in-the-webroot-of-nginx"
and...
"https://github.com/nextcloud/documentation/blob/master/admin_manual/installation/nginx.rst#nextcloud-in-a-subdir-of-nginx".

rewrite ^ /nextcloud/index.php$uri; --> rewrite ^ /nextcloud/index.php$request_uri;
try_files $uri /nextcloud/index.php$uri$is_args$args; --> try_files $uri /nextcloud/index.php$request_uri;
try_files $uri /nextcloud/index.php$uri$is_args$args; --> try_files $uri /nextcloud/index.php$request_uri;

~~I suggest we do some testing to make sure this does not break anything~~
The above change seems to work just browsing around my production site.

I've done the suggested configuration change on my setup now aswell. No negative behavior so far. Additionally I would suggest adding:

# disable autoindex. safety first.
autoindex off;

# filter request methods, deny TRACE and TRACK.
if ($request_method ~* ^(TRACE|TRACK)$) {
	return 403;
}

I do not believe autoindex has ever been enabled by default in NGINX.

I do not think we need to give general configuration tips, only things specific to nextcloud.

I noticed that gzip is enabled in the sample config files. Should we disable it for HTTPS to mitigate attacks like BREACH?

I don't know enough about BREACH to know if our recommendation of SSL and gzip is vulnerable.
Maybe someone can research/comment.

@LukasReschke

@LukasReschke regarding HTTPS and gzip and the potential BREACH attack.

cc @benediktg for the gzip part.

@LukasReschke

Sure, disabling gzip is the most effective way of preventing the BREACH attack. http://breachattack.com says

Unfortunately, we are unaware of a clean, effective, practical solution to the problem. Some of these mitigations are more practical and a single change can cover entire apps, while others are page specific.

The mitigations are ordered by effectiveness (not by their practicality - as this may differ from one application to another).

Disabling HTTP compression

Separating secrets from user input

Randomizing secrets per request

Masking secrets (effectively randomizing by XORing with a random secret per request)

Protecting vulnerable pages with CSRF

Length hiding (by adding random number of bytes to the responses)

Rate-limiting the requests

I think having gzip enabled is still nice to have, hence it'd be great to know what e.g. @LukasReschke can say about the other mitigation options and their applicability in Nextcloud. (5–7 sound like security by obscurity to me).

i just use gzip for static data like js, css, png, jpg, the rest is disabled. thats all. then it is secure. the nexcloud user data should not be compressed.

Secrets such as the CSRF token are XOR'ed in Nextcloud with a random secret per request, but I'd still recommend disabling GZIP for dynamic pages to be honest :)

maybe we should take a look at our recommended gzip_types?

gzip_types application/atom+xml application/javascript application/json application/ld+json application/manifest+json application/rss+xml application/vnd.geo+json application/vnd.ms-fontobject application/x-font-ttf application/x-web-app-manifest+json application/xhtml+xml application/xml font/opentype image/bmp image/svg+xml image/x-icon text/cache-manifest text/css text/plain text/vcard text/vnd.rim.location.xloc text/vtt text/x-component text/x-cross-domain-policy;

user data disabled all. generated can be json or js as well.
gzip only for static nextcloud , but you don't really need gzip.
i need gzip because for angular 4 i got like 3MB js files and now they are 500mb.
gzip is unsafe. just disable. easier disable gzip.

i use like this for nginx the http, the main directive:

    geo $limited {
        default 1;
        172.19.13.0/24 0;
        192.168.78.0/24 0;
    }

    map $limited $limit {
        1 $binary_remote_addr;
        0 "";
    }

    limit_req_zone $limit zone=default_limit:5m rate=1r/s;
    limit_conn_zone $limit zone=default_limit_conn:5m;

secure location

  location /api {
            proxy_pass         "http://127.0.0.1:23501";
            include default-proxy.conf;
            limit_conn default_limit_conn 10;
            limit_req zone=default_limit burst=5;
            gzip off;
  }

/api could be the secrue locations. the rest is gzipped.

It looks like this issue has been closed. Was there any resolution on the gzip discussion? I haven't noticed any changes to the gzip settings in the recommended nginx config

dont use gzip on ssl, otherwise it is unsafe...

Ok, thanks. Should the documentation be updated then? Right now, it does use gzip for SSL.

dont use gzip on ssl, otherwise it is unsafe...

Not necessarily – I simply let gzip activated because it means a measurable performance gain.

@josh4trunks

@josh4trunks
Should we use $request_uri instead of $uri?
At least documents linked above have $uri.

@MeiRos

@MeiRos, yes it looks like my comment #5117 (comment) was not fully implemented.

These two lines in https://github.com/nextcloud/documentation/blob/master/admin_manual/installation/nginx.rst...

rewrite ^ /index.php$uri
rewrite ^ /nextcloud/index.php$uri

Should be changed to...

rewrite ^ /index.php$request_uri;
rewrite ^ /nextcloud/index.php$request_uri;

If someone wants to submit a PR and mention me we can review it there.

BTW:

(5–7 sound like security by obscurity to me).

It's not and these are actually best practices and/or good suggestions. Of course use CSRF tokens. And even adding random data (often as a random padding) to prevent size information leak is actually a thing, which is done in proper modern security protocols. AFAIK TLS 1.3 uses it.
As such, TLS 1.3 may also mitigate it, at least. (note they ordered their suggestions by effectiveness.)

Generally I am also not sure how BREACH may work with HTTP/2, which again does yet different binary data transmission etc. I assume, they only tested with HTTP 1.

Navigation Menu

unsafe nginx config #5117

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

unsafe nginx config #5117

Description

Activity

LukasReschke commented on May 28, 2017

josh4trunks commented on May 29, 2017

patschi commented on May 31, 2017

josh4trunks commented on May 31, 2017

dominic-p commented on Jun 12, 2017

josh4trunks commented on Jun 12, 2017

MorrisJobke commented on Jun 12, 2017

benediktg commented on Jun 12, 2017

p3x-robot commented on Jun 12, 2017

LukasReschke commented on Jun 12, 2017

josh4trunks commented on Jun 12, 2017

p3x-robot commented on Jun 12, 2017

p3x-robot commented on Jun 12, 2017

dominic-p commented on Aug 21, 2017

p3x-robot commented on Aug 21, 2017

dominic-p commented on Aug 21, 2017

benediktg commented on Aug 22, 2017

MeiRos commented on Jun 21, 2018

josh4trunks commented on Jun 21, 2018

rugk commented on Aug 17, 2018

rugk commented on Aug 17, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions