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

Nextcloud 12 - Issues with Integrity Check and "X-Frame-Options"..."SAMEORIGIN" #4605

Closed
ghost opened this issue Apr 29, 2017 · 20 comments
Closed
Assignees

Comments

@ghost
Copy link

ghost commented Apr 29, 2017

Hi Nextclouders,

I just installed Nextcloud 12 alpha (Nextcloud 12.0 alpha Build:2017-04-28T22:01:10+00:00 d4e5b1b), based on an ODroid C2, NGINX 1.13, Ubuntu 16.04.02 LTS x64,, PHP 7.1.4, mariadb 10.0.29 and ran into two issues:

  • Some files have not passed the integrity check. Further information on how to resolve this issue can be found in our documentation. (List of invalid files… / Rescan…)

Raw output
==========
Array
(
[bruteforcesettings] => Array
(
[EXCEPTION] => Array
(
[class] => OC\IntegrityCheck\Exceptions\InvalidSignatureException
[message] => Signature data not found.
)

  • The "X-Frame-Options" HTTP header is not configured to equal to "SAMEORIGIN". This is a potential security or privacy risk and we recommend adjusting this setting.

NGINX is configured properly using "proxy_set_header X-Frame-Options "SAMEORIGIN; always;";"
grafik

grafik

This is a clone of my productiv Nextcloud 11.0.3 environment which is running without the issues above.

Cheers, Carsten

@ghost ghost changed the title Nextcloud 12 - Issues with Integrity Check and SET HEADER X-Frame SAMEORIGIN Nextcloud 12 - Issues with Integrity Check and "X-Frame-Options"..."SAMEORIGIN" Apr 29, 2017
@MorrisJobke
Copy link
Member

The SAMEORIGIN, SAMEORIGN looks wrong, but @LukasReschke can say more on this. In my instance it is only written once:

bildschirmfoto 2017-05-01 um 00 33 55

Maybe @josh4trunks knows why there is same origin listed twice in the headers for Nginx.

@josh4trunks
Copy link
Contributor

josh4trunks commented May 1, 2017

@MorrisJobke Maybe it is being set twice because of proxy_set_header X-Frame-Options "SAMEORIGIN; always;";".
@riegercloud I'm not sure why you say this is "configured properly", I do not see this in the documentation.

The documentation already has add_header X-Frame-Options "SAMEORIGIN";

@ghost
Copy link
Author

ghost commented May 1, 2017

Hi and thanks for all the responses.
I am running NGINX 1.13 as a reverse proxy.
My production system does not show the SAMEORIGIN twice neither.
grafik

My Testcloud is installed in a subfolder, located on a second server. The main gateway.conf points to the second server - maybe it is related to that configuration?

The gateway.conf contains of
...
add_header Referrer-Policy "same-origin" always;
add_header X-Frame-Options "SAMEORIGIN" always;
...

The Nextcloud.conf's consists of:
...
server {
...
proxy_set_header X-Frame-Options "SAMEORIGIN; always;";
...
}
...
location ~* .(?:css|js)$ {
...
proxy_set_header X-Frame-Options "SAMEORIGIN; always;";
...

I would provide more information if you are interested in - please let me know ;-)
...:::carsten

@josh4trunks
Copy link
Contributor

Please try removing proxy_set_header X-Frame-Options "SAMEORIGIN; always;";

@ghost
Copy link
Author

ghost commented May 1, 2017

I removed both entries in the testcloud.conf (Nextcloud 12)

grafik

and restarted the relevant services:
service nginx stop && service redis-server restart && service php7.1-fpm restart && service nginx start

grafik

Unfortunately still the same behaviour. But i recognized the duplicate entries only appear for e.g. the login or file requests grafik
grafik

not for e.g. files

grafik

could that point to tne root cause?

@ghost
Copy link
Author

ghost commented May 1, 2017

@josh4trunks
Copy link
Contributor

josh4trunks commented May 1, 2017

Can you please post your configs in code blocks instead of PDFs?
Why do you have all of these proxy_set_header lines, when the documentation never uses them?
Ohh, I think you explained this above. I'm not familiar with the difference between these. What is your second server running, just PHP?

That is interesting, even with you SAMEORIGIN lines commented out, the header shows up.
Can you show the contents of your fastcgi_params file?

@ghost
Copy link
Author

ghost commented May 1, 2017

The documentation of what? Nextcloud?
The documentation didn't fit all my requirements and we did investigat with others in a more secure and more quicker environement.

fastcgi_params (default - no cahnges applied):

fastcgi_param  QUERY_STRING       $query_string;
fastcgi_param  REQUEST_METHOD     $request_method;
fastcgi_param  CONTENT_TYPE       $content_type;
fastcgi_param  CONTENT_LENGTH     $content_length;

fastcgi_param  SCRIPT_NAME        $fastcgi_script_name;
fastcgi_param  REQUEST_URI        $request_uri;
fastcgi_param  DOCUMENT_URI       $document_uri;
fastcgi_param  DOCUMENT_ROOT      $document_root;
fastcgi_param  SERVER_PROTOCOL    $server_protocol;
fastcgi_param  REQUEST_SCHEME     $scheme;
fastcgi_param  HTTPS              $https if_not_empty;

fastcgi_param  GATEWAY_INTERFACE  CGI/1.1;
fastcgi_param  SERVER_SOFTWARE    nginx/$nginx_version;

fastcgi_param  REMOTE_ADDR        $remote_addr;
fastcgi_param  REMOTE_PORT        $remote_port;
fastcgi_param  SERVER_ADDR        $server_addr;
fastcgi_param  SERVER_PORT        $server_port;
fastcgi_param  SERVER_NAME        $server_name;

fastcgi_param  REDIRECT_STATUS    200;

@josh4trunks
Copy link
Contributor

yes i meant the nedtcloud documentation.
from what i am reading proxy_set_header sends these headers to the backend server (in this case php-fpm), I'm pretty sure you should be using addheader.

ok to clarify. you have an nginx instance running the gateway_conf configuration, then for the testcloud, nginx on another machine?

@josh4trunks
Copy link
Contributor

im not sure hoe this double header is being generated. possibly a bug in nextcloud 12?

Im a bit lost in your setup but i do not think that could cause the double header.
you have a gateway (is this also running the ssl_conf) then 2 different ngjnx servers behind it. you also dodnt shoe you php-handler block, but i doubt that has an issue with it.

@josh4trunks
Copy link
Contributor

ohh, i see where the ssl conf is included. disregard my previous question on that.

@ghost
Copy link
Author

ghost commented May 1, 2017

Server A (nc.c-rieger.de):

  • gateway.conf
  • ssl.conf

points to

  • nextcloud.conf (on Server A / root-folder)
  • testcloud.conf (on Server B / as a subfolder -> nc.c-rieger.de/testcloud)

i will re-install Nextcloud 12 in a root-folder instead of a subfolder and will doublecheck.
But as mentioned before, the same nginx-confguration is running well in the productive environment.
I guess, it is a bug in NC12. Maybe hardcoded?

Will be back with the testresults ... but i have to accompany my twins to their riding first.

@josh4trunks
Copy link
Contributor

sounds good, I'm suspecting a bug here

@ghost
Copy link
Author

ghost commented May 1, 2017

I re-installed NC12 from scratch in the web-root (https://192.168.2.17/login) and took the configuration from Nextclouds-Documentation.

Unfortunately, i still ran in the same issues:

grafik

So this must be related to NC12 and NGINX.
Does anybody have a look at the Integrity Failure as well?

@LukasReschke
Copy link
Member

The X-Frame-Options header has been moved to the PHP processing and outside of .htaccess so that applications can decide themselves if they want to be iframed. For example for iframing a public Nextcloud calendar or so.

So the X-Frame-Options header should be dropped from the NGINX configuration as this would also make it impossible for applications to control themselves if they want to allow iframing.

@ghost
Copy link
Author

ghost commented May 1, 2017

OK, if removed from NGINX it works fine.
This should be documented properly and can be closed from my perspective.
But the other issue remains open.
Thank you @LukasReschke
grafik

@MorrisJobke MorrisJobke self-assigned this May 1, 2017
@LukasReschke LukasReschke self-assigned this May 1, 2017
LukasReschke added a commit that referenced this issue May 1, 2017
Apps that are in shipped.json follow some more requirements such as having a valid code integrity check. This is not something that we require when they come from the appstore as there we verify the download integrity via the signature.

Also the updater treats apps that are shipped differently. We should however handle the apps like any other app from the appstore.

Fixes #4605

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
LukasReschke added a commit that referenced this issue May 1, 2017
Apps that are in shipped.json follow some more requirements such as having a valid code integrity check. This is not something that we require when they come from the appstore as there we verify the download integrity via the signature.

Also the updater treats apps that are shipped differently. We should however handle the apps like any other app from the appstore.

Fixes #4605

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
@LukasReschke
Copy link
Member

Potential patch at #4626

@MorrisJobke
Copy link
Member

See nextcloud/documentation#434 for the documentation update as well as the release notes

@LukasReschke
Copy link
Member

@sergeyklay
Copy link

@LukasReschke Thank you!

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

4 participants