unsafe nginx config

[p3x-robot]

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;
    }

[LukasReschke]

@josh4trunks Any input? :)

[josh4trunks]

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.

[patschi]

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;
}

[josh4trunks]

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.

[dominic-p]

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

[josh4trunks]

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

[MorrisJobke]

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

cc @benediktg for the gzip part.

[benediktg]

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).

1. Disabling HTTP compression
2. Separating secrets from user input
3. Randomizing secrets per request
4. Masking secrets (effectively randomizing by XORing with a random secret per request)
5. Protecting vulnerable pages with CSRF
6. Length hiding (by adding random number of bytes to the responses)
7. 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).

[p3x-robot]

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.

[LukasReschke]

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 :)

[josh4trunks]

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;

[p3x-robot]

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.

[p3x-robot]

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.

[dominic-p]

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

[p3x-robot]

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

[dominic-p]

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

[benediktg]

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

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

[MeiRos]

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

[josh4trunks]

@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.

[rugk]

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.)

[rugk]

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.