[libvirt] [PATCH v1 09/23] lock_protocol: Add two new remote procedures

[Michal Privoznik]

On 16.10.2015 09:17, Peter Krempa wrote:
> On Mon, Oct 12, 2015 at 12:25:54 +0200, Michal Privoznik wrote:
>> These procedures will be used to store and bring back security
>> labels. So far, the idea is that tuple (path, model, label) is
>> enough. Well, certainly for DAC and SELinux. The functions are:
> 
> I'm afraid that a 'path' per-se isn't enough to express storage
> locations in a unique way. Said that a string is fine though, but I'd
> rather call it 'identifier' or something similar and explicitly
> document some formats for possibly other storage systems that might use
> security labeling but don't have a local representation in the host.
> 
> I'd imagine something like:
> STORAGE_SYSTEM_UUID:VOLUME_UUID

I can imagine this, okay.

> and perhaps
> HOST_UUID:/path for local files

Hm... what good it will have to store UUID among with the path? I mean,
we can't know if two paths from two different hosts is the same file or
not anyway.

> 
> One other thing to consider is that if this approach will be used across
> multiple hosts the paths although equal string-wise might not result
> being the same file. Not honoring that would result into security
> implications.

What security implications do you have in mind?

On the other hand, I just realized that this approach will not fly. I
mean, virtlockd is running per host. So consider a file on NFS share,
and to make things more complicated assume it's accessible under
different paths from two different hosts. There is still a race between
those two virtlockd-s - they will not mutually exclude each other and
therefore can store already chown()-ed owership info. For instance:

host1: stat()
host1: store ownership (e.g. 100:20)
host1: chown (e.g. to 20:20)

host2: stat()
host2: store ownership (20:20)

host1: recall ownership (100:20)
host1: restore ownership (100:20)

host2: recall ownership (20:20)
host2: restore ownership (20:20)

Both hosts think that they are the last ones to use the file, therefore
they restore ownerships. However, after all the original ownership is
not preserved.

One way out of this could be that we already require virtlockd in order
to work properly that lockspace is on a shared mountpoint accessible by
both hosts. So instead of keeping seclabel space in memory, we can load
& store it within a file on that mountpoint and guard accesses via
flock(). But this is rather unpleasant as we would have to parse and
format the whole seclabel space just to update a refcounter to some entry.

The other approach would be to have a seclabel space as a directory on
the shared mountpoint and create some file (with internal structure) per
path remembered. The internal structure of the file would then contain
tuple (model, seclabel, refcount) for each model. When updating the
refcount, only the file would need to be flock()-ed getting us much
higher throughput. But we would still need to parse and format a file on
each access (even though the file would be much smaller).

I'm out of ideas here. Anybody has some?

Michal