<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 21, 2018 at 3:23 PM, Christian Ehrhardt <span dir="ltr"><<a href="mailto:christian.ehrhardt@canonical.com" target="_blank">christian.ehrhardt@canonical.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="gmail-h5">On Wed, Mar 21, 2018 at 3:03 PM, Jamie Strandboge <span dir="ltr"><<a href="mailto:jamie@canonical.com" target="_blank">jamie@canonical.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail-m_3491880444434606463HOEnZb"><div class="gmail-m_3491880444434606463h5">On Wed, 2018-03-21 at 13:10 +0100, Christian Ehrhardt wrote:<br>
> d8116b5a "security: Introduce functions for input device hot(un)plug"<br>
> implemented the code (Set|Restore)InputLabel for several security<br>
> modules,<br>
> this patch adds an AppArmor implementation for it as well.<br>
><br>
> That fixes hot-plugging event input devices by generating a rule for<br>
> the<br>
> path that needs to be accessed.<br>
><br>
> Example hot adding:<br>
>   <input type='passthrough' bus='virtio'><br>
>      <source evdev='/dev/input/event0' /><br>
>   </input><br>
> Creates now:<br>
>   "/dev/input/event0" rwk,<br>
><br>
> Fixes: <a href="https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1755153" rel="noreferrer" target="_blank">https://bugs.launchpad.net/ubu<wbr>ntu/+source/libvirt/+bug/17551<wbr>53</a><br>
><br>
> Signed-off-by: Christian Ehrhardt <<a href="mailto:christian.ehrhardt@canonical.com" target="_blank">christian.ehrhardt@canonical.<wbr>com</a>><br>
> ---<br>
>  src/security/security_apparmor<wbr>.c | 45<br>
> ++++++++++++++++++++++++++++++<wbr>++++++++++<br>
>  1 file changed, 45 insertions(+)<br>
><br>
> diff --git a/src/security/security_apparm<wbr>or.c<br>
> b/src/security/security_apparm<wbr>or.c<br>
> index 7509552..3be8eeb 100644<br>
> --- a/src/security/security_apparm<wbr>or.c<br>
> +++ b/src/security/security_apparm<wbr>or.c<br>
> @@ -758,6 +758,48 @@ AppArmorRestoreMemoryLabel(vir<wbr>SecurityManagerPtr<br>
> mgr,<br>
><br>
>  /* Called when hotplugging */<br>
>  static int<br>
> +AppArmorSetInputLabel(virSecu<wbr>rityManagerPtr mgr,<br>
> +                      virDomainDefPtr def,<br>
> +                      virDomainInputDefPtr input)<br>
> +{<br>
> +    switch ((virDomainInputType) input->type) {<br>
> +    case VIR_DOMAIN_INPUT_TYPE_PASSTHRO<wbr>UGH:<br>
> +        if (!virFileExists(input->source.<wbr>evdev)) {<br>
<br>
</div></div>Check if input->type and input->source are NULL?</blockquote><div><br></div></div></div><div>Yes, good idea for defensive coding </div></div></div></div></blockquote><div><br></div><div>Actually input->type (just as <span style="font-family:monospace"><span style="color:rgb(0,0,0);background-color:rgb(255,255,255)">mem->model) are int's</span><br></span></div><div><span style="font-family:monospace"><span style="color:rgb(0,0,0);background-color:rgb(255,255,255)">But the base can be checked for sure.</span></span></div><div> <span style="color:rgb(80,0,80)"> </span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="gmail-h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail-m_3491880444434606463h5">
> +            virReportError(VIR_ERR_INTERNA<wbr>L_ERROR,<br>
> +                           _("%s: \'%s\' does not exist"),<br>
> +                           __func__, input->source.evdev);<br>
> +            return -1;<br>
> +        }<br>
> +        return reload_profile(mgr, def, input->source.evdev, true);<br>
> +        break;<br>
> +<br>
> +    case VIR_DOMAIN_INPUT_TYPE_MOUSE:<br>
> +    case VIR_DOMAIN_INPUT_TYPE_TABLET:<br>
> +    case VIR_DOMAIN_INPUT_TYPE_KBD:<br>
> +    case VIR_DOMAIN_INPUT_TYPE_LAST:<br>
> +        break;<br>
> +    }<br>
> +<br>
> +    return 0;<br>
> +}<br>
> +<br>
> +<br>
> +static int<br>
> +AppArmorRestoreInputLabel(vir<wbr>SecurityManagerPtr mgr,<br>
> +                          virDomainDefPtr def,<br>
> +                          virDomainInputDefPtr input<br>
> ATTRIBUTE_UNUSED)<br>
> +{<br>
> +    virSecurityLabelDefPtr secdef =<br>
> +        virDomainDefGetSecurityLabelDe<wbr>f(def,<br>
> SECURITY_APPARMOR_NAME);<br>
> +<br>
> +    if (!secdef || !secdef->relabel)<br>
> +        return 0;<br>
> +<br>
<br>
</div></div>secdef unneeded due to reload_profile.</blockquote><div><br></div></div></div><div>Thanks, I found why I wondered (being sure I have dropped them).</div><div>I only dropped it on the "Set" functions before.</div><div>Next version will have it dropped on "Restore" as well.</div><span class="gmail-"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail-m_3491880444434606463HOEnZb"><div class="gmail-m_3491880444434606463h5">
> +    return reload_profile(mgr, def, NULL, false);<br>
> +}<br>
> +<br>
> +/* Called when hotplugging */<br>
> +static int<br>
>  AppArmorSetSecurityImageLabel(<wbr>virSecurityManagerPtr mgr,<br>
>                                virDomainDefPtr def,<br>
>                                virStorageSourcePtr src)<br>
> @@ -1158,6 +1200,9 @@ virSecurityDriver virAppArmorSecurityDriver = {<br>
>      .domainSetSecurityMemoryLabel       = AppArmorSetMemoryLabel,<br>
>      .domainRestoreSecurityMemoryLa<wbr>bel   =<br>
> AppArmorRestoreMemoryLabel,<br>
><br>
> +    .domainSetSecurityInputLabel        = AppArmorSetInputLabel,<br>
> +    .domainRestoreSecurityInputLab<wbr>el    = AppArmorRestoreInputLabel,<br>
> +<br>
>      .domainSetSecurityDaemonSocket<wbr>Label =<br>
> AppArmorSetSecurityDaemonSocke<wbr>tLabel,<br>
>      .domainSetSecuritySocketLabel       =<br>
> AppArmorSetSecuritySocketLabel<wbr>,<br>
>      .domainClearSecuritySocketLabe<wbr>l     =<br>
> AppArmorClearSecuritySocketLab<wbr>el,<br>
--<br>
</div></div><span class="gmail-m_3491880444434606463HOEnZb"><font color="#888888">Jamie Strandboge             | <a href="http://www.canonical.com" rel="noreferrer" target="_blank">http://www.canonical.com</a></font></span></blockquote></span></div><span class="gmail-HOEnZb"><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div class="gmail-m_3491880444434606463gmail_signature"><div dir="ltr"><div><div dir="ltr"><span style="color:rgb(136,136,136);font-size:12.8px">Christian Ehrhardt</span><div style="color:rgb(136,136,136);font-size:12.8px">Software Engineer, Ubuntu Server</div><div style="color:rgb(136,136,136);font-size:12.8px">Canonical Ltd</div></div></div></div></div>
</font></span></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><span style="color:rgb(136,136,136);font-size:12.8px">Christian Ehrhardt</span><div style="color:rgb(136,136,136);font-size:12.8px">Software Engineer, Ubuntu Server</div><div style="color:rgb(136,136,136);font-size:12.8px">Canonical Ltd</div></div></div></div></div>
</div></div>