[pulseaudio-discuss] [PATCH] daemon: Don't rely on prctl(PR_SET_KEEPCAPS, 0) for dropping caps.

[Tanu Kaskinen]

On Sun, 2012-12-16 at 10:38 +0200, Tanu Kaskinen wrote:
> Does someone want to review this?

Apparently nobody wants to do that, so I pushed the patch now.

-- 
Tanu

> 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;
> >  }
>