dbus-daemon changes its umask to 0022. gnome-settings-daemon and Firefox processes that I start with the /apps/gnome_settings_daemon/keybindings/www shortcut key are inheriting this umask, which makes Firefox save downloaded files with the wrong permissions.
I think dbus-daemon should not change its umask.
Created attachment 20155 [details] [review]
Created attachment 20156 [details] [review]
Oops, correct bug number in patch
If applying this, it's not necessary to leave an archeology comment saying what code used to be there, just remove the old code with a ChangeLog entry.
That said, this is a potentially dangerous change, and would be ill-advised without at minimum reviewing all places where dbus creates files, and evaluating how this could impact them. Generally daemons set umask to avoid surprises, because umask is well-known to cause surprises. Most daemons do this.
A safer change might be to record the old umask (returned from the umask call), and when we spawn child processes, set the old umask in the child. That way dbus-daemon itself is never affected by the system environment and always gets the permissions it expects.
(In reply to comment #3)
> That said, this is a potentially dangerous change, and would be ill-advised
> without at minimum reviewing all places where dbus creates files, and
> evaluating how this could impact them. Generally daemons set umask to avoid
> surprises, because umask is well-known to cause surprises. Most daemons do
I was just thinking about the per-user daemon and had forgotten that there's a system daemon too. In my view, setting the umask makes perfect sense for the system daemon since it is in control of the security model and knows who it wants to be able to access its files. On the other hand, I don't see why a per-user daemon should override the umask that I have established for all the other processes in my session.
I ran the following to find all the places dbus creates files:
git grep -A 1 -E '\<(O_CREAT\>|(mkdir|symlink|mknod|bind) *\()'
And it appears that there is no created file whose permissions would become looser as a result of not setting the umask. I'm guessing that permissions becoming tighter is not a problem because the daemon is single-user.
> A safer change might be to record the old umask (returned from the umask call),
> and when we spawn child processes, set the old umask in the child. That way
> dbus-daemon itself is never affected by the system environment and always gets
> the permissions it expects.
That would also fix my problem, but please consider whether it would not make sense to drop the umask setting altogether for a per-user daemon.
Agreed, could make sense to drop it for the session daemon. To do this, we'd have to add an option to the config file like <keep_umask> and set it in session.conf.
I was about to hard-code dropping the umask for a session daemon but not a system daemon. I will implement the config option instead.
Created attachment 20160 [details] [review]
Revised patch with keep_umask option
That was an awful lot of work to add one boolean!
Patch looks great to me. Can someone merge this? (I think I still have ssh key problems with freedesktop.org git)
Author: Matt McCutchen <email@example.com>
Date: Mon Nov 10 08:55:27 2008 -0500
Bug 18446: Keep umask for session bus
Signed-off-by: Colin Walters <firstname.lastname@example.org>