<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 25, 2017 at 8:48 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wed, 2017-09-20 at 16:59 +0200, Christian Ehrhardt wrote:<br>
> libvirt allows spaces in vm names, there were issues in the past but<br>
> it<br>
> seems not removed so the assumption has to be that spaces are<br>
> continuing<br>
> to be allowed.<br>
><br>
> Therefore virt-aa-helper should not reject spaces in vm names anymore<br>
> if<br>
> it is goign to be refused causing issues then the parser or xml<br>
> schema<br>
> should do so.<br>
> Apparmor rules are in quotes, so a space in a path based on the name<br>
> works.<br>
><br>
> Signed-off-by: Christian Ehrhardt <<a href="mailto:christian.ehrhardt@canonical.com">christian.ehrhardt@canonical.<wbr>com</a>><br>
> ---<br>
>  src/security/virt-aa-helper.c | 2 +-<br>
>  1 file changed, 1 insertion(+), 1 deletion(-)<br>
><br>
> diff --git a/src/security/virt-aa-helper.<wbr>c b/src/security/virt-aa-<br>
> helper.c<br>
> index d1518ea..5f4519d 100644<br>
> --- a/src/security/virt-aa-helper.<wbr>c<br>
> +++ b/src/security/virt-aa-helper.<wbr>c<br>
> @@ -449,7 +449,7 @@ valid_name(const char *name)<br>
>  {<br>
>      /* just try to filter out any dangerous characters in the name<br>
> that can be<br>
>       * used to subvert the profile */<br>
> -    const char *bad = " /[]*";<br>
> +    const char *bad = "/[]*";<br>
><br>
>      if (strlen(name) == 0)<br>
>          return -1;<br>
<br>
</div></div>Your justification seems reasonable. It does mean that we'll need<br>
always quote rules that use def->name and looking at virt-aa-helper.c,<br>
that seems to be the case.<br>
<br>
All that said, I was surprised that tests/virt-aa-helper-test didn't<br>
need to be updated, but, indeed, this is a testing gap.<br>
<br>
+1 as is, but perhaps in a follow-up patch you could expand bad to be:<br>
<br>
  const char *bad = "/[]{}?^,\"*";<br></blockquote><div><br></div><div>Yep, makes sense.</div><div>Found a (minor) cleanup along the way - patches will follow shortly as new thread (no need to splice it into this thread I think)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
'{', '}', '?', '^', ',' and '"' are characters used in AARE (see<br>
'Globbing' in 'man apparmor.d') and add tests to tests/virt-aa-helper-<br>
test for this.<br>
<br>
Thanks!<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Jamie Strandboge             | <a href="http://www.canonical.com" rel="noreferrer" target="_blank">http://www.canonical.com</a></font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="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>