Ben Pfaff <b...@ovn.org> writes: > Well, is it ever useful to be able to drop unneeded capabilities while > retaining the same uid/gid? It certainly sounds like a reasonable thing > to want to do. I'm reluctant to apply this without at least considering > that possibility.
Let me think about it a bit more. When I originally suggested shunting the setuid code-path, I didn't consider this case. There could be an alternative. I suggested this in response to the original proposal (add CAP_SYS_ADMIN to the list of retained privs). Certainly, I don't want to allow CAP_SYS_ADMIN to be retained (after all, with CAP_NET_ADMIN and CAP_SYS_ADMIN, there's really not much reason to change uid from root at all - for all functional purposes the process will be root). Maybe there's a way to see that the user will be root from the systemd scripts and not pass the "--user=XXX:YYY" option. > On Fri, Feb 02, 2018 at 12:30:50AM -0200, Marcos Felipe Schwarz wrote: >> Is there any reason why they couldn't be the same? >> >> One reason in favor is that the option for running ovs as a non-root user >> can't be deactivated correctly by the means in the documentation [1]. It >> states that setting OVS_USER_ID to 'root:root', or commenting the variable >> out in /etc/sysconfig/openvswitch should revert the behavior back to normal. >> The patch above enforces the expected behaviour without requiring >> tinkering both >> ovs-vswitchd and ovsdb-server systemd scripts to remove the "--user >> OVS_USER_ID" parameter that is hardcoded. >> >> [1] Non-root User Support: >> https://github.com/openvswitch/ovs/blob/master/rhl/README.RHEL.rst >> >> On Thu, Feb 1, 2018 at 10:23 PM, Ben Pfaff <b...@ovn.org> wrote: >> >> > OK, I understand now that the goal is that passing "--user $UID" should >> > not call setuid() or setgid(). In that case, though, why pass the >> > --user option at all? I believe that, with this patch, "--user $UID" >> > and "" yield equivalent behavior. >> > >> > Thanks, >> > >> > Ben. >> > >> > On Thu, Feb 01, 2018 at 09:34:12PM -0200, Marcos Felipe Schwarz wrote: >> > > Hi Ben, >> > > >> > > I agree that the purpose of the canges was not cleared in the patch. The >> > > full discussion is at >> > > https://mail.openvswitch.org/pipermail/ovs-discuss/2018- >> > January/046061.html. >> > > But in a nutshell, this change is a fancy way to avoid the actual >> > problem, >> > > that's why it's not intuitive. The actual problem is the current method >> > to >> > > change the ovs user at (daemon_become_new_user_linux), drops the >> > > CAP_SYS_ADMIN capability, even when you are "changing" from root to >> > itself, >> > > which occurs in rhel based distros when you use OVS_USER="root:toot". My >> > > PoC solution [2] was to force the CAP_SYS_ADMIN during the user change, >> > but >> > > this would give more permission to openvswitch process than necessary on >> > > most of the situations, for instance when DPDK and UIO are not required. >> > > The fix in the current patch was proposed by Aaron [3], which involves to >> > > avoid changing the user when it is already the desired user (and group), >> > > this fixes the problem without the downsides of the initial solution and >> > > avoids unnecessary calls. >> > > >> > > [2] >> > > https://mail.openvswitch.org/pipermail/ovs-discuss/2018- >> > January/045959.html >> > > [3] >> > > https://mail.openvswitch.org/pipermail/ovs-discuss/2018- >> > January/045973.html >> > > Regards, >> > > >> > > Marcos Schwarz >> > > >> > > On Thu, Feb 1, 2018 at 7:27 PM, Ben Pfaff <b...@ovn.org> wrote: >> > > >> > > > On Thu, Feb 01, 2018 at 03:42:03PM -0200, Marcos Felipe Schwarz wrote: >> > > > > Since 2.8.0 OVS runs as non-root user on rhel distros, but the >> > current >> > > > > implementation breaks the ability to run as root with DPDK and as a >> > > > > consequence there is no way possible to use UIO drivers on kernel >> > 4.0 and >> > > > > newer [1, 2]. >> > > > > [1] http://dpdk.org/browse/dpdk/commit/?id=cdc242f260e766 >> > > > > bd95a658b5e0686a62ec04f5b0 >> > > > > [2] https://www.kernel.org/doc/Documentation/vm/pagemap.txt >> > > > > >> > > > > Fixes: e3e738a3d058 ("redhat: allow dpdk to also run as non-root >> > user") >> > > > > Signed-off-by: Marcos Schwarz <marcos.f....@gmail.com> >> > > > > Acked-by: Aaron Conole <acon...@redhat.com> >> > > > >> > > > Thanks for the patch. >> > > > >> > > > I understand from the commit message the ultimate goal of this patch. >> > I >> > > > don't yet understand what change it is actually making. In particular, >> > > > what is the purpose of the following change; what does it do? >> > > > >> > > > > --- a/lib/daemon-unix.c >> > > > > +++ b/lib/daemon-unix.c >> > > > > @@ -1047,5 +1047,6 @@ daemon_set_new_user(const char *user_spec) >> > > > > } >> > > > > } >> > > > > >> > > > > - switch_user = true; >> > > > > + if (!uid_verify(uid) || !gid_verify(gid)) >> > > > > + switch_user = true; >> > > > > } >> > > > >> > > > Thanks, >> > > > >> > > > Ben. >> > > > >> > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
