netdev - Re: [PATCH net] l2tp: Allow duplicate session creation with UDP

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]

Date:   Tue, 21 Jan 2020 17:35:31 +0100
From:   Guillaume Nault <gnault@...hat.com>
To:     Tom Parkin <tparkin@...alix.com>
Cc:     Ridge Kennedy <ridgek@...iedtelesis.co.nz>, netdev@...r.kernel.org
Subject: Re: [PATCH net] l2tp: Allow duplicate session creation with UDP

On Mon, Jan 20, 2020 at 03:09:46PM +0000, Tom Parkin wrote:
> On  Sat, Jan 18, 2020 at 20:13:36 +0100, Guillaume Nault wrote:
> > I've never seen that as a problem in practice since establishing more
> > than one tunnel between two LCCE or LAC/LNS doesn't bring any
> > advantage.
> 
> I think the practical use depends a bit on context -- it might be
> useful to e.g. segregate sessions with different QoS or security
> requirements into different tunnels in order to make userspace
> configuration management easier.
> 
That could be useful for L2TPv2. But that's not going to be more
limitted for L2TPv3 as the tunnel ID isn't visible on the wire.

> > > Since we don't want to arbitrarily limit IP-encap tunnels to on per
> > > pair of peers, it's not practical to stash tunnel context with the
> > > socket in the IP-encap data path.
> > > 
> > Even though l2tp_ip doesn't lookup the session in the context of the
> > socket, it is limitted to one tunnel for a pair of peers, because it
> > doesn't support SO_REUSEADDR and SO_REUSEPORT.
> 
> This isn't the case.  It is indeed possible to create multiple IP-encap
> tunnels between the same IP addresses.
> 
> l2tp_ip takes tunnel ID into account in struct sockaddr_l2tpip when
> binding and connecting sockets.
> 
Yes, sorry. I didn't give this enough thinking and mixed the UDP and IP
transport constraints.

> I think if l2tp_ip were to support SO_REUSEADDR, it would be in the
> context of struct sockaddr_l2tpip.  In which case reusing the address
> wouldn't really make any sense.
> 
Yes, I think we can just forget about it.

> > Thinking more about the original issue, I think we could restrict the
> > scope of session IDs to the 3-tuple (for IP encap) or 5-tuple (for UDP
> > encap) of its parent tunnel. We could do that by adding the IP addresses,
> > protocol and ports to the hash key in the netns session hash-table.
> > This way:
> >  * Sessions would be only accessible from the peer with whom we
> >    established the tunnel.
> >  * We could use multiple sockets bound and connected to the same
> >    address pair, and lookup the right session no matter on which
> >    socket L2TP messages are received.
> >  * We would solve Ridge's problem because we could reuse session IDs
> >    as long as the 3 or 5-tuple of the parent tunnel is different.
> > 
> > That would be something for net-next though. For -net, we could get
> > something like Ridge's patch, which is simpler, since we've never
> > supported multiple tunnels per session anyway.
> 
> Yes, I think this would be possible.  I've been thinking of similar
> schemes.
> 
> I'm struggling with it a bit though.  Wouldn't extending the hash key
> like this get expensive, especially for IPv6 addresses?
> 
>From what I recall, L2TP performances are already quite low. That's
certainly not a reason for making things worse, but I believe that
computing a 3 or 5 tuple hash should be low overhead in comparison.
But checking with real numbers would be interesting.