netdev - Re: [PATCH net-next 14/16] net: Add sk_bind_dev_if to task

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1438099265.20182.63.camel@edumazet-glaptop2.roam.corp.google.com>
Date:	Tue, 28 Jul 2015 18:01:05 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Hannes Frederic Sowa <hannes@...essinduktion.org>
Cc:	"Eric W. Biederman" <ebiederm@...ssion.com>,
	David Ahern <dsa@...ulusnetworks.com>, netdev@...r.kernel.org,
	shm@...ulusnetworks.com, roopa@...ulusnetworks.com,
	gospo@...ulusnetworks.com, jtoppins@...ulusnetworks.com,
	nikolay@...ulusnetworks.com, ddutt@...ulusnetworks.com,
	nicolas.dichtel@...nd.com, stephen@...workplumber.org,
	hadi@...atatu.com, davem@...emloft.net, svaidya@...cade.com,
	mingo@...nel.org, luto@...capital.net
Subject: Re: [PATCH net-next 14/16] net: Add sk_bind_dev_if to task_struct

On Tue, 2015-07-28 at 14:19 +0200, Hannes Frederic Sowa wrote:
> Hello Eric,
> 
> On Mon, 2015-07-27 at 15:33 -0500, Eric W. Biederman wrote:
> > David Ahern <dsa@...ulusnetworks.com> writes:
> > 
> > > Allow tasks to have a default device index for binding sockets. If 
> > > set
> > > the value is passed to all AF_INET/AF_INET6 sockets when they are
> > > created.
> > > 
> > > The task setting is passed parent to child on fork, but can be set 
> > > or
> > > changed after task creation using prctl (if task has CAP_NET_ADMIN
> > > permissions). The setting for a socket can be retrieved using 
> > > prctl().
> > > This option allows an administrator to restrict a task to only 
> > > send/receive
> > > packets through the specified device. In the case of VRF devices 
> > > this
> > > option restricts tasks to a specific VRF.
> > > 
> > > Correlation of the device index to a specific VRF, ie.,
> > >    ifindex --> VRF device --> VRF id
> > > is left to userspace.
> > 
> > Nacked-by: "Eric W. Biederman" <ebiederm@...ssion.com>
> > 
> > Because it is broken by design.  Your routing device is only safe for
> > programs that know it's limitations it is not appropriate for general
> > applications.
> > 
> > Since you don't even seen to know it's limitations I think this is a
> > bad path to walk down.
> 
> Can you please elaborate about the broken by design?
> 
> Different operating systems are already using this approach with good
> success. I read your other mail regarding isolation of different VRFs
> and I agree that all code which persists state depending solely on the
> IP address is affected by this and this must be dealt with and fixed
> (actually, there aren't too many).
> 
> But I wouldn't call that broken by design. This stuff will get fixed
> like e.g. cross-talk between fragmentation queues, icmp rate limiters
> etc, which could already happen in the past.
> 
> What is your opinion on the fundamental approach only from a user
> perspective? Do you think that is broken, too?

I agree with Eric here.

This sk_bind_dev_if on task_struct is quite a hack.

What will be added next ? An array of dev_if ? netfilter support ?
af_packet support ? What about /proc files and netlink dumps ?

We already have network namespaces. Extend this if needed, instead of
bypassing them.

No need to add something else (with lack of proper reporting for various
tools)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html