[libvirt] [PATCH RFC 1/3] security_dac: Remember owner prior chown() and restore on relabel

[Daniel P. Berrange]

On Fri, Mar 01, 2013 at 09:42:57AM +0100, Michal Privoznik wrote:
> On 28.02.2013 02:22, Eric Blake wrote:
> > On 02/27/2013 02:25 AM, Michal Privoznik wrote:
> >>> Are you really planning on storing a string uid:gid?  Wouldn't it be
> >>> simpler to store a uid_t and gid_t as read from struct stat, as long as
> >>> the data is only in memory?  And when storing the data to disk in XML to
> >>> survive libvirtd restarts, it seems like storing two attributes
> >>> user='nnn' group='nnn' is nicer than storing one attribute
> >>> owner='+nnn:+nnn' that requires further parsing back into user and group.
> >>
> >> My idea is; store userName:groupName whenever possible; When one of them
> >> is not accessible, use +NNN instead. The rationale is - whenever a user
> >> or a gropu changes its ID, we will follow it. For instance, a file X has
> >> owner A:B (1:1) but libvirt chowns to C:D. Meanwhile A's ID is changed
> >> from 1 to 2. So when relabeling we should chown to 2:1 instead of 1:1 as
> >> A's ID is 2 not 1. That means I have to do parsing and all the
> >> virAsprintf magic. However, maybe this is not what we want and I should
> >> really remember just numeric values of IDs which has nice tradeoff -
> >> much simpler code.
> > 
> > Migration and shared storage makes this problem so much tougher - the
> > uid on shared storage is common, but the name is not necessarily common.
> >  I'd go for uid only; leave name lookups to each local machine
> > connecting to shared storage, but don't store the name ourselves.
> > Besides, admins tend not to change the name associated with a uid all
> > that frequently (it's generally one-time setup).
> > 
> > Dan has a point that you really need to involve the lock manager or use
> > some persistent storage (extended attribute or additional file in the
> > storage pool) alongside the file whose attributes we want to remember -
> > if a file is on shared storage, then it is the responsibility of the
> > last machine using the image to restore permissions, even if it was not
> > the machine that first did a chmod().  Merely storing a hash in just a
> > single libvirtd instance won't scale.  Does our virlockd interface
> > support attaching attributes to a file as part of locking it?
> 
> Okay. You two has a point. I have not thought about that initially.
> Anyway, if I go with xattrs, would it be safe? I mean, I'd need to
> atomically get extended attribute for refCount. If there's none, get
> current owner of the file and store it - again - as an extended
> attribute among with refCount = 1. Any later libvirtd instance would
> then just atomically increment/decrement refCount. And here's where I
> see the trouble. In case of files protected by sanlock/virlockd we are
> safe - the relabeling is done after we obtain the lock. But what about
> files that are not protected by lock, e.g. <shared/> and <readonly/>?
> Unless we lock them for the time we do the xattr magic, we cannot
> guarantee atomicity, right? But this will extend need of usage of
> sanlock/virlockd to nearly all cases, which is something we might not
> want. Even if you have a couple of domains within a single host and
> you're sharing an ISO image you'd need the lock daemon.

Actually you can't rely on the existing sanlock/virtlockd support.
While during VM startup, you are safe because the lock is held,
for relabelling back to the original after shutdown no locks are
protecting you.

Basically you should think of the current lock support as protecting
the *content* of files. We would need to use a separate lockspace
to protect metadata (ie ownership/xtttr/etc). I would envisage this
unconditionally using virtlockd, regardless of whether sanlock is
used for protecting content.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|