Bug 33241 - UMC runs with umask 0077
UMC runs with umask 0077
Status: CLOSED WONTFIX
Product: UCS
Classification: Unclassified
Component: UMC (Generic)
UCS 3.2
Other Linux
: P2 normal (vote)
: UCS 4.0
Assigned To: Dirk Wiesenthal
Stefan Gohmann
: interim-3
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-11-11 13:02 CET by Arvid Requate
Modified: 2014-11-26 06:54 CET (History)
5 users (show)

See Also:
What kind of report is it?: ---
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Arvid Requate univentionstaff 2013-11-11 13:02:21 CET
In one test environment the update to UCS 3.2 failed running 35univention-management-console-module-appcenter.inst with the following error message:

============================================================================
Waiting for activation of the extension object univention-app:.......................................................ERROR: Master did not mark the extension object active within 180 seconds.
ERROR
ucs_registerLDAPExtension: registraton of /usr/share/univention-management-console-module-appcenter/univention-app.schema failed.
============================================================================

The listener.log shows that the listener (or one of the modules) is not able to open base.conf:

============================================================================
root@ucs-master:~# /usr/sbin/univention-directory-listener -F \
   -b dc=samba4,dc=ucs,dc=demo \
   -m /usr/lib/univention-directory-listener/system \
   -c /var/lib/univention-directory-listener \
   -d 4 -x -ZZ -D cn=admin,dc=samba4,dc=ucs,dc=demo -y /etc/ldap.secret
11.11.13 13:00:01.586  DEBUG_INIT
11.11.13 13:00:01.587  CONFIG      ( ERROR   ) : Error on opening "/etc/univention/base.conf"
11.11.13 13:00:01.587  CONFIG      ( ERROR   ) : Error on opening "/etc/univention/base.conf"
11.11.13 13:00:01.588  CONFIG      ( ERROR   ) : Error on opening "/etc/univention/base.conf"
11.11.13 13:00:01.588  CONFIG      ( ERROR   ) : Error on opening "/etc/univention/base.conf"
Speicherzugriffsfehler
=============================================================================
Comment 1 Arvid Requate univentionstaff 2013-11-11 13:07:34 CET
strace shows:

open("/etc/univention/base.conf", O_RDONLY) = -1 EACCES (Permission denied)


The permissions of base.conf are too strict, maybe the umask of some process is wrong?

root@ucs-master:~# ls -l /etc/univention/base.conf
-rw------- 1 root root 28590 11. Nov 12:58 /etc/univention/base.conf
Comment 2 Philipp Hahn univentionstaff 2013-11-11 13:10:56 CET
Perhaps related:
The UMC-Server runs with umask 0077, see Bug #33157 for a similar issue.
Comment 3 Ingo Steuwer univentionstaff 2013-11-11 13:28:55 CET
mhm, I changed an UCR variable using the UMC UCR module during the update progress (started also in UMC).
Comment 4 Stefan Gohmann univentionstaff 2013-11-21 15:38:43 CET
We should re-check the umask handling of UMC.
Comment 5 Alexander Kläser univentionstaff 2013-11-27 11:28:13 CET
(In reply to Ingo Steuwer from comment #3)
> mhm, I changed an UCR variable using the UMC UCR module during the update
> progress (started also in UMC).

Yes, this is possible. In which way do you think that this is a problem?
Comment 6 Ingo Steuwer univentionstaff 2013-11-27 11:48:41 CET
(In reply to Alexander Kläser from comment #5)
> (In reply to Ingo Steuwer from comment #3)
> > mhm, I changed an UCR variable using the UMC UCR module during the update
> > progress (started also in UMC).
> 
> Yes, this is possible. In which way do you think that this is a problem?

My guess is that the change triggered the wrong file permissions of /etc/univention/base.conf

This might already be fixed, as the machine hadn't the latest UCS 3.1 Erratas.
Comment 7 Stefan Gohmann univentionstaff 2013-11-27 13:26:30 CET
We had a similar problem: Bug #33157.
Comment 8 Dirk Wiesenthal univentionstaff 2013-12-03 11:15:37 CET
I can reproduce the error from Comment 1 with Comment 6 and some preparation (bad timing?):

Log into UMC
Open UCR module
Open a variable's dialog (do not save yet)
mv /etc/univention/base* ~
Save the variable
ls -l /etc/univention/base.conf
-rw------- root root base.conf

But this seems unlikely to happen in the real world.
Comment 9 Dirk Wiesenthal univentionstaff 2013-12-03 11:26:23 CET
Explicitly setting umask to 0077 may cause problems in some situations.

Question is: Why was it done this way in the first place? Not creating a world readable secret file by accident seems to be a good idea.
I am not so sure whether we can easily switch to 0022 in an errata update. Some functions may rely on the fact that only root may read the files created. (/etc/$service.secret? /var/cache/univention-management-console/acls/?). 0022 will certainly not break anything, but maybe expose some files.

Maybe we can protect certain directories manually, e.g. /var/tmp/univention-management-console-frontend/ (where all the upload goes).
Comment 10 Philipp Hahn univentionstaff 2013-12-03 12:28:19 CET
(In reply to Dirk Wiesenthal from comment #9)
> Explicitly setting umask to 0077 may cause problems in some situations.

In general daemons are advised to set umask(0), just to guarantee deterministic behavior. See <http://www-theorie.physik.unizh.ch/~dpotter/howto/daemonize>.
Here's some more background: <http://stackoverflow.com/questions/7141793/file-creation-mode-mask>

> Question is: Why was it done this way in the first place? Not creating a
> world readable secret file by accident seems to be a good idea.

See Bug #25162 AKA r29782.
Comment 11 Dirk Wiesenthal univentionstaff 2013-12-03 15:08:26 CET
umask was set for Bug #25162

If we fix the issues mentioned there, maybe we can change the umask back to 0?

Issues were:
  * some files in /var/run/ (e.g. umc-server.pid.lock)
  * acl file
  * log files (presumably already resolved, but should be double checked)

As I said before, check the uploaded files. Maybe /var/cache/u-m-c, too (out of paranoia. Currently used for App Center ini files and acls files, where the App Center is not ciritical and the acls may be handled separately).

Searching for (writable) file handles in the code is difficult. One should grep at least for "with open" ("open" would be better but yields a lot of udm noise) and "shutil" and "os" (for os.rename, os.mkdir, etc)
Comment 12 Florian Best univentionstaff 2013-12-03 15:38:33 CET
(In reply to Dirk Wiesenthal from comment #11)
> Searching for (writable) file handles in the code is difficult. One should
> grep at least for "with open" ("open" would be better but yields a lot of
> udm noise) and "shutil" and "os" (for os.rename, os.mkdir, etc)
open([^)], fd, fn, file
Comment 13 Dirk Wiesenthal univentionstaff 2013-12-04 15:59:25 CET
The profile script of system setup should be protected
Comment 14 Dirk Wiesenthal univentionstaff 2013-12-04 22:16:30 CET
There are two options:

1. Fix it now, fix it forever: Change umask from 0077 to 0000 or 0022. This would solve all problems regarding file permissions being too restrictive. All spawned processes would have reasonable defaults for new files.
Problem: Backward compatibility; we will have problems finding all files that may require rw-------. We will not see any problems (everything that worked before will work afterwards, i.e. all existent tests will most probably run successfully, they are more or less useless here), we really need to be mindful with every file created in any directory by any UMC module: Is is okay, that this file is world readable?!

2. Fix it now, be backwards compatible: This could be done by adding UCR variables for the umask. Set it in postinst to 0077 for existing umc-server (upgrade) and to 0022 for new installations.
Problem: Same permission problems as in (1), plus: We will have from now on two divergent UMC versions with security problems in one group and not services refusing to start in the other. And at some point all Univention Developers will have their systems reinstalled and only test with 0022 while there are many 0077 systems out there.



This bug really needs to be fixed, we cannot leave it this way. univention-run-join-scripts runs with a different umask as UMC/Join. univention-add-app runs with different umask as UMC/App Center.

But changing it now is too dangerous. We need the whole Product Tests, we need some time to find files that need proper protection.

The most important things seem to be resolved by workarounds by now. Let's hope that no new problems arise during UCS 3.2 lifetime and wait for the next major release.
Comment 15 Dirk Wiesenthal univentionstaff 2014-10-17 15:25:08 CEST
Removed the umask in univention-management-console-server scripts. UCR problem is gone.

I have looked at some files created by UMC and the sensitive ones (ACLs, profile, uploads, socket, even logs) are all protected.

RESOLVED for now.
Comment 16 Stefan Gohmann univentionstaff 2014-10-18 22:57:07 CEST
The Jenkins check permissions check fails since a few days. Maybe this change is the cause:

 ***Searching for files and directories with write permissions for other:
 drwxrwxrwx 2 root root 40 Okt 18 00:53 /run/univention-management-console

See 
http://jenkins.knut.univention.de:8080/job/UCS-4.0/job/UCS-4.0-0/job/Autotest%20MultiEnv/34/SambaVersion=s4,Systemrolle=master/testReport/junit/00_base/25check-permissions/test/
Comment 17 Florian Best univentionstaff 2014-10-20 13:18:18 CEST
ls -l /var/www/univention-directory-reports/univention-directory-reports-970utE.csv 
-rw-r--r-- 1 www-data www-data 934 Oct 20 13:15 /var/www/univention-directory-reports/univention-directory-reports-970utE.csv

ls -l /var/log/univention/directory-*cleanup*log
-rw-r--r-- 1 root root 0 Aug 27 06:00 /var/log/univention/directory-reports-cleanup.log
Comment 18 Dirk Wiesenthal univentionstaff 2014-10-22 15:07:52 CEST
As discussed: Too many possible side effects regarding security. Better fix modules that are buggy with this custom umask separately.

Fixes reverted.
Comment 19 Dirk Wiesenthal univentionstaff 2014-10-22 15:15:49 CEST
I have added Bug#36271 and Bug#36270 for test cases regarding the two issues related to this bug: Files created during UCR updates and files created during join.
Comment 20 Stefan Gohmann univentionstaff 2014-11-17 09:15:07 CET
OK, commits have been reverted.
Comment 21 Stefan Gohmann univentionstaff 2014-11-26 06:54:41 CET
UCS 4.0-0 has been released:
 http://docs.univention.de/release-notes-4.0-0-en.html
 http://docs.univention.de/release-notes-4.0-0-de.html

If this error occurs again, please use "Clone This Bug".