[nova] Privsep is not giving us any security

Matthew Booth mbooth at redhat.com
Mon Apr 1 14:23:03 UTC 2019


On Sat, 30 Mar 2019 at 08:32, Michael Still <mikal at stillhq.com> wrote:
>
> On Sat., 30 Mar. 2019, 6:28 pm Thierry Carrez, <thierry at openstack.org> wrote:
>>
>> Michael Still wrote:
>> > The reality is that privsep was always going to be a process. It's taken
>> > more than 80 patches to get close to removing rootwrap.
>> >
>> > There are other advantages to removing rootwrap, mainly around
>> > performance, the integration of library code, and general
>> > non-bonkersness (cat to tee to write to a file as root), etc.
>> >
>> > There is president in the code to mark calls as undesirable, and others
>> > could be marked like that as well, but ultimately someone needs to do an
>> > audit and fix things... That's more than one person can reasonably do.
>> >
>> > So, who wants to help try and improve this? Patches welcome.
>>
>> It's been on my priority-2 TODO list for a while to help with that...
>> Now if people would stop adding to my priority-1 TODO list...
>>
>> Agree that's definitely more than a one-person job, but migrating a
>> specific call is also a reasonably self-contained unit of work that (1)
>> does not require a deep understanding of all the code around it, and (2)
>> does not commit you for a lifelong feature maintenance duty... So maybe
>> it would be a good thing to suggest newcomers / students to get a poke
>> at? I'm happy to help with the reviewing if we can come up with a topic
>> name that helps finding those.
>
>
> One concern I have is that I am not sure it's always as simple as it looks. For example, we could enforce that device files are always in /dev, but is that always true on all architectures with all hypervisors? How do we know that?

I think the answer here is that we don't have privsep functions like
that *at all*. So instead of a generic
privsep.data.discombobulate(device) you have
privsep.libvirt.volume.foo.discombobulate(volume). I mentioned this as
a general principal in my original mail which has since been snipped,
that we should probably *never* pass a reference to a system resource
(the most obvious example being a path) to a privsep function.
Instead, privsep has its own config and can work that out for itself.
This does mean a proliferation of specific privsep functions over a
few generic ones, but that's how the security model works. In general
we should expect classes to have a shadow privsep class containing
security-sensitive logic.

> We could add enforcement and at the same time add a workaround flag to turn it off, which is immediately deprecated so people have a release to notice a breakage. Do we do that per enforcement rule? That's a lot of flags!
>
> Finally, the initial forklift has been a series of relatively simple code swaps (which is really the root of Mr Booth's concern). Even with taking the easy path there haven't been heaps of volunteers helping and some of this code has been in review for quite a long time. Do we really think that volunteers are going to show up now?

Exactly. Most folks acknowledge the importance of this stuff, but not
to the extend of prioritising it for review: it's boring, involves
large code churn, and doesn't immediately add anything obvious. Worse
the code churn means that unless it lands quickly it's very soon going
to be in merge conflict hell. Writing the code is likely the easy bit.
Unless we can agree on this as a priority theme for a while I don't
see us making any progress on it in practise.

Matt
-- 
Matthew Booth
Red Hat OpenStack Engineer, Compute DFG

Phone: +442070094448 (UK)



More information about the openstack-discuss mailing list