[v2] Apparmor support for less common devices

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

Posted by Christian Ehrhardt 7 years, 10 months ago

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@canonical.com>
---
 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)) {
+            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)
@@ -1158,6 +1200,9 @@ virSecurityDriver virAppArmorSecurityDriver = {
     .domainSetSecurityMemoryLabel       = AppArmorSetMemoryLabel,
     .domainRestoreSecurityMemoryLabel   = AppArmorRestoreMemoryLabel,
 
+    .domainSetSecurityInputLabel        = AppArmorSetInputLabel,
+    .domainRestoreSecurityInputLabel    = AppArmorRestoreInputLabel,
+
     .domainSetSecurityDaemonSocketLabel = AppArmorSetSecurityDaemonSocketLabel,
     .domainSetSecuritySocketLabel       = AppArmorSetSecuritySocketLabel,
     .domainClearSecuritySocketLabel     = AppArmorClearSecuritySocketLabel,
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

Posted by Jamie Strandboge 7 years, 10 months ago

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@canonical.com>
> ---
>  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?

> +            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.

> +    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--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

Posted by Christian Ehrhardt 7 years, 10 months ago

On Wed, Mar 21, 2018 at 3:03 PM, Jamie Strandboge <jamie@canonical.com>
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@canonical.com>
> > ---
> >  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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

Posted by Christian Ehrhardt 7 years, 10 months ago

On Wed, Mar 21, 2018 at 3:23 PM, Christian Ehrhardt <
christian.ehrhardt@canonical.com> wrote:

>
>
> On Wed, Mar 21, 2018 at 3:03 PM, Jamie Strandboge <jamie@canonical.com>
> 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@canonical.com>
>> > ---
>> >  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
>

Actually input->type (just as mem->model) are int's
But the base can be checked for sure.


> > +            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
>



-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 1/5] security, apparmor: add (Set|Restore)MemoryLabel
[libvirt] [PATCH v2 2/5] security, apparmor: add (Set|Restore)InputLabel
[libvirt] [PATCH v2 3/5] virt-aa-helper: generate rules for passthrough input devices
[libvirt] [PATCH v2 4/5] virt-aa-helper: generate rules for nvdimm memory
[libvirt] [PATCH v2 5/5] virt-aa-helper: test: check for expected profile content