[libvirt] [PATCH 03/18] security: Include security_util

[Michal Privoznik]

On 11/28/18 11:00 AM, Daniel P. Berrangé wrote:
> On Wed, Nov 28, 2018 at 09:35:49AM +0100, Michal Privoznik wrote:
>> On 11/27/18 4:20 PM, Daniel P. Berrangé wrote:
>>> On Fri, Nov 23, 2018 at 09:43:21AM +0100, Michal Privoznik wrote:
>>>
>>>> +/* There are four namespaces available (xattr(7)):
>>>
>>> s/available/available on Linux/
>>>
>>> FreeBSD only supports 'user' and 'system' namespaces
>>
>> D'oh!
>>
>>>
>>>> + *
>>>> + *  user - can be modified by anybody,
>>>> + *  system - used by ACLs
>>>> + *  security - used by SELinux
>>>> + *  trusted - accessibly by CAP_SYS_ADMIN processes only
>>>> + *
>>>> + * Looks like the last one is way to go.
>>>
>>> That prevents the QEMU driver using this functionality on any
>>> non-Linux host.
>>>
>>> The key problem we obviously face is that of the QEMU process
>>> being able to modify the xattrs maliciously. 'trusted' namespace
>>> solves this for Linux but unsolved for BSD/macOS.
>>>
>>> I can only think of two alternative ways to deal with this
>>>
>>>  - Use a sidecar file. eg $FILEPATH.libvirt.json
>>>    Works ok for plain files. Troublesome for device nodes.
>>>    Would have to use a file in /var/run/libvirt/devs/$DEVNODE
>>>    perhaps ?
>>
>> I rather not create files. If there is a bug and libvirt doesn't remove
>> the file on the last restore (or it gets refcounting wrong or something)
>> then it requires user intervention.
> 
> Isn't that true of this series too - ie if we update the xattr for
> refcount wrong, or fail to remove the xattr on last restore ?

Yes, this the same. If there is a bug then unlink() and setxattr() don't
differ from this POV.

> 
> Feels like we have the same possible source of bugs, just against
> an unlink() call rather than a setxattr() call.
> 
> The existance of libvirt's sidecar files on disk would be more
> obviously to an admin.
> 
> That said tellin people to "rm" the file is kinda dangerous if
> they typo and rm the real disk file that's alongside.  Using a
> completely 100% separate directory would solve that.

Except we can't do that. If there is a file accessible to two daemons
(e.g. running on the same host in two different containers) the daemons
can't have their private DB, it has to be shared -> because of
refcounting. If the DB was private then one daemon could not
increment/decrement the refcounter inside the other daemon (consider two
daemons, one starting domain A, the other starting domain B, then the
first destroying domain A).

I mean, this is the sole reason I chose XATTRs and not some in memory
list/mapping of original owners.

> 
>> Not only that, but the file would need to be owned by root:root (in
>> order to avoid malicious users mangling its contents) which might not be
>> possible at all times (e.g. root squashed NFS). On the other hand, NFS
>> itself doesn't support XATTRs currently so these patches do not with
>> that filesystem.
> 
> Yeah, root sqaush  NFS is horrible :-(
> 
>>
>>>
>>>  - Use 'user' label but add a cryptographic signature
>>>    as a further attribute. Doesn't prevent tampering
>>>    but lets us throw away the data when tempering is
>>>    detected.
>>
>> I'm not sure how this would work with multiple daemons. They would have
>> to have the key shared among them. I mean, if one daemon sets the
>> attribute and signs it with its own key and then comes another daemon
>> and updates the attribute it needs to sign it with the very same key
>> otherwise the signature would be invalidated from the first daemon POV.
> 
> Yeah, it would need shared keys in some manner which is a very big
> pain point.
> 
>>
>>>
>>> Did you consider either of these, or any other possible
>>> options ?  I'm still loathe to bake in a solution that will
>>> only work on Linux, despite 99% of our userbase being Linux.
>>
>> Not really. I focused on Linux and haven't realized BSD doesn't support
>> trusted.
>>
>> Now that I am playing with this it looks like 'system' while being
>> available on both Linux and BSD can't really be set on Linux. But it can
>> be set on BSD (for privileged user only of course). I'm wondering if
>> using 'trusted' on Linux and 'system' on BSD is the way to go. This
>> could work except for cases where BSD and Linux meet, which can be only
>> on NFS (right?) which doesn't support XATTRs anyway. Therefore I think
>> we are safe, aren't we?
> 
> I expect eventually NFSv4 may get xattr support - there's a 1 year old
> RFC for it:
> 
>   https://tools.ietf.org/html/rfc8276
> 
> Any app using xattr is going to face this same problem unless they only
> use the common "user" namespace
> 
> Maybe justusing trusted (Linux) and system (BSD) is good enough for
> us to get started, and we can allow for a different system (side car
> files, or attribute signatures) in future if it becomes needed.

Agreed.

Michal