libcap vs libcap-ng mess

libcap vs libcap-ng mess

[Dr. David Alan Gilbert]

Hi,
  We seem to have a bit of a mess with libcap and libcap-ng; and I'm not
sure if we should try and untangle it.

a) Our configure script has tests for both libcap and libcap-ng
  for libcap it says $cap, for libcap-ng it says $cap_ng (ok)
  If $cap is set - nothing happens?
  If $cap_ng is set - we define CONFIG_LIBCAP  (!)

b) We use both
  1) pr-helper and bridge-helper use CONFIG_LIBCAP and use cap-ng
  2) 9p's virtfs-proxy-helper uses libcap - it's got a check in
     configure to make sure you have libcap if you've asked for 9p

c) Our gitlab-ci.yml installs libcap-dev to get the 9p stuff tested
  but never installes libcap-ng-dev

I hit this because we're using libcap in virtiofsd at the moment.

So hmm how to fix?
I'm tempted to:
  x) Replace CONFIG_LIBCAP by CONFIG_LIBCAPNG to make it clear
  y) Should we flip over to only using one or the other - what
     are the advantages?
  z) We should probably add the other one to the ci.

Dave

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

Re: libcap vs libcap-ng mess

[Daniel P. Berrangé]

On Thu, Nov 28, 2019 at 07:04:08PM +0000, Dr. David Alan Gilbert wrote:
> Hi,
>   We seem to have a bit of a mess with libcap and libcap-ng; and I'm not
> sure if we should try and untangle it.
> 
> a) Our configure script has tests for both libcap and libcap-ng
>   for libcap it says $cap, for libcap-ng it says $cap_ng (ok)
>   If $cap is set - nothing happens?
>   If $cap_ng is set - we define CONFIG_LIBCAP  (!)

Wow, confusing to the max.

> b) We use both
>   1) pr-helper and bridge-helper use CONFIG_LIBCAP and use cap-ng
>   2) 9p's virtfs-proxy-helper uses libcap - it's got a check in
>      configure to make sure you have libcap if you've asked for 9p
> 
> c) Our gitlab-ci.yml installs libcap-dev to get the 9p stuff tested
>   but never installes libcap-ng-dev

Opps.

> 
> I hit this because we're using libcap in virtiofsd at the moment.
> 
> So hmm how to fix?
> I'm tempted to:
>   x) Replace CONFIG_LIBCAP by CONFIG_LIBCAPNG to make it clear

Definitely

>   y) Should we flip over to only using one or the other - what
>      are the advantages?

In libvirt we use libcap-ng. We picked this originally as its API
design allows you do write simpler code than libcap in some cases
You can see some docs & examples here:

  https://people.redhat.com/sgrubb/libcap-ng/

So I vote for changing the 9p code to use libcap-ng.

>   z) We should probably add the other one to the ci.

Yep.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: libcap vs libcap-ng mess

[Paolo Bonzini]

On 29/11/19 10:34, Daniel P. Berrangé wrote:
>>   y) Should we flip over to only using one or the other - what
>>      are the advantages?
> In libvirt we use libcap-ng. We picked this originally as its API
> design allows you do write simpler code than libcap in some cases
> You can see some docs & examples here:
> 
>   https://people.redhat.com/sgrubb/libcap-ng/
> 
> So I vote for changing the 9p code to use libcap-ng.

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
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.

In the meanwhile, if someone else wants to look at the CI I would
appreciate that.

Paolo

Re: libcap vs libcap-ng mess

[Dr. David Alan Gilbert]

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> On 29/11/19 10:34, Daniel P. Berrangé wrote:
> >>   y) Should we flip over to only using one or the other - what
> >>      are the advantages?
> > In libvirt we use libcap-ng. We picked this originally as its API
> > design allows you do write simpler code than libcap in some cases
> > You can see some docs & examples here:
> > 
> >   https://people.redhat.com/sgrubb/libcap-ng/
> > 
> > So I vote for changing the 9p code to use libcap-ng.
> 
> 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
> 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.
> 
> In the meanwhile, if someone else wants to look at the CI I would
> appreciate that.

Yeh I'll fix up the config and ci.

Dave

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

Re: libcap vs libcap-ng mess

[Dr. David Alan Gilbert]

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> On 29/11/19 10:34, Daniel P. Berrangé wrote:
> >>   y) Should we flip over to only using one or the other - what
> >>      are the advantages?
> > In libvirt we use libcap-ng. We picked this originally as its API
> > design allows you do write simpler code than libcap in some cases
> > You can see some docs & examples here:
> > 
> >   https://people.redhat.com/sgrubb/libcap-ng/
> > 
> > So I vote for changing the 9p code to use libcap-ng.
> 
> 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
> 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.

Dave

> In the meanwhile, if someone else wants to look at the CI I would
> appreciate that.
> 
> Paolo
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: libcap vs libcap-ng mess

[Paolo Bonzini]

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.

Does virtiofsd have to do uid/gid dances like virtfs-proxy-helper?

Thanks,

Paolo

Re: libcap vs libcap-ng mess

[Dr. David Alan Gilbert]

* 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?)

> 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.

Dave

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

Re: libcap vs libcap-ng mess

[Paolo Bonzini]

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.

>> 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.

Paolo

Re: libcap vs libcap-ng mess

[Dr. David Alan Gilbert]

* 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

Re: libcap vs libcap-ng mess

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1742 bytes --]

Il ven 29 nov 2019, 19:54 Dr. David Alan Gilbert <dgilbert@redhat.com> ha
scritto:

> > 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.
>
> So, I think what you're saying is I need to:
>   a) Before we sandbox do the capng_get_caps_process
>

Why not after sandboxing? If the code is in any way similar to the 9p
proxy, you have two states, "sandboxed with capabilities" and "sandboxed
without capabilities". The former (permitted=effective) is what you get
after setresuid/setresgid, the other can be computed after sandboxing and
saved using capng_save_state. The FSETID capability can be updated
explicitly before/after capng_apply.

  b) Before we start a new thread do a capng_save_state and restore it
> in the thread
>

Or just save after (a), and restore always before capng_apply.

  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?)
>

Yes it would. For uid!=0 the kernel clears the effective capabilities so it
shouldn't need to do anything, unless virtiodsd restores capabilities after
setresuid/setresgid.

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

No, only the effective caps.

Paolo

[-- Attachment #2: Type: text/html, Size: 2933 bytes --]

Re: libcap vs libcap-ng mess

[Dr. David Alan Gilbert]

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> Il ven 29 nov 2019, 19:54 Dr. David Alan Gilbert <dgilbert@redhat.com> ha
> scritto:
> 
> > > 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.
> >
> > So, I think what you're saying is I need to:
> >   a) Before we sandbox do the capng_get_caps_process
> >
> 
> Why not after sandboxing?

Because in our sandbox we don't have /proc and capng_get_caps_process
tries to read /proc/.../status and fails. The old libcap code doesn't
use /proc, it just uses capget (which the new one also uses).

> If the code is in any way similar to the 9p
> proxy, you have two states, "sandboxed with capabilities" and "sandboxed
> without capabilities". The former (permitted=effective) is what you get
> after setresuid/setresgid, the other can be computed after sandboxing and
> saved using capng_save_state. The FSETID capability can be updated
> explicitly before/after capng_apply.
> 
>   b) Before we start a new thread do a capng_save_state and restore it
> > in the thread
> >
> 
> Or just save after (a), and restore always before capng_apply.

Hmm yes, that's easier.

>   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?)
> >
> 
> Yes it would. For uid!=0 the kernel clears the effective capabilities so it
> shouldn't need to do anything, unless virtiodsd restores capabilities after
> setresuid/setresgid.
> 
> But are you suggesting I need to change something other than the
> > effective caps in that case?
> >
> 
> No, only the effective caps.

OK, thanks.

Dave

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

Re: libcap vs libcap-ng mess

[Paolo Bonzini]

On 02/12/19 11:07, Dr. David Alan Gilbert wrote:
> * Paolo Bonzini (pbonzini@redhat.com) wrote:
>> Il ven 29 nov 2019, 19:54 Dr. David Alan Gilbert <dgilbert@redhat.com> ha
>> scritto:
>>
>>>> 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.
>>>
>>> So, I think what you're saying is I need to:
>>>   a) Before we sandbox do the capng_get_caps_process
>>>
>>
>> Why not after sandboxing?
> 
> Because in our sandbox we don't have /proc and capng_get_caps_process
> tries to read /proc/.../status and fails. The old libcap code doesn't
> use /proc, it just uses capget (which the new one also uses).

"In the middle of sandboxing" would have been a more precise suggestion.
 In the virtio-9p proxy I set up the state right after the sandboxing
capng_apply, so that there is no need to do capng_get_caps_process (the
libcap virtio-9p also used cap_get_proc).  So you could capng_save_state
there.

Paolo