On  Fri, Jan 17, 2020 at 17:36:27 +0100, Guillaume Nault wrote:

On Thu, Jan 16, 2020 at 09:23:32PM +0000, Tom Parkin wrote:

quoted

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

quoted

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.

In the UDP case, we have a socket connected to the peer (or at least
bound to a local address). That is, some local setup is needed. With
l2tp_ip, we don't even need to have an open socket. Everything is
triggered remotely. And here, "remotely" means "any host on any IP
network the LCCE is connected", because the destination address can
be any address assigned to the LCCE, even if it's not configured to
handle any kind of L2TP. But well, after thinking more about our L2TPv3
discussiong, I guess that's how the designer's of the protocol wanted.

I note that RFC 3931 provides for a cookie in the data packet header
to mitigate against data packet spoofing (section 8.2).

More generally I've not tried to see what could be done to spoof
session data packets, so I can't really comment further.  It would be
interesting to try spoofing packets and see if the kernel code could
do more to limit any potential problems.

quoted

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.

To summarise, my understanding is that global session IDs would follow
the spirit of RFC 3931 and would allow establishing multiple L2TPv3
connections (tunnels) over the same 5-tuple (or 3-tuple for IP encap).
Per socket session IDs don't, but would allow fixing Ridge's case.

I'm not 100% certain what "per socket session IDs" means here.  Could
you clarify?