[libvirt] [PATCH] virt-aa-helper: disallow VNC socket read permissions
From: Simon Arlott <bugzilla.redhat.simon@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; } -- 2.5.5
On Fri, Apr 08, 2016 at 09:01:33AM -0400, Cole Robinson wrote:
From: Simon Arlott <bugzilla.redhat.simon@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. Cheers, -- Guido
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@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? -- Cedric
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@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) Thanks, Cole
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@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
participants (3)
-
Cedric Bosdonnat -
Cole Robinson -
Guido Günther