The configure script for pulseaudio has this: CAP_LIBS='' AC_ARG_WITH([caps], AS_HELP_STRING([--without-caps],[Omit support for POSIX capabilities.])) if test "x${with_caps}" != "xno"; then AC_SEARCH_LIBS([cap_init], [cap], [], [ if test "x${with_caps}" = "xyes" ; then AC_MSG_ERROR([*** POSIX caps libraries not found]) fi]) AC_CHECK_HEADERS([sys/capability.h], [], [ if test "x${with_caps}" = "xyes" ; then AC_MSG_ERROR([*** POSIX caps headers not found]) fi]) fi Then in the daemon's source: void pa_drop_caps(void) { #ifdef HAVE_SYS_CAPABILITY_H cap_t caps; pa_assert_se(caps = cap_init()); pa_assert_se(cap_clear(caps) == 0); pa_assert_se(cap_set_proc(caps) == 0); pa_assert_se(cap_free(caps) == 0); #else pa_log_warn("Normally all extra capabilities would be dropped now, but " "that's impossible because this Pulseaudio was built without " "libcap support."); #endif } POSIX does not specify what such a file should contain -- the attempt to standardise it seems to have failed, as evidenced by the comment at the top of Linux's version of this file: * defunct POSIX.1e Standard: 25.2 Capabilities Meanwhile, the combination of the two checks above produces the wrong behaviour. Imagine a system where sys/capability.h exists, but not cap_init (FreeBSD is such a system, for example). The first check fails due to missing cap_init, but because --with-caps=yes was not explicitly given, the failure is ignored. The second check, which is independent of the first check then passes, because we do find sys/capability.h. This results in HAVE_SYS_CAPABILITY_H being defined, and then the caps code gets enabled in pa_drop_caps(). Inside the #ifdef for HAVE_SYS_CAPABILITY_H there should probably also be an #ifdef __linux before using that Linux-style capabilities code. There could then ideally be another branch for BSD-style sys/capability.h.
Created attachment 90593 [details] [review] [PATCH] Add support for FreeBSD <sys/capability.h> cap_init() and friends are Linux-specific, so only use them if we're on Linux. Add support for FreeBSD capabilities if we find <sys/capability.h> to be available there. Add an #else (not Linux or FreeBSD) case with an #error requesting contributions for other platforms. This patch keeps the cap_init check in configure.ac but removes the error if it fails. This will ensure we link to -lcap if needed, but won't fail for the case that capabilities are part of the core system (as on FreeBSD). We do however, modify the header check to ensure we fail if there is no <sys/capability.h> at all, forcing --without-caps to be explicitly specified to make the failure go away.
Thanks for the patch. I have an improvement request: I think it would be better to not fail hard on systems that do not provide the capability functionality, unless --with-caps has been explicitly specified.
I consider it to be an automake "best practice" to fail instead of silently disabling a feature. The reason for that is simple: the person who downloads and pulseaudio may simply not have the libcap development headers installed. I think it's better to give them a chance to choose to install them or to explicitly specify --without-cap. If I make your suggested modification then the build will just go ahead without capabilities support, which would be a reduction in functionality (or in this case security) just because the user happened not to have a particular package installed. The same logic applies to people who are making distro packages as well... I think that it's always better to let them know.
We treat all other dependencies in the same way: if the user didn't enable or disable an optional feature explicitly, then it's enabled or disabled depending on whether it's available in the system or not. While I didn't come up with this scheme, I think it's a good way to deal with optional dependencies. I want ./configure without arguments to just work, as long as all mandatory dependencies are satisfied. At the end of configure, we print a list of features that are enabled or disabled. Capability support isn't currently in that list, so that could be fixed. Since capabilities are a security feature, it might be a good idea to make an exception in this case, and require explicit --without-caps to make sure that the user really wants to build PulseAudio without capability support, but that should only be done on platforms on which capabilities are supported. We don't support capabilities on Windows, for example, and I don't want to require all Windows users to pass --without-caps.
That's a fair point. I'll add a check for being on Linux or FreeBSD before giving the error and upload a new patch soon.
Created attachment 90814 [details] [review] Add support for FreeBSD <sys/capability.h> cap_init() and friends are Linux-specific, so only use them if we're on Linux. Add support for FreeBSD capabilities if we find <sys/capability.h> to be available there. Add an #else (not Linux or FreeBSD) case with an #error requesting contributions for other platforms. This patch keeps the cap_init check in configure.ac but removes the error if it fails. This will ensure we link to -lcap if needed, but won't fail for the case that capabilities are part of the core system (as on FreeBSD). We do however, modify the header check to ensure we fail if there is no <sys/capability.h> at all and we are on a system where it could be installed. The logic here is that it is better to give the user the chance to install it than it is to proceed silently with a disabled security feature on a system where it could easily be supported. --without-caps remains an option if the user wants to force it.
Thanks again for the updated patch! configure.ac looks good now. It looks like you forgot to remove the #error from caps.c, or was leaving it there intentional?
That's intentional. It will catch that case that there is some other platform out there that has <sys/capability.h> but we don't know how to use it. Hopefully they will then file a bug telling us how to do so. It will not hit platforms that don't have <sys/capability.h> (Windows, etc.) They can always workaround the issue with --without-caps.
OK, that makes sense. I had some misunderstanding of the logic. Patch applied: cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=1da34e99b203de14a45c94ec768faf04d41eca5b
Thanks very much.
So this patch is not right. I assumed from my reading of the Linux code ("cap_clear()...") that it was clearing all capabilities of the process when in fact it is only clearing the "special to root" capabilities. The FreeBSD version of the code indeed clears _all_ capabilities beyond ones that the process already has (ie: cannot open any new files, create sockets, etc.) This has a pretty obvious adverse effect on pulseaudio's ability to do what it needs to do -- indeed, it bombs out pretty quickly due to an inability to read its own config file. We need to find out if we can drop the special-to-root caps on FreeBSD without the others. My initial research into this makes it look like it may not be possible...
I removed the broken code: http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=11e71e3990c5880ff5e52cc33741d71a0ba21d6c
This is suboptimal because now we #error out on FreeBSD... The crux of the problem here comes back down to this check in configure.ac that assumes that sys/capability.h means anything in particular. It turns out that the presence of this file on FreeBSD is related to the existence of a very different feature than on Linux...
So it turns out that FreeBSD simply has no mechanism similar to Linux capabilities. The correct fix is to add back the #elif case for FreeBSD and have it do nothing, perhaps with a comment about "known not to be supported".
Ok, I'll make that change soon.
Done: http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=97c27202ca71f5f3f6792535eef1e26f5369e9cc
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.