[libvirt] [PATCH 0/4] Apparmor support for less common devices
So far users added manual rules for most of these uncommon devices, but recent changes made some of the callbacks mandatory for hotplug so we should take shot at implementing them as those callbacks as well as for the initial start of a guest via virt-aa-helper. Christian Ehrhardt (4): security, apparmor: add (Set|Restore)MemoryLabel security, apparmor: add (Set|Restore)InputLabel virt-aa-helper: generate rules for passthrough input devices virt-aa-helper: generate rules for nvdimm memory src/security/security_apparmor.c | 101 +++++++++++++++++++++++++++++++++++++++ src/security/virt-aa-helper.c | 14 ++++++ 2 files changed, 115 insertions(+) -- 2.7.4
Recent changes have made implementing this mandatory to hot add any memory. Implementing this in apparmor fixes this as well as allows hot-add of nvdimm tpye memory with an nvdimmPath set generating a AppArmor rule for that path. Example hot adding: <memory model='nvdimm'> <source> <path>/tmp/nvdimm-test</path> </source> <target> <size unit='KiB'>524288</size> <node>0</node> </target> </memory> Creates now: "/tmp/nvdimm-test" rwk, Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/security_apparmor.c | 50 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index a989992..4ae1e3d 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -718,6 +718,53 @@ AppArmorRestoreSecurityDiskLabel(virSecurityManagerPtr mgr, /* Called when hotplugging */ static int +AppArmorSetMemoryLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ + virSecurityLabelDefPtr secdef; + + switch ((virDomainMemoryModel) mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + if (!secdef || !secdef->relabel) + return 0; + + if (!virFileExists(mem->nvdimmPath)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: \'%s\' does not exist"), + __func__, mem->nvdimmPath); + return -1; + } + + return reload_profile(mgr, def, mem->nvdimmPath, true); + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; + } + + return 0; +} + + +static int +AppArmorRestoreMemoryLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainMemoryDefPtr mem ATTRIBUTE_UNUSED) +{ + virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef || !secdef->relabel) + return 0; + + return reload_profile(mgr, def, NULL, false); +} + +/* Called when hotplugging */ +static int AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, virStorageSourcePtr src) @@ -1115,6 +1162,9 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainSetSecurityImageLabel = AppArmorSetSecurityImageLabel, .domainRestoreSecurityImageLabel = AppArmorRestoreSecurityImageLabel, + .domainSetSecurityMemoryLabel = AppArmorSetMemoryLabel, + .domainRestoreSecurityMemoryLabel = AppArmorRestoreMemoryLabel, + .domainSetSecurityDaemonSocketLabel = AppArmorSetSecurityDaemonSocketLabel, .domainSetSecuritySocketLabel = AppArmorSetSecuritySocketLabel, .domainClearSecuritySocketLabel = AppArmorClearSecuritySocketLabel, -- 2.7.4
On Tue, 2018-03-20 at 13:08 +0100, Christian Ehrhardt wrote:
Recent changes have made implementing this mandatory to hot add any memory. Implementing this in apparmor fixes this as well as allows hot-add of nvdimm tpye memory with an nvdimmPath set generating a AppArmor rule for that path.
Example hot adding: <memory model='nvdimm'> <source> <path>/tmp/nvdimm-test</path> </source> <target> <size unit='KiB'>524288</size> <node>0</node> </target> </memory> Creates now: "/tmp/nvdimm-test" rwk,
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/security_apparmor.c | 50 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index a989992..4ae1e3d 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -718,6 +718,53 @@ AppArmorRestoreSecurityDiskLabel(virSecurityManagerPtr mgr,
/* Called when hotplugging */ static int +AppArmorSetMemoryLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ + virSecurityLabelDefPtr secdef; + + switch ((virDomainMemoryModel) mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + if (!secdef || !secdef->relabel) + return 0; + + if (!virFileExists(mem->nvdimmPath)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: \'%s\' does not exist"), + __func__, mem->nvdimmPath); + return -1; + } + + return reload_profile(mgr, def, mem->nvdimmPath, true); + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; + } + + return 0; +} + + +static int +AppArmorRestoreMemoryLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainMemoryDefPtr mem ATTRIBUTE_UNUSED) +{ + virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef || !secdef->relabel) + return 0; +
This secdef check is done already in reload_profile.
+ return reload_profile(mgr, def, NULL, false);
Calling this with NULL means that virt-aa-helper will be called and remove the mem file, but will it also remove anything else that was hotplugged and not present in 'xml = virDomainDefFormat(def, ...)'? I've not looked at this in a while, so maybe it's ok (it seems like this is what the various other AppArmorRestore* functions do after all). A test case for hotplugging all these different devices and hotunplugging each would be good to show if hotunplugging one inadvertently removes rules for other devices that are still hotplugged.
+} + +/* Called when hotplugging */ +static int AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, virStorageSourcePtr src) @@ -1115,6 +1162,9 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainSetSecurityImageLabel = AppArmorSetSecurityImageLabel, .domainRestoreSecurityImageLabel = AppArmorRestoreSecurityImageLabel,
+ .domainSetSecurityMemoryLabel = AppArmorSetMemoryLabel, + .domainRestoreSecurityMemoryLabel = AppArmorRestoreMemoryLabel, + .domainSetSecurityDaemonSocketLabel = AppArmorSetSecurityDaemonSocketLabel, .domainSetSecuritySocketLabel = AppArmorSetSecuritySocketLabel, .domainClearSecuritySocketLabel = AppArmorClearSecuritySocketLabel, -- Jamie Strandboge | http://www.canonical.com
On Tue, Mar 20, 2018 at 9:50 PM, Jamie Strandboge <jamie@canonical.com> wrote:
On Tue, 2018-03-20 at 13:08 +0100, Christian Ehrhardt wrote:
Recent changes have made implementing this mandatory to hot add any memory. Implementing this in apparmor fixes this as well as allows hot-add of nvdimm tpye memory with an nvdimmPath set generating a AppArmor rule for that path.
Example hot adding: <memory model='nvdimm'> <source> <path>/tmp/nvdimm-test</path> </source> <target> <size unit='KiB'>524288</size> <node>0</node> </target> </memory> Creates now: "/tmp/nvdimm-test" rwk,
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/security_apparmor.c | 50 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index a989992..4ae1e3d 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -718,6 +718,53 @@ AppArmorRestoreSecurityDiskLabel(virSecurityManagerPtr mgr,
/* Called when hotplugging */ static int +AppArmorSetMemoryLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ + virSecurityLabelDefPtr secdef; + + switch ((virDomainMemoryModel) mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + if (!secdef || !secdef->relabel) + return 0; + + if (!virFileExists(mem->nvdimmPath)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: \'%s\' does not exist"), + __func__, mem->nvdimmPath); + return -1; + } + + return reload_profile(mgr, def, mem->nvdimmPath, true); + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; + } + + return 0; +} + + +static int +AppArmorRestoreMemoryLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainMemoryDefPtr mem ATTRIBUTE_UNUSED) +{ + virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef || !secdef->relabel) + return 0; +
This secdef check is done already in reload_profile.
I was following other examples, but you are right - I'll drop it in a V2
+ return reload_profile(mgr, def, NULL, false);
Calling this with NULL means that virt-aa-helper will be called and remove the mem file, but will it also remove anything else that was hotplugged and not present in 'xml = virDomainDefFormat(def, ...)'? I've not looked at this in a while, so maybe it's ok (it seems like this is what the various other AppArmorRestore* functions do after all).
Yeah I was following other examples that were present in the file already. It is a complex case to follow "what will happen"
A test case for hotplugging all these different devices and hotunplugging each would be good to show if hotunplugging one inadvertently removes rules for other devices that are still hotplugged.
I agree, first I ensured I actually test the requested code. Disks should be going through AppArmorRestoreSecurityImageLabel I ensured with GDB that they do on detach. reload_profile (mgr=0x7f96741060e0, def=0x7f967418b480, fn=0x0, append=false) at ../../../src/security/security_apparmor.c:287 That way we can be sure that attach/detach disks is a valid test for the reload_profile call with NULL. Test with two disks $ qemu-img create /var/lib/libvirt/images/hp-test.qcow 10M $ qemu-img create /var/lib/libvirt/images/hp-test2.qcow 10M root@b:~# cat hotplug-disk cat: hotplug-disk: No such file or directory root@b:~# cat hotplug-disk.xml <!-- qemu-img create /var/lib/libvirt/images/hp-test.qcow 10M --> <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/var/lib/libvirt/images/hp-test.qcow'/> <target dev='vdy' bus='virtio'/> </disk> root@b:~# cat hotplug-disk2.xml <!-- qemu-img create /var/lib/libvirt/images/hp-test.qcow 10M --> <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/var/lib/libvirt/images/hp-test2.qcow'/> <target dev='vdz' bus='virtio'/> </disk> root@b:~# cat /etc/apparmor.d/libvirt/libvirt-$(virsh dominfo b-test | awk '/^UUID:/ {print $2}').files | grep hp root@b:~# virsh attach-device b-test hotplug-disk.xml --live Device attached successfully root@b:~# cat /etc/apparmor.d/libvirt/libvirt-$(virsh dominfo b-test | awk '/^UUID:/ {print $2}').files | grep hp "/var/lib/libvirt/images/hp-test.qcow" rwk, root@b:~# virsh attach-device b-test hotplug-disk2.xml --live Device attached successfully root@b:~# cat /etc/apparmor.d/libvirt/libvirt-$(virsh dominfo b-test | awk '/^UUID:/ {print $2}').files | grep hp "/var/lib/libvirt/images/hp-test.qcow" rwk, "/var/lib/libvirt/images/hp-test2.qcow" rwk, root@b:~# virsh detach-device b-test hotplug-disk.xml --live Device detached successfully root@b:~# cat /etc/apparmor.d/libvirt/libvirt-$(virsh dominfo b-test | awk '/^UUID:/ {print $2}').files | grep hp "/var/lib/libvirt/images/hp-test2.qcow" rwk, The one not detached remains in the profile as it should. So it seems safe against the mentioned "accidental" removal of the other hotplugged devices. I can't see how to do so in a unit test atm, maybe down the road in a dep8 or qa test. But the test above should reassure you that it is not a generic problem currently.
+} + +/* Called when hotplugging */ +static int AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, virStorageSourcePtr src) @@ -1115,6 +1162,9 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainSetSecurityImageLabel = AppArmorSetSecurityImageLabel, .domainRestoreSecurityImageLabel = AppArmorRestoreSecurityImageLabel,
+ .domainSetSecurityMemoryLabel = AppArmorSetMemoryLabel, + .domainRestoreSecurityMemoryLabel = AppArmorRestoreMemoryLabel, + .domainSetSecurityDaemonSocketLabel = AppArmorSetSecurityDaemonSocketLabel, .domainSetSecuritySocketLabel = AppArmorSetSecuritySocketLabel, .domainClearSecuritySocketLabel = AppArmorClearSecuritySocketLabel, -- Jamie Strandboge | http://www.canonical.com
-- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd
d8116b5a "security: Introduce functions for input device hot(un)plug" implemented the code (Set|Restore)InputLabel for several security modules, this patch adds an AppArmor implementation for it as well. That fixes hot-plugging event input devices by generating a rule for the path that needs to be accessed. Example hot adding: <input type='passthrough' bus='virtio'> <source evdev='/dev/input/event0' /> </input> Creates now: "/dev/input/event0" rwk, Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/security_apparmor.c | 51 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 4ae1e3d..7924b9a 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -765,6 +765,54 @@ AppArmorRestoreMemoryLabel(virSecurityManagerPtr mgr, /* Called when hotplugging */ static int +AppArmorSetInputLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainInputDefPtr input) +{ + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + if (!secdef || !secdef->relabel) + return 0; + + switch ((virDomainInputType) input->type) { + case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: + if (!virFileExists(input->source.evdev)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: \'%s\' does not exist"), + __func__, input->source.evdev); + return -1; + } + return reload_profile(mgr, def, input->source.evdev, true); + break; + + case VIR_DOMAIN_INPUT_TYPE_MOUSE: + case VIR_DOMAIN_INPUT_TYPE_TABLET: + case VIR_DOMAIN_INPUT_TYPE_KBD: + case VIR_DOMAIN_INPUT_TYPE_LAST: + break; + } + + return 0; +} + + +static int +AppArmorRestoreInputLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainInputDefPtr input ATTRIBUTE_UNUSED) +{ + virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef || !secdef->relabel) + return 0; + + return reload_profile(mgr, def, NULL, false); +} + +/* Called when hotplugging */ +static int AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, virStorageSourcePtr src) @@ -1165,6 +1213,9 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainSetSecurityMemoryLabel = AppArmorSetMemoryLabel, .domainRestoreSecurityMemoryLabel = AppArmorRestoreMemoryLabel, + .domainSetSecurityInputLabel = AppArmorSetInputLabel, + .domainRestoreSecurityInputLabel = AppArmorRestoreInputLabel, + .domainSetSecurityDaemonSocketLabel = AppArmorSetSecurityDaemonSocketLabel, .domainSetSecuritySocketLabel = AppArmorSetSecuritySocketLabel, .domainClearSecuritySocketLabel = AppArmorClearSecuritySocketLabel, -- 2.7.4
On Tue, 2018-03-20 at 13:08 +0100, Christian Ehrhardt wrote:
d8116b5a "security: Introduce functions for input device hot(un)plug" implemented the code (Set|Restore)InputLabel for several security modules, this patch adds an AppArmor implementation for it as well.
That fixes hot-plugging event input devices by generating a rule for the path that needs to be accessed.
Example hot adding: <input type='passthrough' bus='virtio'> <source evdev='/dev/input/event0' /> </input> Creates now: "/dev/input/event0" rwk,
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/security_apparmor.c | 51 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 4ae1e3d..7924b9a 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -765,6 +765,54 @@ AppArmorRestoreMemoryLabel(virSecurityManagerPtr mgr,
/* Called when hotplugging */ static int +AppArmorSetInputLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainInputDefPtr input) +{ + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + if (!secdef || !secdef->relabel) + return 0; + + switch ((virDomainInputType) input->type) { + case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: + if (!virFileExists(input->source.evdev)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: \'%s\' does not exist"), + __func__, input->source.evdev); + return -1; + } + return reload_profile(mgr, def, input->source.evdev, true); + break; + + case VIR_DOMAIN_INPUT_TYPE_MOUSE: + case VIR_DOMAIN_INPUT_TYPE_TABLET: + case VIR_DOMAIN_INPUT_TYPE_KBD: + case VIR_DOMAIN_INPUT_TYPE_LAST: + break; + } + + return 0; +} + + +static int +AppArmorRestoreInputLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainInputDefPtr input ATTRIBUTE_UNUSED) +{ + virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef || !secdef->relabel) + return 0; + + return reload_profile(mgr, def, NULL, false); +} + +/* Called when hotplugging */ +static int AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, virStorageSourcePtr src) @@ -1165,6 +1213,9 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainSetSecurityMemoryLabel = AppArmorSetMemoryLabel, .domainRestoreSecurityMemoryLabel = AppArmorRestoreMemoryLabel,
+ .domainSetSecurityInputLabel = AppArmorSetInputLabel, + .domainRestoreSecurityInputLabel = AppArmorRestoreInputLabel, + .domainSetSecurityDaemonSocketLabel = AppArmorSetSecurityDaemonSocketLabel, .domainSetSecuritySocketLabel = AppArmorSetSecuritySocketLabel, .domainClearSecuritySocketLabel = AppArmorClearSecuritySocketLabel,
Same comments on this as for '[PATCH 1/4] security, apparmor: add (Set|Restore)MemoryLabel'. -- Jamie Strandboge | http://www.canonical.com
Input devices can passthrough an event device. This currently works only via hotplug where the AppArmor label is created via the domain label callbacks. This adds the virt-aa-helper support for passthrough input devices to generate rules for the needed paths from the initial guest definition as well. Example in domain xml: <input type='passthrough' bus='virtio'> <source evdev='/dev/input/event0' /> </input> Works to start now and creates: "/dev/input/event0" rw, Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1757085 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/virt-aa-helper.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 456cfce..1fc482d 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1157,6 +1157,13 @@ get_files(vahControl * ctl) } } + for (i = 0; i < ctl->def->ninputs; i++) { + if (ctl->def->inputs[i]->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) { + if (vah_add_file(&buf, ctl->def->inputs[i]->source.evdev, "rw") != 0) + goto cleanup; + } + } + for (i = 0; i < ctl->def->nnets; i++) { if (ctl->def->nets[i] && ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && -- 2.7.4
On Tue, 2018-03-20 at 13:08 +0100, Christian Ehrhardt wrote:
> Input devices can passthrough an event device. This currently works
> only via
> hotplug where the AppArmor label is created via the domain label
> callbacks.
>
> This adds the virt-aa-helper support for passthrough input devices to
> generate
> rules for the needed paths from the initial guest definition as well.
>
> Example in domain xml:
> <input type='passthrough' bus='virtio'>
> <source evdev='/dev/input/event0' />
> </input>
> Works to start now and creates:
> "/dev/input/event0" rw,
>
> Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1757085
>
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> ---
> src/security/virt-aa-helper.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-
> helper.c
> index 456cfce..1fc482d 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -1157,6 +1157,13 @@ get_files(vahControl * ctl)
> }
> }
>
> + for (i = 0; i < ctl->def->ninputs; i++) {
> + if (ctl->def->inputs[i]->type ==
> VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) {
'if (ctl->def->inputs[i] && ctl->def->inputs[i]->type ==
VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH)', no?
> + if (vah_add_file(&buf, ctl->def->inputs[i]-
> >source.evdev, "rw") != 0)
> + goto cleanup;
> + }
> + }
> +
> for (i = 0; i < ctl->def->nnets; i++) {
> if (ctl->def->nets[i] &&
> ctl->def->nets[i]->type ==
> VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
Adding test cases for this would be good.
--
Jamie Strandboge | http://www.canonical.com
On Tue, Mar 20, 2018 at 10:00 PM, Jamie Strandboge <jamie@canonical.com>
wrote:
> On Tue, 2018-03-20 at 13:08 +0100, Christian Ehrhardt wrote:
> > Input devices can passthrough an event device. This currently works
> > only via
> > hotplug where the AppArmor label is created via the domain label
> > callbacks.
> >
> > This adds the virt-aa-helper support for passthrough input devices to
> > generate
> > rules for the needed paths from the initial guest definition as well.
> >
> > Example in domain xml:
> > <input type='passthrough' bus='virtio'>
> > <source evdev='/dev/input/event0' />
> > </input>
> > Works to start now and creates:
> > "/dev/input/event0" rw,
> >
> > Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1757085
> >
> > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> > ---
> > src/security/virt-aa-helper.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-
> > helper.c
> > index 456cfce..1fc482d 100644
> > --- a/src/security/virt-aa-helper.c
> > +++ b/src/security/virt-aa-helper.c
> > @@ -1157,6 +1157,13 @@ get_files(vahControl * ctl)
> > }
> > }
> >
> > + for (i = 0; i < ctl->def->ninputs; i++) {
> > + if (ctl->def->inputs[i]->type ==
> > VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) {
>
> 'if (ctl->def->inputs[i] && ctl->def->inputs[i]->type ==
> VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH)', no?
>
Yes, checking the pointer to be valid is certainly better
Will be in a V2 of the series.
> > + if (vah_add_file(&buf, ctl->def->inputs[i]-
> > >source.evdev, "rw") != 0)
> > + goto cleanup;
> > + }
> > + }
> > +
> > for (i = 0; i < ctl->def->nnets; i++) {
> > if (ctl->def->nets[i] &&
> > ctl->def->nets[i]->type ==
> > VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
>
> Adding test cases for this would be good.
>
Yes, I agree - will be in a V2
> --
> Jamie Strandboge | http://www.canonical.com
--
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd
nvdimm memory is backed by a path on the host. This currently works only via hotplug where the AppArmor label is created via the domain label callbacks. This adds the virt-aa-helper support for nvdimm memory devices to generate rules for the needed paths from the initial guest definition as well. Example in domain xml: <memory model='nvdimm'> <source> <path>/tmp/nvdimm-base</path> </source> <target> <size unit='KiB'>524288</size> <node>0</node> </target> </memory> Works to start now and creates: "/tmp/nvdimm-base" rw, Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1757085 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/virt-aa-helper.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 1fc482d..0d0db0b 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1176,6 +1176,13 @@ get_files(vahControl * ctl) } } + for (i = 0; i < ctl->def->nmems; i++) { + if (ctl->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { + if (vah_add_file(&buf, ctl->def->mems[i]->nvdimmPath, "rw") != 0) + goto cleanup; + } + } + if (ctl->def->virtType == VIR_DOMAIN_VIRT_KVM) { for (i = 0; i < ctl->def->nnets; i++) { virDomainNetDefPtr net = ctl->def->nets[i]; -- 2.7.4
On Tue, 2018-03-20 at 13:08 +0100, Christian Ehrhardt wrote:
nvdimm memory is backed by a path on the host. This currently works only via hotplug where the AppArmor label is created via the domain label callbacks.
This adds the virt-aa-helper support for nvdimm memory devices to generate rules for the needed paths from the initial guest definition as well.
Example in domain xml: <memory model='nvdimm'> <source> <path>/tmp/nvdimm-base</path> </source> <target> <size unit='KiB'>524288</size> <node>0</node> </target> </memory> Works to start now and creates: "/tmp/nvdimm-base" rw,
Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1757085
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/virt-aa-helper.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa- helper.c index 1fc482d..0d0db0b 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1176,6 +1176,13 @@ get_files(vahControl * ctl) } }
+ for (i = 0; i < ctl->def->nmems; i++) { + if (ctl->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
'if (ctl->def->mems[i] && ctl->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM)', no?
+ if (vah_add_file(&buf, ctl->def->mems[i]->nvdimmPath, "rw") != 0) + goto cleanup; + } + } + if (ctl->def->virtType == VIR_DOMAIN_VIRT_KVM) { for (i = 0; i < ctl->def->nnets; i++) { virDomainNetDefPtr net = ctl->def->nets[i];
Adding a test case for this would be good. -- Jamie Strandboge | http://www.canonical.com
participants (2)
-
Christian Ehrhardt -
Jamie Strandboge