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

NC17 admin setup check incorrectly reports 'nosniff' header is not set #16476

Closed
pgnd opened this issue Jul 20, 2019 · 17 comments · Fixed by #19002
Closed

NC17 admin setup check incorrectly reports 'nosniff' header is not set #16476

pgnd opened this issue Jul 20, 2019 · 17 comments · Fixed by #19002
Labels
3. to review Waiting for reviews enhancement
Milestone

Comments

@pgnd
Copy link

pgnd commented Jul 20, 2019

I'm running

NC 17
	git log | head
		commit 602a48c411cb242840a25f59e5eff44e40d0094c
		Author: Nextcloud bot <bot@nextcloud.com>
		Date:   Sat Jul 20 02:14:33 2019 +0000

		    [tx-robot] updated from transifex

		commit a085a88205ccf833c347c1b0a5021ca2ee8db484
		Merge: c00d6f4eac 2c5ebac458
		Author: Morris Jobke <hey@morrisjobke.de>
		Date:   Fri Jul 19 18:03:37 2019 +0200

php -v
	PHP 7.4.0-dev (cli) (built: Jul 20 2019 07:05:36) ( NTS )
	Copyright (c) The PHP Group
	Zend Engine v3.4.0-dev, Copyright (c) Zend Technologies
	    with Zend OPcache v7.4.0-dev, Copyright (c), by Zend Technologies

nginx -v
	nginx version: nginx/1.17.1 (Local Build)

on the NC 17/head's admin setup-check page,

/admin/overview

it currently reports here,

The "X-Content-Type-Options" HTTP header is not set to "nosniff". This is a potential security or privacy risk, as it is recommended to adjust this setting accordingly.

in my webserver config, I've

	add_header X-Content-Type-Options "nosniff" always;

checking document response headers on that NC admin page,

	HTTP/2.0 200 OK
	date: Sat, 20 Jul 2019 14:55:39 GMT
	content-type: text/html; charset=UTF-8
	vary: Accept-Encoding
	x-powered-by: PHP/7.4.0-dev
	expires: Thu, 19 Nov 1981 08:52:00 GMT
	pragma: no-cache
	x-frame-options: SAMEORIGIN
	cache-control: no-cache, no-store, must-revalidate
	content-security-policy: default-src 'none';base-uri 'none';manifest-src 'self';script-src 'nonce-UjB...ST0=';style-src 'self' 'unsafe-inline';img-src 'self' data: blob:;font-src 'self' data:;connect-src 'self';media-src 'self';frame-ancestors 'self'
	nextcloud: App
>>	x-content-type-options: nosniff
	strict-transport-security: max-age=315360000; includeSubDomains; preload
	x-content-type-options: nosniff
	x-xss-protection: 1; mode=block
	x-robots-tag: none
	x-download-options: noopen
	x-permitted-cross-domain-policies: none
	content-encoding: br
	X-Firefox-Spdy: h2
@pgnd pgnd added 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug labels Jul 20, 2019
@kesselb
Copy link
Contributor

kesselb commented Jul 20, 2019

Warning is shown because X-Content-Type-Options is there 2x.

@pgnd
Copy link
Author

pgnd commented Jul 20, 2019

twice?

It's set only once in my config.
It's shown only once in the response headers, above.

Can you clarify?

@kesselb
Copy link
Contributor

kesselb commented Jul 20, 2019

image

@pgnd
Copy link
Author

pgnd commented Jul 20, 2019

ok, now that we've established that I'm blind as a bat ...

my bad. sry.

@pgnd
Copy link
Author

pgnd commented Jul 20, 2019

although, that doesn't explain WHY there are 2x in the response headers.

there's only one instance of the header-set in my config ...

@kesselb
Copy link
Contributor

kesselb commented Aug 12, 2019

if(getenv('modHeadersAvailable') !== 'true') {
header('X-XSS-Protection: 1; mode=block'); // Enforce browser based XSS filters
header('X-Content-Type-Options: nosniff'); // Disable sniffing the content type for IE

X-Content-Type-Options is added by nextcloud to the response if modHeadersAvailable is not true. https://docs.nextcloud.com/server/16/admin_manual/installation/nginx.html for more details about the nginx configuration.

@eehmke
Copy link

eehmke commented Aug 19, 2019

I have this problem since upgrading to 17.0.0 Beta1 with Apache. X-Content-Type-Options is set to nosniff in .htaccess, as can be checked by https://www.webconfs.com/http-header-check.php. There are no double entries.

@ghost
Copy link

ghost commented Sep 18, 2019

This issue has been automatically marked as stale because it has not had recent activity and seems to be missing some essential information. It will be closed if no further activity occurs. Thank you for your contributions.

@ghost ghost added the stale Ticket or PR with no recent activity label Sep 18, 2019
@ghost ghost closed this as completed Oct 2, 2019
@eehmke
Copy link

eehmke commented Oct 4, 2019

Please reopen the issue. This is still a problem when the apache configuration has set the header. In Debian 10.1, Apache Apache 2.4.38, the header is set in
/etc/apache2/conf-available/security.conf
The statement in .htaccess should be conditional, so it is only set when not already present.

Header setifempty X-Content-Type-Options "nosniff"
in .htaccess does the trick. It works whether the option is set in the apache config or not.

Beside of this, the error message is misleading. If the option is set twice, it should not say "header is not set".

@kesselb
Copy link
Contributor

kesselb commented Oct 4, 2019

Header setifempty X-Content-Type-Options "nosniff"
in .htaccess does the trick. It works whether the option is set in the apache config or not.

Looks good to me. Would you mind to create a pr with your changes? https://github.com/nextcloud/server/edit/master/.htaccess

@eehmke
Copy link

eehmke commented Oct 4, 2019

That's fine, thanks for quick response. Alert: this problem might apply for other headers too.

@cyBea
Copy link

cyBea commented Dec 10, 2019

I have the same issue using apache on buster. Since there is no change in master and no pull request the issue is not resolved.
@kesselb Do you think replacing every occurrence of always set with setifempty in .htaccess is a good solution? Could it have any side effects?

@kesselb
Copy link
Contributor

kesselb commented Dec 10, 2019

Could it have any side effects?

I don't know. Fixing your setup is probably the better way. There are some threads at https://help.nextcloud.com about this warning.

@zertrin
Copy link
Member

zertrin commented Jan 19, 2020

After upgrading to NC 17 I had the case that the headers were set twice. That is because in by case the server admin has configured the apache host server to already set these headers site-wise (across all virtualhosts, not only nextcloud). Then the .htaccess of nextcloud adds them again...

Solved the warnings and double setting of headers by replacing every occurrence of always set with setifempty in .htaccess. This works well.

I think here Nextcloud is being too aggressive with header setting. When the host server already has a security policy in place, and already set those headers, Nextcloud should not always set its own headers on top.

Thus I also petition to change the default .htaccess to use setifempty.

zertrin added a commit to zertrin/nextcloud-server that referenced this issue Jan 19, 2020
The headers might already be set by the system administrator at the http server level (apache or nginx) for some or all virtualhosts.
Using "always set" in the .htaccess of Nextcloud leads to the situation where the headers are set twice! Which leads to warnings in the admin area.
Using "setifempty" solves the problem. In the default case where the system admin didn't do anything, Nextcloud will add them automatically. On the other hand, when the system administrator has already set security headers, we should not add ours on top.
See github issues nextcloud#16893 nextcloud#16476 nextcloud#16938 nextcloud#18017
@zertrin
Copy link
Member

zertrin commented Jan 19, 2020

Looks good to me. Would you mind to create a pr with your changes? https://github.com/nextcloud/server/edit/master/.htaccess

Opened PR #19002 😉

zertrin added a commit to zertrin/nextcloud-server that referenced this issue Jan 19, 2020
The headers might already be set by the system administrator at the http server level (apache or nginx) for some or all virtualhosts.
Using "always set" in the .htaccess of Nextcloud leads to the situation where the headers are set twice! Which leads to warnings in the admin area.
Using "setifempty" solves the problem. In the default case where the system admin didn't do anything, Nextcloud will add them automatically. On the other hand, when the system administrator has already set security headers, we should not add ours on top.
See github issues nextcloud#16893 nextcloud#16476 nextcloud#16938 nextcloud#18017

Signed-off-by: zertrin <zertrin@gmail.com>
zertrin added a commit to zertrin/nextcloud-server that referenced this issue Jan 19, 2020
The headers might already be set by the system administrator at the http server level (apache or nginx) for some or all virtualhosts.
Using "always set" in the .htaccess of Nextcloud leads to the situation where the headers are set twice! Which leads to warnings in the admin area.
Using "setifempty" solves the problem. In the default case where the system admin didn't do anything, Nextcloud will add them automatically. On the other hand, when the system administrator has already set security headers, we should not add ours on top.
See github issues nextcloud#16893 nextcloud#16476 nextcloud#16938 nextcloud#18017

Signed-off-by: Marc Gallet <zertrin@gmail.com>
zertrin added a commit to zertrin/nextcloud-server that referenced this issue Jan 19, 2020
The headers might already be set by the system administrator at the http server level (apache or nginx) for some or all virtualhosts.
Using "always set" in the .htaccess of Nextcloud leads to the situation where the headers are set twice! Which leads to warnings in the admin area.
Using "setifempty" solves the problem. In the default case where the system admin didn't do anything, Nextcloud will add them automatically. On the other hand, when the system administrator has already set security headers, we should not add ours on top.
See github issues nextcloud#16893 nextcloud#16476 nextcloud#16938 nextcloud#18017

Signed-off-by: zertrin <zertrin@gmail.com>
zertrin added a commit to zertrin/nextcloud-server that referenced this issue Jan 20, 2020
The headers might already be set by the system administrator at the http server level (apache or nginx) for some or all virtualhosts.
Using "set" in the .htaccess of Nextcloud leads to the situation where the headers are set twice! Which leads to warnings in the admin area.
Using "setifempty" solves the problem. In the default case where the system admin didn't do anything, Nextcloud will add them automatically. On the other hand, when the system administrator has already set security headers, we should not add ours on top.
See github issues nextcloud#16893 nextcloud#16476 nextcloud#16938 nextcloud#18017

Signed-off-by: zertrin <zertrin@gmail.com>
zertrin added a commit to zertrin/nextcloud-server that referenced this issue Jan 28, 2020
The headers might already be set by the system administrator at the http server
level (apache or nginx) for some or all virtualhosts.

Using "always set" in the .htaccess of Nextcloud leads to the situation where
the headers might be set twice (once in the default 'onsuccess' table and once
in the 'always' table)! Which leads to warnings in the admin area.

Adding "onsuccess unset" solves the problem, and forces the header in
the 'onsucess' table to be unset, and the header in the 'always' table to be set.

NOTE: with this change, Nextcloud overrides whatever the system administrator
might have already set

See github issues nextcloud#16893 nextcloud#16476 nextcloud#16938 nextcloud#18017 and discussion in PR nextcloud#19002

Signed-off-by: zertrin <zertrin@gmail.com>
zertrin added a commit to zertrin/nextcloud-server that referenced this issue Jan 28, 2020
The headers might already be set by the system administrator at the http server
level (apache or nginx) for some or all virtualhosts.

Using "always set" in the .htaccess of Nextcloud leads to the situation where
the headers might be set twice (once in the default 'onsuccess' table and once
in the 'always' table)! Which leads to warnings in the admin area.

Adding "onsuccess unset" solves the problem, and forces the header in
the 'onsucess' table to be unset, and the header in the 'always' table to be set.

NOTE: with this change, Nextcloud overrides whatever the system administrator
might have already set

See github issues nextcloud#16893 nextcloud#16476 nextcloud#16938 nextcloud#18017 and discussion in PR nextcloud#19002

Signed-off-by: zertrin <zertrin@gmail.com>
zertrin added a commit to zertrin/nextcloud-server that referenced this issue Mar 5, 2020
The headers might already be set by the system administrator at the http server
level (apache or nginx) for some or all virtualhosts.

Using "always set" in the .htaccess of Nextcloud leads to the situation where
the headers might be set twice (once in the default 'onsuccess' table and once
in the 'always' table)! Which leads to warnings in the admin area.

Adding "onsuccess unset" solves the problem, and forces the header in
the 'onsucess' table to be unset, and the header in the 'always' table to be set.

NOTE: with this change, Nextcloud overrides whatever the system administrator
might have already set

See github issues nextcloud#16893 nextcloud#16476 nextcloud#16938 nextcloud#18017 and discussion in PR nextcloud#19002

Signed-off-by: zertrin <zertrin@gmail.com>
zertrin added a commit to zertrin/nextcloud-server that referenced this issue Mar 5, 2020
The headers might already be set by the system administrator at the http server
level (apache or nginx) for some or all virtualhosts.

Using "always set" in the .htaccess of Nextcloud leads to the situation where
the headers might be set twice (once in the default 'onsuccess' table and once
in the 'always' table)! Which leads to warnings in the admin area.

Adding "onsuccess unset" solves the problem, and forces the header in
the 'onsucess' table to be unset, and the header in the 'always' table to be set.

NOTE: with this change, Nextcloud overrides whatever the system administrator
might have already set

See github issues nextcloud#16893 nextcloud#16476 nextcloud#16938 nextcloud#18017 and discussion in PR nextcloud#19002

Signed-off-by: zertrin <zertrin@gmail.com>
@MichaIng MichaIng reopened this Mar 5, 2020
@ghost ghost removed the stale Ticket or PR with no recent activity label Mar 5, 2020
@MichaIng MichaIng added this to the Nextcloud 19 milestone Mar 5, 2020
@MichaIng MichaIng added 3. to review Waiting for reviews enhancement and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap 17-feedback bug needs info labels Mar 5, 2020
@eehmke
Copy link

eehmke commented Apr 17, 2020

This is still an issue with NC 19.0.0 beta 4. The "onsuccess unset" line is not in .htaccess.

@zertrin
Copy link
Member

zertrin commented Apr 17, 2020

The pull request is ready to merge but was not yet included in the latest beta apparently.

Hope it can make it this round.

backportbot-nextcloud bot pushed a commit that referenced this issue Apr 25, 2020
The headers might already be set by the system administrator at the http server
level (apache or nginx) for some or all virtualhosts.

Using "always set" in the .htaccess of Nextcloud leads to the situation where
the headers might be set twice (once in the default 'onsuccess' table and once
in the 'always' table)! Which leads to warnings in the admin area.

Adding "onsuccess unset" solves the problem, and forces the header in
the 'onsucess' table to be unset, and the header in the 'always' table to be set.

NOTE: with this change, Nextcloud overrides whatever the system administrator
might have already set

See github issues #16893 #16476 #16938 #18017 and discussion in PR #19002

Signed-off-by: zertrin <zertrin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants