Hi.

Not sure whether the problem here is actually in virt-manager, libvirt
or spice-client-glib-usb-acl-helper.
So pleace redirect as necessary.


I've just noted a very serious behaviour (which is also why I marked
it as critical and root security hole):

It seems that when plugging an USB device into ay computer where
I run virtmanager and where I'm connected to some VMs via SPICE,
that such USB devices are forwarded to that VM. o.O

I wonder how it chooses to which the device is redirected if there
are more VMs connected.


Now SPICE seemst to be the default for newly created VMs via libvirt
and the SPICE USB Redirector devices are created per default as well.
Also this isn't like the "USB Host Device" hardware in
virtmanager/libvirt/qemu, where one at least has to select *which*
USB device is connected.


Now since VM's are often used by people as kind of jails, e.g. running
untrustworthy OSes or programs in it, or since the VM may be just on
any remote server (from work or wherever), redirecting USB devices
without asking is IMHO a great security hole.
The USB device could contain just anything, my most recent hard disk
backup (and thus root passwords, dmcrypt keys etc). or my private
picture collection.


The 2nd critical security aspect of this:
A normal user(!) is apparently allowed to redirect a hardware device.
Not sure whether this is the typical policykit problem that locally
logged in users are handled as if they were root... but hell, one
cannot simply give normal users full access to USB devices if root
hasn't manually allowed them.


Cheers,
Chris

After some thinking I'd guess that there are actually two bugs:

1) That virt-manager automatically redirects without asking (because
apparently it now has a menu (Virtual Machine/Redirect USB Device) which
should do just that.
Actually IMHO it should generally happen only manually, because it's
quite annoying if everytime I put in some USB device, all my
virt-manager consoles would ask for it.

2) A security hole in the polkit configuration, in that it allows any
user actually redirect - and via that - gain full access to such USB
device.


So only if root has manually (via configuration) allowed a user to
redirect all or specific USB devices polkit should even grant the whole
thing.
But even then, it shouldn't happen automatically, but only when the user
really says "oh, yeah,... go and redirect".


Cheers,
Chris.

severity 764894 important
thanks

You can turn off usb auto redirecton in virt-manager's preferences. I
I'm open for discussion to changing this to off by default but until
then please let's not block the testing migration (the version in
jessie is affected by the same bug).
Cheers,
 -- Guido

To be honest, I'm quite surprised (or should I say shocked) how much
this "culture" of hiding away serious issues has taken it's way serious
issues.

1) critical & grave are basically the only real way for a user to see
about such issues on upgrade (when using apt-listbugs)
2) not having stuff moved to testing is probably just what one want (at
least if the affected versions aren't in yet)
3) having an issue release critical is probably again just what one
wants, if the issue is severe enough to justify it as that

and apart from that

4) I'd guess no-one would "count" the number of grave/critical bugs to
denigrate their maintainers. It's quite clear that packages like
iceweasel or likely also VM related stuff will out of their always have
more serious bugs open than something like the "tree" package.
This if course doesn't mean in any way that their maintainers would be
less capable or less passionate or whatever.



And the two issues in this bug quite clearly qualify for being severe
enough.
It's basically as if firefox would export parts of your harddisk to some
(e.g. https) websites automatically, just that this would affect even
more users.
No one would expect such behaviour, no one could say "well you triggered
that yourself by going to an https site" and especially no one would
accept if it was exporting data (and had the capability to) from
anywhere on the system ... like other users.
But that's basically what happens here, imagine that people still have
multi-user-systems (not everything in the world is a tablet),... now I
stick in some USB stick, and while the other user is doing stuff with
his VMs, it's exported to it. Even though root, never mounted it for one
of the two, or gave permissions on the device.
Well I don't think that this solves either of the two bugs I've reported
here.

AFAIU you mean the option in Edit/Preference/New VM/Add spice USB
redirection, right?
AFAICS this only controls what happens on the VM (i.e. server-side),...
and for the server it's absolutely no security problem to allow
redirections (since it's not his USB devices, but the client's).

The two problems we have here:
a) virt-manager (and perhaps virt-viewer as well?) exports the device
unconditionally, as long as it's allowed by the server (but a rogue
server will of course always allow).
On the VM window, there is the "Virtual Machine/Redirect USB Device"
menu entry, but here my devices are exported before I even go there.

b) The second, IMHO even more severe issue is:
Why does a normal user get permissions to redirect USB devices?
Even if virt-manager behaves buggy as described in (1), the user still
shouldn't have any permissions by default that polkit grants him access
to the USB device.
And access to the "full" USB device is granted! Not only to e.g. the
users own files on some filesystem *on* the USB device (in case it was a
mass storage device).
Well I already expected that which I wrote (2) above, but IMHO we have
some problem here than with the migration procedures.
Or will it migrate if we mark the current testing version as affected as
well?
Cause then we could keep the current severity, mark the testing version
and still have the new one migrated.


Oh and btw: Do you know where the issue (b) comes from? I'd guess it's
polkit, or rather some rules added by some package to it,... is it
spice-client-glib-usb-acl-helper as I've guessed (my polkit knowledge is
a little bit rusty ^^),... cause then I could clone the bug there and
every package could just deal with its own part of the two issues here.



Cheers,
Chris.

Hi,
much (which is good) please do all the work and figure out the
affected versions (I've just done so).

[..snip..]

No. I mean the confer key that handles usb redirection, see

  d81fd3c3af1abde1fa0e2bf3b79643f36836f45b

on https://anonscm.debian.org/cgit/pkg-libvirt/virt-manager.git/

See above. This should be fixed now with redirection defaulting to off
by default.

http://forums.fedoraforum.org/showthread.php?t=290933

which is

  /usr/share/polkit-1/actions/org.spice-space.lowlevelusbaccess.policy

and therefore allowed for interactive users (which makes sense). Feel
free to dup this to spice an keep me on cc. Thanks for raising this.
 -- Guido

We believe that the bug you reported is fixed in the latest version of
virt-manager, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 764894@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Guido Günther <agx@sigxcpu.org> (supplier of updated virt-manager package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)
Format: 1.8
Date: Sun, 12 Oct 2014 19:30:57 +0200
Source: virt-manager
Binary: virt-manager virtinst
Architecture: source all
Version: 1:1.0.1-3
Distribution: unstable
Urgency: medium
Maintainer: Debian Libvirt Maintainers <pkg-libvirt-maintainers@lists.alioth.debian.org>
Changed-By: Guido Günther <agx@sigxcpu.org>
Description:
 virt-manager - desktop application for managing virtual machines
 virtinst   - Programs to create and clone virtual machines
Closes: 764880 764894
Changes:
 virt-manager (1:1.0.1-3) unstable; urgency=medium
 .
   * [da12f60] details: Fix changing graphics type (bz 1083903)
     (Closes: #764880)
   * [d81fd3c] Don't enable usb redirection into the guest by default
     (Closes: #764894)
Checksums-Sha1:
 de213b8aacecc5fa64d62b6ace34e27341910511 2075 virt-manager_1.0.1-3.dsc
 8260f8c69bf813856e2919c7e19a2be494dd65fd 13884 virt-manager_1.0.1-3.debian.tar.xz
 7510541b4744fc17e0970fcef89a95b76a004ff9 881064 virt-manager_1.0.1-3_all.deb
 bbe3eb068c52899d2df430e7f4a299e33a3af2c0 163834 virtinst_1.0.1-3_all.deb
Checksums-Sha256:
 81ee5cd20ac53e0bcfd7f9f1d409f9b0d98468e5cbd53e7db585a2e07a6993bb 2075 virt-manager_1.0.1-3.dsc
 978651452016747133bf794488dd31d730bb371f42a20815a23f9c83b5cf27e5 13884 virt-manager_1.0.1-3.debian.tar.xz
 34402014c3786f6da10d2cb4f92eaff60870b0ee7f25654093f8f31ae74ff9b5 881064 virt-manager_1.0.1-3_all.deb
 9228497b1df56fc8c6f0f783dedd84f7d2b9ee99d4ddf40e277baf05a1b00282 163834 virtinst_1.0.1-3_all.deb
Files:
 ac717c34928bc676753eb808d710aa9e 2075 admin optional virt-manager_1.0.1-3.dsc
 7d57e28daffc433d62017999115b9d47 13884 admin optional virt-manager_1.0.1-3.debian.tar.xz
 b4a11bd0c348937dc9ecd6ac5b15cbe4 881064 admin optional virt-manager_1.0.1-3_all.deb
 fdcb8f9538381f02dc015cbf583d1059 163834 admin optional virtinst_1.0.1-3_all.deb
iQIVAwUBVDq78Qe4t7DqmBILAQgIZA//WthcR9JPOXMsrXxo7RsytpOBDwEJV+GG
wPj+67ddkuJIA/NN7ofFCiLnDmhpA/5B355yyj5ZMMD+5jTAi/1aJpPVeZUvmY8Q
dn8/Z4ulcrT12St+rSXz9eueyTosA8h9QnCrs24NyWn3LJwloirayFm+bVj/dX/M
7wGlLtKfGj4CpYdBF0WRqIbuboMy/Ao1VR9TVQVJoqgQOprupsR4CHXMGVJsCw4T
Z77auCKVldjO/SiL7FL6mikVFxWL8Tur4+FxC9Ndh9ecCpWrRi62Vm6lpdqsWCLx
96cklpbA2hj9GOAr/Rhq91117VQyMS6vw91wFUqnALFtOEF8Hs2CvlDYx1j/w6Xt
YV7LQJL1zWEHrjfw4gulf7FnFZny0LIW/OZ6yFTFk/1Os2J9AVduUIfhspU/Y73e
vHPzr3c7TunyrqDuqyupGgeWS10ykEeKTpfWJUO+84i8yPpw2KmGLWFEAbZ2fmEI
l/qbUYuFFbHspzNqLPMary6z1PQPTsoUErfrllbJp/QvNeZiqLYNUdcVLwb4i4o7
ZPa4XkBczVwBRH/5otkgl5yo9wOD4qegbV0EgzHePs9awB3kczqQmEZiLpJBizWZ
cIdB4hKHEHIqnUxPL6n/ojyMchGKTahjjGHN7GpBI+xAcxwwPMXIh46gtkWEIs2g
XpT7qS0SKE8=
=UVcb
-----END PGP SIGNATURE-----

clone 764894 -1 -2
reassign -1 virt-viewer
retitle -1 SECURITY - automatically redirects USB devices to guests
reassign -2 spice-client-glib-usb-acl-helper
retitle -2 SECURITY - normal users are allowed full access to USB devices per default
stop


Hi Guido.
Yeah I only read that in the end of your mail and added it only there :)
Thanks a lot :)

Still, to be honest, I think one should rather prevent migration from
testing (even if it's annoying) until one knows which versions are
affected... and better have a chance to warn the users :)
Great, thanks a lot...
just tested and verified it... it no longer redirects automatically,...

But it still does in virt-viewer, so I clone the bug there.
Yeah,... already expected spice-client-glib-usb-acl-helper to be the
"bad guy" ;-)
Well to be honest I think it's really dangerous what is done recently
there with polkit (even though it's not really polkit's fault itslef).

Rules for it are mostly written, so that interactive users have a lot
rights, which may sound okay on the first glance and which may work out
for *some* usage scenarios (tablets, single user desktops, etc.)...
But I think there are many valid usage scenarios which silently break
(in the security PoV) by that.

Apart from that, we've often seen bugs in polkit and/or the rules in the
past, and where these lax privileges caused even more pain and troubles
(I remember a case when polkit allowed local users to access the master
keys of dm-crypt devices o.O).
I'm cloning this now... hope that keeps you on CC.
Sure... thanks for dealing with it so quickly :)



Cheers,
Chris.

reopen 765017
stop

Hi Liung.
(keeping Guido CCed at his request)


I just cloned this bug from virt-manager to
spice-client-glib-usb-acl-helper.

As you can see from the previous discussion, there was an issue (the one
I usually named (1)) that virt-manager automatically redirected USB
devices fully to guests.

And there is another issue (I've numbered (2)) that, even if
virt-manager had a security-problematic default, it should have never
gotten the rights to do so.

Apparently these rights are automatically granted by the polkit rules
from your package.

IMHO, as laid out in e.g. my message #39, even interactive users
shouldn't be granted such powerful rights per se,... only if root has
really manually granted this in the policy (e.g. per user or for all
interactive users).


Cheers,
Chris.

Hello,

This problem is no longer present with current version of
libspice-client-glib-2.0-8

This is probably due to a change in the policykit and/or the helper
included with the library.

Either way, I cannot access devices for which I have write permission
since policykit denies the access and opening the device directly is not
attempted.

I guess you can close this.

Thanks

Michal

Is it possible that libspice-client-glib-2.0-8 should merely Recommend:
spice-client-glib-usb-acl-helper, rather than Depend: ing on it?

spice-client-glib-usb-acl-helper is one of the few setuid binaries on
debian systems, and if it isn't installed, it seems like the attack
surface would be reduced.

I'd be a perfectly happy spice-client user *without* the ability to
redirect USB devices to the VM i'm playing with, if it meant i didn't
have to worry about yet another setuid binary on my system.

Hi,

I recently adopted spice-gtk and usbredir so I looked into this. In v0.39, upstream removed setuid root from usb-acl-helper and replaced it with the CAP_FOWNER permission. The merge request was here for reference. I'm going to see if I can remove some of the custom debian/rules logic for usb-acl-helper to accommodate that change, which should fix this issue.

Additionally if anyone has problems with the CAP_FOWNER permissions, there's a second issue here to remove CAP_FOWNER which is currently open upstream which would further minimize the privileges needed.

Lance Lin <lqi254@protonmail.com>
GPG Fingerprint:  8CAD 1250 8EE0 3A41 7223  03EC 7096 F91E D75D 028F