On Thu, 16 Jan 2020, Tom Parkin wrote:

On  Thu, Jan 16, 2020 at 20:05:56 +0100, Guillaume Nault wrote:

quoted

On Thu, Jan 16, 2020 at 01:12:24PM +0000, Tom Parkin wrote:

quoted

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.

quoted

quoted

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.

quoted

quoted

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.