Re: libcap vs libcap-ng mess
On 02/12/19 11:07, Dr. David Alan Gilbert wrote: > * Paolo Bonzini (pbonz...@redhat.com) wrote: >> Il ven 29 nov 2019, 19:54 Dr. David Alan Gilbert 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
Re: libcap vs libcap-ng mess
* Paolo Bonzini (pbonz...@redhat.com) wrote: > Il ven 29 nov 2019, 19:54 Dr. David Alan Gilbert 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 / dgilb...@redhat.com / Manchester, UK
Re: libcap vs libcap-ng mess
Il ven 29 nov 2019, 19:54 Dr. David Alan Gilbert 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
Re: libcap vs libcap-ng mess
* Paolo Bonzini (pbonz...@redhat.com) wrote: > On 29/11/19 19:20, Dr. David Alan Gilbert wrote: > > * Paolo Bonzini (pbonz...@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//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 / dgilb...@redhat.com / Manchester, UK
Re: libcap vs libcap-ng mess
On 29/11/19 19:20, Dr. David Alan Gilbert wrote: > * Paolo Bonzini (pbonz...@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//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
* Paolo Bonzini (pbonz...@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//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 / dgilb...@redhat.com / Manchester, UK
Re: libcap vs libcap-ng mess
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//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
* Paolo Bonzini (pbonz...@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//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 / dgilb...@redhat.com / Manchester, UK
Re: libcap vs libcap-ng mess
* Paolo Bonzini (pbonz...@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 / dgilb...@redhat.com / Manchester, UK
Re: libcap vs libcap-ng mess
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
On Thu, Nov 28, 2019 at 07:04:08PM +, 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 :|