[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 6/6] Try much harder to restore disk file labels



On 09/01/2009 02:20 PM, Stephen Smalley wrote:
> On Tue, 2009-09-01 at 13:33 -0400, Stephen Smalley wrote:
>> On Tue, 2009-09-01 at 18:10 +0100, Daniel P. Berrange wrote:
>>> On Tue, Sep 01, 2009 at 01:00:13PM -0400, Stephen Smalley wrote:
>>>> On Tue, 2009-09-01 at 16:28 +0100, Daniel P. Berrange wrote:
>>>>> * src/security_selinux.c: matchpath() may well return NULL for many
>>>>>   directories, to try and fallback to using parent directory label
>>>>>   in that scenario.
>>>>
>>>> When have you seen this happen?  matchpathcon() ultimately should fall
>>>> back to the top-level regex (/.*) and map any otherwise unmatched files
>>>> to default_t, and should generally have a fallback regex for each
>>>> subtree (e.g. any file under /dev that isn't otherwise matched would get
>>>> device_t).  So I wouldn't expect this to happen.
>>>
>>> That describes what I always imagined would happen, but in my recent
>>> testing it doesn't appear to be doing that in practice in some cases
>>>
>>> eg, running matchpathcon("/media/usbdisk/virtual-images/demo.img") returned
>>> an error. Likewise for /sys/bus/pci/devices/NNNN.NN.NN.NN/*.   I was testing
>>> this on a Fedora 11 host. Perhaps the policy is simply missing some rules
>>
>> Ah, I had forgotten about the behavior in the case where the path is
>> marked with <<none>> in the file_contexts configuration, in which case
>> matchpathcon() does return NULL as you describe.  That gets used for two
>> situations:
>> - mount point directories for filesystems that do not support labeling,
>> like /sys and /selinux.  That should be obsoleted by the support in
>> kernels >= 2.6.30 to report seclabel support in /proc/mounts, which gets
>> used by setfiles when available.  So we can likely remove those <<none>>
>> entries going forward.
>> - directories that contain runtime-created files whose labels we do not
>> wish to reset, like /tmp or /var/run.  Those entries are still necessary
>> to avoid clobbering runtime labels on such files upon a relabel.
>>
>> So I guess you do need this.
> 
> Actually, why can't you just save the original file label somewhere
> (i.e. call getfilecon() and store that) and then just use that original
> label when restoring the label.  Then you don't need to use
> matchpathcon() or this extra logic at all.
> 
>>
>>>> Also, files will inherit their SELinux type from the parent directory by
>>>> default upon creation unless a type transition rule is specified, so it
>>>> isn't clear why you need to replicate this copying from parent behavior
>>>> in the application.
>>>
>>> This code is being run upon VM shutdown, to get rid of the dynamic label
>>> that was assigned at VM startup. THe file already exists, so the default
>>> rule for file creation wouldn't come into effect at this point, so I was
>>> aiming to replicate that behaviour for the existing file.
>>>
>>> If we could guarentee that matchpathcon($PATH) would always return
>>> something usable, I would happily kill this code
>>
In Rawhide, these are the files that are labeled as <<none>>

Which ones should I eliminate.

I tend to agree with Steven, the best solution to the problem would be

MAKEFILE/DEVICE
getfilecon(path, oldcon)
setfilecon(path, svirtcon)
run virtual machine,
end virtual machine
setfilecon(path, oldcon)


/sys/.*	<<none>>
/tmp/.*	<<none>>
/mnt/[^/]*/.*	<<none>>
/proc/.*	<<none>>
/media/[^/]*/.*	<<none>>
/dev/pts(/.*)?	<<none>>
/var/tmp/.*	<<none>>
/usr/tmp/.*	<<none>>
/selinux/.*	<<none>>
/var/run/.*\.*pid	<<none>>
/lost\+found/.*	<<none>>
/tmp/\.ICE-unix/.*	-s	<<none>>
/tmp/\.X11-unix/.*	-s	<<none>>
/var/lost\+found/.*	<<none>>
/usr/lost\+found/.*	<<none>>
/tmp/lost\+found/.*	<<none>>
/var/spool/cron/[^/]*	--	<<none>>
/var/spool/fcron/.*	<<none>>
/boot/lost\+found/.*	<<none>>
/var/tmp/lost\+found/.*	<<none>>
/usr/local/lost\+found/.*	<<none>>
/var/named/chroot/proc(/.*)?	<<none>>
/var/lib/nfs/rpc_pipefs(/.*)?	<<none>>
/var/spool/cron/crontabs/.*	--	<<none>>
/sys	-d	<<none>>
/proc	-d	<<none>>
/selinux	-d	<<none>>
/\.journal	<<none>>
/var/\.journal	<<none>>
/usr/\.journal	<<none>>
/tmp/\.journal	<<none>>
/boot/\.journal	<<none>>
/usr/local/\.journal	<<none>>

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]