Bug 14564 - Incorrect permissions prevent web download of configuration backups
Summary: Incorrect permissions prevent web download of configuration backups
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: David Cook
QA Contact: Josef Moravec
URL:
Keywords:
: 22143 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-07-19 14:14 UTC by Mirko Tietgen
Modified: 2021-06-14 21:35 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.01,20.05.07,19.11.13


Attachments
Bug 14564 -- Export configuration: Unfortunately, no backups are available (1.24 KB, patch)
2015-07-19 15:23 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 14564: Set permissions on configuration backups (1.51 KB, patch)
2020-11-16 22:31 UTC, David Cook
Details | Diff | Splinter Review
Bug 14564: Set permissions on configuration backups (1.56 KB, patch)
2020-12-16 17:01 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14564: Set permissions on configuration backups (1.56 KB, patch)
2020-12-18 11:11 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mirko Tietgen 2015-07-19 14:14:54 UTC
With koha-conf.xml setting

<backup_db_via_tools>1</backup_db_via_tools> <backup_conf_via_tools>1</backup_conf_via_tools>

open Tools > Export data

Check tab »Export database«: available files appear.
Check tab »Export configuration«: »Unfortunately, no backups are available.«

Export configuration should show available files instead.
Comment 1 Mirko Tietgen 2015-07-19 14:52:35 UTC
Looks like a permission problem:

Backups of database files are owned by root:<koha-user>.
Backups of configuration files are owned by root:root
Comment 2 Mirko Tietgen 2015-07-19 15:23:09 UTC
Created attachment 41092 [details] [review]
Bug 14564 -- Export configuration: Unfortunately, no backups are available

Configuration files are not shown in the export data tool due to file
ownership/ permissions.

Test plan:

To reproduce:
- check file permissions on the server in /var/spool/koha/sitename/
  config files have root:root and 600
- try to use the export tool to download configuration files: they
  are not available

Then
- apply patch
- use koha-dump
- check file permissions for config files, should be root:$site-koha
  and 640 now
- try to use the export tool to download configuration files: they
  should be available for download now
Comment 3 Mirko Tietgen 2015-07-19 15:27:44 UTC
Not sure if this was intended for security reasons, but then we cannot offer to download the config files via staff client then.

Maybe this could also go to commandline tools, but the broken functionality is in tools.
Comment 4 Robin Sheat 2015-07-20 00:19:22 UTC
(In reply to Mirko Tietgen from comment #3)
> Not sure if this was intended for security reasons, but then we cannot offer
> to download the config files via staff client then.
> 
> Maybe this could also go to commandline tools, but the broken functionality
> is in tools.

I don't remember the reasoning behind this. I think it was that when we built it, it wasn't possible to download configuration.

I'm also not sure how I feel about downloading the config in general, it can have things like SSL keys and what not that probably should be protected more than usual. Bit I'm open to opinions.
Comment 5 Mirko Tietgen 2015-07-20 06:23:12 UTC
I think there is rarely use for a hosting client to download the configuration, you can't just use it on a different server.

On the other hand, it requires <backup_conf_via_tools>1</backup_conf_via_tools> in the config and is not available by default, so I'd say it is protected more than usual. There may be use cases for it.
Comment 6 Robin Sheat 2015-07-20 22:27:27 UTC
(In reply to Mirko Tietgen from comment #5)
> I think there is rarely use for a hosting client to download the
> configuration, you can't just use it on a different server.
> 
> On the other hand, it requires
> <backup_conf_via_tools>1</backup_conf_via_tools> in the config and is not
> available by default, so I'd say it is protected more than usual. There may
> be use cases for it.

Yeah, I think you're right. My thinking is that we shouldn't open up the permissions by default though as they do contain things that we'd not like people to see. On the other hand, if they get to a point where they can download that file, we probably have bigger problems anyway. But it still could contain SSL keys that they wouldn't otherwise have permissions to get.
Comment 7 Mirko Tietgen 2015-07-22 13:35:01 UTC
Maybe the question really is, what is the purpose of this.

Being able to download the data is very important and something we allow easily, others don't. It makes perfect sense to have in Koha.

Being able to download server specific configuration, while not being the administrator of that particular server… does that really help anyone, is it just something that looks »good« and »open« or is it just something that ended in the code analogue to the other part of the backup (the actual data)?

The longer I think about it, the more I lean towards keeping the backup function as it is and either taking away the »Export configuration« part or leaving it up to the admin to change the permissions if they really want to allow download. It should probably be documented then that setting backup_conf_via_tools is not enough though.
Comment 8 Robin Sheat 2015-07-22 23:04:10 UTC
I think that pretty much jibes with my opinion. No point removing it, people may have cause to use it, but documenting that it's going to be more than just flicking an option if you want to use it.
Comment 9 Mirko Tietgen 2015-07-23 07:57:17 UTC
Where should the additional information go? koha-conf.xml? It looks like the whole thing (export of db and conf) is not documented in the manual.
Comment 10 Robin Sheat 2015-07-24 01:27:09 UTC
(In reply to Mirko Tietgen from comment #9)
> Where should the additional information go? koha-conf.xml? It looks like the
> whole thing (export of db and conf) is not documented in the manual.

If we're really keen we could create a man page: koha-conf.xml(5) that describes what can go in there.
Comment 11 Magnus Enger 2019-01-17 08:36:33 UTC
*** Bug 22143 has been marked as a duplicate of this bug. ***
Comment 12 Tomáš Wünsch 2020-05-14 09:54:28 UTC
IMHO it would be at least mentioned at GUI (/cgi-bin/koha/tools/export.pl), next to "Unfortunately, no backups are available.".
Comment 13 David Cook 2020-11-16 22:18:22 UTC
I dislike the export configuration functionality, but I dislike even more the fact that it's current unusable.

Either we remove it or we fix it, and so far the consensus seems to be to keep it, since the default is to have it turned off.
Comment 14 David Cook 2020-11-16 22:22:12 UTC
Patch doesn't apply. 

I don't think Mirko is around anymore, so I'll take this one over.
Comment 15 David Cook 2020-11-16 22:31:30 UTC
Created attachment 113685 [details] [review]
Bug 14564: Set permissions on configuration backups

This patch sets the group and group read permissions on
configuration backups, so that they can be downloaded
via the web staff interface.

Test plan:
0. Apply the patch
1. cp debian/scripts/koha-dump /usr/sbin/koha-dump
2. koha-run-backups --days 2 --output /var/spool/koha
3. Note *.tar.gz backups in /var/spool/koha/kohadev
now have root:kohadev-koha ownership and 640 permissions
4. vi /etc/koha/sites/kohadev/koha-conf.xml
5. Set backup_db_via_tools and backup_conf_via_tools to 1
6. echo 'flush_all' | nc -q 1 memcached 1121
7. koha-plack --restart kohadev
8. Go to http://localhost:8081/cgi-bin/koha/tools/export.pl
9. Click on "Export configuration"
10. Choose a file
11. Click Download configuration
12. Note that *.tar.gz file downloaded correctly

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 16 Josef Moravec 2020-12-16 17:01:29 UTC
Created attachment 114454 [details] [review]
Bug 14564: Set permissions on configuration backups

This patch sets the group and group read permissions on
configuration backups, so that they can be downloaded
via the web staff interface.

Test plan:
0. Apply the patch
1. cp debian/scripts/koha-dump /usr/sbin/koha-dump
2. koha-run-backups --days 2 --output /var/spool/koha
3. Note *.tar.gz backups in /var/spool/koha/kohadev
now have root:kohadev-koha ownership and 640 permissions
4. vi /etc/koha/sites/kohadev/koha-conf.xml
5. Set backup_db_via_tools and backup_conf_via_tools to 1
6. echo 'flush_all' | nc -q 1 memcached 1121
7. koha-plack --restart kohadev
8. Go to http://localhost:8081/cgi-bin/koha/tools/export.pl
9. Click on "Export configuration"
10. Choose a file
11. Click Download configuration
12. Note that *.tar.gz file downloaded correctly

Signed-off-by: David Cook <dcook@prosentient.com.au>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 17 Josef Moravec 2020-12-16 17:03:03 UTC
This is definitely a bug. Now at least both backup files have same permissions which is good.

Passing QA
Comment 18 Jonathan Druart 2020-12-18 11:11:00 UTC
Created attachment 114519 [details] [review]
Bug 14564: Set permissions on configuration backups

This patch sets the group and group read permissions on
configuration backups, so that they can be downloaded
via the web staff interface.

Test plan:
0. Apply the patch
1. cp debian/scripts/koha-dump /usr/sbin/koha-dump
2. koha-run-backups --days 2 --output /var/spool/koha
3. Note *.tar.gz backups in /var/spool/koha/kohadev
now have root:kohadev-koha ownership and 640 permissions
4. vi /etc/koha/sites/kohadev/koha-conf.xml
5. Set backup_db_via_tools and backup_conf_via_tools to 1
6. echo 'flush_all' | nc -q 1 memcached 1121
7. koha-plack --restart kohadev
8. Go to http://localhost:8081/cgi-bin/koha/tools/export.pl
9. Click on "Export configuration"
10. Choose a file
11. Click Download configuration
12. Note that *.tar.gz file downloaded correctly

Signed-off-by: David Cook <dcook@prosentient.com.au>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 19 Jonathan Druart 2020-12-18 11:11:17 UTC
Adjusted commit date.
Comment 20 Jonathan Druart 2020-12-18 11:23:39 UTC
Shouldn't it be $name-koha:$name-koha?
Comment 21 Tomás Cohen Arazi 2020-12-18 11:34:18 UTC
(In reply to Jonathan Druart from comment #20)
> Shouldn't it be $name-koha:$name-koha?

Backups are a sysadmin thing, and there shouldn't be permissions allowing the user to delete them. St first glance. I agree with the proposed root:$name-koha and g+r combination.
Comment 22 Jonathan Druart 2020-12-21 09:19:56 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 23 Fridolin Somers 2020-12-28 14:39:37 UTC
Pushed to 20.11.x for 20.11.01
Comment 24 Andrew Fuerste-Henry 2020-12-28 21:36:17 UTC
Pushed to 20.05.x for 20.05.07
Comment 25 Victor Grousset/tuxayo 2020-12-30 14:30:29 UTC
Backported to 19.11.x branch for 19.11.13