On Mon, 2010-01-11 at 18:48 +0100, Karel Klic wrote:
here is a patch that fixes /var/cache/abrt/* permissions by allowing
users to read, but not to change their crash data. It adds abrt user,
changes abrt-hook-python to use suid instead of sgid bit (uid=abrt),
sets /var/cache/abrt and every dump subdirectory to be owned by abrt
user. Read access for users and their own crashes is provided by group
(/var/cache/abrt/ccpp-xxxx-xx has user's group).
Yes, I was thinking about it and having abrt user seems to be a workable
Current git version is broken. We create a debug dump directory
write access for user, and applications run by that user
(abrt-python-hook) cannot create files in that directory, even when they
run with the right group (=abrt group, which has the write access)
Furthermore, user can chmod directories he owns, so he can add write
access himself. So abrt group is useless in this case.
(I am a bit surprised that process with uid:gid 111:222 cannot create
new files in a directory owned by 111:222 with mode r-xrwx---)
The only problem with this patch is that it assumes that
groups" are used on the system. That is, a user "karel" belongs to group
"karel", and not to a common group "users". I do not know whether
is true for all desktop and server deployments.
Yes, unix permission model does not allow us to give two _uids_
read access to files, thus if dump files are owned by user "abrt",
there is no way to open them for reading for user "ufoo" too,
we can only open it for reading for his group "gfoo". I think this is
good enough for now.
(If all users share the same group, they will also be able to read
user's crashes, and we do not want that.)
We may try to use ACL on filesystems which have them. Lets leave it as a
An alternative to this patch is setting the option MakeCompatCore
default and make everything in /var/cache/abrt/ readable only by ABRT,
as it was proposed by Jiri
Well, we want to run gdb etc on the corefile, and we do not trust gdb
enough to run it under root. Thus we have to make at least corefile
readable by processes running under user's uid:gid.
+ /* Was used to override umask, which might remove write access to group.
+ * but currently we do not need it.
+ if (chmod(m_sDebugDumpDir.c_str(), 0750) == -1)
Why "was"? Perhaps "mkdir's mode (above) can be affected by umask, fix
- throw CABRTException(EXCEP_DD_SAVE, "Can't open file '%s'",
+ throw CABRTException(EXCEP_DD_SAVE, "Can't open file '%s':
%s", pPath, errno ? strerror(errno) : "errno == 0");
Lets keep indentation consistent at least within the same file.
(Wwe already have different styles across files, but at least
within one file it is so far consistent)
Apart from these nits, patch looks ok. Please apply.