Re: [libvirt] New QEMU daemon for persistent reservations

On 27/11/2017 13:18, Daniel P. Berrange wrote: > On Mon, Nov 27, 2017 at 12:51:33PM +0100, Paolo Bonzini wrote: >> On 27/11/2017 12:37, Daniel P. Berrange wrote: >>> On Mon, Nov 27, 2017 at 12:13:24PM +0100, Paolo Bonzini wrote: >>>> Hm, I see what you mean now. But it would be "just" a qemu-pr-helper >>>> bug that it trusts the caller to have "ioctl" permissions on the file >>>> descriptor, wouldn't it? >>>> >>>> And it could be a feature even, since the remote QEMU process also has >>>> to have "connect" permissions on the qemu-pr-helper socket. So you >>>> could give it ioctl access *limited to persistent reservations* by >>>> granting the appropriate permissions on the socket. >>> >>> We can't grant access to the persistent reservation helper's socket on a >>> per QEMU basis. Permissions are granted on the domain type svirt_t, and >>> we don't want to invent a new domain type just for having access to the >>> PR helper. >> >> You can do so via DAC and MAC on the socket itself, or is that not enough? > > Yes, you can do MAC on the socket, but that merely gives you protection > betweeen the host & guest. The goal of sVirt is to give protection > between VMs, and MAC on the socket alone doesn't guarantee that. > In the shared-PR helper the PR helper context would not have an > MAC level applied, so the MAC check is now > > process context == qemu_pr_helper_t:s0 > file context == svirt_image_t:s0,c340,c524 > > This is still a MAC check, but it is a weaker check than what sVirt promises, > because a QEMU with MCS c340,c524 could pass a file descriptor with a MCS > level c123,c422. QEMU would be denied to use this bogus FD, but the PR > helper would happily allow this bad access. So the issue could be that QEMU could have received this bogus FD from an attacker. Got it. > So to satisfy sVirt separation of QEMU instances, we need the PR helper to > be able to validate the MCS levels, as the kernel no longer has enough info > todo this automatically. > > In the PR helper this is achieved with something like > > getpeercon(socketfd, &qemucon) => returns svirt_t:s0,c340,c524 > fgetfilecont(filefd, &diskcon) => returns svirt_image_t:s0,c340,c524 > > Then it would call something like > > security_compute_av(qemucon, diskcon, ...file class..., ...ioctl perm...) > > to get a MAC decision from the kernel. This preserves the sVirt isolation > of individual QEMU instances. Got it. My problem here is that ioctl permission might be too strict. One use case for the helper is to bypass the ioctl permission, and only grant SCSI passthrough access for the specific case of persistent reservation commands. Would it make sense to also allow e.g. "lock" (and would it pass the SELinux policy)?

Also, what about AppArmor?