Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

securing log file locations #8851

Closed
iseries opened this issue Jan 9, 2023 · 34 comments
Closed

securing log file locations #8851

iseries opened this issue Jan 9, 2023 · 34 comments

Comments

@iseries
Copy link

iseries commented Jan 9, 2023

Good day!

I am working as a security researcher. I found during a review of web servers of German municipalities and authorities that many Roundcube instances in production are not configured correctly. Also a recent investigation of German universities showed the same picture. I was able to view log files and temporary email attachments on over 30 instances.

The data that was drained during my investigation amounted to several GB. Among them were ID cards, passports, medical cards, various applications, birth certificates, and sick notes.

In other words, highly sensitive personal data.

Of course, the responsibility for a correct configuration lies with the operator of the application. After exchanging information with the CCC and various data protection authorities, I would like to point out this issue and suggest that it would make sense to change the software in future Roundcube versions so that log files and temporary attachments are stored outside the documentRoot by default. This would not only contribute to the security of data, but also strengthen the Roundcube software in general in its confidentiality.

Best regards,
Rene Rehme

@iseries iseries changed the title Confidentiality of the roundcube software securing log file locations Jan 9, 2023
@ghost
Copy link

ghost commented Jan 17, 2023

So in other words, you have access to their roundcube /logs and /temp directories? If that is so, it implies one of the following:

  • the .htaccess files have been removed
  • AllowOverride is set to none (or not enough permissions)
  • a required Apache module is not loaded
  • they don't use Apache, thus there is no support for .htaccess files

I would guess, that it is the administrators responsibility to install roundcube correctly and if they don't use .htaccess files then they should take the appropriate action to block access to /logs and /temp directories.

Your suggestion to store these things out of the public_html directory is reasonable, but it could complicate roundcube's installation?

@alecpl
Copy link
Member

alecpl commented Jan 17, 2023

There's public_html folder in Roundcube for a reason. We do not force users to use it. I'm not sure we should (e.g. by providing index.php file under public_html only). But we recommend to use it in the INSTALL file (SECURE YOUR INSTALLATION section).

@alecpl alecpl closed this as completed Jan 17, 2023
@iseries
Copy link
Author

iseries commented Jan 20, 2023

There's public_html folder in Roundcube for a reason. We do not force users to use it. I'm not sure we should (e.g. by providing index.php file under public_html only). But we recommend to use it in the INSTALL file (SECURE YOUR INSTALLATION section).

Sorry to bring this up again. The question is: Why should you not provide an index.php file under public_html only?

Setting up a virtual host that points to a public_html directory to ensure that all other directories are not accessible via the internet should simply be standard and would be a huge contribution to security. This is standard with every common php framework.

If you really can't see that this is currently a critical thing, then have a look on google, check the first 50 pages and think about it again. Here is an excerpt from my list of "misconfigured" Roundcube instances.
image

Best

@ghost
Copy link

ghost commented Jan 20, 2023

So all those websites had the .htaccess file removed or made inoperable in some way.

I don't see how this is roundcube's fault.

@iseries
Copy link
Author

iseries commented Jan 20, 2023

Stop. It is not about who is to blame. It's about modifying software so that a standard installation guarantees maximum security. As you can easily see, it is a problem if you only give "additional" installation hints how to do a secure installation. It should be the standard from the beginning. Especially when it comes to sensitive files being processed.

@ghost
Copy link

ghost commented Jan 20, 2023

Sure I agree, which is why roundcube comes prepared with .htaccess files to block all requests under /temp and /logs.

I did some research and I found many roundcube installations that allow access to all subdirectories like logs and temp, in addition to that, these websites are very badly configured and allow directory listing by default, thus I can see the complete contents of those directories!

@beckerr-rzht
Copy link

I think this is not about why these mistakes are made.
The question is rather: Why is it possible to make these mistakes?

I see the folder structure used as the cause here.
Why are $RC/config, $RC/log, $RC/vendor etc. in the DocumentRoot by default?

Using .htaccess as the only protection can be problematic. For example, the file $RC/temp/.htaccess could be deleted from the web server because it or at least $RC/temp belong to the user www-data.

For self-testing: dirb

For me it was an eye opener to scan the web server via dirb with a custom word file.

You may try it yourself:

find /var/www/html -printf "%f\n" | sort -u >/tmp/rc-words.txt
dirb https://example.com/roundcube /tmp/rc-words.txt -X ",.orig,.rej,.bak,~,.n,.o,.old,.off,.save"

Where /var/www/html and https://example.com/roundcube must of course be adapted to your own environment.
It might also make sense to temporarily switch off the Apache log:

CustomLog /dev/null vhost_combined

@ghost
Copy link

ghost commented Jan 20, 2023

I don't see any difference with other open source projects, Wordpress has its config file in the DocumentRoot, along with all the admin files.

I run dirb the way you run it, and all it found was publicly accessible files, nothing of interest or of value. Same thing that would happen if I run it on a Wordpress installation, theme files, CSS, JS, etc.

Again, all I see, is some idiot administrator who does not understand what .htaccess files are all about. So the question is, does Roundcube want/care to protect those idiots? Or should be move on to more important issues, like releasing version 1.6.1? :D

@ghost
Copy link

ghost commented Jan 20, 2023

You are dancing around the issue:

Those admins removed or disabled .htaccess files...

My comparison is sound, you can butcher a Wordpress installation and make it insecure, is Wordpress at fault because of the idiot admin?

@iseries
Copy link
Author

iseries commented Jan 20, 2023

I made my point.

@haslersn
Copy link

haslersn commented Jan 24, 2023

I think the roundcube devs should reconsider their opinion about secure defaults. This issue should be reopened.

@david-sawatzke
Copy link

david-sawatzke commented Jan 24, 2023

The ./INSTALL file mentions mod_rewrite, but https://github.com/roundcube/roundcubemail/wiki/Installation#protect-your-installation doesn't, leading to some people not enabling it.

If it isn't enabled, directory listing is disabled but log access isn't and thus cursory checks if this issue is fixed don't show any problem.

This isn't your fault, but the roundcube-core package from debian buster doesn't include the public_html folder: https://packages.debian.org/buster/all/roundcube-core/filelist, leading to further confusion (This isn't the case anymore in bullseye, thankfully).

@thomascube
Copy link
Member

The folder structure of Roundcube is still the same as in the beginning of the project 15 years ago where the goal was to make it as easy as possible to install Roundcube by simply unzipping and uploading it to a web server via FTP. This was one factor that made it popular. Of course times have changed with great popularity also comes responsibility.

I agree to the concerns of @iseries et al that defaults matter. Personally I think we as the developers of a software package should do our best "so that a standard installation guarantees maximum security". Removing the index.php in the root folder and updating the documentation may already help a lot and should be a simple task. But @alecpl is in charge of technical decisions and I leave it up to him to re-open this issue.

@alecpl
Copy link
Member

alecpl commented Jan 29, 2023

If others are ok with this switch, I'll be too. However, I foresee some problems with these links in public_html directory. I remember someone saying that on windows they might not be created when you unpack the release package. This could be documented, but I wonder what other issues we can have.

We can try this in master.

@alecpl alecpl added this to the 1.7-beta milestone Jan 29, 2023
@alecpl alecpl reopened this Jan 29, 2023
@iseries
Copy link
Author

iseries commented Feb 2, 2023

@alecpl That is a justified counter-argument. Of course, the system should run without errors - even under Windows. Would it perhaps be a useful workaround to simply add a simple check that checks the sensitive directories for accessibility?

@alecpl
Copy link
Member

alecpl commented Feb 2, 2023

How would you check that? I don't think you can do that with a CLI script. And if that would be a CLI tool, you assume users will use it, which I doubt.

Another argument: access to the Installer. It would have to be a link too. Then probably the .htaccess file should be moved (the link replaced).

Thinking about these links. Maybe we should just move these directories there, instead of using links. But then it becomes more complicated, and migration probably would not be easy (not mentioning switching between older releases for development purposes).

@iseries
Copy link
Author

iseries commented Feb 2, 2023

I thought about a check in application's runtime like the garbage collector is done. I thought about something like this:

$headers = @get_headers($tempDirectory);
if($headers[0] === 'HTTP/1.1 401 Unauthorized' || $headers[0] === 'HTTP/1.0 401 Unauthorized') {
    // protected
}

Maybe also check to read an index.php file in the directory, too.
How exactly this can then really look in the end should of course be carefully considered again.

@ner00
Copy link

ner00 commented Feb 20, 2023

From a Windows stand-point, I'm curious how this would work without needing additional mandatory configurations by hand. For one, Unix-like symlinks won't work under Windows, they can't even be extracted from a release archive due to the different implementation of soft-links between Windows and *nix.

Currently I have Apache properly configured and .htaccess protects the resources. Installations, upgrades and migrations are a breeze - even cross-platform; but I see how that might be a problem if the target webserver is mis-configured and unsecured, which then shifts the onus of security entirely onto software developers, as is now the case.

I took a quick look at Squirrel mail and I personally wouldn't use that as an example, although it validates the original point. For example, logging is not enabled by default. It's not that it's not enabled, it's more like it literally does not exist in the core, you must install a 10 year old third-party plugin, so that's a win, I guess. The documentation on this portion is also outstanding, judging by the volume of questions on the matter over at StackExchange, probably as outstanding as the outdated release packages.

So this topic is of interest, I'm curious how much complexity and side-steps need to be introduced to cater to the fallibility of a few server admins. Or, contrary to my assumption, could turn out to be very easy to implement and maintain, which I admit would be the ideal outcome of this issue.

@ghost
Copy link

ghost commented Feb 20, 2023

In my humble opinion, if the issue is with the logs and temp directories, then those two can be easily moved out of the public_html by just using sane defaults, like PHP's /tmp directory. Thus, we avoid any .htaccess problems and misconfigured web servers.

For example, PHP function sys_get_temp_dir returns the actual PHP temporary location.

@ner00
Copy link

ner00 commented Feb 20, 2023

Sure, no problem.
Although an option to keep the current folder layout would be nice, I personally don't feel like keeping those folders in an external location if I know that the server is properly configured. And if by chance www-data is compromised, I have a bigger problem on my hands.

@ozgurkazancci
Copy link

ozgurkazancci commented Jul 26, 2023

The documentation was/is already clear enough.

@iseries I truly see your good point, however, web/server administrators should know what they install, how they install, what to protect and how to protect.
They shouldn't be idiots. Especially the government ones -> too much critical data.

Just my two cents.

@Neustradamus
Copy link

To follow this ticket.

@pabzm
Copy link
Member

pabzm commented Dec 10, 2024

I've written a small script, that takes a base-URL/hostname as argument (because Roundcubemail is – AFAIK – agnostic to the hostname it's running under, and in a CLI environment we don't have HTTP headers to read it from) and checks whether it can read a created file via HTTP for both, temp_dir and log_dir.

I would suggest to integrate this script into update.sh and release that for at least the tracks v1.6 and v1.5.

This is not an alternative to reducing the number of files that lie in the docroot but an additional means, targetting old installations (that probably wouldn't like breaking change releases that move files around).

@alecpl Do you hvae any thoughts on this?

@alecpl
Copy link
Member

alecpl commented Dec 10, 2024

How do you integrate it if it requires an URL on input? Users would have to give the URL as argument, so it might not be a good change for stable release lines. The script has some issues too, it's not PHP7 compatible, and I'd not use emoji in console output. But in general terms, you assume the URL is accessible from localhost, which might not be the case.

If we just provide the script (what we can always do), how do you tell users to use it?

On the other hand next major version should include my #9294 solution (hopefully I can finish it soon) where it will not be an issue anymore.

@pabzm
Copy link
Member

pabzm commented Dec 10, 2024

How do you integrate it

I would call the script from update.sh – which admins are told be execute after an update.

it might not be a good change for stable release lines

Why do you think that? It's just one interaction during the update-process for admins (users don't have to do anything), I wouldn't classify that as a breaking change.

next major version should include my #9294 solution

Your solution is useful for future releases, but it is not helping the many installations out there that still run on v1.5, or want to stay on v1.6 for longer.

it's not PHP7 compatible

I'll see to that, thank you for the hint.

you assume the URL is accessible from localhost, which might not be the case

I'll extend the script so that it puts out instructions how to check the URL from a remote computer in case of network failures 👍

@alecpl
Copy link
Member

alecpl commented Dec 10, 2024

it might not be a good change for stable release lines

Why do you think that? It's just one interaction during the update-process for admins (users don't have to do anything), I wouldn't classify that as a breaking change.

By users I meant admins, of course. If you make the URL an optional argument then ok, but if it becomes a required argument, you change the way they used update.sh before. That kind of change is better to not be introduced in a minor release. So, I don't mind as long as it's optional, but if it's optional will it help much? I guess most users will not even notice it (and not everyone is using update.sh).

@pabzm
Copy link
Member

pabzm commented Dec 10, 2024

The URL is a required argument because there's no other way to know the URL/hostname – or is there?

not everyone is using update.sh

I just want to reach as many admins as possible, even if we can't reach every one of them.

Do you maybe have an alternative suggestion? I think it's our (Roundcube's) responsibility to act soon, and provide admins with a simple tool to check if they're vulnerable.

@alecpl
Copy link
Member

alecpl commented Dec 10, 2024

Alternative is to make it optional. And of course document it. Documentation is clear about securing an installation, though. Users just ignore it. So, the only sensible solution is to force them to use public_html, but this can be done in a major release only.

@alecpl
Copy link
Member

alecpl commented Dec 10, 2024

BTW, I don't share the responsibility to act soon regarding this issue, it's not new.

@pabzm
Copy link
Member

pabzm commented Dec 10, 2024

Alternative is to make it optional

I just realised that I hadn't committed and pushed the latest changes to that script. Now it asks interactively for a base URL if none is specified. Would that maybe be more acceptable to you to be run from update.sh?

@ner00
Copy link

ner00 commented Dec 10, 2024

I'm probably off-topic here... I don't use a Linux environment, so I never use update.sh at all, but I'm aware that I'm a minority in that regard. I use my custom batch script with rsync to upgrade versions (+ php installer script to finish). I think #9294 is the way to go in general, especially a major release. I'll also have to adapt my script to it, but once that's done I won't need to jump around for between the best methods, which can potentially only target Linux systems that run bash. So, I guess that what I'm saying is that we should strictly stick to one solution that becomes the norm from there on.

BTW, I don't share the responsibility to act soon regarding this issue, it's not new.

Literally not new at all, no rush required.

@Tragen
Copy link

Tragen commented Dec 10, 2024

Same here, I don't use Linux. Would you share your batch script?

@ner00
Copy link

ner00 commented Dec 10, 2024

Same here, I don't use Linux. Would you share your batch script?

I won't clutter this issue much, and I'm not proud of what I'm sharing, be sure to look at the script in detail to know what you need or don't need (especially the "custom stuff", plugins, etc). Essentially you'll have an UPGRADE.bat script in the root folder of Roundcube, which is roughly this: https://gist.github.com/ner00/bb637aed77af1e1b630a1e7403eef61d

You'll also have another batch script rsync.bat in the root folder of Roundcube, with this single line:
@"C:\rsync\usr\bin\rsync.exe" %*

The previous script will point to the location of the actual rsync binary, which for me is in c:\rsync\, there is a zip attached here:
rsync_win.zip

P.S: Probably some binaries/dlls are not up to date, but very close to the most recent ones from https://repo.msys2.org/msys/x86_64/.

P.S.2: UPGRADE.bat derives some variables, namely date/time, using the assumption of a certain regional configuration, at the very least, you might need to tweak that to your own regional configuration to get the date right.

@alecpl
Copy link
Member

alecpl commented Feb 9, 2025

Use of public_html is now required. Closing.

@alecpl alecpl closed this as completed Feb 9, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests