Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

How to hide the data directory from external access #62

Closed
jurgenhaas opened this issue May 20, 2018 · 36 comments
Closed

How to hide the data directory from external access #62

jurgenhaas opened this issue May 20, 2018 · 36 comments
Labels

Comments

@jurgenhaas
Copy link

Thanks a lot for this great plugin and for the ongoing maintenance.

Since I installed the latest update,when I'm going to the RainLoop admin panel, I'm getting a warning, that the data directory should be hidden from external access:

screenshot-cloud haas-gottmadingen de-2018-05-20-14-27-37

Is there some advise on how to do that for the integrated instance or is it even necessary in the Nextcloud context?

@enboig
Copy link

enboig commented May 21, 2018

Also interested in this

@pierre-alain-b
Copy link
Owner

Hello all,
Yes, as a matter of fact I spotted this as well yesterday after the update. But I had no time to investigate thoroughly but a quick test on my instance showed that the data/ folder was apparently not leaked outside of Nextcloud. If confirmed, this would be a very minor issue for mono-user instance, presumably a bigger one for multi-user instances. I shall report as soon as possible, I hope later today. If some of you can test and help me evaluate what is effectively accessible in the nextcloud context, it would immensely help!

@jurgenhaas
Copy link
Author

I was under the same impression, that the data directory is not leaked but I wasn't quite sure. At least the warning produced by the RainLoop library must have some means to determine the fact and if they thought it was true, there might be some tricky way, I just couldn't find out.

@zinsserzh
Copy link

The warning disappeared after I blocked this url in the Chrome developer tools.

/apps/rainloop/app/data/VERSION*

I don't have time to check the code, but I assume that a 403 is expected while nextcloud generates a 302 redirect.

@pierre-alain-b
Copy link
Owner

The code responsible for this in Rainloop is here: line 32
https://github.com/RainLoop/rainloop-webmail/blob/90a3d2b62a626b05b914cdf597fd0b9466803ddc/dev/Stores/Admin/App.js

if (settingsGet('Auth')) {
    $.get('./data/VERSION?' + window.Math.random()).then(() => this.dataFolderAccess(true));
}

And as said previously, Nextcloud does reply to this by redirecting the request to the Files application. So Rainloop wrongly considers the data folder is open.

@pierre-alain-b
Copy link
Owner

And
$.ajax({url: './data/VERSION?' + window.Math.random(),type: 'get'}).done(function(data, statusText, xhr){var status = xhr.status;console.log(status);});
logs the status code 200... because it seems it follows the redirect and gives the status code of the redirected query.

@zinsserzh
Copy link

Yeah, ajax always follow redirects silently. It's part of the XMLHttpRequest and can't be overridden. I reasonable solution would be to check if the final response URL is the same as the original one (meaning redirects happened). I'm not sure if it is safe to treat redirect as a passed test tho.

@pierre-alain-b
Copy link
Owner

Or we may test the content of the retrieved file and see if this is what is expected (the current content of the VERSION file is "1.12.0"). What do you think?

@zinsserzh
Copy link

If we are going to verify the content, I think at least the "expected" content should not be a hard-coded string. Either retrieve its content via a legal method or get the version number from somewhere. If so, I think it's fine.

@feutl
Copy link

feutl commented Jul 5, 2018

Is there a fix now?
Are there any implications?
Do I need a .htaccess file in the "rainloop-data" directory or not? based on my ticket #69 this questions are still unanswered.

@pierre-alain-b
Copy link
Owner

  • No, we discussed the fix but I have not implemented it yet.
  • As far as I (and others) have analyzed, there is no implication and in fact it is a false positive
  • No need of a special .htaccess file

@feutl
Copy link

feutl commented Jul 5, 2018

Great, this is what I was unable to "figure out" based on the discussion.

Thanks

@truhi
Copy link

truhi commented Jul 6, 2018

I've just discovered that web-folders after /data/_data_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx are fully readable without any authentication.
For example: https://xyz.com/rainloop/data/_data_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/_default_/storage
Which I believe reproduces the issue.

@pierre-alain-b
Copy link
Owner

I can't replicate this on my instance.
I am everytime caught by the Rainloop login page. Could others test?

@pierre-alain-b
Copy link
Owner

Dear All,

Because of the lack of time and as I was never the developer of Rainloop or of this plugin, I have decided to cease maintaining the plugin.
I cannot guarantee the security of the plugin and I cannot update it to work with NC 14 that is coming.

If one of you is willing to fork the project and take the lead, I will assist the best I can.

Best regards
Pierre-Alain

@jkalousek
Copy link

I have also made some tests and I ended up always on either RL or NC login page.

@feutl
Copy link

feutl commented Jul 9, 2018

@pierre-alain-b
This is bad, but that's how such projects work sometimes :(
I would have loved to see Rainloop being used instead of the "official" Mail app which is by far not as smooth and nice to use as Rainloop but, I need to figure out another way to have it running for me and the others.

@pierre-alain-b
Copy link
Owner

OK, summertime was good for thinking again and here I am back. I am going to look over this to fix this in NC14 version. Maybe not in the first NC14 version to be published today (not to prevent users from upgrading) but I will try to fix the threatening error message which is wrong.

@tabp0le
Copy link
Contributor

tabp0le commented Sep 17, 2018

This isn't actually a security issue if you use an alternate location for data.
If you aren't using alternate location for data then the following will fix this:

nginx:

location ^~/apps/rainloop/app/data {
    deny all;
}

apache: Should work as it should use .htaccess to deny

@meminens
Copy link

I am getting the same error message. What should I do with Apache2?

@andriusign
Copy link

andriusign commented Mar 10, 2019

So what about the fix? It has been nearly a year, has the developer managed to fix this error?

@pierre-alain-b
Copy link
Owner

There is no security issue it seems, it is mainly a UI glitch that is worrying. Do not hesitate to send a patch if you fixed this on your side, we will be happy to review any Merge Request.

@paulcalabro
Copy link

paulcalabro commented Sep 26, 2019

I wonder if this tip could be of use for identifying the redirection and handling it accordingly:

https://stackoverflow.com/a/29023433/517137

@pierre-alain-b
Copy link
Owner

Interesting, I will give it a try.

@aaronjlawson
Copy link

@pierre-alain-b I had read previously that this error that pops up is a false positive. Are we still under the assumption that the analysis prior is still true? Also, is there anywhere that you or anyone else have posted thorough instructions regarding how to fix this error with Apache? Thanks for your work on this.

@pierre-alain-b
Copy link
Owner

To my knowledge yes. The issue is that, rather than rejecting the test, Nextcloud is offering the login page so the Rainloop tools wrongly interpretes that the folder is accessible.

@leanbardelli
Copy link

In my case my server answer with a 304, but is still forbidden. Is a false positive also?

@paulcalabro
Copy link

paulcalabro commented Oct 24, 2019

@AaronADev

I was able to resolve this with Nginx by adding the following location block to my nginx.conf file:

location /apps/rainloop/app/data {
    deny all;
}

This results in a 403 HTTP status code instead of a 302 redirect to the login page, which the test was interpreting as a failure.

Screen Shot 2019-10-23 at 11 39 58 PM

That approach should work with Apache as well.

@pierre-alain-b
Copy link
Owner

Good, this is a nice fix as well.

@pinpox
Copy link

pinpox commented Jul 19, 2021

Issue is still not fixed with 926ae7a . The workaround using nginx works as expected though.

@pciavald
Copy link

pciavald commented Sep 9, 2021

Hi, I understand this is not actually a security issue, but i'm wondering if there's a way to block the data folder using Traefik as the reverse-proxy manager? My installation seems to only have a .htaccess in nextcloud's html folder, would there be something to insert here?

Thank you

@IanChok
Copy link

IanChok commented Dec 13, 2021

How can this be resolved using apache2? I've tried solutions discussed here but none seem to work.

@JordanDIG
Copy link

Issue is still not fixed with 926ae7a . The workaround using nginx works as expected though.

Yes, but we still have the warning...

@tabp0le
Copy link
Contributor

tabp0le commented Dec 31, 2021 via email

@JordanDIG
Copy link

You guys, it's a WARNING. Just get over it. It's only your OCD that's the issue.

On Thu, Dec 30, 2021 at 12:58 AM Jordan D. @.> wrote: Issue is still not fixed with 926ae7a <926ae7a> . The workaround using nginx works as expected though. Yes, but we still have the warning... — Reply to this email directly, view it on GitHub <#62 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABT3VMYW4FV6TLEFW53JD7TUTQGJDANCNFSM4FAYHSSA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you commented.Message ID: @.>

Hello,
Thank you for your reply, I have solved this problem by setting up a NGINX rule that blocks access to the directory.

@sgutermann
Copy link

How can this be resolved using apache2? I've tried solutions discussed here but none seem to work.

This is what I include in my NextCloud Apache Installations. Blocks the folder but does not get rid of the message:

<DirectoryMatch "^.*/rainloop/app/data/.*">
# Apache 2.2
<IfModule !authz_core_module>
Order Deny,Allow
Deny from all
</IfModule>
# Apache 2.4+
<IfModule authz_core_module>
Require all denied
</IfModule>
</DirectoryMatch>

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests