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(a)canonical.com&gt; > --- > 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

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(a)canonical.com&gt; --- 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,

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(a)canonical.com&gt; --- 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 &&