[PATCH v2] virt-aa-helper: disallow graphics socket read permissions

[Christian Ehrhardt]

On Thu, May 28, 2020 at 12:45 PM Simon Arlott <libvirt at octiron.net> wrote:
>
> The VM does not need read permission for its own sockets to create,
> bind(), listen(), accept() connections or to recv(), send(), etc. on
> those connections.
>
> This was fixed in ab9569e5460d1e4737fe8b625c67687dc2204665
> (virt-aa-helper: disallow VNC socket read permissions),
> but then b6465e1aa49397367a9cd0f27110b9c2280a7385
> (graphics: introduce new listen type 'socket')
> and acc83afe333bfadd3f7f79091d38ca3d7da1eeb2
> (acc83afe333bfadd3f7f79091d38ca3d7da1eeb2) reverted it.
>
> Unless the read permission is omitted, VMs can connect to each other's
> VNC/graphics sockets.

As only the defined path from the XML is allowed that would only apply
if one specified the same path in each VM definition right?
That would be borderline to "intentionally configured that way".

> Signed-off-by: Simon Arlott <libvirt at octiron.net>
> ---
> Updated version that changes the test case too.
>
>  src/security/virt-aa-helper.c | 2 +-
>  tests/virt-aa-helper-test     | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index 6e6dd1b1db..fddbdafc41 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
[...]

>  <graphics type='vnc' socket='/var/lib/libvirt/qemu/myself.vnc'><listen type='address' address='0.0.0.0'/></graphics>

Isn't that option above converted to "-vnc unix:path"?
Hmm, no as ./tests/qemuxml2argvdata/graphics-vnc-remove-generated-socket.args
shows it is actually removed later.

But the attribute that virt-aa-helper parses in the lines you touched
is used for good cases as well, cases like these:
./tests/qemuxml2argvdata/graphics-vnc-socket-new-cmdline.args
./tests/qemuxml2argvdata/graphics-vnc-socket.args

In which case qemu would need to write to it as it is meant to [1]:
"... Rather than using listen/port, QEMU supports a socket attribute
for listening on a unix domain socket path Since 0.8.8 . ..."

An example would be:
./tests/qemuxml2argvdata/graphics-vnc-socket.xml-22-    <graphics type='vnc'>
./tests/qemuxml2argvdata/graphics-vnc-socket.xml:23:      <listen
type='socket' socket='/tmp/vnc.sock'/>
./tests/qemuxml2argvdata/graphics-vnc-socket.xml-24-    </graphics>
=>
./tests/qemuxml2argvdata/graphics-vnc-socket.args:27:-vnc unix:/tmp/vnc.sock \

And if I bluntly try that there are cases where qemu then creates that socket:
  $ qemu-system-x86_64 -vnc socket:/tmp/foobar
creates:
  srwxrwxr-x 1 paelzer paelzer 0 Sep  1 11:43 /tmp/foobar=

Therefore qemu would need the write permission to that value IMHO.
And as I said the concern of "VMs can connect to each other" would
only be true if the admin specifies the same path in each of them
intentionally.


[1]: https://libvirt.org/formatdomain.html#graphical-framebuffers

-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd