Re: libcap vs libcap-ng mess

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> On 29/11/19 19:20, Dr. David Alan Gilbert wrote:
> > * Paolo Bonzini (pbonzini@redhat.com) wrote:
> >> On 29/11/19 19:01, Dr. David Alan Gilbert wrote:
> >>>> It's not entirely trivial because fsdev-proxy-helper wants to keep the
> >>>> effective set and clear the permitted set; in libcap-ng you can only
> >>                      ^^^^^
> >>
> >> (Wrong, this is "modify" the permitted set.  The permitted set is
> >> already cleared by setresuid/setresgid).
> >>
> >>>> apply both sets at once, and you cannot choose only one of them in
> >>>> capng_clear/capng_get_caps_process.  But it's doable, I'll take a look.
> >>> I'm having some difficulties making the same conversion for virtiofsd;
> >>> all it wants to do is drop (and later recover) CAP_FSETID
> >>> from it's effective set;  so I'm calling capng_get_caps_process
> >>> (it used to be cap_get_proc).  While libcap survives just using the
> >>> capget syscall, libcap-ng wants to read /proc/<TID>/status - and
> >>> that's a problem because we're in a sandbox without /proc mounted
> >>> at that point.
> >>
> >> The state of libcap-ng persists after capng_apply.  So you can just call
> >> capng_update({CAP_ADD,CAP_DROP}) followed by capng_apply.
> > 
> > But the internal state needs initialising doesn't it? So that when you
> > capng_update it tweaks a set that was originally read from somewhere?
> > (and that's per-thread?)
> 
> Yes, it's per thread.  The state can be built from
> capng_clear/capng_get_caps_process + capng_update, and left in there
> forever.  There is also capng_save_state/capng_restore_state which, as
> far as I can see from the sources, can be used across threads.

OK that's a lot more complex than the current code, and a bit fragile -
but probably more efficient.
So, I think what you're saying is I need to:
  a) Before we sandbox do the capng_get_caps_process
  b) Before we start a new thread do a capng_save_state and restore it
in the thread

I've got to be pretty careful that I do (a) at the write point so
I've not gained anything we later try and drop.
(But we do save doing the capget() on every time we do this drop/restore
dance).

> >> Does virtiofsd have to do uid/gid dances like virtfs-proxy-helper?
> > 
> > It looks like it; I can see setresuid calls to save and restore
> > euid/egid.
> 
> Ok, then perhaps you can take a look at my virtfs-proxy-helper patch.
> The important part is that after setresuid/setresgid PERM=EFF if
> uid=0/gid=0 and PERM=0 otherwise.

I think we're ok because:
  a) This code is very local - it does a drop FSETID, a write, restore
FSETID
  b) I'm not sure but I suspect it's used only in the non-uid=0 case; 
the whole thing is just a hack to cause setuid/setgid to be dropped
in the case where it's written by a process that doesn't have FSETID
(hmm I guess if the guest was root but didn't have fsetid then it would
be 0?)

But are you suggesting I need to change something other than the
effective caps in that case?

Dave

> Paolo
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK