Message-ID: <alpine.DEB.2.21.2001171027090.9038@ridgek-dl.ws.atlnz.lc>
Date:   Fri, 17 Jan 2020 10:50:55 +1300 (NZDT)
From:   Ridge Kennedy <ridgek@...iedtelesis.co.nz>
To:     Tom Parkin <tparkin@...alix.com>
cc:     Guillaume Nault <gnault@...hat.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net] l2tp: Allow duplicate session creation with UDP



On Thu, 16 Jan 2020, Tom Parkin wrote:

> On  Thu, Jan 16, 2020 at 20:05:56 +0100, Guillaume Nault wrote:
>> On Thu, Jan 16, 2020 at 01:12:24PM +0000, Tom Parkin wrote:
>>> I agree with you about the possibility for cross-talk, and I would
>>> welcome l2tp_ip/ip6 doing more validation.  But I don't think we should
>>> ditch the global list.
>>>
>>> As per the RFC, for L2TPv3 the session ID should be a unique
>>> identifier for the LCCE.  So it's reasonable that the kernel should
>>> enforce that when registering sessions.
>>>
>> I had never thought that the session ID could have global significance
>> in L2TPv3, but maybe that's because my experience is mostly about
>> L2TPv2. I haven't read RFC 3931 in detail, but I can't see how
>> restricting the scope of sessions to their parent tunnel would conflict
>> with the RFC.
>
> Sorry Guillaume, I responded to your other mail without reading this
> one.
>
> I gave more detail in my other response: it boils down to how RFC 3931
> changes the use of IDs in the L2TP header.  Data packets for IP or UDP
> only contain the 32-bit session ID, and hence this must be unique to
> the LCCE to allow the destination session to be successfully
> identified.
>
>>> When you're referring to cross-talk, I wonder whether you have in mind
>>> normal operation or malicious intent?  I suppose it would be possible
>>> for someone to craft session data packets in order to disrupt a
>>> session.
>>>
>> What makes me uneasy is that, as soon as the l2tp_ip or l2tp_ip6 module
>> is loaded, a peer can reach whatever L2TPv3 session exists on the host
>> just by sending an L2TP_IP packet to it.
>> I don't know how practical it is to exploit this fact, but it looks
>> like it's asking for trouble.
>
> Yes, I agree, although practically it's only a slightly easier
> "exploit" than L2TP/UDP offers.
>
> The UDP case requires a rogue packet to be delivered to the correct
> socket AND have a session ID matching that of one in the associated
> tunnel.
>
> It's a slightly more arduous scenario to engineer than the existing
> L2TPv3/IP case, but only a little.
>
> I also don't know how practical this would be to leverage to cause
> problems.
>
>>> For normal operation, you just need to get the wrong packet on the
>>> wrong socket to run into trouble of course.  In such a situation
>>> having a unique session ID for v3 helps you to determine that
>>> something has gone wrong, which is what the UDP encap recv path does
>>> if the session data packet's session ID isn't found in the context of
>>> the socket that receives it.
>> Unique global session IDs might help troubleshooting, but they also
>> break some use cases, as reported by Ridge. At some point, we'll have
>> to make a choice, or even add a knob if necessary.
>
> I suspect we need to reach agreement on what RFC 3931 implies before
> making headway on what the kernel should ideally do.
>
> There is perhaps room for pragmatism given that the kernel
> used to be more permissive prior to dbdbc73b4478, and we weren't
> inundated with reports of session ID clashes.
>

A knob (module_param?) to enable the permissive behaviour would certainly
work for me.

Powered by blists - more mailing lists