Re: [libvirt] [PATCH v2 2/5] security, apparmor: add (Set|Restore)InputLabel

On Wed, Mar 21, 2018 at 3:03 PM, Jamie Strandboge <jamie(a)canonical.com&gt; wrote: > On Wed, 2018-03-21 at 13:10 +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, > > > > Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1755153 > > > > Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com&gt; > > --- > > src/security/security_apparmor.c | 45 > > ++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 45 insertions(+) > > > > diff --git a/src/security/security_apparmor.c > > b/src/security/security_apparmor.c > > index 7509552..3be8eeb 100644 > > --- a/src/security/security_apparmor.c > > +++ b/src/security/security_apparmor.c > > @@ -758,6 +758,48 @@ AppArmorRestoreMemoryLabel(virSecurityManagerPtr > > mgr, > > > > /* Called when hotplugging */ > > static int > > +AppArmorSetInputLabel(virSecurityManagerPtr mgr, > > + virDomainDefPtr def, > > + virDomainInputDefPtr input) > > +{ > > + switch ((virDomainInputType) input->type) { > > + case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: > > + if (!virFileExists(input->source.evdev)) { > > Check if input->type and input->source are NULL? Yes, good idea for defensive coding

> + 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; > > + > > secdef unneeded due to reload_profile. Thanks, I found why I wondered (being sure I have dropped them). I only dropped it on the "Set" functions before. Next version will have it dropped on "Restore" as well. > > + return reload_profile(mgr, def, NULL, false); > > +} > > + > > +/* Called when hotplugging */ > > +static int > > AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr, > > virDomainDefPtr def, > > virStorageSourcePtr src) > > @@ -1158,6 +1200,9 @@ virSecurityDriver virAppArmorSecurityDriver = { > > .domainSetSecurityMemoryLabel = AppArmorSetMemoryLabel, > > .domainRestoreSecurityMemoryLabel = > > AppArmorRestoreMemoryLabel, > > > > + .domainSetSecurityInputLabel = AppArmorSetInputLabel, > > + .domainRestoreSecurityInputLabel = AppArmorRestoreInputLabel, > > + > > .domainSetSecurityDaemonSocketLabel = > > AppArmorSetSecurityDaemonSocketLabel, > > .domainSetSecuritySocketLabel = > > AppArmorSetSecuritySocketLabel, > > .domainClearSecuritySocketLabel = > > AppArmorClearSecuritySocketLabel, > -- > Jamie Strandboge | http://www.canonical.com -- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd