Does it make sense to drop priviledges from root:root? If yes, how can it
be decide which are the unneeded capabilities?
To fix UIO support, the minimal change required would be to keep
CAP_SYS_ADMIN when --user=root:root. Is this strategy preferable?

On Fri, Feb 2, 2018 at 9:22 PM, Ben Pfaff <b...@ovn.org> wrote:

> 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.
>
> 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

Reply via email to