Does someone want to review this?
On Thu, 2012-04-05 at 15:37 +0300, Tanu Kaskinen wrote:
> Capability dropping when changing the user in the system
> mode was previously implemented by calling
> prctl(PR_SET_KEEPCAPS, 0), but that doesn't necessarily
> work. It's possible that the KEEPCAPS flag is locked to 1,
> in which case the prctl() call fails with EPERM (this
> happens at least on Harmattan). This patch implements
> explicit capability dropping after changing the user.
> ---
> src/daemon/caps.c | 27 +++++++++++++--------------
> src/daemon/caps.h | 2 ++
> src/daemon/main.c | 4 +++-
> 3 files changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/src/daemon/caps.c b/src/daemon/caps.c
> index 3759388..36b76a9 100644
> --- a/src/daemon/caps.c
> +++ b/src/daemon/caps.c
> @@ -36,10 +36,6 @@
> #include <sys/capability.h>
> #endif
>
> -#ifdef HAVE_SYS_PRCTL_H
> -#include <sys/prctl.h>
> -#endif
> -
> #include "caps.h"
>
> /* Glibc <= 2.2 has broken unistd.h */
> @@ -78,17 +74,20 @@ void pa_drop_root(void) {
> pa_assert_se(getegid() == gid);
> #endif
>
> -#ifdef HAVE_SYS_PRCTL_H
> - pa_assert_se(prctl(PR_SET_KEEPCAPS, 0, 0, 0, 0) == 0);
> -#endif
> + if (uid != 0)
> + pa_drop_caps();
> +}
>
> +void pa_drop_caps(void) {
> #ifdef HAVE_SYS_CAPABILITY_H
> - if (uid != 0) {
> - 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);
> - }
> + 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
> }
> diff --git a/src/daemon/caps.h b/src/daemon/caps.h
> index 5d0ee62..e9cd7cb 100644
> --- a/src/daemon/caps.h
> +++ b/src/daemon/caps.h
> @@ -26,4 +26,6 @@
>
> void pa_drop_root(void);
>
> +void pa_drop_caps(void);
> +
> #endif
> diff --git a/src/daemon/main.c b/src/daemon/main.c
> index da1e0cf..0ba51cd 100644
> --- a/src/daemon/main.c
> +++ b/src/daemon/main.c
> @@ -251,6 +251,8 @@ static int change_user(void) {
> return -1;
> }
>
> + pa_drop_caps();
> +
> pa_set_env("USER", PA_SYSTEM_USER);
> pa_set_env("USERNAME", PA_SYSTEM_USER);
> pa_set_env("LOGNAME", PA_SYSTEM_USER);
> @@ -266,7 +268,7 @@ static int change_user(void) {
> if (!getenv("PULSE_STATE_PATH"))
> pa_set_env("PULSE_STATE_PATH", PA_SYSTEM_STATE_PATH);
>
> - pa_log_info(_("Successfully dropped root privileges."));
> + pa_log_info(_("Successfully changed user to \"" PA_SYSTEM_USER "\"."));
>
> return 0;
> }
--
Tanu
