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

X-Frame-Options warning in NC17beta2 #16893

Closed
nursoda opened this issue Aug 27, 2019 · 8 comments · Fixed by #19002
Closed

X-Frame-Options warning in NC17beta2 #16893

nursoda opened this issue Aug 27, 2019 · 8 comments · Fixed by #19002
Labels
3. to review Waiting for reviews enhancement
Milestone

Comments

@nursoda
Copy link

nursoda commented Aug 27, 2019

After upgrading from NC 16 to NC 17 beta2 settings/admin/overview urges me to set the X-Frame-Options header to SAMEORIGIN in the WEBSERVER? config. No explanation or link is given, docs/admin handbook says nothing. Here is what I see:

Der „X-Frame-Options“-HTTP-Header ist nicht so konfiguriert, dass er „SAMEORIGIN“ entspricht. Dies ist ein potentielles Sicherheitsrisiko und es wird empfohlen, diese Einstellung zu ändern.

So I added it to my vhost config and double checked: It's now twice in the headers, and even then the warning is displayed, so is this an update issue or a regression (see #8207)?

Upon kesselb's request, I use the template to provide information. I omitted irrelevant entries:

Steps to reproduce

Install/Update Nextcloud to 17beta2, have it installed in a subdirectory (here: /cloud), so there are OTHER paths served. Check HTTP headers for URLs inside /cloud and outside /cloud.

Expected behaviour

X-Frame-Options header should be set to SAMEORIGIN by Nexcloud. That IS the case, so all fine for URLs inside /cloud. But no warning should be issued IMHO.
If that warning aims to nudge admins to set the header globally (which is fine also), then if the header is already set globally, it should not be set TWICE.

Actual behaviour

Although X-Frame-Options header IS set to SAMEORIGIN in the server config a warning is issued and URLs within /cloud are served with the twi identical X-Frame-Options header lines.

Server configuration

Operating system: debian 8 'jessie', current
Web server: Apache 2.4.10-10+deb8u15
Database: mySQL 5.5.62-0+deb8u1
PHP version: PHP 7.3.8-1+0-20190807.43+debian8-1.gbp7731bf via libapache2-mod-php7.3
Nextcloud version: 17beta2 (17.0.0.4)
Updated from an older Nextcloud/ownCloud or fresh install: updated from 16.0.4
Where did you install Nextcloud from: Updater app
Signing status: No errors have been found.
Nextcloud configuration: (removed irrelevant parts)

{ "system": { "trusted_domains": [ "mydomain.com" ], "overwrite.cli.url": "https:\/\/mydomain.com\/cloud", "dbtype": "mysql", "version": "17.0.0.4", "logtimezone": "Europe\/Berlin", "installed": true, "filelocking.enabled": "true", "maintenance": false, "loglevel": 2, "theme": "", "updater.release.channel": "beta", } }
@nursoda nursoda added 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug labels Aug 27, 2019
@kesselb
Copy link
Contributor

kesselb commented Aug 27, 2019

Would you mind to add some basic information about your setup? Apache or Nginx version, how is PHP connected to the webserver, etc...

@kesselb
Copy link
Contributor

kesselb commented Aug 29, 2019

Mind to add this information to your first post? Please also add the exact apache 2.4.x version (sometimes things are changed with minor versions). Its much easier to triage an issue with the issue template.

@kesselb
Copy link
Contributor

kesselb commented Sep 24, 2019

Would you mind to share the output of

curl -I https://yourcloud.com/index.php/login

@nursoda
Copy link
Author

nursoda commented Sep 25, 2019

HTTP/1.1 200 OK Date: Wed, 25 Sep 2019 08:26:04 GMT Server: Apache/2.4.10 Strict-Transport-Security: max-age=31536000; includeSubDomains; preload X-Content-Type-Options: nosniff X-Frame-Options: SAMEORIGIN Referrer-Policy: no-referrer X-Download-Options: noopen X-Permitted-Cross-Domain-Policies: none X-Robots-Tag: none X-XSS-Protection: 1; mode=block Set-Cookie: oclnu97rv47u=b9jmuu8va3j71fcqg6oqdqecm0; path=/cloud; secure; HttpOnly Expires: Thu, 19 Nov 1981 08:52:00 GMT Cache-Control: no-cache, no-store, must-revalidate Pragma: no-cache Set-Cookie: oc_sessionPassphrase=opjhda07Gyrf1gTO5TArSHIMt68ggrRckcSaru2jMlfhB3nTewafZzK4E3VyKav2%2Faz2apLsUR5jQIvmZOyxK1w1rU5f5N4qL%2BujFKC%2BiUZNERtp5CexNnak1MkeMaj0; path=/cloud; secure; HttpOnly Content-Security-Policy: default-src 'none';base-uri 'none';manifest-src 'self';script-src 'self';style-src 'self' 'unsafe-inline';img-src 'self' data: blob:;font-src 'self' data:;connect-src 'self' stun.nextcloud.com:443;media-src 'self';frame-src data:;child-src 'self';frame-ancestors 'self';worker-src 'self' blob:;form-action 'self' Set-Cookie: nc_sameSiteCookielax=true; path=/cloud; httponly;secure; expires=Fri, 31-Dec-2100 23:59:59 GMT; SameSite=lax Set-Cookie: nc_sameSiteCookiestrict=true; path=/cloud; httponly;secure; expires=Fri, 31-Dec-2100 23:59:59 GMT; SameSite=strict Feature-Policy: autoplay 'self';camera 'self';fullscreen 'self';geolocation 'none';microphone 'self';payment 'none' Content-Length: 6467 X-Frame-Options: SAMEORIGIN Content-Type: text/html; charset=UTF-8
I'm setting up a new server, so the culprit server will be offline in the next days.

@kesselb
Copy link
Contributor

kesselb commented Sep 25, 2019

OK. I suspect that the heartbeat request is sent to the wrong url. Could you visit settings/admin/overview, open the development tools (e.g https://developers.google.com/web/tools/chrome-devtools/), go to network tab and reload the current page?

image

If the request is going to yourdomin.com/heartbeat (without the subdirectory) please try overwritewebroot.

@nursoda
Copy link
Author

nursoda commented Sep 25, 2019

Seems to be sent to the correct URL:
heartbeat

@kesselb
Copy link
Contributor

kesselb commented Sep 25, 2019

OK. The behaviour changed from 16.04 to 17. Please remove the option from your webserver configuration.

@nursoda
Copy link
Author

nursoda commented Oct 1, 2019

Thanks. Installed NC17 on Arch/NGINX, no issues/warnings there. Security setting are in NGINGX site config though, as proposed in https://docs.nextcloud.com/server/latest/admin_manual/installation/nginx.html#nextcloud-in-the-webroot-of-nginx

@nursoda nursoda closed this as completed Oct 1, 2019
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 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 added enhancement 3. to review Waiting for reviews and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug needs info labels Mar 5, 2020
@MichaIng MichaIng added this to the Nextcloud 19 milestone Mar 5, 2020
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.

3 participants