[libvirt] [PATCH] virt-aa-helper: disallow VNC socket read permissions

[Cole Robinson]

On 04/13/2016 08:15 PM, Cole Robinson wrote:
> On 04/11/2016 03:06 AM, Cedric Bosdonnat wrote:
>> On Fri, 2016-04-08 at 18:55 +0200, Guido Günther wrote:
>>> On Fri, Apr 08, 2016 at 09:01:33AM -0400, Cole Robinson wrote:
>>>> From: Simon Arlott <bugzilla.redhat.simon at arlott.org>
>>>>
>>>> The VM does not need read permission for its own VNC socket to
>>>> create(),
>>>> bind(), accept() connections or to receive(), send(), etc. on
>>>> connections.
>>>>
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1312573
>>>> ---
>>>>  src/security/virt-aa-helper.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa
>>>> -helper.c
>>>> index 50d2a08..a4cb409 100644
>>>> --- a/src/security/virt-aa-helper.c
>>>> +++ b/src/security/virt-aa-helper.c
>>>> @@ -1062,7 +1062,7 @@ get_files(vahControl * ctl)
>>>>      for (i = 0; i < ctl->def->ngraphics; i++) {
>>>>          if (ctl->def->graphics[i]->type ==
>>>> VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
>>>>              ctl->def->graphics[i]->data.vnc.socket &&
>>>> -            vah_add_file(&buf, ctl->def->graphics[i]
>>>> ->data.vnc.socket, "rw"))
>>>> +            vah_add_file(&buf, ctl->def->graphics[i]
>>>> ->data.vnc.socket, "w"))
>>>>              goto cleanup;
>>>>      }
>>>
>>> ACK. I'd be great if Jamie or Cedric would comment also though on
>>> this
>>> security sensitive stuff.
>>
>> I'm not a socket expert, but it sounds weird to me that recv doesn't
>> need read access to the socket file. ACK to this patch as long a read
>> is really not needed.
>>
>> Jamie, any thoughts?
>>
> 
> Maybe someone with an apparmor setup can give it a test?
> 
>   <graphics type='vnc' socket='/tmp/foo.vnc'/>
> 
> Then open the VM locally in virt-manager (virt-viewer probably has a URI for
> it too but I didn't figure it out. Make sure the app isn't using the libvirt
> OpenGraphics API though since that won't go through the actual socket)
> 

Since there was no response (except ACKs), I've pushed this patch. If it's
wrong we can always revert

Thanks,
Cole