Hi!
In order to have non-crippled networking (ie, better than the user mode
stack), one needs to either use tap (via qemu-bridge-helper) or a far more
complex setup using vlan.  The helper requires the setuid bit to work, yet
is shipped without it.  I understand the rationale, yet in the current way
the package works, the setuid bit disappears after every qemu upgrade,
making networking "mysteriously" stop working.

Thus, could you please preserve this bit of configuration in some way?

This configuration is unsupported, until proper security model is implemented
upstream.

Thanks,

/mjt

19.10.2014 16:34, Michael Tokarev wrote:

Actually this is the same theme as #766664, merging the two.

Thanks,

/mjt

Hi,

From what I have read elsewhere, it looks like that you need to whitelist
the bridges on which qemu-system-helper can attach new devices (via
/etc/qemu/bridge.conf).

This seems like a reasonable balance if the whitelist is empty by default?

Or if the whitelist only includes the "virbr0" that libvirt setups
for basic NAT networking in VM...

Note that GNOME Boxes for example needs this helper to work in order
to provide a sane networking setup where the host can access the VM:
https://bugzilla.gnome.org/show_bug.cgi?id=677688

At the very least, you could document how users can opt to re-enable
permanently the setuid bit with "dpkg-statoverride" (and ensure that
the package doesn't mess with this).

Please reconsider your position on this topic.

Cheers,

Hi,

Having the binary only executable by group kvm (which is needed for
sensible virt performance) and doing a

     setcap cap_net_admin+ep /usr/lib/qemu/qemu-bridge-helper

and not a full u+s would further limit the attack surface. Having
gnome-boxes work out of the box in Stretch would be great.
Cheers,
 -- Guido

Hi,

Any chance this can get fixed?
 -- Guido

Hello,

I've reached this issue after experiencing the same issue.

Everytime I upgrade the `qemu-system-common` package (currently I have
1:6.1+dfsg-8) the suid bit on /usr/lib/qemu/qemu-bridge-helper is reset
and I have to manually set that again by applying the suggestion that is
most commonly found around:

$ sudo chmod u+s /usr/lib/qemu/qemu-bridge-helper

It would be great if the upgrade process would not interfere with this
decision of the user.

Also is this comment still relevant to the issue?

Thanks!

Best,

The original rationale is stated here:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=691138#10

...with a specific complaint:
 > For example, initially the helper always created interfaces in the
 > form tapNN (picking up next free number).  This way, it weren't
 > possible to know which iface will be in use by which guest, and
 > hence, for example, firewall rules can't be created for this
 > iface by system administrator.  Recently upstream added ability
 > to specify interface name from command line, but now it accepts
 > any interface, including ones which are used by the system -
 > eg, pppNN, for which firewall rules already created.

I'm by no means an expert on this, but I looked into this as best I
could. My findings were:

1. As stated earlier in this bug, the name of the target bridge
interface must be allowed via /etc/qemu/bridge.conf. This is sort-of
documented in:
https://wiki.qemu.org/Features/HelperNetworking

...with more details about per-user access control in the commit message:
https://github.com/qemu/qemu/commit/bdef79a2994d6f0383e07e9597675711662b3031

My testing showed that:
* When /etc/qemu/bridge.conf does not exist, the helper errors out.
* When the conf is empty, the helper errors out.
* When the conf has no matching ACL, the helper errors out.
* When the conf has a matching ACL, the helper succeeds.

I didn't test the per-user ACL functionality, but at least for baseline
functionality, the handling of the user-supplied bridge name seems secure.


2. The actual complaint quoted above was about the creation of the tap
devices, though, not the bridge. As far as I can tell, the helper does
not actually accept any user specification of the tap device naming.
Devices are still created as "tapN" (with the kernel picking the next
available number).
https://github.com/qemu/qemu/blob/f761b41a62c8ac12f26727dddc7ae1dd3ca9802b/qemu-bridge-helper.c#L348

I don't see anywhere in the code that renames the tap interface either,
and my testing via libvirt shows that interfaces remain named e.g.
"tap0" (libvirt doesn't seem to rename them either).


3. I also checked the usage of the supplied FD, which could have
potentially been used for user input. This socket is never read from,
however; the only use is to send the FD for /dev/net/tun back to the
parent via a control message.
https://github.com/qemu/qemu/blob/f761b41a62c8ac12f26727dddc7ae1dd3ca9802b/qemu-bridge-helper.c#L442

This FD is opened read/write, but that is not a problem, since the
socket is globally writable anyway (the kernel restricts some operations
when they are called).
https://www.kernel.org/doc/Documentation/networking/tuntap.txt


Am I missing any further unsafe operation of the helper? If there aren't
any specific known problems, then I suggest that the binary be given
setuid, or (if possible) CAP_NET_ADMIN. This would be consistent with
upstream and would alleviate this recurring bug.

It would also be helpful to provide a default /etc/qemu/bridge.conf; I
don't know what would be a good default, but even an empty file would be
a good starting point.

Thanks,
Corey

Is it sufficient to use

   dpkg-statoverride --add root root 04755 /usr/lib/qemu/qemu-bridge-helper

to fix this on a particular system?

(see man dpkg-statoverride for more info).

BTW, this helper should be moved to /usr/libexec/qemu/ now..

Thanks,

/mjt

1. This does persist across a reinstallation of qemu-system-common.
2. This does not apply immediately.

I suggest instead:

sudo dpkg-statoverride --update --add root root 04755 /usr/lib/qemu/qemu-bridge-helper

That will immediately change the mode of the existing file.


I also tested granting CAP_NET_ADMIN via:
sudo setcap cap_net_admin=ep /usr/lib/qemu/qemu-bridge-helper

This works as well, but I don't see any way to make it persist
across package reinstallation. Some research indicates this must
be done by post-install script.

https://superuser.com/questions/1606800/how-can-capabilities-be-set-for-a-file-across-upgrades-in-dpkg-like-for-permissi
https://serverfault.com/questions/784426/preserve-linux-capabilities-in-debian-package

Thanks,
Corey