Re: [Question] Should `CAP_NET_ADMIN` be needed when opening `/dev/ppp`?

[Guillaume Nault <g.nault@alphalink.fr>]

On Tue, May 03, 2016 at 01:23:34PM +0200, Hannes Frederic Sowa wrote:
> On Tue, May 3, 2016, at 12:35, Richard Weinberger wrote:
> > On Tue, May 3, 2016 at 12:12 PM, Guillaume Nault <g.nault@alphalink.fr>
> > wrote:
> > > On Sun, May 01, 2016 at 09:38:57PM +0800, Wang Shanker wrote:
> > >> static int ppp_open(struct inode *inode, struct file *file)
> > >> {
> > >>       /*
> > >>        * This could (should?) be enforced by the permissions on /dev/ppp.
> > >>        */
> > >>       if (!capable(CAP_NET_ADMIN))
> > >>               return -EPERM;
> > >>       return 0;
> > >> }
> > >> ```
> > >>
> > >> I wonder why CAP_NET_ADMIN is needed here, rather than leaving it to the
> > >> permission of the device node. If there is no need, I suggest that the
> > >> CAP_NET_ADMIN check be removed.
> > >>
> > > If this test was removed here, then it'd have to be added again in the
> > > PPPIOCNEWUNIT ioctl, at the very least, because creating a netdevice
> > > should require CAP_NET_ADMIN. Therefore that wouldn't help for your
> > > case.
> > > I don't know why the test was placed in ppp_open() in the first place,
> > > but changing it now would have side effects on user space. So I'd
> > > rather leave the code as is.
> > 
> > I think the question is whether we really require having CAP_NET_ADMIN
> > in the initial namespace and not just in the current one.
> > Is ppp not network namespace aware?
> 
> I agree, ns_capable(net->user_ns, CAP_NET_ADMIN), would probably make
> more sense.
>
I guess you assume net is set to current->nsproxy->net_ns here.
Why about ns_capable(current_user_ns(), CAP_NET_ADMIN)?

>From my understanding of the code (I currently have no practical
experience with user namespaces), net->user_ns points to the userns in
which the current netns was created, while current_user_ns() refers to
the caller's userns. Shouldn't we check the later? Otherwise, any
process running in the netns would have the same capabilities regarding
PPP ioctls().

But I'm certainly missing important points. Interactions between netns
and userns are something I never investigated before, and using
net->user_ns seems to be way more common than using current_user_ns()
for checking capabilities in the networking stack.