Bug 45656

Sorry, for now it's rather up to ALT package differences: http://git.altlinux.org/people/sbolshakov/packages/?p=pulseaudio.git;a=commitdiff;h=b83171ee668c832a750c96b77866827fa9e43130;hp=3cea9f3032a4ae65030e74c4ffdf1547e3f3a2d3

Although it would be nice to check if the directory's fine before rushing to craft it into shape.

For the record, ALT-related fix was: http://git.altlinux.org/people/sbolshakov/packages/?p=pulseaudio.git;a=commitdiff;h=97559e61b8223f7f9a4f79ca9d74bfefd6525dbd

This should be fairly easy to catch for 2.0

OK, so I've just tested this.

I started "pulseaudio --system" as root and it correctly dropped privs to the pulse user.

The runtime dir was set to /var/run/pulse which I had pre-created to be owned by pulse.pulse. Likewise the pulse users homedir was set to be /var/lib/pulse which was also the state dir as per compilation.

When the user was added, the dir had these permissions:
drwxr-xr-x 5 pulse pulse 4096 Apr 10 10:47 /var/lib/pulse/


After launching PA, it changed to these permissions:
drwx------ 5 pulse pulse 4096 Apr 10 10:48 /var/lib/pulse/


The runtime dir was also handled fine. PA ensured the permissions were:
drwxr-xr-x 2 pulse pulse 4096 Apr 10 10:47 /var/run/pulse/


Where I could replicate failure is when /var was a read-only filesystem.

Now this is, in itself, not really something that should be supported. The whole point of /var is that it is variable. It does not make sense to mount /var read-only. /usr yes, but not /var:

/var/lib

    Files that change while the system is running normally.

So I'm not sure we should go out of our way to support this. The runtime dir is even more variable, as /var/run is used for multiple applications to record transient state. These days it is a symlink to /run which is mounted in tmpfs.


That said, I agree that we could try harder to not fail when the perms are correct to begin with. i.e. do a check first and miss out the chown/chmods/mkdir if all is correct already.

I'd happily take a patch for that, but the one on arch is incorrect in this regard. We should still insist on the known-good perms, we just shouldn't fail if they are already like that and we cannot call mkdir.

Anyway, this is a very specific use case (the readonly /var - NFS root systems should use e.g. tmpfs, aufs or unionfs to make these directories r/w even if the changes are ultimately lost - there are various scripts to do this on Fedora and Mageia etc - Personally I run just such a setup for my own media centre), and as such it's something I'm not going to be able to personally look at for v2.0. If someone wants to provide a good patch, I'll happily merge it. Removing from 2.0 blockers.

Hi,

I'm piggybacking on this issue to comment that pa_make_secure_dir still doesn't handle the pulse dir being a symlink (as of 5.0). I tested this and pulseaudio errored with "too many levels of symbolic links" when ~/.pulse is a symlink.

Created attachment 97826 [details] [review]
Proposed (partial) fix to chown problem


The attached patch only attempts to issue the chown if the target does not have the desired uid or gid.

It fixes the 'Failed to create secure directory' error I was receiving on my NFS mounted home directory.

Please test it out.

(In reply to comment #6)
> Created attachment 97826 [details] [review] [review]
> Proposed (partial) fix to chown problem
> 
> 
> The attached patch only attempts to issue the chown if the target does not
> have the desired uid or gid.
> 
> It fixes the 'Failed to create secure directory' error I was receiving on my
> NFS mounted home directory.
> 
> Please test it out.

Patch applied: http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=5610d41482df995baaf510308e07ccbe04c9e18b

this should probably be set to 'resolved'; what is missing?

Should be fine, I didn't have a chance to re-test it within that specific NFS-root environment but feel free to close the bug based on Bradley's patch having been applied -- I'll revisit it if still needed.

Thanks everyone involved :)

Pulseaudio still fails when ~/.pulse is a symlink.

For some reason pa_make_secure_dir opens with O_NOFOLLOW if it is available. As I do not know the reason I cannot say if simply removing the flag is reasonable, but removing it makes pa work when ~/.pulse is a symlink to a directory.

Sorry, I closed this bug prematurely. I don't know either if there's some valid reason to use O_NOFOLLOW. Until there's clarity, it makes sense to keep this bug open.

I'll pile on here, for the moment; there's one other related issue; 
I've recently ended up on a new configured-to-be-ultra-secure(?) 
shared NFS home area where I cannot change group ownership.
(it's actually kerberos nfs4 ACL's, and thus they want the group 
permissions turned off...) 

Now pulseaudio complains that it cannot make a secure directory,
because it tries to chgrp it, and that fails.  The funny thing is,
it makes it mode 0700, so the group ownership is moot anyhow.

So I propose the following:
*** ./src/pulsecore/core-util.c.orig	2016-05-16 15:13:39.281228492 -0500
--- ./src/pulsecore/core-util.c	2016-05-16 14:39:39.193214811 -0500
***************
*** 268,274 ****
  #ifndef OS_IS_WIN32
      if (!S_ISDIR(st.st_mode) ||
          (st.st_uid != uid) ||
!         (st.st_gid != gid) || 
          ((st.st_mode & 0777) != m)) {
          errno = EACCES;
          goto fail;
--- 268,275 ----
  #ifndef OS_IS_WIN32
      if (!S_ISDIR(st.st_mode) ||
          (st.st_uid != uid) ||
!          /* we only care about gid if its being used... */
!         (st.st_gid != gid && st.st_mode & 070 != 0) || 
          ((st.st_mode & 0777) != m)) {
          errno = EACCES;
          goto fail;

This works great for me; basically we don't check the group ownership
if the group bits are turned off in the mode; and voila, lovely audio again.

This came up on IRC again, and henrk pointed out this old bug that explains why we might want to keep O_NOFOLLOW:

  https://web.archive.org/web/20101129201521/http://pulseaudio.org/ticket/662

So we have two choices, I think -- continue to not allow symlinks and close this out, or have throw out a message about possibly opening up your PA for access by any user if the directory is a symlink.

Thoughts?

Hello, the use case for the symlinked ~/.config/pulse is following:

I use GNU Stow for managing my dotfiles and I set it up in the following way. I have a git repo in "dotflies_location" where I track "dotflies_location/base/.config/pulse/default.pa" and then I use stow to install it on my machines by "stow -t ~ base". That creates symlink to the pulse folder in ~/.config, if the pulse folder is not there.

I re-read the discussion in this bug, and Marc's patch in comment 12 seems like a sensible thing to do (sorry, Marc, for ignoring you for so long).

Now, regarding O_NOFOLLOW, I don't quite understand why Lennart was so strongly against symlinks. As far as I can tell, the purpose of O_NOFOLLOW is just to avoid trouble *after* the user has already shot himself in the foot by creating a symlink that others can modify. That's a nice thing to do, but if the extra caution causes trouble, then it's probably not worth it. I don't see any security problems if the symlink is in the user's home directory where others can't write. I'm in favour of removing the O_NOFOLLOW flag. (Maybe it should be removed from pa_lock_file() and pid.c:open_pid_file() too?)

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/issues/242.