On Wed, Jan 17, 2018 at 06:53:30PM +0800, Jason Wang wrote: > > > On 2018年01月17日 18:31, Daniel P. Berrange wrote: > > On Tue, Jan 16, 2018 at 03:18:24PM -0800, Shaun Reitan wrote: > > > This patch replaces the patch I sent yesturday. This one fixes > > > a bug in my original code as well as corrects a few styling > > > issues. Hopfully this one comes out correct! Sorry for the > > > inconvienece. > > > When currently using -netdev bridge or -netdev tap with a helper > > > you are unable to set an ifname. This patch adds that ability so > > > that you can now specify an ifname. > > I really don't think users should be allowed to override the > > ifname when using the setuid bridge helper. This allows an > > unprivileged users to create tap devices that may confuse the > > sysadmin, and/or network mgmt scripts. > > Ok, I drop it from my queue. > > > > > eg consider the user asks for a tap device called eth1. To the > > sysadmin the user's tap device now looks like a physical NIC. > > This can be even worse if the host does physical NIC hotplug, > > or uses SRIOV. eg consider the host as eth0 -> eth7 for SRIOV > > NICs, and eth3 is given to a guest. Now a user uses the setuid > > helper to ask for a TAP called eth3. When the SRIOV device is > > later released by the guest it will end up called eth8, as the > > TAP device occupies eth3. In bad cases this could even cause > > the host mgmt layer to configure bogus addresses on the eth3 > > TAP device instead of the SRIOV device. > > It looks to me that mgmt should not assume the type or location of device > just from its name. Ethtool should be used to do this.
Yes, a well written tool, or a prudent administrator should be careful and validate the devices they use, but most have assumptions they make which could easily be violated here. > > If we want to allow ifname to be set via the setuid helper, then IMHO, > > the config file for the helper *must* whitelist the various permitted > > naming patterns. > > Good point but this only work for e.g default helper. Qemu allows 3rd helper > which can do anything they want. If anyone writes a custom setuid helper, they are responsible for writing to in a way that doesn't open security holes, or expose the admin to intentionally misleading setup. IOW, that is somebody else's problem. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|