Daniele Di Proietto <diproiet...@ovn.org> writes: > Hi Aaron, apologies for the delay, > > I took another look at the series, and I'm not convinced this is the right > solution to the problem. > > This obviously has nothing to do with the quality of the code. > > While the hardness amplification might help against symlinks, it doesn't > protect against some > type of race conditions. > > There's still a window between 'rte_vhost_driver_register()' and > 'vhost_set_permissions()' that > can be exploited to change permissions of other sockets in the ovs rundir > (which is the default > vhostuser socket directory). ovs_kchown() doesn't guarantee that the socket > is the one we > intended. By introducing a sleep between 'rte_vhost_driver_register()' and > 'vhost_set_permissions()' (and restarting some daemons in between) I was able > to trick > ovs-vswitchd into changing the permissions of the ovn nb database socket. I > suspect someone > more expert than me could find similar and more dangerous exploits.
Okay. > Alternative approaches: > > 1) vhostuser client mode. It prevents this (and other) problem(s) and it is > IMHO the best way to > deploy vhostuser in production. I know that it requires a recent version of > QEMU, but this whole > stack is still pretty new, I don't see many ways around this. > 2) Do we want to do chmod/chown in the lower layers of the software stack? > Then we need > some support in DPDK. Using fchmod() and fchown() in DPDK seems better to me, > because it's > possible to do it without any race conditions. The same thing is not > possible in OVS (it would be > possible if DPDK gave us access to the fd before binding). > 3) If it's not possible to do it in the lower layers without any races, I > think it's best not to hide the > hack, but to do it in outside of Open vSwitch, where there might be more > context about the > other running daemons, or the vhost-sock-dir. As mentioned before, a manual > intervention will > not survive an ovs-vswitchd restart, but an ovs restart will also break the > connection to the > existing VM (that's why I think client mode is a much better approach). Agreed. Let's drop this effort. > Again, this is just my opinion, but I'm really not comfortable maintaining > this. > > Daniele > > 2016-08-19 16:48 GMT-07:00 Aaron Conole <acon...@redhat.com>: > > Currently, when using Open vSwitch with DPDK and qemu guests, the recommended > method for joining the guests is via the dpdkvhostuser interface. This > interface uses Unix Domain sockets to communicate. When these sockets are > created, they inherit the permissions and ownership from the vswitchd > process. > This can lead to an undesirable state where the QEMU process cannot use the > socket file until manual intervention is performed (via `chown` and/or > `chmod` > calls). > > This patchset gives the ability to set the permissions and ownership of all > dpdkvhostuser sockets from the database, avoiding the manual intervention > required to connect QEMU and OVS via DPDK. > > The first patch adds chmod and chown calls to lib, with unit tests. The > second patch adds a hardness amplification version as described in the > paper "Portably Solving File TOCTTOU Races with Hardness Amplification" > found at > > https://www.usenix.org/legacy/event/fast08/tech/full_papers/tsafrir/tsafrir_html/index.html, > while the third patch hooks those calls into the > netdev_dpdk_vhost_user_construct function, after the socket is created. > > Changes from v3: > * Replaced patch 2/3 with hardness amplification version. Retested on RHEL7 > and validated the travis builds. > > Changes from v2: > * Added a new 2nd patch to series for chmod/chown on already opened files. > There exist known implementations for other systems, including FreeBSD, but > only linux is implemented. ENOTSUP is set when these calls fail on > non-linux > systems. > > Aaron Conole (3): > chutil: introduce a new change-utils lib > chutil: Add hardness amplification versions of chmod/chown > netdev-dpdk: Support user-defined socket attribs > > INSTALL.DPDK.md | 8 + > configure.ac | 2 +- > lib/automake.mk | 2 + > lib/chutil-unix.c | 652 > +++++++++++++++++++++++++++++++++++++++++++++++++++ > lib/chutil.h | 36 +++ > lib/daemon-unix.c | 149 +----------- > lib/netdev-dpdk.c | 37 +++ > tests/automake.mk | 2 + > tests/library.at | 5 + > tests/test-chutil.c | 297 +++++++++++++++++++++++ > vswitchd/vswitch.xml | 23 ++ > 11 files changed, 1068 insertions(+), 145 deletions(-) > create mode 100644 lib/chutil-unix.c > create mode 100644 lib/chutil.h > create mode 100644 tests/test-chutil.c > > -- > 2.5.5 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
