Bug 1371823

Summary: Should report "permission denied" when attach disk failed due to lack of permission
Product: Red Hat Enterprise Linux 7 Reporter: Fangge Jin <fjin>
Component: libvirtAssignee: Michal Privoznik <mprivozn>
Status: CLOSED NOTABUG QA Contact: Virtualization Bugs <virt-bugs>
Severity: medium Docs Contact:
Priority: medium    
Version: 7.3CC: dyuan, mzhan, rbalakri, yafu, yanqzhan, zpeng
Target Milestone: rc   
Target Release: ---   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2016-09-13 07:28:26 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:

Description Fangge Jin 2016-08-31 08:17:02 UTC 
Description of problem:
Prepare a image with root user:
# ll /var/lib/libvirt/images/test.qcow2 -Z
-rw-rw-rw-. root root system_u:object_r:virt_image_t:s0 /var/lib/libvirt/images/test.qcow2

With non-root user, start a guest, and attach the disk to guest:
$ virsh start rhel7
Domain rhel7 started

$ virsh attach-disk rhel7 --source /var/lib/libvirt/images/test.qcow2 --target vdb --mode readonly
error: Failed to attach disk
error: internal error: unable to execute QEMU command '__com.redhat_drive_add': Device 'drive-virtio-disk1' could not be initialized

libvirt log
error : virSecuritySELinuxSetFileconHelper:920 : unable to set security context 'system_u:object_r:virt_content_t:s0' on '/var/lib/libvirt/images/test.qcow2': Operation not permitted

qemu log:
Could not open '/var/lib/libvirt/images/test.qcow2': Permission denied

Version-Release number of selected component:
libvirt-2.0.0-6.el7.x86_64

How reproducible:
100%

Steps to Reproduce:
As the description

Actual results:
Attach disk failed with unclear error

Expected results:
Attach disk failed and output error "Permission denied"
Comment 3 Michal Privoznik 2016-09-13 07:28:26 UTC 
Hm, now that I'm checking the code, I'm not sure this is an error. I'd say this is expected behaviour. From src/security/security_selinux.c:

static int
virSecuritySELinuxSetFileconHelper(const char *path, char *tcon,
                                   bool optional, bool privileged)
{
    /* ... */
            virReportSystemError(setfilecon_errno,
                                 _("unable to set security context '%s' on '%s'"),
                                 tcon, path);
            /* However, don't claim error if SELinux is in Enforcing mode and
             * we are running as unprivileged user and we really did see EPERM.
             * Otherwise we want to return error if SELinux is Enforcing. */
            if (security_getenforce() == 1 && (setfilecon_errno != EPERM || privileged))
                return -1;
    /* ... */
    return 0;
}


http://libvirt.org/git/?p=libvirt.git;a=blob;f=src/security/security_selinux.c;h=4be946d2e872657ce5a01be786f9236e393d1699;hb=HEAD#l883

IOW, when daemon is running unde non-privileged user, we just do our best and hope it works. We can't be sure that even if relabelling fails qemu is denied access. I mean, libvirt would need to parse selinux policies, all the linux users & groups, AppArmor rules, ACLs to be 100% sure whether qemu will or will not have the access (when relabelling fails). So we don't undertake all the burden and just let qemu find out itself.