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

Reply via email to